| Message ID | 20200821024231.16256-1-marcos@mpdesouza.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: block-group: Fix free-space bitmap threshould | expand |
On 2020/8/21 上午10:42, Marcos Paulo de Souza wrote: > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > [BUG] > After commit 9afc66498a0b ("btrfs: block-group: refactor how we read one > block group item"), cache->length is being assigned after calling > btrfs_create_block_group_cache. This causes a problem since > set_free_space_tree_thresholds is calculate the free-space threshould to > decide is the free-space tree should convert from extents to bitmaps. > > The current code calls set_free_space_tree_thresholds with cache->length > being 0, which then makes cache->bitmap_high_thresh being zero. This > implies the system will always use bitmap instead of extents, which is > not desired if the block group is not fragmented. > > This behavior can be seen by a test that expects to repair systems > with FREE_SPACE_EXTENT and FREE_SPACE_BITMAP, but the current code only > created FREE_SPACE_BITMAP. > > [FIX] > Call set_free_space_tree_thresholds after setting cache->length. > > Link: https://github.com/kdave/btrfs-progs/issues/251 > Fixes: 9afc66498a0b ("btrfs: block-group: refactor how we read one block group item") > CC: stable@vger.kernel.org # 5.8+ > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> It would be even nicer if you could add some warning or self-test on cache->length to prevent such problem from happening again. Thanks, Qu > --- > fs/btrfs/block-group.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c > index 44fdfa2eeb2e..01e8ba1da1d3 100644 > --- a/fs/btrfs/block-group.c > +++ b/fs/btrfs/block-group.c > @@ -1798,7 +1798,6 @@ static struct btrfs_block_group *btrfs_create_block_group_cache( > > cache->fs_info = fs_info; > cache->full_stripe_len = btrfs_full_stripe_len(fs_info, start); > - set_free_space_tree_thresholds(cache); > > cache->discard_index = BTRFS_DISCARD_INDEX_UNUSED; > > @@ -1908,6 +1907,8 @@ static int read_one_block_group(struct btrfs_fs_info *info, > > read_block_group_item(cache, path, key); > > + set_free_space_tree_thresholds(cache); > + > if (need_clear) { > /* > * When we mount with old space cache, we need to > @@ -2128,6 +2129,7 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans, u64 bytes_used, > return -ENOMEM; > > cache->length = size; > + set_free_space_tree_thresholds(cache); > cache->used = bytes_used; > cache->flags = type; > cache->last_byte_to_unpin = (u64)-1; >
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index 44fdfa2eeb2e..01e8ba1da1d3 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -1798,7 +1798,6 @@ static struct btrfs_block_group *btrfs_create_block_group_cache( cache->fs_info = fs_info; cache->full_stripe_len = btrfs_full_stripe_len(fs_info, start); - set_free_space_tree_thresholds(cache); cache->discard_index = BTRFS_DISCARD_INDEX_UNUSED; @@ -1908,6 +1907,8 @@ static int read_one_block_group(struct btrfs_fs_info *info, read_block_group_item(cache, path, key); + set_free_space_tree_thresholds(cache); + if (need_clear) { /* * When we mount with old space cache, we need to @@ -2128,6 +2129,7 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans, u64 bytes_used, return -ENOMEM; cache->length = size; + set_free_space_tree_thresholds(cache); cache->used = bytes_used; cache->flags = type; cache->last_byte_to_unpin = (u64)-1;