Re: [PATCH 02/15] btrfs: push ->owner_root check into btrfs_read_extent_buffer

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2024/4/20 03:46, Josef Bacik 写道:
> Currently we're only doing this in read_tree_block(), however
> btrfs_check_eb_owner() properly deals with ->owner_root being set to 0,
> and in fact we're duplicating this check in read_block_for_search().
> Push this check up into btrfs_read_extent_buffer() and fixup
> read_block_for_search() to just return the result from
> btrfs_read_extent_buffer() and drop the duplicate check.

Since end_bbio_meta_read() is already calling
btrfs_validate_extent_buffer() with bbio->parent_check copied from the
callers, can we just remove the btrfs_check_eb_owner() calls directly
from all the higher layer callers?

Even the check in btrfs_read_extent_buffer() seems unnecessary now.

Thanks,
Qu
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>   fs/btrfs/ctree.c   | 7 +------
>   fs/btrfs/disk-io.c | 6 ++----
>   2 files changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 1a49b9232990..48aa14046343 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -1551,12 +1551,7 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p,
>   		if (ret) {
>   			free_extent_buffer(tmp);
>   			btrfs_release_path(p);
> -			return -EIO;
> -		}
> -		if (btrfs_check_eb_owner(tmp, btrfs_root_id(root))) {
> -			free_extent_buffer(tmp);
> -			btrfs_release_path(p);
> -			return -EUCLEAN;
> +			return ret;
>   		}
>
>   		if (unlock_up)
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index c2dc88f909b0..64523dc1060d 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -251,6 +251,8 @@ int btrfs_read_extent_buffer(struct extent_buffer *eb,
>   	if (failed && !ret && failed_mirror)
>   		btrfs_repair_eb_io_failure(eb, failed_mirror);
>
> +	if (!ret)
> +		ret = btrfs_check_eb_owner(eb, check->owner_root);
>   	return ret;
>   }
>
> @@ -635,10 +637,6 @@ struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr,
>   		free_extent_buffer_stale(buf);
>   		return ERR_PTR(ret);
>   	}
> -	if (btrfs_check_eb_owner(buf, check->owner_root)) {
> -		free_extent_buffer_stale(buf);
> -		return ERR_PTR(-EUCLEAN);
> -	}
>   	return buf;
>
>   }