Re: [PATCH 2/7] btrfs: unset reloc control if we fail to recover

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

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



On 2020/3/3 上午2:47, Josef Bacik wrote:
> If we fail to load an fs root, or fail to start a transaction we can
> bail without unsetting the reloc control, which leads to problems later
> when we free the reloc control but still have it attached to the file
> system.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/relocation.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 507361e99316..173fc7628235 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4678,6 +4678,7 @@ int btrfs_recover_relocation(struct btrfs_root *root)
>  		fs_root = read_fs_root(fs_info, reloc_root->root_key.offset);
>  		if (IS_ERR(fs_root)) {
>  			err = PTR_ERR(fs_root);
> +			unset_reloc_control(rc);
>  			list_add_tail(&reloc_root->root_list, &reloc_roots);
>  			goto out_free;
>  		}


Shouldn't the unset_reloc_control() also get called for all related
errors after set_reloc_control()?

That includes btrfs_join_transaction() (the one after
set_reloc_contrl(), which looks missing), the read_fs_root() and the
commit transaction error you're addressing.

Thanks,
Qu

> @@ -4689,8 +4690,10 @@ int btrfs_recover_relocation(struct btrfs_root *root)
>  	}
>  
>  	err = btrfs_commit_transaction(trans);
> -	if (err)
> +	if (err) {
> +		unset_reloc_control(rc);
>  		goto out_free;
> +	}
>  
>  	merge_reloc_roots(rc);
>  
> 


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