Re: [PATCH v3]btrfs: round down stripe size and chunk size to pow of 2

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2022/8/17 22:58, Wang Yugui wrote:
> In decide_stripe_size_regular(), when new disk is added to RAID0/RAID10/RAID56,
> it is better to free-then-reuse the free space if stripe size is kept or
> changed to 1/2. so stripe size of pow of 2 will be more friendly. Although
> roundup_pow_of_two() match better with orig round_up(), but
> rounddown_pow_of_two() is better to make sure <=ctl->max_chunk_size here.

Why insist on round*_pow_of_two()?

I see no reason that a power of 2 sized chunk has any benefit to btrfs.

>
> In another rare case that file system is quite small, we calc max chunk size
> in pow of 2 too, so that max chunk size / chunk size /stripe size are same or
> match easy in some case.
>
> Signed-off-by: Wang Yugui <wangyugui@e16-tech.com>
> ---
> changes since v2:
> 	restore to rounddown_pow_of_two() from roundup_pow_of_two()
> changes since v1:
>   - change rounddown_pow_of_two() to roundup_pow_of_two() to match better with
>     orig roundup().
>
>   fs/btrfs/volumes.c | 20 +++++++++-----------
>   1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6595755..fab9765 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -5083,9 +5083,9 @@ static void init_alloc_chunk_ctl_policy_regular(
>   	if (ctl->type & BTRFS_BLOCK_GROUP_SYSTEM)
>   		ctl->devs_max = min_t(int, ctl->devs_max, BTRFS_MAX_DEVS_SYS_CHUNK);
>
> -	/* We don't want a chunk larger than 10% of writable space */
> -	ctl->max_chunk_size = min(div_factor(fs_devices->total_rw_bytes, 1),
> -				  ctl->max_chunk_size);
> +	/* We don't want a chunk larger than 1/8 of writable space */
> +	ctl->max_chunk_size = min_t(u64, ctl->max_chunk_size,
> +			rounddown_pow_of_two(fs_devices->total_rw_bytes >> 3));
>   	ctl->dev_extent_min = BTRFS_STRIPE_LEN * ctl->dev_stripes;
>   }
>
> @@ -5143,10 +5143,9 @@ static void init_alloc_chunk_ctl_policy_zoned(
>   		BUG();
>   	}
>
> -	/* We don't want a chunk larger than 10% of writable space */
> -	limit = max(round_down(div_factor(fs_devices->total_rw_bytes, 1),
> -			       zone_size),
> -		    min_chunk_size);
> +	/* We don't want a chunk larger than 1/8 of writable space */
> +	limit = max_t(u64, min_chunk_size,
> +		rounddown_pow_of_two(fs_devices->total_rw_bytes >> 3));
>   	ctl->max_chunk_size = min(limit, ctl->max_chunk_size);
>   	ctl->dev_extent_min = zone_size * ctl->dev_stripes;
>   }
> @@ -5284,13 +5283,12 @@ static int decide_stripe_size_regular(struct alloc_chunk_ctl *ctl,
>   	 */
>   	if (ctl->stripe_size * data_stripes > ctl->max_chunk_size) {
>   		/*
> -		 * Reduce stripe_size, round it up to a 16MB boundary again and
> +		 * Reduce stripe_size, round it down to pow of 2 boundary again and
>   		 * then use it, unless it ends up being even bigger than the
>   		 * previous value we had already.
>   		 */
> -		ctl->stripe_size = min(round_up(div_u64(ctl->max_chunk_size,
> -							data_stripes), SZ_16M),
> -				       ctl->stripe_size);
> +		ctl->stripe_size = min_t(u64, ctl->stripe_size,
> +			rounddown_pow_of_two(div_u64(ctl->max_chunk_size, data_stripes)));
>   	}
>
>   	/* Align to BTRFS_STRIPE_LEN */