| Message ID | 20191008044909.157750-3-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Introduce new incompat feature BG_TREE to hugely reduce mount time | expand |
Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
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
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
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); }
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(-)