Re: [PATCH v3 49/54] btrfs: do proper error handling in merge_reloc_roots

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

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



On 2020/12/3 上午3:51, Josef Bacik wrote:
> We have a BUG_ON() if we get an error back from btrfs_get_fs_root().
> This honestly should never fail, as at this point we have a solid
> coordination of fs root to reloc root, and these roots will all be in
> memory.  But in the name of killing BUG_ON()'s remove this one and
> handle the error properly.  Change the remaining BUG_ON() to an
> ASSERT().
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/relocation.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 91479979d2a7..099a64b47020 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -1949,9 +1949,18 @@ void merge_reloc_roots(struct reloc_control *rc)
>  
>  		root = btrfs_get_fs_root(fs_info, reloc_root->root_key.offset,
>  					 false);
> +		if (IS_ERR(root)) {
> +			/*
> +			 * This likely won't happen, since we would have failed
> +			 * at a higher level.  However for correctness sake
> +			 * handle the error anyway.
> +			 */

Maybe another ASSERT(0)?

Despite that looks good.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> +			ret = PTR_ERR(root);
> +			goto out;
> +		}
> +
>  		if (btrfs_root_refs(&reloc_root->root_item) > 0) {
> -			BUG_ON(IS_ERR(root));
> -			BUG_ON(root->reloc_root != reloc_root);
> +			ASSERT(root->reloc_root == reloc_root);
>  			ret = merge_reloc_root(rc, root);
>  			btrfs_put_root(root);
>  			if (ret) {
> 


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