Skip to content

Commit f1b6b64

Browse files
Sun YangKaikdave
authored andcommitted
btrfs: fix nonzero lowest level handling in btrfs_search_forward()
Commit 323ac95 ("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 323ac95 ("Btrfs: don't read leaf blocks containing only checksums during truncate") Signed-off-by: Sun YangKai <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 4959038 commit f1b6b64

File tree

1 file changed

+19
-25
lines changed

1 file changed

+19
-25
lines changed

fs/btrfs/ctree.c

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4636,38 +4636,28 @@ int btrfs_search_forward(struct btrfs_root *root, struct btrfs_key *min_key,
46364636
goto out;
46374637
}
46384638

4639-
/* at the lowest level, we're done, setup the path and exit */
4640-
if (level == path->lowest_level) {
4641-
if (slot >= nritems)
4642-
goto find_next_key;
4643-
ret = 0;
4644-
path->slots[level] = slot;
4645-
/* Save our key for returning back. */
4646-
btrfs_item_key_to_cpu(cur, min_key, slot);
4647-
goto out;
4648-
}
4649-
if (sret && slot > 0)
4639+
/*
4640+
* Not at the lowest level and not a perfect match,
4641+
* go back a slot if possible to search lower level.
4642+
*/
4643+
if (sret && slot > 0 && level > path->lowest_level)
46504644
slot--;
46514645
/*
4652-
* check this node pointer against the min_trans parameters.
4646+
* Check this node pointer against the min_trans parameters.
46534647
* If it is too old, skip to the next one.
46544648
*/
4655-
while (slot < nritems) {
4656-
u64 gen;
4657-
4658-
gen = btrfs_node_ptr_generation(cur, slot);
4659-
if (gen < min_trans) {
4649+
if (level > 0) {
4650+
while (slot < nritems) {
4651+
if (btrfs_node_ptr_generation(cur, slot) >= min_trans)
4652+
break;
46604653
slot++;
4661-
continue;
46624654
}
4663-
break;
46644655
}
4665-
find_next_key:
4656+
path->slots[level] = slot;
46664657
/*
4667-
* we didn't find a candidate key in this node, walk forward
4668-
* and find another one
4658+
* We didn't find a candidate key in this node, walk forward
4659+
* and find another one.
46694660
*/
4670-
path->slots[level] = slot;
46714661
if (slot >= nritems) {
46724662
sret = btrfs_find_next_key(root, path, min_key, level,
46734663
min_trans);
@@ -4678,12 +4668,16 @@ int btrfs_search_forward(struct btrfs_root *root, struct btrfs_key *min_key,
46784668
goto out;
46794669
}
46804670
}
4671+
/* At the lowest level, we're done. Set the key and exit. */
46814672
if (level == path->lowest_level) {
46824673
ret = 0;
4683-
/* Save our key for returning back. */
4684-
btrfs_node_key_to_cpu(cur, min_key, slot);
4674+
if (level == 0)
4675+
btrfs_item_key_to_cpu(cur, min_key, slot);
4676+
else
4677+
btrfs_node_key_to_cpu(cur, min_key, slot);
46854678
goto out;
46864679
}
4680+
/* Search down to a lower level. */
46874681
cur = btrfs_read_node_slot(cur, slot);
46884682
if (IS_ERR(cur)) {
46894683
ret = PTR_ERR(cur);

0 commit comments

Comments
 (0)