public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Boris Burkov <boris@bur.io>
To: Qu Wenruo <wqu@suse.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: pass a valid btrfs_tree_parent_check when possible
Date: Thu, 9 Apr 2026 17:03:37 -0700	[thread overview]
Message-ID: <20260410000337.GA1018385@zen.localdomain> (raw)
In-Reply-To: <64f82fae3b8c690a3ad27d21fa80401f62b9e0ac.1775776582.git.wqu@suse.com>

On Fri, Apr 10, 2026 at 08:46:40AM +0930, Qu Wenruo wrote:
> Commit 6e181cfe2409 ("btrfs: revalidate cached tree blocks on the
> uptodate path") introduced the @check parameter for
> btrfs_buffer_uptodate() to allow re-validation of a cached extent
> buffer.
> 
> But there are still call sites that doesn't utilize this parameter,
> which exposes them to possible corrupted tree blocks, e.g. an empty
> child leaf of a parent node, which should be rejected by
> btrfs_verify_level_key() but if @check is NULL such check will be
> skipped and cause problems.
> 
> Thankfully for a lot of cases there is already an existing @check
> structure around and we can pass it directly to btrfs_buffer_uptodate().
> 
Reviewed-by: Boris Burkov <boris@bur.io>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/ctree.c       | 16 +++++-----------
>  fs/btrfs/extent-tree.c | 11 ++++++++---
>  fs/btrfs/extent_io.c   |  2 +-
>  3 files changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index d70da290bedf..829d8be7f423 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -1497,17 +1497,11 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p,
>  		if (p->reada == READA_FORWARD_ALWAYS)
>  			reada_for_search(fs_info, p, parent_level, slot, key->objectid);
>  
> -		/* first we do an atomic uptodate check */
> -		if (btrfs_buffer_uptodate(tmp, check.transid, NULL) > 0) {
> -			/*
> -			 * Do extra check for first_key, eb can be stale due to
> -			 * being cached, read from scrub, or have multiple
> -			 * parents (shared tree blocks).
> -			 */
> -			if (unlikely(btrfs_verify_level_key(tmp, &check))) {
> -				ret = -EUCLEAN;
> -				goto out;
> -			}
> +		/* Check if the cached eb is uptodate. */
> +		ret = btrfs_buffer_uptodate(tmp, check.transid, &check);
> +		if (unlikely(ret < 0))
> +			goto out;
> +		if (ret > 0) {
>  			*eb_ret = tmp;
>  			tmp = NULL;
>  			ret = 0;
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 391fad41c3b6..5d5b42ea4ade 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -5781,16 +5781,21 @@ static int check_next_block_uptodate(struct btrfs_trans_handle *trans,
>  
>  	generation = btrfs_node_ptr_generation(path->nodes[level], path->slots[level]);
>  
> -	if (btrfs_buffer_uptodate(next, generation, NULL))
> -		return 0;
> -
>  	check.level = level - 1;
>  	check.transid = generation;
>  	check.owner_root = btrfs_root_id(root);
>  	check.has_first_key = true;
>  	btrfs_node_key_to_cpu(path->nodes[level], &check.first_key, path->slots[level]);
>  
> +	ret = btrfs_buffer_uptodate(next, generation, &check);
> +	if (ret > 0)
> +		return 0;
>  	btrfs_tree_unlock(next);
> +	if (ret < 0) {
> +		free_extent_buffer(next);
> +		return ret;
> +	}
> +
>  	if (level == 1)
>  		reada_walk_down(trans, root, wc, path);
>  	ret = btrfs_read_extent_buffer(next, &check);
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index a8887285deda..3f59fb0a9ee5 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4648,7 +4648,7 @@ void btrfs_readahead_tree_block(struct btrfs_fs_info *fs_info,
>  	if (IS_ERR(eb))
>  		return;
>  
> -	if (btrfs_buffer_uptodate(eb, gen, NULL)) {
> +	if (btrfs_buffer_uptodate(eb, gen, &check)) {
>  		free_extent_buffer(eb);
>  		return;
>  	}
> -- 
> 2.53.0
> 

      reply	other threads:[~2026-04-10  0:03 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-09 23:16 [PATCH] btrfs: pass a valid btrfs_tree_parent_check when possible Qu Wenruo
2026-04-10  0:03 ` Boris Burkov [this message]

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=20260410000337.GA1018385@zen.localdomain \
    --to=boris@bur.io \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=wqu@suse.com \
    /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