From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754462AbbIBBKu (ORCPT ); Tue, 1 Sep 2015 21:10:50 -0400 Received: from mail-ig0-f175.google.com ([209.85.213.175]:33794 "EHLO mail-ig0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753629AbbIBBKq (ORCPT ); Tue, 1 Sep 2015 21:10:46 -0400 Date: Wed, 2 Sep 2015 09:10:27 +0800 From: Boqun Feng To: Oleg Nesterov Cc: "Paul E. McKenney" , Michal Hocko , Peter Zijlstra , LKML , David Howells , Linus Torvalds , Jonathan Corbet Subject: Re: wake_up_process implied memory barrier clarification Message-ID: <20150902011027.GB8007@fixme-laptop.cn.ibm.com> References: <20150828160637.GA4393@redhat.com> <20150829092514.GA3240@fixme-laptop.cn.ibm.com> <20150829142707.GA19263@redhat.com> <20150831003719.GC924@fixme-laptop.cn.ibm.com> <20150831183335.GA26333@redhat.com> <20150831203739.GX4029@linux.vnet.ibm.com> <20150901034014.GD1071@fixme-laptop.cn.ibm.com> <20150901095923.GB31368@redhat.com> <20150901145024.GA8007@fixme-laptop.cn.ibm.com> <20150901163923.GA20055@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="eAbsdosE1cNLO4uF" Content-Disposition: inline In-Reply-To: <20150901163923.GA20055@redhat.com> User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --eAbsdosE1cNLO4uF Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 01, 2015 at 06:39:23PM +0200, Oleg Nesterov wrote: > 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 n= eed > > > 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 "norm= al" > > > wait_event()-like code. >=20 > Looks like, you have missed this part of my previous email. See below. I guess I need to think through this, though I haven't found any problem in wake_up() if we remove the STORE-LOAD barrier in try_to_wake_up(). And I know that in wake_up(), try_to_wake_up() will be called with holding wait_queue_head_t->lock, however, only part of wait_event() holds the same lock, I can't figure out why the barrier is not needed because of the lock.. I will read the code carefully to see whether I miss something. >=20 > > I think maybe I have a misunderstanding of barrier pairing. >=20 > Or me. I can only say how it is supposed to work. >=20 > > think that a barrier pairing can only happen: >=20 > Well, no. See for example https://lkml.org/lkml/2014/7/16/310 This helps a lot, so does the LWN article it references: https://lwn.net/Articles/573436/ The barrier pairing here is scenario 10 in that article. I'm sure that is my misunderstanding of barrier pairing now, thank you! > Or, say, the comment in completion_done(). >=20 > 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. >=20 Your explanation and references help a lot, now I understand how the barrier works in try_to_wake_up() ;-) > Plus I didn't sleep today, not sure I can understand you correctly > and/or answer your question ;) >=20 > > One example of #2 pairing is the following sequence of events: > > > > Initially X =3D 0, Y =3D 0 > > > > CPU 1 CPU 2 > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > WRITE_ONCE(Y, 1); > > smp_mb(); > > r1 =3D READ_ONCE(X); // r1 =3D=3D 0 > > WRITE_ONCE(X, 1); > > smp_mb(); > > r2 =3D READ_ONCE(Y); > > > > ---------------------------------------------------------------- > > { assert(!(r1 =3D=3D 0 && r2 =3D=3D 0)); // means if r1 =3D=3D 0 then r= 2 =3D=3D 1} > > > > If CPU 1 _fails_ to read the value of X written by CPU 1, r2 is > > guaranteed to 1, which means assert(!(r1 =3D=3D 0 && r2 =3D=3D 0)) afte= rwards > > 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, >=20 > See above, wake_up/wait_event are fine in any case because they use > the same lock. >=20 I think I wanted to say wake_up_proccess() here, which calls try_to_wake_up() directly, sorry about that mistake.. > But as for try_to_wake_up() you are right, we rely on STORE-MB-LOAD. > Now let me quote another previous email, >=20 > So in fact try_to_wake_up() needs mb() before it does >=20 > if (!(p->state & state)) > goto out; >=20 > 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 sec= tion > and thus it can't be reordered with LOAD(p->state). >=20 This also helps a lot, thank you ;-) > And I think that smp_mb__before_spinlock() + spin_lock() should pair > correctly with mb(). If not - we should redefine it. >=20 > > X is the condition and Y is the task state, >=20 > Yes, >=20 > > and replace smp_mb() with really necessary barriers, right? >=20 > Sorry, can't understand this part... >=20 I just want to be accurate by saying that, because the barrier in try_to_wake_up() is not a smp_mb(), is a smp_mb__before_spinlock() + spin_lock(). Regards, Boqun --eAbsdosE1cNLO4uF Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJV5kx6AAoJEEl56MO1B/q4PNUH/RjXCkvzCLAIOfH328NZNUMh kBgY0E4/UZ3FDZ0hfrjf4uTGJ30M4whEOdxQZSyI03eZmoNBo4V6KLSEtbESJFgJ Tu0wQ7QDfgmf1bnWax/NRPP5qCuzZ0O4jRXwxShjvV5YJEgzTujbwrratauxeAyu JTK+6sIMqyWz77nqOR5QNt5kLpd88DC+/EQrRrqek6WM/rn/fUBfFh1nkFPl7sK7 +vOy2Iy5EamoTSRbfe87Hd8iJk2Cqkezg09+51lU2luSL20dU/mMv5j+6h38dEXb kTdgKHhox/Ik0rmA7PhbAQ1i8nyV8e5i8XrA3LKfYZCcDP5b+Z9+n2KJ1Tu2FjU= =qRtO -----END PGP SIGNATURE----- --eAbsdosE1cNLO4uF--