* Re: [PATCH] btrfs: fix the max chunk size and stripe length calculation
2022-08-18 7:06 [PATCH] btrfs: fix the max chunk size and stripe length calculation Qu Wenruo
@ 2022-08-18 8:04 ` Wang Yugui
2022-08-18 8:13 ` Qu Wenruo
2022-09-06 16:01 ` David Sterba
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Wang Yugui @ 2022-08-18 8:04 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] btrfs: fix the max chunk size and stripe length calculation
2022-08-18 8:04 ` Wang Yugui
@ 2022-08-18 8:13 ` Qu Wenruo
0 siblings, 0 replies; 7+ messages in thread
From: Qu Wenruo @ 2022-08-18 8:13 UTC (permalink / raw)
To: Wang Yugui, Qu Wenruo; +Cc: linux-btrfs
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
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: fix the max chunk size and stripe length calculation
2022-08-18 7:06 [PATCH] btrfs: fix the max chunk size and stripe length calculation Qu Wenruo
2022-08-18 8:04 ` Wang Yugui
@ 2022-09-06 16:01 ` David Sterba
2022-09-07 10:37 ` Wang Yugui
2022-09-14 21:54 ` Filipe Manana
3 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2022-09-06 16:01 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Wang Yugui
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.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] btrfs: fix the max chunk size and stripe length calculation
2022-08-18 7:06 [PATCH] btrfs: fix the max chunk size and stripe length calculation Qu Wenruo
2022-08-18 8:04 ` Wang Yugui
2022-09-06 16:01 ` David Sterba
@ 2022-09-07 10:37 ` Wang Yugui
2022-09-07 10:44 ` Qu Wenruo
2022-09-14 21:54 ` Filipe Manana
3 siblings, 1 reply; 7+ messages in thread
From: Wang Yugui @ 2022-09-07 10:37 UTC (permalink / raw)
To: linux-btrfs; +Cc: Qu Wenruo
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] btrfs: fix the max chunk size and stripe length calculation
2022-09-07 10:37 ` Wang Yugui
@ 2022-09-07 10:44 ` Qu Wenruo
0 siblings, 0 replies; 7+ messages in thread
From: Qu Wenruo @ 2022-09-07 10:44 UTC (permalink / raw)
To: Wang Yugui, linux-btrfs; +Cc: 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: fix the max chunk size and stripe length calculation
2022-08-18 7:06 [PATCH] btrfs: fix the max chunk size and stripe length calculation Qu Wenruo
` (2 preceding siblings ...)
2022-09-07 10:37 ` Wang Yugui
@ 2022-09-14 21:54 ` Filipe Manana
3 siblings, 0 replies; 7+ messages in thread
From: Filipe Manana @ 2022-09-14 21:54 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Wang Yugui
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.”
^ permalink raw reply [flat|nested] 7+ messages in thread