From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Zijlstra Subject: Re: [RFC][PATCH] wake_up_var() memory ordering Date: Tue, 25 Jun 2019 12:34:30 +0200 Message-ID: <20190625103430.GW3402@hirez.programming.kicks-ass.net> References: <20190624165012.GH3436@hirez.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+glpam-linux-mediatek=m.gmane.org-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org To: Andreas Gruenbacher Cc: Martin Brandenburg , linux-cachefs-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, Mike Snitzer , linux-aio-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, David Airlie , samba-technical , Joonas Lahtinen , Will Deacon , dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, David Howells , Chris Mason , dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, keyrings-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar , linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Alasdair Kergon , Mike Marshall , linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, rds-devel-N0ozoZBvEnrZJqsBc5GL+g@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, James Morris , cluster-devel , Antti Palosaari , Matthias Brugger , Paul McKenney , intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.orgd List-Id: intel-gfx@lists.freedesktop.org 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. 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. > > } > >