Re: [PATCH v3 48/54] btrfs: handle extent corruption with select_one_root properly

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

[-- Attachment #1.1: Type: text/plain, Size: 1822 bytes --]



On 2020/12/3 上午3:51, Josef Bacik wrote:
> In corruption cases we could have paths from a block up to no root at
> all, and thus we'll BUG_ON(!root) in select_one_root.  Handle this by
> adding an ASSERT() for developers, and returning an error for normal
> users.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/relocation.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index d4656a8f507d..91479979d2a7 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -2200,7 +2200,16 @@ struct btrfs_root *select_one_root(struct btrfs_backref_node *node)
>  		cond_resched();
>  		next = walk_up_backref(next, edges, &index);
>  		root = next->root;
> -		BUG_ON(!root);
> +
> +		/*
> +		 * This can occur if we have incomplete extent refs leading all
> +		 * the way up a particular path, in this case return -EUCLEAN.
> +		 * However leave as an ASSERT() for developers, because it could
> +		 * indicate a bug in the backref code.
> +		 */
> +		ASSERT(root);
> +		if (!root)
> +			return ERR_PTR(-EUCLEAN);

Just the same comment on using ASSERT(0) in the error branch.

Despite that looks OK to me.

Thanks,
Qu
>  
>  		/* No other choice for non-shareable tree */
>  		if (!test_bit(BTRFS_ROOT_SHAREABLE, &root->state))
> @@ -2598,8 +2607,12 @@ static int relocate_tree_block(struct btrfs_trans_handle *trans,
>  
>  	BUG_ON(node->processed);
>  	root = select_one_root(node);
> -	if (root == ERR_PTR(-ENOENT)) {
> -		update_processed_blocks(rc, node);
> +	if (IS_ERR(root)) {
> +		ret = PTR_ERR(root);
> +		if (ret == -ENOENT) {
> +			ret = 0;
> +			update_processed_blocks(rc, node);
> +		}
>  		goto out;
>  	}
>  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]