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

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

[Qu Wenruo]

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>
---
 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,
 	};
 	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;
@@ -4671,9 +4677,13 @@ void btrfs_readahead_tree_block(struct btrfs_fs_info *fs_info,
  */
 void btrfs_readahead_node_child(struct extent_buffer *node, int slot)
 {
+	struct btrfs_key node_key;
+
+	btrfs_node_key_to_cpu(node, &node_key, slot);
 	btrfs_readahead_tree_block(node->fs_info,
 				   btrfs_node_blockptr(node, slot),
 				   btrfs_header_owner(node),
 				   btrfs_node_ptr_generation(node, slot),
-				   btrfs_header_level(node) - 1);
+				   btrfs_header_level(node) - 1,
+				   &node_key);
 }
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index b284aee1bfb0..7b4152387d88 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -287,7 +287,8 @@ static inline void wait_on_extent_buffer_writeback(struct extent_buffer *eb)
 }
 
 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);
 void btrfs_readahead_node_child(struct extent_buffer *node, int slot);
 
 /* Note: this can be used in for loops without caching the value in a variable. */
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 1c42c5180bdd..c42ef4bed8d3 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2607,7 +2607,7 @@ int relocate_tree_blocks(struct btrfs_trans_handle *trans,
 		if (!block->key_ready)
 			btrfs_readahead_tree_block(fs_info, block->bytenr,
 						   block->owner, 0,
-						   block->level);
+						   block->level, NULL);
 	}
 
 	/* Get first keys */
-- 
2.53.0

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

[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?

>  	};
>  	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;

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

[Qu Wenruo]

在 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;