From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andreas Gruenbacher Subject: Re: [RFC][PATCH] wake_up_var() memory ordering Date: Tue, 25 Jun 2019 14:12:22 +0200 Message-ID: References: <20190624165012.GH3436@hirez.programming.kicks-ass.net> <20190625103430.GW3402@hirez.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20190625103430.GW3402@hirez.programming.kicks-ass.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Peter Zijlstra Cc: Martin Brandenburg , linux-cachefs@redhat.com, Mike Snitzer , linux-aio@kvack.org, David Airlie , samba-technical , Will Deacon , dri-devel@lists.freedesktop.org, David Howells , Chris Mason , dm-devel@redhat.com, keyrings@vger.kernel.org, Ingo Molnar , linux-afs@lists.infradead.org, Alasdair Kergon , Mike Marshall , linux-cifs@vger.kernel.org, rds-devel@oss.oracle.com, linux-rdma@vger.kernel.org, James Morris , cluster-devel , Antti Palosaari , Matthias Brugger , Paul McKenney , intel-gfx@lists.freedesktop.org, devel@lists.orangefs.org, "Serge E. Hallyn" List-Id: linux-rdma@vger.kernel.org T24gVHVlLCAyNSBKdW4gMjAxOSBhdCAxMjozNiwgUGV0ZXIgWmlqbHN0cmEgPHBldGVyekBpbmZy YWRlYWQub3JnPiB3cm90ZToKPiBPbiBUdWUsIEp1biAyNSwgMjAxOSBhdCAxMToxOTozNUFNICsw MjAwLCBBbmRyZWFzIEdydWVuYmFjaGVyIHdyb3RlOgo+ID4gPiBkaWZmIC0tZ2l0IGEvZnMvZ2Zz Mi9nbG9wcy5jIGIvZnMvZ2ZzMi9nbG9wcy5jCj4gPiA+IGluZGV4IGNmNGM3NjcwMDViMS4uNjY2 NjI5ZWE1ZGE3IDEwMDY0NAo+ID4gPiAtLS0gYS9mcy9nZnMyL2dsb3BzLmMKPiA+ID4gKysrIGIv ZnMvZ2ZzMi9nbG9wcy5jCj4gPiA+IEBAIC0yMjcsNiArMjI3LDcgQEAgc3RhdGljIHZvaWQgZ2Zz Ml9jbGVhcl9nbG9wX3BlbmRpbmcoc3RydWN0IGdmczJfaW5vZGUgKmlwKQo+ID4gPiAgICAgICAg ICAgICAgICAgcmV0dXJuOwo+ID4gPgo+ID4gPiAgICAgICAgIGNsZWFyX2JpdF91bmxvY2soR0lG X0dMT1BfUEVORElORywgJmlwLT5pX2ZsYWdzKTsKPiA+ID4gKyAgICAgICBzbXBfbWJfX2FmdGVy X2F0b21pYygpOwo+ID4gPiAgICAgICAgIHdha2VfdXBfYml0KCZpcC0+aV9mbGFncywgR0lGX0dM T1BfUEVORElORyk7Cj4gPgo+ID4gVGhpcyBzaG91bGQgYmVjb21lIGNsZWFyX2FuZF93YWtlX3Vw X2JpdCBhcyB3ZWxsLCByaWdodD8gVGhlcmUgYXJlCj4gPiBzZXZlcmFsIG1vcmUgaW5zdGFuY2Vz IG9mIHRoZSBzYW1lIHBhdHRlcm4uCj4KPiBPbmx5IGlmIHdlIGRvIGFzIERhdmlkIHN1Z2dlc3Rl ZCBhbmQgbWFrZSBjbGVhbl9hbmRfd2FrZV91cF9iaXQoKQo+IHByb3ZpZGUgdGhlIFJFTEVBU0Ug YmFycmllci4KCihJdCdzIGNsZWFyX2FuZF93YWtlX3VwX2JpdCwgbm90IGNsZWFuX2FuZF93YWtl X3VwX2JpdC4pCgo+IFRoYXQgaXMsIGN1cnJlbnRseSBjbGVhcl9hbmRfd2FrZV91cF9iaXQoKSBp cwo+Cj4gICAgICAgICBjbGVhcl9iaXQoKQo+ICAgICAgICAgc21wX21iX19hZnRlcl9hdG9taWMo KTsKPiAgICAgICAgIHdha2VfdXBfYml0KCk7Cj4KPiBCdXQgdGhlIGFib3ZlIGlzOgo+Cj4gICAg ICAgICBjbGVhcl9iaXRfdW5sb2NrKCk7Cj4gICAgICAgICBzbXBfbWJfX2FmdGVyX2F0b21pYygp Owo+ICAgICAgICAgd2FrZV91cF9iaXQoKQo+Cj4gdGhlIGRpZmZlcmVuY2UgaXMgdGhhdCBfdW5s b2NrKCkgdXNlcyBSRUxFQVNFIHNlbWFudGljcywgd2hlcmUKPiBjbGVhcl9iaXQoKSBkb2VzIG5v dC4KPgo+IFRoZSBkaWZmZXJlbmNlIGlzIGlsbHVzdHJhdGVkIHdpdGggc29tZXRoaW5nIGxpa2U6 Cj4KPiAgICAgICAgIGNvbmQgPSB0cnVlOwo+ICAgICAgICAgY2xlYXJfYml0KCkKPiAgICAgICAg IHNtcF9tYl9fYWZ0ZXJfYXRvbWljKCk7Cj4gICAgICAgICB3YWtlX3VwX2JpdCgpOwo+Cj4gSW4g dGhpcyBjYXNlLCBhIHJlbW90ZSBDUFUgY2FuIGZpcnN0IG9ic2VydmUgdGhlIGNsZWFyX2JpdCgp IGFuZCB0aGVuCj4gdGhlICdjb25kID0gdHJ1ZScgc3RvcmUuIFdoZW4gd2UgdXNlIGNsZWFyX2Jp dF91bmxvY2soKSB0aGlzIGlzIG5vdAo+IHBvc3NpYmxlLCBiZWNhdXNlIHRoZSBSRUxFQVNFIGJh cnJpZXIgZW5zdXJlcyB0aGF0IGV2ZXJ5dGhpbmcgYmVmb3JlLAo+IHN0YXlzIGJlZm9yZS4KCk5v dyBJJ20gY29uZnVzZWQgYmVjYXVzZSBjbGVhcl9hbmRfd2FrZV91cF9iaXQoKSBpbiBtYWlubGlu ZSBkb2VzIHVzZQpjbGVhcl9iaXRfdW5sb2NrKCksIHNvIGl0J3MgdGhlIGV4YWN0IG9wcG9zaXRl IG9mIHdoYXQgeW91IGp1c3Qgc2FpZC4KClRoYW5rcywKQW5kcmVhcwpfX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGluZyBsaXN0Cklu dGVsLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5v cmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZng= From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andreas Gruenbacher Date: Tue, 25 Jun 2019 14:12:22 +0200 Subject: [Cluster-devel] [RFC][PATCH] wake_up_var() memory ordering In-Reply-To: <20190625103430.GW3402@hirez.programming.kicks-ass.net> References: <20190624165012.GH3436@hirez.programming.kicks-ass.net> <20190625103430.GW3402@hirez.programming.kicks-ass.net> Message-ID: List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Tue, 25 Jun 2019 at 12:36, Peter Zijlstra wrote: > On Tue, Jun 25, 2019 at 11:19:35AM +0200, Andreas Gruenbacher wrote: > > > diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c > > > index cf4c767005b1..666629ea5da7 100644 > > > --- a/fs/gfs2/glops.c > > > +++ b/fs/gfs2/glops.c > > > @@ -227,6 +227,7 @@ static void gfs2_clear_glop_pending(struct gfs2_inode *ip) > > > return; > > > > > > clear_bit_unlock(GIF_GLOP_PENDING, &ip->i_flags); > > > + smp_mb__after_atomic(); > > > wake_up_bit(&ip->i_flags, GIF_GLOP_PENDING); > > > > This should become clear_and_wake_up_bit as well, right? There are > > several more instances of the same pattern. > > Only if we do as David suggested and make clean_and_wake_up_bit() > provide the RELEASE barrier. (It's clear_and_wake_up_bit, not clean_and_wake_up_bit.) > That is, currently clear_and_wake_up_bit() is > > clear_bit() > smp_mb__after_atomic(); > wake_up_bit(); > > But the above is: > > clear_bit_unlock(); > smp_mb__after_atomic(); > wake_up_bit() > > the difference is that _unlock() uses RELEASE semantics, where > clear_bit() does not. > > The difference is illustrated with something like: > > cond = true; > clear_bit() > smp_mb__after_atomic(); > wake_up_bit(); > > In this case, a remote CPU can first observe the clear_bit() and then > the 'cond = true' store. When we use clear_bit_unlock() this is not > possible, because the RELEASE barrier ensures that everything before, > stays before. Now I'm confused because clear_and_wake_up_bit() in mainline does use clear_bit_unlock(), so it's the exact opposite of what you just said. Thanks, Andreas