Re: [PATCH] It may not be assumed that wake_up(), finish_wait() and co. imply a memory barrier

Oleg Nesterov
2009-04-24T17:14:17Z

On 04/24, Paul E. McKenney wrote:
>
> One question, assuming that this documentation intends to guide the
> reader on where to put the locking and/or memory-barrier primitives...
>
> Suppose we have the following sequence of events:
>
> 1. The waiter does "set_current_state(TASK_UNINTERRUPTIBLE);".
> This implies a full memory barrier.
>
> 2. The awakener updates some shared state.
>
> 3. The awakener does "event_indicated = 1;".
>
> 4. The waiter does "if (event_indicated)", and, finding that
> the event has in fact been indicated, does "break".
>
> 5. The waiter accesses the shared state set in #2 above.
>
> 6. Some time later, the awakener does "wake_up(&event_wait_queue);"
> This does not awaken anyone, so no memory barrier.
>
> Because there is no memory barrier between #2 and #3, reordering by
> either the compiler or the CPU might cause the awakener to update the
> event_indicated flag in #3 -before- completing its update of shared
> state in #2. Less likely (but still possible) optimizations might
> cause the waiter to access the shared state in #5 before checking
> the event_indicated flag in #4.

Do you mean something like

awakener:

DATA = value;
DATA_IS_READY = true;
wake_up(wq);

waiter:

set_current_state(UNINTERRUPTIBLE);
if (DATA_IS_READY)
do_something(DATA);

?

Imho, the code above is just buggy and should be ignored by documentation ;)

Or do I miss your point?

Oleg.