Re: [PATCH] btrfs: apply first key check for readahead when possible

[Qu Wenruo <wqu@suse.com>]

在 2026/4/14 03:26, David Sterba 写道:
> On Sun, Apr 12, 2026 at 08:31:05PM +0930, Qu Wenruo wrote:
>> Currently for tree block readahead we never pass a
>> btrfs_tree_parent_check with @has_first_key set.
>>
>> Without @has_first_key set, btrfs will skip the following extra
>> checks:
>>
>> - Header generation check
>>    This is a minor one.
>>
>> - Empty leaf/node checks
>>    This is more serious, for certain trees like the csum tree, they are
>>    allowed to be empty, thus an empty leaf can pass the tree checker.
>>    But if there is a parent node for such an empty leaf, it indicates
>>    corruption.
>>
>>    Without @has_first_key set, we can no longer detect such a problem.
>>
>>    In fact there is already a fuzzed image report that a corrupted csum
>>    leaf which has zero nritems but still has a parent node can trigger
>>    a BUG_ON() during csum deletion.
>>
>> However there are only two call sites of btrfs_readahead_tree_block():
>>
>> - Inside relocate_tree_blocks()
>>    At this call site we are trying to grab the first key of the tree
>>    block, thus we are not able to pass a @first_key parameter.
>>
>> - Inside btrfs_readahead_node_child()
>>    This is the more common call site, where we have the parent node and
>>    want to readahead the child tree blocks.
>>
>>    In this case we can easily grab the node key and pass it for checks.
>>
>> Add a new parameter @first_key to btrfs_readahead_tree_block() and pass
>> the node key to it inside btrfs_readahead_node_child().
>>
>> This should plug the gap in empty leaf detection during readahead.
>>
>> Link: https://lore.kernel.org/linux-btrfs/20260409071255.3358044-1-gality369@gmail.com/
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Reviewed-by: David Sterba <dsterba@suse.com>
> 
>> ---
>>   fs/btrfs/extent_io.c  | 16 +++++++++++++---
>>   fs/btrfs/extent_io.h  |  3 ++-
>>   fs/btrfs/relocation.c |  2 +-
>>   3 files changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index 3f59fb0a9ee5..4d5605aaf827 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -4635,15 +4635,21 @@ int try_release_extent_buffer(struct folio *folio)
>>    * to read the block we will not block on anything.
>>    */
>>   void btrfs_readahead_tree_block(struct btrfs_fs_info *fs_info,
>> -				u64 bytenr, u64 owner_root, u64 gen, int level)
>> +				u64 bytenr, u64 owner_root, u64 gen, int level,
>> +				const struct btrfs_key *first_key)
>>   {
>>   	struct btrfs_tree_parent_check check = {
>>   		.level = level,
>> -		.transid = gen
>> +		.transid = gen,
> 
> It's not necessary to add the "," or was there another reason?

My bad, I was originally also setting @has_first_key there, but removed 
later without reverting this part back.

Will remove this change during merge.

Thanks,
Qu

> 
>>   	};
>>   	struct extent_buffer *eb;
>>   	int ret;
>>   
>> +	if (first_key) {
>> +		memcpy(&check.first_key, first_key, sizeof(struct btrfs_key));
>> +		check.has_first_key = true;
>> +	}
>> +
>>   	eb = btrfs_find_create_tree_block(fs_info, bytenr, owner_root, level);
>>   	if (IS_ERR(eb))
>>   		return;