[v2,2/3] btrfs: disk-io: Remove unnecessary check before freeing chunk root
diff mbox series

Message ID 20191008044909.157750-3-wqu@suse.com
State New
Headers show
Series
  • btrfs: Introduce new incompat feature BG_TREE to hugely reduce mount time
Related show

Commit Message

Qu Wenruo Oct. 8, 2019, 4:49 a.m. UTC
In free_root_pointers(), free_root_extent_buffers() has checked if the
@root parameter is NULL.
So there is no need checking chunk_root before freeing it.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/disk-io.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Johannes Thumshirn Oct. 8, 2019, 8:30 a.m. UTC | #1
Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Anand Jain Oct. 10, 2019, 2 a.m. UTC | #2
On 8/10/19 12:49 PM, Qu Wenruo wrote:
> In free_root_pointers(), free_root_extent_buffers() has checked if the
> @root parameter is NULL.
> So there is no need checking chunk_root before freeing it.

um..

> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   fs/btrfs/disk-io.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 044981cf6df9..bfeeac83b952 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2038,8 +2038,7 @@ static void free_root_pointers(struct btrfs_fs_info *info, int chunk_root)
>   	free_root_extent_buffers(info->csum_root);
>   	free_root_extent_buffers(info->quota_root);
>   	free_root_extent_buffers(info->uuid_root);
> -	if (chunk_root)
> -		free_root_extent_buffers(info->chunk_root);
> +	free_root_extent_buffers(info->chunk_root);
>   	free_root_extent_buffers(info->free_space_root);
>   }
>   
> 

This will cause the regression and fails mount from the backup root.

We have %recovery_tree_root: which shall re-read all the trees root
except the chunk root.

I don't think your idea was to re-read the chunk root as well?.

-----------------
recovery_tree_root:
         if (!btrfs_test_opt(fs_info, USEBACKUPROOT))
                 goto fail_tree_roots;

         free_root_pointers(fs_info, 0);
-----------------

Thanks, Anand
Qu Wenruo Oct. 10, 2019, 2:21 a.m. UTC | #3
On 2019/10/10 上午10:00, Anand Jain wrote:
> On 8/10/19 12:49 PM, Qu Wenruo wrote:
>> In free_root_pointers(), free_root_extent_buffers() has checked if the
>> @root parameter is NULL.
>> So there is no need checking chunk_root before freeing it.
> 
> um..

Oh, my bad, I get confused with the parameter @chunk_root against
@fs_info->chunk_root.

So please discard the patch.

Thanks,
Qu

> 
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   fs/btrfs/disk-io.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 044981cf6df9..bfeeac83b952 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -2038,8 +2038,7 @@ static void free_root_pointers(struct
>> btrfs_fs_info *info, int chunk_root)
>>       free_root_extent_buffers(info->csum_root);
>>       free_root_extent_buffers(info->quota_root);
>>       free_root_extent_buffers(info->uuid_root);
>> -    if (chunk_root)
>> -        free_root_extent_buffers(info->chunk_root);
>> +    free_root_extent_buffers(info->chunk_root);
>>       free_root_extent_buffers(info->free_space_root);
>>   }
>>  
> 
> This will cause the regression and fails mount from the backup root.
> 
> We have %recovery_tree_root: which shall re-read all the trees root
> except the chunk root.
> 
> I don't think your idea was to re-read the chunk root as well?.
> 
> -----------------
> recovery_tree_root:
>         if (!btrfs_test_opt(fs_info, USEBACKUPROOT))
>                 goto fail_tree_roots;
> 
>         free_root_pointers(fs_info, 0);
> -----------------
> 
> Thanks, Anand

Patch
diff mbox series

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 044981cf6df9..bfeeac83b952 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2038,8 +2038,7 @@  static void free_root_pointers(struct btrfs_fs_info *info, int chunk_root)
 	free_root_extent_buffers(info->csum_root);
 	free_root_extent_buffers(info->quota_root);
 	free_root_extent_buffers(info->uuid_root);
-	if (chunk_root)
-		free_root_extent_buffers(info->chunk_root);
+	free_root_extent_buffers(info->chunk_root);
 	free_root_extent_buffers(info->free_space_root);
 }