diff mbox series

btrfs: fix nonzero lowest level handling in btrfs_search_forward()

Message ID 20250313053509.31946-1-sunk67188@gmail.com (mailing list archive)
State New
Headers show
Series btrfs: fix nonzero lowest level handling in btrfs_search_forward() | expand

Commit Message

Sun YangKai March 13, 2025, 5:34 a.m. UTC
Commit 323ac95bce44 ("Btrfs: don't read leaf blocks containing only
checksums during truncate") changed the condition from `level == 0` to
`level == path->lowest_level`, while its origional purpose is just to do
some leaf nodes handling (calling btrfs_item_key_to_cpu()) and skip some
code that doesn't fit leaf nodes.

After changing the condition, the code path
1. also handle the non-leaf nodes when path->lowest_level is nonzero,
   which is wrong. However, it seems that btrfs_search_forward() is never
   called with a nonzero path->lowest_level, which makes this bug not
   found before.
2. makes the later if block with the same condition, which is origionally
   used to handle non-leaf node (calling btrfs_node_key_to_cpu()) when
   lowest_level is not zero, dead code.

So Use if conditions to skip the non-leaf handling code instead of using
goto to make it more clear, and handle both leaf and non-leaf node in the
lowest_level loop exit logic.

This changes the behavior when btrfs_search_forward() is called with
nonzero path->lowest_level. But this never happens in the current code
base, and the previous behavior is wrong. So the change of behavior will
not be a problem.

Fix: commit 323ac95bce44 ("Btrfs: don't read leaf blocks containing only checksums during truncate")
Signed-off-by: Sun YangKai <sunk67188@gmail.com>
---
 fs/btrfs/ctree.c | 44 +++++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)
diff mbox series

Patch

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index c982960d8a91..77e33120c3f7 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -4637,38 +4637,28 @@  int btrfs_search_forward(struct btrfs_root *root, struct btrfs_key *min_key,
 			goto out;
 		}
 
-		/* at the lowest level, we're done, setup the path and exit */
-		if (level == path->lowest_level) {
-			if (slot >= nritems)
-				goto find_next_key;
-			ret = 0;
-			path->slots[level] = slot;
-			/* Save our key for returning back. */
-			btrfs_item_key_to_cpu(cur, min_key, slot);
-			goto out;
-		}
-		if (sret && slot > 0)
+		/*
+		 * Not at the lowest level and not a perfect match,
+		 * go back a slot if possible to search lower level.
+		 */
+		if (sret && slot > 0 && level > path->lowest_level)
 			slot--;
 		/*
-		 * check this node pointer against the min_trans parameters.
+		 * Check this node pointer against the min_trans parameters.
 		 * If it is too old, skip to the next one.
 		 */
-		while (slot < nritems) {
-			u64 gen;
-
-			gen = btrfs_node_ptr_generation(cur, slot);
-			if (gen < min_trans) {
+		if (level > 0) {
+			while (slot < nritems) {
+				if (btrfs_node_ptr_generation(cur, slot) >= min_trans)
+					break;
 				slot++;
-				continue;
 			}
-			break;
 		}
-find_next_key:
+		path->slots[level] = slot;
 		/*
-		 * we didn't find a candidate key in this node, walk forward
-		 * and find another one
+		 * We didn't find a candidate key in this node, walk forward
+		 * and find another one.
 		 */
-		path->slots[level] = slot;
 		if (slot >= nritems) {
 			sret = btrfs_find_next_key(root, path, min_key, level,
 						  min_trans);
@@ -4679,12 +4669,16 @@  int btrfs_search_forward(struct btrfs_root *root, struct btrfs_key *min_key,
 				goto out;
 			}
 		}
+		/* At the lowest level, we're done. Set the key and exit. */
 		if (level == path->lowest_level) {
 			ret = 0;
-			/* Save our key for returning back. */
-			btrfs_node_key_to_cpu(cur, min_key, slot);
+			if (level == 0)
+				btrfs_item_key_to_cpu(cur, min_key, slot);
+			else
+				btrfs_node_key_to_cpu(cur, min_key, slot);
 			goto out;
 		}
+		/* Search down to a lower level. */
 		cur = btrfs_read_node_slot(cur, slot);
 		if (IS_ERR(cur)) {
 			ret = PTR_ERR(cur);