On 2020/8/21 上午10:42, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@xxxxxxxx>
>
> [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@xxxxxxxxxxxxxxx # 5.8+
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@xxxxxxxx>
Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>
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;
>
Attachment:
signature.asc
Description: OpenPGP digital signature
