Messages in this thread | | | Date | Tue, 1 Sep 2015 18:39:23 +0200 | From | Oleg Nesterov <> | Subject | Re: wake_up_process implied memory barrier clarification |
| |
On 09/01, Boqun Feng wrote: > > On Tue, Sep 01, 2015 at 11:59:23AM +0200, Oleg Nesterov wrote: > > > > And just in case, wake_up() differs in a sense that it doesn't even need > > that STORE-LOAD barrier in try_to_wake_up(), we can rely on > > wait_queue_head_t->lock. Assuming that wake_up() pairs with the "normal" > > wait_event()-like code.
Looks like, you have missed this part of my previous email. See below.
> I think maybe I have a misunderstanding of barrier pairing.
Or me. I can only say how it is supposed to work.
> think that a barrier pairing can only happen:
Well, no. See for example https://lkml.org/lkml/2014/7/16/310 Or, say, the comment in completion_done().
And please do not assume I can answer authoritatively the questions in this area. Fortunately we have paulmck/peterz in CC, they can correct me.
Plus I didn't sleep today, not sure I can understand you correctly and/or answer your question ;)
> One example of #2 pairing is the following sequence of events: > > Initially X = 0, Y = 0 > > CPU 1 CPU 2 > =========================== ================================ > WRITE_ONCE(Y, 1); > smp_mb(); > r1 = READ_ONCE(X); // r1 == 0 > WRITE_ONCE(X, 1); > smp_mb(); > r2 = READ_ONCE(Y); > > ---------------------------------------------------------------- > { assert(!(r1 == 0 && r2 == 0)); // means if r1 == 0 then r2 == 1} > > If CPU 1 _fails_ to read the value of X written by CPU 1, r2 is > guaranteed to 1, which means assert(!(r1 == 0 && r2 == 0)) afterwards > wouldn't be triggered in any case. > > And this is actually the case of wake_up/wait, assuming that > prepare_to_wait() is called on CPU 1 and wake_up() is called on CPU 2,
See above, wake_up/wait_event are fine in any case because they use the same lock.
But as for try_to_wake_up() you are right, we rely on STORE-MB-LOAD. Now let me quote another previous email,
So in fact try_to_wake_up() needs mb() before it does
if (!(p->state & state)) goto out;
But mb() is heavy, we can use wmb() instead, but only in this particular case: before spin_lock(), which guarantees that LOAD(p->state) can't leak out of the critical section. And since spin_lock() itself is the STORE, this guarantees that STORE(CONDITION) can't leak _into_ the critical section and thus it can't be reordered with LOAD(p->state).
And I think that smp_mb__before_spinlock() + spin_lock() should pair correctly with mb(). If not - we should redefine it.
> X is the condition and Y is the task state,
Yes,
> and replace smp_mb() with really necessary barriers, right?
Sorry, can't understand this part...
Oleg.
| |