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

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

[Qu Wenruo]

[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;
-- 
2.37.1

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

[Wang Yugui]

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. */

Currently we  limit  the stripe size to SIZE_1G.

Is there some technical limit such as 'used as signed 32bit, so max
SIZE_1G or SIZE_2G?' or 'used as unsigned 32bit, so max SIZE_2G or
SIZE_4G?'

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/08/18


> +	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;
> -- 
> 2.37.1

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

[Qu Wenruo]

On 2022/8/18 16:04, Wang Yugui wrote:
> 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. */
>
> Currently we  limit  the stripe size to SIZE_1G.
>
> Is there some technical limit such as 'used as signed 32bit, so max
> SIZE_1G or SIZE_2G?' or 'used as unsigned 32bit, so max SIZE_2G or
> SIZE_4G?'

AFAIK the only problem with larger stripe size is for unbalanced
data/metadata case it will be harder to reclaim block groups.

Other than that I'm not aware of certain problems related the stripe size.

Thanks,
Qu
>
> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2022/08/18
>
>
>> +	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;
>> --
>> 2.37.1
>
>

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

[David Sterba]

On Thu, Aug 18, 2022 at 03:06:44PM +0800, Qu Wenruo wrote:
> [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>

Added to misc-next, thanks. And thanks to Wang Yugui for the report.

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

[Wang Yugui]

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

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

[Qu Wenruo]

On 2022/9/7 18:37, Wang Yugui wrote:
> 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);

Doing that won't cause much difference AFAIK.

As in decide_stripe_size_regular() we never bothered
ctl->max_stripe_size from the beginning anyway...

Thanks,
Qu

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

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

[Filipe Manana]

On Thu, Aug 18, 2022 at 8:22 AM Qu Wenruo <wqu@suse.com> wrote:
>
> [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>

Btw, btrfs/253 now fails.
Probably needs to be updated after this patch.

Thanks.

> ---
>  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;
> --
> 2.37.1
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”