Date:	Mon, 31 Aug 2015 08:37:20 +0800
From:	Boqun Feng <boqun.feng@...il.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Michal Hocko <mhocko@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	LKML <linux-kernel@...r.kernel.org>,
	David Howells <dhowells@...hat.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Jonathan Corbet <corbet@....net>
Subject: Re: wake_up_process implied memory barrier clarification

Hi Oleg,

On Sat, Aug 29, 2015 at 04:27:07PM +0200, Oleg Nesterov wrote:
> Hello Boqun,
> 
...
> > By this, I think you actually means the example below the added text,
> > i.e. the example for "to repeat..", right?
> 
> And above. Even
> 
> 	The barrier occurs before the task state is cleared
> 
> is not actually right. This is misleading. What is really important is that
> we have a barrier before we _read_ the task state. And again, again, the
> fact that we actually have the write barrier is just the implementation
> detail.
> 

Yes, agree with you. However, I didn't realize this problem before, I
will see what I can do ;-)

> > Subject: Documentation: call out conditional barriers of wait_*() and wake_up*()
> >
> > The memory barriers in some sleep and wakeup functions are conditional,
> > there are several situations that there is no barriers:
> >
> > 1.	If wait_event() and co. actually don't prepare to sleep, there
> > 	may be no barrier in them.
> 
> And thus (imo) we should not even try to document this. I mean, a user
> should not make any assumptions about the barriers inside wait_event().
> 

Yep.

> > 2.	No matter whether a sleep occurs or not, there may be no barrier
> > 	between a successful wait-condition checking(the result of which
> > 	is true) in wait_event() and the following instructions.
> 
> Yes, this is true in any case. Not sure this deserves addtionional
> documentation, but see below.
> 
> > 3.	If wake_up() and co. actually wake up no one, there may be no
> > 	write barrier in them.
> 
> See above.
> 
> > --- a/Documentation/memory-barriers.txt
> > +++ b/Documentation/memory-barriers.txt
> > @@ -1975,7 +1975,8 @@ set_current_state() may be wrapped by:
> >
> >  which therefore also imply a general memory barrier after setting the state.
> >  The whole sequence above is available in various canned forms, all of which
> > -interpolate the memory barrier in the right place:
> > +imply a general barrier if and only if a sleep is at least about to happen,
> > +i.e. prepare_to_wait*() is called.
> >
> >  	wait_event();
> >  	wait_event_interruptible();
> > @@ -1986,6 +1987,9 @@ interpolate the memory barrier in the right place:
> >  	wait_on_bit();
> >  	wait_on_bit_lock();
> >
> > +Further more, no barrier is guaranteed after the successful wait condition
> > +checkings, whose results are true, in wait_*() and before the instructions
> > +following wait_*().
> 
> Yes, perhaps this makes sense, but (to me) only because the explanation above
> looks a bit confusing to me. I simply can't understand why memory-barriers.txt
> mentions that barrier implied by set_current_state(). You need to know this
> if you want to understand how wait_event() works. You do not need to know
> about this barrier if you want to use wait_event(). If nothing else, because
> you can never rely on this barrier. Anf your examples below shows this.
> 

Fair enough, I went too far. How about just a single paragraph saying
that:

The wake_up(), wait_event() and their friends have proper barriers in
them, but these implicity barriers are only for the correctness for
sleep and wakeup. So don't rely on these barriers for things that are
neither wait-conditons nor task states.

Is that OK to you?

Thank you for your comments ;-)

Regards,
Boqun


Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)

Powered by blists - more mailing lists