Re: [PATCH 9/9] btrfs: Add RAID 6 recovery for a btrfs filesystem.

[Goffredo Baroncelli]

On 25/09/2018 21.20, Daniel Kiper wrote:
> On Wed, Sep 19, 2018 at 08:40:40PM +0200, Goffredo Baroncelli wrote:
>> From: Goffredo Baroncelli <address@hidden>
>>
[....]
>>             *  - stripe_offset is the disk offset,
>>             *  - csize is the "potential" data to read. It will be reduced to
>>             *    size if the latter is smaller.
>> +           *  - parities_pos is the position of the parity inside a row (
> 
> s/inside/in/> 
>> +           *    2 for P1, 3 for P2...)

+              *  - nparities is the number of parities (1 for RAID5, 2 for 
RAID6);
+              *    used only in RAID5/6 code.

>>             */
>>            block_nr = grub_divmod64 (off, chunk_stripe_length, &low);
>>
>> @@ -1030,6 +1069,9 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data, 
>> grub_disk_addr_t addr,
>>             */
>>            grub_divmod64 (high + stripen, nstripes, &stripen);
>>
>> +          grub_divmod64 (high + nstripes - nparities, nstripes,
>> +                         &parities_pos);
> 
> I think that this math requires a bit of explanation in the comment
> before grub_divmod64(). Especially I am interested in why high +
> nstripes - nparities works as expected.


What about

/*
 * parities_pos is equal to "(high - nparities) % nstripes" (see the diagram 
above).
 * However "high - nparities" might be negative (eg when high == 0) leading to 
an
 * incorrect computation.
 * Instead "high + nstripes - nparities" is always positive and in modulo 
nstripes is
 * equal to "(high - nparities) % nstripes
 */
> 
> Daniel
> 
BR
G.Baroncelli

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5