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

Josef Bacik
2020-08-21T13:59:29Z

On 8/21/20 3:31 AM, Nikolay Borisov wrote:
>
>
> 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 ?

It's a global root, so it's final put happens in btrfs_free_fs_info(),
we just need to drop the radix tree ref here. Thanks,

Josef