On Wed, Dec 11, 2019 at 01:00:01PM +0800, Qu Wenruo wrote:
> Due to commit d2311e698578 ("btrfs: relocation: Delay reloc tree
> deletion after merge_reloc_roots"), reloc tree lifespan is extended.
>
> Although we always set root->reloc_root to NULL before we drop the reloc
> tree, but that's not multi-core safe since we have no proper memory
> barrier to ensure other cores can see the same root->reloc_root.
>
> The proper root fix should be some proper root refcount, and make
> btrfs_drop_snapshot() to wait for all other root owner to release the
> root before dropping it.
This would block cleaning deleted subvolumes, no? We can skip the dead
tree (and add it back to the list) in that can and not wait. The
cleaner thread is able to process the list repeatedly.
> But for now, let's just check the DEAD_RELOC_ROOT bit before accessing
> root->reloc_root.
Ok, the bit is safe way to sync that as long as the correct order of
setting/clearing is done. The ops are atomic wrt to the value itself
but need barriers around as they're simple atomic ops (not RMW,
according to Documentation/atomic_bitops.txt) and there's no outer
synchronization.
Check:
smp_mb__before_atomic();
if (test_bit() ...)
return;
Set:
set_bit()
smp_mb__after_atomic();
(delete reloc_root)
reloc_root = NULL
Clearing of the bit is done when there are not potential other users so
that part does not need the barrier (I think).
The checking part could use a helper so we don't have barriers scattered
around code.
