Re: [PATCH v2 4/4] btrfs: ctree: cleanup btrfs_prev_leaf()

[Qu Wenruo <wqu@suse.com>]

在 2026/2/8 13:12, Sun YangKai 写道:

> 
> Sorry, I got a little confused here. When btrfs_search_slot() returns an 
> empty leaf, path->slots[0] will be 0 and btrfs_prev_leaf() will return 
> 1. Therefore, if btrfs_prev_leaf() returns 0, the leaf we get should 
> always be non-empty. Which part of this reasoning is incorrect? Or maybe 
> I don't get what you want to say.

Because the old btrfs_prev_leaf() can return 0 if the tree got emptied 
before btrfs_search_slot() call, if you're doing a cleanup, just don't 
change that behavior.

If you think this behavior is not correct, do it in another patch and 
explain it correctly.

>>
>> Although the change is mostly fine, if you want to keep the existing 
>> behavior, you should return 0 for empty tree to keep the old behavior.
> 
> I've not thought about the empty tree case before. But I think the 
> previous behavior in empty tree case is not proper and I don't want to 
> keep the old behavior in this case.

Then it's no longer a cleanup, and you're mixing a cleanup with a 
behavior change.

> All callers is doing
> path->slots[0]-- if btrfs_prev_leaf() returns 0 and it will cause an 
> unexpected underflow although they'll check if nritems is 0 later.

They all have proper nritems check or the tree will never be empty.

For extent tree it will never be empty, at least without extent-tree-v2 
feature.
So btrfs_previous_extent_item() should never hit an empty tree.

For btrfs_search_slot_for_read() the qgroup callers are never blindly 
decreasing slots[0], for send callers the subvolume tree will never be 
empty.

Thus all your underflow argument makes no sense.

If you don't like this behavior, again change it in a dedicated patch, 
not mixing it into the so called "cleanup" patch.