On Fri, May 17, 2019 at 12:06:05PM +0200, Hans van Kranenburg wrote:
> > -   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?

I did the cleanup only in the function and was not aware of the above,
but the ifs did not feel right so I'm glad you pointed that out.

And actually I think there must be an ultimate formula that also
includes the sub_stripes (raid10) and devs_increment (dup), this could
clean up the rest of the special cases.

> 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.

Yeah the comment does not help much, it was introduced by the monster
raid56 commit but I don't think there's anything to be fixed, regarding
raid1 or raid10.

Reply via email to