Messages in this thread | | | Date | Fri, 24 Apr 2009 20:30:29 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH] It may not be assumed that wake_up(), finish_wait() and co. imply a memory barrier |
| |
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.
| |