From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754031AbbIAOuy (ORCPT ); Tue, 1 Sep 2015 10:50:54 -0400 Received: from mail-pa0-f42.google.com ([209.85.220.42]:34277 "EHLO mail-pa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752411AbbIAOuw (ORCPT ); Tue, 1 Sep 2015 10:50:52 -0400 Date: Tue, 1 Sep 2015 22:50:24 +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: <20150901145024.GA8007@fixme-laptop.cn.ibm.com> References: <20150827182654.GA12191@redhat.com> <20150828145121.GG5301@dhcp22.suse.cz> <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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="J/dobhs11T7y2rNN" Content-Disposition: inline In-Reply-To: <20150901095923.GB31368@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 --J/dobhs11T7y2rNN Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 01, 2015 at 11:59:23AM +0200, Oleg Nesterov wrote: > On 09/01, Boqun Feng wrote: > > > > But I'm still a little confused at Oleg's words: > > > > "What is really important is that we have a barrier before we _read_ the > > task state." > > > > I read is as "What is really important is that we have a barrier before > > we _read_ the task state and _after_ we write the CONDITION", if I don't > > misunderstand Oleg, this means a STORE-barrier-LOAD sequence, >=20 > Yes, exactly. >=20 > Let's look at this trivial code again, >=20 > CONDITION =3D 1; > wake_up_process(); >=20 > note that try_to_wake_up() does >=20 > if (!(p->state & state)) > goto out; >=20 > If this LOAD could be reordered with STORE(CONDITION) above we can obviou= sly > race with >=20 > set_current_state(...); > if (!CONDITION) > schedule(); >=20 > See the comment at the start of try_to_wake_up(). And again, again, please Thank you for your detailed explanation! I read the comment, but just couldn't understand how the pairing happens, I think I have a misunderstanding of pairing, please see below. > note that initially the only documented behaviour of smp_mb__before_spinl= ock() > was the STORE - LOAD serialization. This is what try_to_wake_up() needs, = it > doesn't actually need the write barrier after STORE(CONDITION). >=20 > 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. >=20 > > which IIUC > > can't pair with anything. >=20 > It pairs with the barrier implied by set_current_state(). I think maybe I have a misunderstanding of barrier pairing. I used to think that a barrier pairing can only happen: 1. between a "MEM-barrier-STORE" and "LOAD-barrier-MEM", when the LOAD reads the value which the STORE writes where MEM is a memory operation(STORE or LOAD). However, wait and wakeup don't fit in the #1 barrier pairing, so I guess I was wrong, there is a kind of barrier pairings which also happen: 2. between a "MEM-barrier-LOAD" and "STORE-barrier-MEM", when the LOAD _fails_ to read the value which the STORE writes, #1 pairing is easy to understand and memory-barriers.txt already has some examples. I admit that I haven't seen any usage of #2 pairing other than wait_event() and wake_up(). Of course, this may be because I'm not an expert of parallel programming and don't know too much.. Have to ask Paul, when we are talking about barrier pairing, we are actually talking about the combination of #1 and #2, right? And Oleg, the barrier pairing we are talking here is the kind of #2, right? Add two examples, in case that I fail to make clear the difference of two barrier pairings. One example of #1 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(); WRITE_ONCE(X, 1); r1 =3D READ_ONCE(X); // r1 =3D=3D 1 smp_mb(); r2 =3D READ_ONCE(Y); ---------------------------------------------------------------- { assert(!(r1 =3D=3D 1 && r2 =3D=3D 0)); // means if r1 =3D=3D 1 then r2 = =3D=3D 1} If CPU 2 reads the value of X written by CPU 1, r2 is guaranteed to be 1, which means assert(!(r1 =3D=3D 1 && r2 =3D=3D 0)) afterwards wouldn't be triggered in any case. 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 r2 = =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)) afterwar= ds 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, X is the condition and Y is the task state, and replace smp_mb() with really necessary barriers, right? Regards, Boqun --J/dobhs11T7y2rNN Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJV5bsrAAoJEEl56MO1B/q49pcIALSLJt3/XAjbpFnzTsgtRm+r nZjemWSkbLjmd9kjlsrlHcNEHKWsO46Cnknc+2iqvdrXwwtx37TTZwc3l/Qkr/Eo uXFfKwqRG0VWqYFXanJQfY7myawFCtkEN4QRUfoxIBrFHTKjeakGBYYTDtA0AaEC nNGHX9OYhoAOS7CvOA028jLyS6oJ0LY8m6Bz4azz9AvUGZ0YceCQH8uFYmDnL4Rw fkmqN7SEZMbCFt5owe4VlW/uTxkhn8rVX2Y2Rk2+MmMn3cN7Pu3fuUMBy8W6xIi9 gJnvz2xDf4ymSpd2nokSNjhCgC7byl1KMr9X5JinhFfWEExvoCUmaGdOWq7S/zE= =rRyN -----END PGP SIGNATURE----- --J/dobhs11T7y2rNN--