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

[Oleg Nesterov <oleg@redhat.com>]

On 04/24, David Howells wrote:
>
> Oleg Nesterov <oleg@redhat.com> wrote:
>
> > Suppose that "event_indicated = 1" leaks into try_to_wake_up() after we
> > read p->state.
>
> In that case, it's entirely possible that the smp_wmb() in try_to_wake_up()
> should actually be an smp_mb(), but that on whichever arch patch:
>
> 	commit 04e2f1741d235ba599037734878d72e57cb302b5
> 	Author: Linus Torvalds <torvalds@woody.linux-foundation.org>
> 	Date:   Sat Feb 23 18:05:03 2008 -0800
> 	Subject: Add memory barrier semantics to wake_up() & co
>
> was tested on, it made no difference.

I think that, from the correctness pov, this patch does not depend on arch.
>From the changelog:

    However, adding a smp_wmb() to before the spinlock should now order the
    writing of CONDITION wrt the lock itself, which in turn is ordered wrt
    the accesses within the spinlock (which includes the reading of the old
    state).

IOW, spin_lock() itself is STORE, and has the acquire semantics. This means
that "wmb() + spin_lock()" must correctly serialize "STOREs bfore" and
"STOREs/LOADs after". At least this is my understanding.


But does it really "equal" to mb() on all arches ? I don't know, but I guess
it is not possible to derive from documentation. So, at least in theory,
try_to_wake_up() doesn't provide rmb() afaics, and

	LOAD_1;
	try_to_wake_up();
	LOAD_2;

this can be re-ordered.


But I know nothing about how mb/lock _actually_ play with hardware, even on
x86.

Oleg.