IRQ and sleep_on

IRQ and sleep_on

[christophe barbe]

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
www.lineo.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: IRQ and sleep_on

[David Woodhouse]

christophe.barbe@inup.com 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


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: IRQ and sleep_on

[Manfred Spraul]

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 majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: IRQ and sleep_on

[christophe barbe]

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 majordomo@vger.kernel.org
> 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
www.lineo.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: IRQ and sleep_on

[Anton Blanchard]

> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/