[RFC][PATCH] wake_up_var() memory ordering
agruenba at redhat.com
Tue Jun 25 05:12:22 PDT 2019
On Tue, 25 Jun 2019 at 12:36, Peter Zijlstra <peterz at infradead.org> 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
> But the above is:
> the difference is that _unlock() uses RELEASE semantics, where
> clear_bit() does not.
> The difference is illustrated with something like:
> cond = true;
> 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.
More information about the Linux-mediatek