Re: [ANNOUNCE] Kernel Janitor's TODO list

[Andrea Arcangeli <andrea@suse.de>]

On Mon, Jan 29, 2001 at 08:47:50PM +0100, Roman Zippel wrote:
> Hi,
> 
> On Mon, 29 Jan 2001, Andi Kleen wrote:
> 
> > You can miss wakeups. The standard pattern is:
> > 
> > 	get locks
> > 
> > 	add_wait_queue(&waitqueue, &wait);
> > 	for (;;) { 
> > 		if (condition you're waiting for is true) 
> > 			break; 
> > 		unlock any non sleeping locks you need for condition
> > 		__set_task_state(current, TASK_UNINTERRUPTIBLE); 
> > 		schedule(); 
> > 		__set_task_state(current, TASK_RUNNING); 
> > 		reaquire locks
> > 	}
> > 	remove_wait_queue(&waitqueue, &wait); 
> 
> You still miss wakeups. :)
> Always set the task state first, then check the condition. See the
> wait_event*() macros you mentioned for the right order.

If the wakeup happens with the spinlock acquired (as the above code seems to
assume) you don't need to set the task state as uninterruptible before checking
the condition, however the above is wrong anyways because it should do
__set_task_state _before_ releasing the lock and not after.

Andrea