From: Sun YangKai <sunk67188@gmail.com>
To: linux-btrfs@vger.kernel.org
Cc: Sun YangKai <sunk67188@gmail.com>, Chris Mason <clm@fb.com>,
Josef Bacik <josef@toxicpanda.com>,
David Sterba <dsterba@suse.com>,
linux-kernel@vger.kernel.org (open list)
Subject: [PATCH v2] btrfs: fix nonzero lowest level handling in btrfs_search_forward()
Date: Tue, 22 Apr 2025 20:56:42 +0800 [thread overview]
Message-ID: <20250422125807.30218-1-sunk67188@gmail.com> (raw)
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 --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index a2e7979372cc..3e69e705b5dc 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -4636,38 +4636,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);
@@ -4678,12 +4668,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);
--
2.49.0
next reply other threads:[~2025-04-22 12:58 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-22 12:56 Sun YangKai [this message]
2025-04-29 6:57 ` [PATCH v2] btrfs: fix nonzero lowest level handling in btrfs_search_forward() Sun YangKai
2025-04-29 15:27 ` David Sterba
[not found] ` <6048084.MhkbZ0Pkbq@saltykitkat>
2025-05-17 13:33 ` Sun YangKai
2025-05-17 13:47 ` [PATCH v3] btrfs: remove " Sun YangKai
2025-05-18 11:25 ` Qu Wenruo
[not found] ` <12674804.O9o76ZdvQC@saltykitkat>
[not found] ` <4d02fad5-07b2-47b6-9e18-30f45bc67163@suse.com>
[not found] ` <5890818.DvuYhMxLoT@saltykitkat>
2025-05-19 5:30 ` Qu Wenruo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250422125807.30218-1-sunk67188@gmail.com \
--to=sunk67188@gmail.com \
--cc=clm@fb.com \
--cc=dsterba@suse.com \
--cc=josef@toxicpanda.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox