Re: [PATCH] btrfs: fix the max chunk size and stripe length calculation

[Wang Yugui <wangyugui@e16-tech.com>]

Hi,

> [BEHAVIOR CHANGE]
> Since commit f6fca3917b4d ("btrfs: store chunk size in space-info
> struct"), btrfs no longer can create larger data chunks than 1G:
> 
>   mkfs.btrfs -f -m raid1 -d raid0 $dev1 $dev2 $dev3 $dev4
>   mount $dev1 $mnt
> 
>   btrfs balance start --full $mnt
>   btrfs balance start --full $mnt
>   umount $mnt
> 
>   btrfs ins dump-tree -t chunk $dev1 | grep "DATA|RAID0" -C 2
> 
> Before that offending commit, what we got is a 4G data chunk:
> 
> 	item 6 key (FIRST_CHUNK_TREE CHUNK_ITEM 9492758528) itemoff 15491 itemsize 176
> 		length 4294967296 owner 2 stripe_len 65536 type DATA|RAID0
> 		io_align 65536 io_width 65536 sector_size 4096
> 		num_stripes 4 sub_stripes 1
> 
> Now what we got is only 1G data chunk:
> 
> 	item 6 key (FIRST_CHUNK_TREE CHUNK_ITEM 6271533056) itemoff 15491 itemsize 176
> 		length 1073741824 owner 2 stripe_len 65536 type DATA|RAID0
> 		io_align 65536 io_width 65536 sector_size 4096
> 		num_stripes 4 sub_stripes 1
> 
> This will increase the number of data chunks by the number of devices,
> not only increase system chunk usage, but also greatly increase mount
> time.
> 
> Without a properly reason, we should not change the max chunk size.
> 
> [CAUSE]
> Previously, we set max data chunk size to 10G, while max data stripe
> length to 1G.
> 
> Commit f6fca3917b4d ("btrfs: store chunk size in space-info struct")
> completely ignored the 10G limit, but use 1G max stripe limit instead,
> causing above shrink in max data chunk size.
> 
> [FIX]
> Fix the max data chunk size to 10G, and in decide_stripe_size_regular()
> we limit stripe_size to 1G manually.
> 
> This should only affect data chunks, as for metadata chunks we always
> set the max stripe size the same as max chunk size (256M or 1G
> depending on fs size).
> 
> Now the same script result the same old result:
> 
> 	item 6 key (FIRST_CHUNK_TREE CHUNK_ITEM 9492758528) itemoff 15491 itemsize 176
> 		length 4294967296 owner 2 stripe_len 65536 type DATA|RAID0
> 		io_align 65536 io_width 65536 sector_size 4096
> 		num_stripes 4 sub_stripes 1
> 
> Reported-by: Wang Yugui <wangyugui@e16-tech.com>
> Fixes: f6fca3917b4d ("btrfs: store chunk size in space-info struct")
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/space-info.c | 2 +-
>  fs/btrfs/volumes.c    | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 477e57ace48d..b74bc31e9a8e 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -199,7 +199,7 @@ static u64 calc_chunk_size(const struct btrfs_fs_info *fs_info, u64 flags)
>  	ASSERT(flags & BTRFS_BLOCK_GROUP_TYPE_MASK);
>  
>  	if (flags & BTRFS_BLOCK_GROUP_DATA)
> -		return SZ_1G;
> +		return BTRFS_MAX_DATA_CHUNK_SIZE;
>  	else if (flags & BTRFS_BLOCK_GROUP_SYSTEM)
>  		return SZ_32M;
>  
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 8c64dda69404..e0fd1aecf447 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -5264,6 +5264,9 @@ static int decide_stripe_size_regular(struct alloc_chunk_ctl *ctl,
>  				       ctl->stripe_size);
>  	}
>  
> +	/* Stripe size should never go beyond 1G. */
> +	ctl->stripe_size = min_t(u64, ctl->stripe_size, SZ_1G);
> +
>  	/* Align to BTRFS_STRIPE_LEN */
>  	ctl->stripe_size = round_down(ctl->stripe_size, BTRFS_STRIPE_LEN);
>  	ctl->chunk_size = ctl->stripe_size * data_stripes;

Is it a better place to do this SZ_1G limit?
   init_alloc_chunk_ctl_policy_regular()
	ctl->max_stripe_size = min_t(u64, SZ_1G, ctl->max_stripe_size);

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/09/07