[PATCH] btrfs: fix root-in-trans fast-path ordering

[PATCH] btrfs: fix root-in-trans fast-path ordering

[Cen Zhang]

btrfs_record_root_in_trans() has a lockless fast path for shareable
roots. It skips reloc_mutex when root->last_trans matches the current
transaction and BTRFS_ROOT_IN_TRANS_SETUP is clear.

The writer side publishes that state in two phases: it sets
IN_TRANS_SETUP before updating root->last_trans, then clears the bit
after btrfs_init_reloc_root() finishes. However, the reader-side
smp_rmb() is before both loads, so it does not order the last_trans load
against the later bit test. A reader can observe the new last_trans value
while missing the setup bit and return before the relocation-root setup
is complete.

Read root->last_trans first, then issue the read barrier before testing
IN_TRANS_SETUP. Also use clear_bit_unlock() for the writer's final clear
and test_bit_acquire() for the successful fast path, so the lockless
return observes the setup done before the bit was cleared.

Fixes: 7585717f304f ("Btrfs: fix relocation races")
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 fs/btrfs/transaction.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 8dd77c4..ac9ffa8 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -454,12 +454,12 @@ static int record_root_in_trans(struct btrfs_trans_handle *trans,
 		 *
 		 * When this is zero, they can trust root->last_trans and fly
 		 * through btrfs_record_root_in_trans without having to take the
-		 * lock.  smp_wmb() makes sure that all the writes above are
-		 * done before we pop in the zero below
+		 * lock. smp_wmb() makes sure readers that see the last_trans
+		 * update also see IN_TRANS_SETUP set, and clear_bit_unlock()
+		 * publishes the relocation setup before we clear the bit.
 		 */
 		ret = btrfs_init_reloc_root(trans, root);
-		smp_mb__before_atomic();
-		clear_bit(BTRFS_ROOT_IN_TRANS_SETUP, &root->state);
+		clear_bit_unlock(BTRFS_ROOT_IN_TRANS_SETUP, &root->state);
 	}
 	return ret;
 }
@@ -497,10 +497,12 @@ int btrfs_record_root_in_trans(struct btrfs_trans_handle *trans,
 	 * see record_root_in_trans for comments about IN_TRANS_SETUP usage
 	 * and barriers
 	 */
-	smp_rmb();
-	if (btrfs_get_root_last_trans(root) == trans->transid &&
-	    !test_bit(BTRFS_ROOT_IN_TRANS_SETUP, &root->state))
-		return 0;
+	if (btrfs_get_root_last_trans(root) == trans->transid) {
+		/* Order the last_trans load before testing IN_TRANS_SETUP. */
+		smp_rmb();
+		if (!test_bit_acquire(BTRFS_ROOT_IN_TRANS_SETUP, &root->state))
+			return 0;
+	}
 
 	mutex_lock(&fs_info->reloc_mutex);
 	ret = record_root_in_trans(trans, root, 0);
-- 
2.43.0