Re: [PATCH 1/2] btrfs: free fs roots on failed mount

[Nikolay Borisov <nborisov@suse.com>]

On 20.08.20 г. 23:00 ч., Josef Bacik wrote:
> While testing a weird problem with -o degraded, I noticed I was getting
> leaked root errors
> 
> BTRFS warning (device loop0): writable mount is not allowed due to too many missing devices
> BTRFS error (device loop0): open_ctree failed
> BTRFS error (device loop0): leaked root -9-0 refcount 1
> 
> This is the DATA_RELOC root, which gets read before the other fs roots,
> but is included in the fs roots radix tree.  Handle this by adding a
> btrfs_drop_and_free_fs_root() on the data reloc root if it exists.  This
> is ok to do here if we fail further up because we will only drop the ref
> if we delete the root from the radix tree, and all other cleanup won't
> be duplicated.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/disk-io.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 814f8de395fe..ac6d6fddd5f4 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3418,6 +3418,8 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>  	btrfs_put_block_group_cache(fs_info);
>  
>  fail_tree_roots:
> +	if (fs_info->data_reloc_root)
> +		btrfs_drop_and_free_fs_root(fs_info, fs_info->data_reloc_root);

But will this really free the root? So the newly allocated
data_reloc_root has it's ref set to 1 from
btrfs_get_root_ref->btrfs_read_tree_root->btrfs_alloc_root and to 2 from
being added to the radix tree in btrfs_insert_fs_root().

But btrfs_drop_and_free_fs_root makes a single call to btrfs_put_root.
So won't the reloc tree be left with a refcount of 1 ?

>  	free_root_pointers(fs_info, true);
>  	invalidate_inode_pages2(fs_info->btree_inode->i_mapping);
>  
>