Hi,
Great cleanup series!
On 5/17/19 11:43 AM, David Sterba wrote:
> The table is already used for ncopies, replace open coding of stripes
> with the raid_attr value.
>
> Signed-off-by: David Sterba <dste...@suse.com>
> ---
> fs/btrfs/volumes.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 995a15a816f2..743ed1f0b2a6 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -6652,19 +6652,14 @@ 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;
> + int nparity = btrfs_raid_array[index].nparity;
> int data_stripes;
>
> - switch (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
> - case BTRFS_BLOCK_GROUP_RAID5:
> - data_stripes = num_stripes - 1;
> - break;
> - case BTRFS_BLOCK_GROUP_RAID6:
> - data_stripes = num_stripes - 2;
> - break;
> - default:
> + if (nparity)
> + data_stripes = num_stripes - nparity;
> + else
> data_stripes = num_stripes / ncopies;
> - break;
> - }
A few lines earlier in that file we have this:
/*
* this will have to be fixed for RAID1 and RAID10 over
* more drives
*/
data_stripes = (num_stripes - nparity) / ncopies;
1) I changed the calculation in b50836edf9 and did it in one statement,
I see you use and extra if here. Which one do you prefer and why?
2) Back then I wanted to get rid of that comment, because I don't
understand it. "this will have to be fixed" does not tell me what should
be fixed, so I left it there. Maybe now is the time? Do you know what
this comment/warning means and if it can be removed? I mean, even with
raid1c3 the calculation would be correct. There's no parity and three
copies.
> +
> return div_u64(chunk_len, data_stripes);
> }
>
>
Hans
