Re: [PATCH 08/10] btrfs: simplify data stripe calculation helpers

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

On 2021/7/26 下午8:15, David Sterba wrote:
> There are two helpers doing the same calculations based on nparity and
> ncopies. calc_data_stripes can be simplified into one expression, so far
> we don't have profile with both copies and parity, so there's no
> effective change. calc_stripe_length should reuse the helper and not
> repeat the same calculation.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Just one nitpick inlined below.
> ---
>   fs/btrfs/volumes.c | 15 ++-------------
>   1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index d98e29556d79..78dd013d0ee3 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3567,10 +3567,7 @@ static u64 calc_data_stripes(u64 type, int num_stripes)
>   	const int ncopies = btrfs_raid_array[index].ncopies;
>   	const int nparity = btrfs_raid_array[index].nparity;
>
> -	if (nparity)
> -		return num_stripes - nparity;
> -	else
> -		return num_stripes / ncopies;

I would prefer an ASSERT() here to be extra sure.
But it's my personal taste (and love for tons of ASSERT()).

Thanks,
Qu

> +	return (num_stripes - nparity) / ncopies;
>   }
>
>   /* [pstart, pend) */
> @@ -6878,15 +6875,7 @@ static void btrfs_report_missing_device(struct btrfs_fs_info *fs_info,
>
>   static u64 calc_stripe_length(u64 type, u64 chunk_len, int num_stripes)
>   {
> -	int index = btrfs_bg_flags_to_raid_index(type);
> -	int ncopies = btrfs_raid_array[index].ncopies;
> -	const int nparity = btrfs_raid_array[index].nparity;
> -	int data_stripes;
> -
> -	if (nparity)
> -		data_stripes = num_stripes - nparity;
> -	else
> -		data_stripes = num_stripes / ncopies;
> +	const int data_stripes = calc_data_stripes(type, num_stripes);
>
>   	return div_u64(chunk_len, data_stripes);
>   }
>