Thread (2 messages) 2 messages, 2 authors, 2021-05-26

Re: [PATCH] btrfs: relocation: fix misplaced barrier in reloc_root_is_dead

From: Qu Wenruo <hidden>
Date: 2021-05-26 06:13:42
Also in: lkml


On 2021/5/26 下午2:09, Baptiste Lepers wrote:
Fix a misplaced barrier in reloc_root_is_dead. The
BTRFS_ROOT_DEAD_RELOC_TREE bit should be checked before accessing
reloc_root.

The fix forces the order documented in the original commit:
"In the cross section C-D, either caller gets the DEAD bit set, avoiding
access reloc_root no matter if it's safe or not.  Or caller get the DEAD
bit cleared, then access reloc_root, which is already NULL, nothing will
be wrong."

static bool reloc_root_is_dead()
     smp_rmb(); <--- misplaced
     if (test_bit(BTRFS_ROOT_DEAD_RELOC_TREE, &root->state))
           return true;
     return false;
}
Exactly what I originally purposed to David, we didn't reach an
agreement on where the barrier should be.

At least I'm completely fine with this patch.

Reviewed-by: Qu Wenruo <redacted>

Thanks,
Qu
quoted hunk ↗ jump to hunk
static bool have_reloc_root(struct btrfs_root *root)
{
        if (reloc_root_is_dead(root))
                return false;
        <--- the barrier should happen here
        if (!root->reloc_root)
                return false;
        return true;
}

Fixes: 6282675e6708e ("btrfs: relocation: fix reloc_root lifespan and access")
Signed-off-by: Baptiste Lepers <redacted>
---
  fs/btrfs/relocation.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index b70be2ac2e9e..8cddcce56bf4 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -289,15 +289,14 @@ static int update_backref_cache(struct btrfs_trans_handle *trans,

  static bool reloc_root_is_dead(struct btrfs_root *root)
  {
+	bool is_dead = test_bit(BTRFS_ROOT_DEAD_RELOC_TREE, &root->state);
  	/*
  	 * Pair with set_bit/clear_bit in clean_dirty_subvols and
  	 * btrfs_update_reloc_root. We need to see the updated bit before
  	 * trying to access reloc_root
  	 */
  	smp_rmb();
-	if (test_bit(BTRFS_ROOT_DEAD_RELOC_TREE, &root->state))
-		return true;
-	return false;
+	return is_dead;
  }

  /*
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help