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

Nikolay Borisov
2020-08-21T07:32:05Z

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);
>
>