Messages in this thread | | | Date | Mon, 29 Sep 2014 23:02:21 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 02/11] wait: Provide infrastructure to deal with nested blocking |
| |
On 09/24, Peter Zijlstra wrote: > > There are a few places that call blocking primitives from wait loops, > provide infrastructure to support this without the typical > task_struct::state collision. > > We record the wakeup in wait_queue_t::flags which leaves > task_struct::state free to be used by others.
Sorry for delay. FWIW,
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> +/* > + * DEFINE_WAIT_FUNC(wait, woken_wait_func); ^^^^^^^^^^^^^^^^ woken_wake_function ;)
> +int woken_wake_function(wait_queue_t *wait, unsigned mode, int sync, void *key) > +{ > + /* > + * Although this function is called under waitqueue lock, LOCK > + * doesn't imply write barrier and the users expects write > + * barrier semantics on wakeup functions. The following > + * smp_wmb() is equivalent to smp_wmb() in try_to_wake_up() > + * and is paired with set_mb() in wait_woken(). > + */ > + smp_wmb(); /* C */ > + wait->flags |= WQ_FLAG_WOKEN;
Perhaps it is just me, but I was a bit confused by the comment above wmb(). Afaics, it is not that "users expects write barrier semantics", just we need to ensure that
CONDITION = true; wait->flags |= WQ_FLAG_WOKEN;
can't be reordered (and this differs from smp_wmb() in try_to_wake_up()). Otherwise we can obviously race with
// wait_woken() -> set_mb() wait->flags &= ~WQ_FLAG_WOKEN; mb();
if (CONDITION) break;
Oleg.
| |