Re: [PATCH] Document that wake_up(), complete() and co. imply a full memory barrier

Ingo Molnar
2009-04-22T13:52:21Z

* David Howells <dhowells@redhat.com> wrote:

> Oleg Nesterov <oleg@redhat.com> wrote:
>
> > > That's an interesting question. Should wake_up() imply a barrier of any
> > > sort, I wonder. Well, __wake_up() does impose a barrier as it uses a
> > > spinlock, but I wonder if that's sufficient.
> >
> > wake_up() does imply the barrier. Note the smp_wmb() in try_to_wake_up().
> > And in fact this wmb() implies mb(), because spin_lock() itself is STORE,
> > and the futher LOADs can't leak up before spin_lock().
> >
> > But afaics, this doesn't matter? prepare_to_wait() sets
> > task->state under wait_queue_head_t->lock and wake_up() takes
> > this look too, so we can't miss the event.
> >
> > Or I completely misunderstood the issue...
>
> The problem is not what wake_up() and co. do, it's what you are
> allowed to assume that they do.
>
> However, I think you're right, and that we can assume they imply a
> full memory barrier. To this end, I've attached a patch to
> document this.
>
> David
> ---
> From: David Howells <dhowells@redhat.com>
> Subject: [PATCH] Document that wake_up(), complete() and co. imply a full memory barrier
>
> Add to the memory barriers document to note that wake_up(), complete() and
> co. all imply a full memory barrier.

No. They dont generally imply a full memory barrier versus any
arbitrary prior (or following) memory access.

try_to_wake_up() has an smp_wmb() so it is a write memory barrier
(but not necessarily a read memory barrier). Otherwise there are
spinlocks there but spinlocks are not explicit 'full memory
barriers'.

Also, there's a sub-detail wrt. the wake_up() variants in that they
have a fastpath for the !q case - then they dont have any atomics at
all - they just return straight away.

Another sub-detail: wakeups using a special wakeup handler might not
even call try_to_wake_up() - so in their case not even a write
barrier can be assumed.

So your patch is misleading in a number of areas here.

Ingo