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.