I've missed the thread "avoiding bad sleeps" last week. I've had a similar problem and I would like to discuss the solution I've used to avoid it.
I want to wake up a sleeping process from an IRQ handler. In the process, if I use a interruptible_sleep_on(), I need first to restore flags (otherwise the process will sleep forever).
restore_flags(flags);
// <<== here IRQ handler possibly call wake_up()
interruptible_sleep_on(&my_queue);
If I first enable interrupts and then call sleep_on, sometimes (often) the wake_up is called (due to a pending interrupt) when the wait_queue is empty.
I've written a modified version of interruptible_sleep_on which takes an additionnal argument : flags to be restored.
my_interruptible_sleep_on(&my_queue, flags);
It seems to be ok. I've no more bad sleeps or more exactly rarely and that why I submit this to you. Is my way to do it correct ?
I've joined at the end of this mail the modified function.
Thanks,
Christophe Barb?
long my_interruptible_sleep_on_timeout(struct wait_queue **p, long timeout, unsigned long oflags)
{
unsigned long flags;
struct wait_queue wait;
current->state = TASK_INTERRUPTIBLE;
// SLEEP_ON_HEAD
wait.task = current;
write_lock_irq(&waitqueue_lock);
__add_wait_queue(p, &wait);
write_unlock(&waitqueue_lock);
restore_flags(oflags);
timeout = schedule_timeout(timeout);
// SLEEP_ON_TAIL
write_lock_irqsave(&waitqueue_lock,flags);
__remove_wait_queue(p, &wait);
write_unlock_irqrestore(&waitqueue_lock,flags);
return timeout;
}
--
Christophe Barb?
Software Engineer
Lineo High Availability Group
42-46, rue M?d?ric
92110 Clichy - France
phone (33).1.41.40.02.12
fax (33).1.41.40.02.01
http://www.lineo.com
[email protected] said:
> It seems to be ok. I've no more bad sleeps or more exactly rarely and
> that why I submit this to you. Is my way to do it correct ? I've
> joined at the end of this mail the modified function.
You can't restore flags in a different function to the one you saved them
in. It'll break.
You should probably be using the wait_event() macro instead, which does
something similar - actually it performs the check after setting up the wait
queue, rather than releasing the lock.
--
dwmw2
christophe barbe wrote:
>
> I've missed the thread "avoiding bad sleeps" last week. I've had a similar problem
> and I would like to discuss the solution I've used to avoid it.
>
> I want to wake up a sleeping process from an IRQ handler. In the process, if I use
> a interruptible_sleep_on(), I need first to restore flags (otherwise the process
> will sleep forever).
>
> restore_flags(flags);
> // <<== here IRQ handler possibly call wake_up()
> interruptible_sleep_on(&my_queue);
>
> [...]
> I've written a modified version of interruptible_sleep_on which takes an
> additionnal argument : flags to be restored.
That's possible, but it will crash on Sparc: you cannot restore the
interrupt flag saved in one function in another function.
The solution is very simple: do not call restore_flags() before
interruptible_sleep_on(), the schedule internally reenables interrupts.
>>>>>>>>>>
for(;;) {
cli();
if(condition) {
sti();
break;
}
interruptible_sleep_on();
sti(); /* required! */
}
>>>>>>>>>
But if you are writing new code, then DO NOT USE sleep_on(), use
add_wait_queue(), and a spinlock instead of cli().
Look at wait_event_irq in <linux/raid/md_k.h> from the 2.4 kernel as an
example.
--
Manfred
Ok thank you for your help.
I've followed your first advice. My solution was ok on my target (ppc and x86) but was not a good solution.
I'm very interesting to know why it's bad to restore flags in a sub-function. I imagine it should be due to an optimisation in the restore function.
Thank you,
Christophe Barb?
On lun, 05 f?v 2001 13:59:28 Manfred Spraul wrote:
> christophe barbe wrote:
> >
> > I've missed the thread "avoiding bad sleeps" last week. I've had a similar problem
> > and I would like to discuss the solution I've used to avoid it.
> >
> > I want to wake up a sleeping process from an IRQ handler. In the process, if I use
> > a interruptible_sleep_on(), I need first to restore flags (otherwise the process
> > will sleep forever).
> >
> > restore_flags(flags);
> > // <<== here IRQ handler possibly call wake_up()
> > interruptible_sleep_on(&my_queue);
> >
> > [...]
> > I've written a modified version of interruptible_sleep_on which takes an
> > additionnal argument : flags to be restored.
>
> That's possible, but it will crash on Sparc: you cannot restore the
> interrupt flag saved in one function in another function.
>
> The solution is very simple: do not call restore_flags() before
> interruptible_sleep_on(), the schedule internally reenables interrupts.
>
> >>>>>>>>>>
> for(;;) {
> cli();
> if(condition) {
> sti();
> break;
> }
> interruptible_sleep_on();
> sti(); /* required! */
> }
> >>>>>>>>>
>
> But if you are writing new code, then DO NOT USE sleep_on(), use
> add_wait_queue(), and a spinlock instead of cli().
> Look at wait_event_irq in <linux/raid/md_k.h> from the 2.4 kernel as an
> example.
>
> --
> Manfred
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Christophe Barb?
Software Engineer
Lineo High Availability Group
42-46, rue M?d?ric
92110 Clichy - France
phone (33).1.41.40.02.12
fax (33).1.41.40.02.01
http://www.lineo.com
> I'm very interesting to know why it's bad to restore flags in a sub-function.
> I imagine it should be due to an optimisation in the restore function.
On sparc32 the flags includes the window pointer which tells us where in
the register windows we are. If you restore flags in a sub function
the kernel will become very confused :)
Forcing cli/sti etc to be in the same function also helps readability.
Anton