Re: [patch 4/4] char/snsc: reorder set_current_state() and add_wait_queue()

[Nishanth Aravamudan <nacc@us.ibm.com>]

On Mon, Nov 22, 2004 at 09:56:20PM +0100, Martin Waitz wrote:
> hoi :)
> 
> On Sat, Nov 20, 2004 at 03:47:03AM +0100, janitor@sternwelten.at wrote:
> > Description: Reorder add_wait_queue() and set_current_state() as a
> > signal could be lost in between the two functions.
> 
> couldn't you loose a wake event that way?

The patch in question was made specifically because the given code may
miss wake events. Hence the reorder and my description. Since I'm not an
expert on locking, I asked Oliver Neukum about this exact issue a while
ago. Here is his response:

--------

>Am Mittwoch, 15. September 2004 19:34 schrieb Nishanth Aravamudan:
> Oliver,
> 
> It was recommended to me to ask you a question about the proper ordering
> of add_wait_queue() and set_current_state().
> 
> In some drivers the order is
> 
> set_current_state(TASK_INTERRUPTIBLE);
> add_wait_queue(...);

That is correct.

> and in others it is
> 
> add_wait_queue(...);
Here in between is a window in which a wake up would be missed.
> set_current_state(TASK_INTERRUPTIBLE);
> 
> It seems like one of these should be oorrect and not the other, but I'm
> not sure which is which. Any insight you could provide would be greatly
> appreciated.

Iff you test whether you should indeed sleep before you call schedule,
it doesn't matter. In the other case, you need to use the first form.

-------

Hope that clears things up a bit.

-Nish