Do not deprecate binary semaphore or do allow mutex in software interrupt contexts
The following code seems to me to be a valid example of a binary semaphore
(mutex) in a timer:
//timer called 10 times a second
static void status_timer(unsigned long device)
{
struct etp_device_private *dp = (struct etp_device_private *)device;
if (unlikely(dp->status_interface == 0))
dp->status_interface = INTERFACES_PER_DEVICE - 1;
else
dp->status_interface--;
//DBG_PRINT ("%s: In status timer, interface:0x%x.\n",etp_NAME,
dp->status_interface);
idt_los_interrupt_1(dp, dp->status_interface);
if (likely(!dp->reset))
// reset the timer:
mod_timer(&dp->status_timer, jiffies + HZ / 10);
}
static inline void read_idt_register_interrupt(struct etp_device_private *dp,
unsigned reg)
{
DBG_PRINT("read_idt_register_interrupt to mutex_lock.\n");
if (unlikely(down_trylock(&dp->semaphore)))
return;/* Do not read because failed to lock. */
if (likely
(!dp->status
&& !(inl((void *)(dp->ioaddr + REG_E1_CTRL)) & E1_ACCESS_ON))) {
outl(((reg << E1_REGISTER_SHIFT) & E1_REGISTER_MASK)
| E1_DIR_READ | E1_ACCESS_ON,
(void *)(dp->ioaddr + REG_E1_CTRL));
dp->status = 1;
DBG_PRINT("read_idt_register_interrupt set status read.\n");
} else
DBG_PRINT
("read_idt_register_interrupt did not set status %u read.\n",
dp->status);
DBG_PRINT
("read_idt_register_interrupt do not wait for result here, read in
tasklet.\n");
}
//for getting los information with interrupt:
void idt_los_interrupt_1(struct etp_device_private *dp, unsigned interface)
{
read_idt_register_if_interrupt(dp, E1_TRNCVR_LINE_STATUS0_REG,
interface);
}
static void e1_access_task(unsigned long data)//called after
e1_access_interrupt
{
struct etp_device_private *dp = (struct etp_device_private *)data;
struct etp_interface_private *ip;
unsigned int interface, error;
bool los;
//check if los status was read:
if (unlikely(!dp->status)) {
DBG_PRINT("e1_access_task wakes up user.\n");
wake_up(&dp->e1_access_q);
return;
}
error =
idt_los_interrupt_2(dp->ioaddr, &interface, &los,
dp->pci_dev->device);
//DBG_PRINT ("%s: In e1 task, error:0x%x, interface:0x%x, los:0x%x.\n",
// etp_NAME, error, interface, los);
dp->status = 0;
up(&dp->semaphore);
DBG_PRINT("e1_access_task got error %u.\n", error);
if (unlikely(error))
return;
//update los status:
ip = &dp->interface_privates[interface];
ip->los = los;
//update status:
if ((ip->if_mode == IF_MODE_CLOSED) ||//interface closed or
(ip->los)) {//link down
set_led(LED_CTRL_OFF, ip);
if (netif_carrier_ok(ip->ch_priv.this_netdev))
netif_carrier_off(ip->ch_priv.this_netdev);
} else {//link up and interface opened
if (!netif_carrier_ok(ip->ch_priv.this_netdev))
netif_carrier_on(ip->ch_priv.this_netdev);
if (ip->if_mode == IF_MODE_HDLC) {
set_led(LED_CTRL_TRAFFIC, ip);
} else {//ip->if_mode == IF_MODE_TIMESLOT
set_led(LED_CTRL_ON, ip);
}
}
}
int idt_los_interrupt_2(u8 * ioaddr, unsigned *interface, bool * los,
unsigned pci_device_id)
{//returns 0 in success
unsigned int value = inl((void *)(ioaddr + REG_E1_CTRL));
//if access not ended:
if (value & E1_ACCESS_ON) {
return 1;
}
//if access not to los status register
if ((value & E1_REGISTER_MASK_NO_IF) !=
(E1_TRNCVR_LINE_STATUS0_REG << E1_REGISTER_SHIFT)) {
return 1;
}
//get interface
*interface =
idt_if_to_if((value & E1_REGISTER_MASK_IF) >>
E1_REGISTER_SHIFT_IF, pci_device_id);
*los = value & 0x1;
return 0;
}
int write_idt_register_lock(unsigned device, unsigned reg, u32 value)
{
struct etp_device_private *etp = get_dev_priv(device);
unsigned ctrl;
DBG_PRINT("write_idt_register_lock to mutex lock device %u.\n", device);
down(&etp->semaphore);
if (unlikely(etp->reset)) {
up(&etp->semaphore);
DBG_PRINT
("write_idt_register_lock device %u unusable.\n", device);
return -ENXIO;
}
DBG_PRINT("write_idt_register_lock mutex locked device %u.\n", device);
do {
DBG_PRINT
("write_idt_register_lock to wait_event_timeout device %u.\n",
device);
wait_event_timeout(etp->e1_access_q,
!((ctrl =
inl((void *)(etp->ioaddr + REG_E1_CTRL)))
& E1_ACCESS_ON), HZ / 500);
}
while (ctrl & E1_ACCESS_ON);
DBG_PRINT("write_idt_register_lock to outl device %u.\n", device);
outl(((reg << E1_REGISTER_SHIFT) & E1_REGISTER_MASK) |
E1_DIR_WRITE | E1_ACCESS_ON | (value & E1_DATA_MASK
Re: Do not deprecate binary semaphore or do allow mutex in software interrupt contexts
On Tue, 11 Sep 2007 10:29:19 -0700 (PDT) Matti Linnanvuori <[EMAIL PROTECTED]> wrote: > > Not being too familiar with the timer stuff, it smells wrong what > > you say. > > Why? timers and sleeping locks really shouldn't be mixed. It ends up in general as more complex, fragile and weird... But feel free to prove us otherwise with real code... - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Do not deprecate binary semaphore or do allow mutex in software interrupt contexts
On Tue, 11 Sep 2007 09:20:23 -0700 (PDT) Matti Linnanvuori <[EMAIL PROTECTED]> wrote: > Arjan van de Ven: > > what do you do if the trylock fails? > > Just do not read the status variable now but modify the timer to run > later. > > > to be honest, the scenario describe really smells of broken > > locking, in fact it really sounds like it wants to use spinlocks > > instead > > No, I don't think it is broken. > Spinlocks can be used, but I don't see them being obviously better in > all cases. If access takes a long time, it is better to sleep during > it. And if you sleep, you might just end up creating a new mutex > implementation with a spinlock. at this point the discussion has gone so theoretical that I think it's better to go with a real example. What actual source code do you think is a legit case for this? I still think that whatever case you have in mind is better served with something else, but until we see the actual complete drier we're both talking air. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Do not deprecate binary semaphore or do allow mutex in software interrupt contexts
[re-adding CCs, please do not drop these]
On Tue, 2007-09-11 at 09:20 -0700, Matti Linnanvuori wrote:
> Arjan van de Ven:
> > what do you do if the trylock fails?
>
> Just do not read the status variable now but modify the timer to run later.
>
> > to be honest, the scenario describe really smells of broken locking, in
> > fact it really sounds like it wants to use spinlocks instead
>
> No, I don't think it is broken.
Yes it is.
> Spinlocks can be used, but I don't see them being obviously better in all
> cases.
> If access takes a long time, it is better to sleep during it.
> And if you sleep, you might just end up creating a new mutex
> implementation with a spinlock.
If you have to wait a long time in an atomic context you've done
something wrong. If you're only reading it from an atomic context you
might consider storing a copy that can be quickly updated and protect
that using a spinlock.
do_update ()
{
mutex_lock(&my_device_mutex);
my_device_frob_state(&my_state); /* <-- this takes a _long_ while */
spin_lock_irq(&my_shadow_state_lock);
my_shadow_state = state; /* <-- this is a quick memcopy */
spin_unlock_irq(&my_shadow_state_lock);
mutex_unlock(&my_device_mutex);
}
do_read()
{
spin_lock_irq(&my_shadow_state);
do_something_with_shadow_state(&mt_shadow_state);
spin_unlock_irq(&my_shadow_state);
return foo;
}
> Alan Cox:
> > For polling and timer based code its often simpler to do
> >
> >del_timer_sync(&my_timer);
> >FrobStuff
> >add_timer(&my_timer);
> >
> > especially if "FrobStuff" is likely to change when you next need to poll.
>
> In the scenario I presented, the timer modifies itself to run later.
> Therefore, simply calling del_timer_sync is not enough but you have to
> set an atomic variable to prevent the timer from adding itself again.
Not being too familiar with the timer stuff, it smells wrong what you
say.
As for the whole polling method, consider what Alan said, don't do it if
you don't need to. You'll annoy people at no end. Try to push state
changes where possible.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Do not deprecate binary semaphore or do allow mutex in software interrupt contexts
Arjan van de Ven: > what do you do if the trylock fails? Just do not read the status variable now but modify the timer to run later. > to be honest, the scenario describe really smells of broken locking, in > fact it really sounds like it wants to use spinlocks instead No, I don't think it is broken. Spinlocks can be used, but I don't see them being obviously better in all cases. If access takes a long time, it is better to sleep during it. And if you sleep, you might just end up creating a new mutex implementation with a spinlock. Alan Cox: > For polling and timer based code its often simpler to do > >del_timer_sync(&my_timer); >FrobStuff >add_timer(&my_timer); > > especially if "FrobStuff" is likely to change when you next need to poll. In the scenario I presented, the timer modifies itself to run later. Therefore, simply calling del_timer_sync is not enough but you have to set an atomic variable to prevent the timer from adding itself again. Again, you end up creating a new mutex implementation, which is not good. __ Yahoo! Clever - Der einfachste Weg, Fragen zu stellen und Wissenswertes mit Anderen zu teilen. www.yahoo.de/clever - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Do not deprecate binary semaphore or do allow mutex in software interrupt contexts
> to be honest, the scenario describe really smells of broken locking, in > fact it really sounds like it wants to use spinlocks instead For polling and timer based code its often simpler to do del_timer_sync(&my_timer); FrobStuff add_timer(&my_timer); especially if "FrobStuff" is likely to change when you next need to poll. And don't forget regular polling is a good way to really annoy laptop users as it burns power Alan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Do not deprecate binary semaphore or do allow mutex in software interrupt contexts
On Tue, 11 Sep 2007 06:20:51 -0700 (PDT) Matti Linnanvuori <[EMAIL PROTECTED]> wrote: Hi, > I thought of a scenario where it seems appropriate to use a binary > semaphore or a mutex in a software interrupt context. If a device > cannot interrupt when some important variable changes, it can be > polled occasionally to update e.g. LEDs to indicate status. Such > polling can be done most efficiently in timers that are software > interrupts. Timers are more efficient than works because they do not > have so much context switching overhead. If the access to the > variables of the device must be serialized, a binary semaphore or a > mutex is a natural choice. If user-space writing to the device is > likely to change the status, it can make sense not to poll the status > of the device at the same time. The timer could therefore sensibly > call mutex_trylock. what do you do if the trylock fails? > Therefore, it seems wrong to me to deprecate > binary semaphores and disallow the use of mutexes in software > interrupt contexts. to be honest, the scenario describe really smells of broken locking, in fact it really sounds like it wants to use spinlocks instead Greetings, Arjan van de Ven - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Do not deprecate binary semaphore or do allow mutex in software interrupt contexts
I thought of a scenario where it seems appropriate to use a binary semaphore or a mutex in a software interrupt context. If a device cannot interrupt when some important variable changes, it can be polled occasionally to update e.g. LEDs to indicate status. Such polling can be done most efficiently in timers that are software interrupts. Timers are more efficient than works because they do not have so much context switching overhead. If the access to the variables of the device must be serialized, a binary semaphore or a mutex is a natural choice. If user-space writing to the device is likely to change the status, it can make sense not to poll the status of the device at the same time. The timer could therefore sensibly call mutex_trylock. Therefore, it seems wrong to me to deprecate binary semaphores and disallow the use of mutexes in software interrupt contexts. __ Yahoo! Clever: Sie haben Fragen? Yahoo! Nutzer antworten Ihnen. www.yahoo.de/clever - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
