On Mon, 10 Apr 2017, Paul E. McKenney wrote:
> > But I would like to get this matter settled first. Is the explicit
> > barrier truly necessary?
>
> If you are using wait_event()/wake_up() or friends, the explicit
> barrier -is- necessary. To see this, look at v4.10's wait_event():
>
> #define wait_event(wq, condition) \
> do { \
> might_sleep(); \
> if (condition) \
> break; \
> __wait_event(wq, condition); \
> } while (0)
>
> As you can see, if the condition is set just before the wait_event()
> macro checks it, there is no ordering whatsoever.
This is true, but it is not relevant to the question I was asking.
> And if wake_up()
> finds nothing to wake up, there is no relevant ordering on that side,
> either.
>
> So you had better supply your own ordering, period, end of story.
The question is: Exactly what ordering do I need to supply? The
ordering among my own variables is okay; I know how to deal with that.
But what about the ordering between my variables and current->state?
For example, __wait_event() calls prepare_to_wait(), which calls
set_current_state(), which calls smp_store_mb(), thereby inserting a
full memory barrier between setting current->state and checking the
condition. But I didn't see any comparable barrier inserted by
wake_up(), between setting the condition and checking task->state.
However, now that I look more closely, I do see that wakeup_process()
calls try_to_wake_up(), which begins with:
/*
* If we are going to wake up a thread waiting for CONDITION we
* need to ensure that CONDITION=1 done by the caller can not be
* reordered with p->state check below. This pairs with mb() in
* set_current_state() the waiting thread does.
*/
smp_mb__before_spinlock();
raw_spin_lock_irqsave(&p->pi_lock, flags);
if (!(p->state & state))
So it does insert a full barrier after all, and there is nothing to
worry about.
This also means that the analysis provided by Thinh Nguyen in the
original patch description is wrong.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
