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

[David Sterba <dsterba@suse.cz>]

On Mon, Jul 26, 2021 at 08:38:16PM +0800, Qu Wenruo wrote:
> 
> 
> 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()).

Assert for what exactly? I had a thought about that too but was not sure
what to put there.