Re: [PATCH v2] btrfs: relocation: Fix KASAN reports caused by extended reloc tree lifespan

[David Sterba <dsterba@suse.cz>]

On Wed, Jan 08, 2020 at 04:08:41PM +0100, David Sterba wrote:
> > >> +static bool have_reloc_root(struct btrfs_root *root)
> > >> +{
> > >> +    if (test_bit(BTRFS_ROOT_DEAD_RELOC_TREE, &root->state))
> > >> +        return false;
> > > 
> > > You still need a smp_mb__after_atomic() here, test_bit is unordered. 
> > 
> > Nope, that won't do anything, since smp_mb__(After|before)_atomic only
> > orders RMW operations and test_bit is not an RMW operation. From
> > atomic_bitops.txt:
> > 
> > 
> > Non-RMW ops:
> > 
> > 
> > 
> >   test_bit()
> > 
> > Furthermore from atomic_t.txt:
> > 
> > The barriers:
> > 
> > 
> > 
> >   smp_mb__{before,after}_atomic()
> > 
> > 
> > 
> > only apply to the RMW atomic ops and can be used to augment/upgrade the
> > 
> > ordering inherent to the op.
> 
> The way I read it is more like smp_rmb/smp_wmb, but for bits in this
> case, so the smp_mb__before/after_atomic was only a syntactic sugar to
> match that it's atomic bitops. I realize this could have caused some
> confusion, however I still think that some sort of barrier is needed.

There's an existing pattern used for serializing set/clear of
BTRFS_ROOT_IN_TRANS_SETUP (record_root_in_trans,
btrfs_record_root_in_trans).

Once upon a time there were barriers like smp_mb__before_clear_bit but
they got unified to just smp_mb__before_atomic for all set/clear
operations, so I believe I was not all wrong with using them.