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

David Howells
2009-04-28T10:22:07Z

Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> But I would strongly suggest at least a note calling this out, preferably a
> "don't do this" example.

How about I add this to the bottom of the new section:

[!] Note that the memory barriers implied by the sleeper and the waker do _not_
order multiple stores before the wake-up with respect to loads of those stored
values after the sleeper has called set_current_state(). For instance, if the
sleeper does:

set_current_state(TASK_INTERRUPTIBLE);
if (event_indicated)
break;
__set_current_state(TASK_RUNNING);
do_something(my_data);

and the waker does:

my_data = value;
event_indicated = 1;
wake_up(&event_wait_queue);

there's no guarantee that the change to event_indicated will be perceived by
the sleeper as coming after the change to my_data. In such a circumstance, the
code on both sides must interpolate its own memory barriers between the
separate data accesses. Thus the above sleeper ought to do:

set_current_state(TASK_INTERRUPTIBLE);
if (event_indicated) {
smp_rmb();
do_something(my_data);
}

and the waker should do:

my_data = value;
smp_wmb();
event_indicated = 1;
wake_up(&event_wait_queue);

David