On 27.02.2018 12:31, Qu Wenruo wrote:
>
>
> On 2018年02月27日 18:01, Su Yue wrote:
>>
>>
>> On 02/27/2018 05:12 PM, Qu Wenruo wrote:
>>> Original --check-data-csum option will skip the extra copy if the first
>>> copy matches csum.
>>>
>>> Since offline scrub (with recoverability report) is still out-of-tree, at
>>> least enhance --check-data-csum option to handle multiple copies.
>>>
>>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>>> ---
>>> check/main.c | 65
>>> ++++++++++++++++++++++++++++++------------------------------
>>> 1 file changed, 32 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/check/main.c b/check/main.c
>>> index 97baae583f04..f25fdc765d63 100644
>>> --- a/check/main.c
>>> +++ b/check/main.c
>>> @@ -5381,42 +5381,37 @@ static int check_extent_csums(struct
>>> btrfs_root *root, u64 bytenr,
>>> if (!data)
>>> return -ENOMEM;
>>> + num_copies = btrfs_num_copies(root->fs_info, bytenr, num_bytes);
>>> while (offset < num_bytes) {
>>> - mirror = 0;
>>> -again:
>>> - read_len = num_bytes - offset;
>>> - /* read as much space once a time */
>>> - ret = read_extent_data(fs_info, data + offset,
>>> - bytenr + offset, &read_len, mirror);
>>> - if (ret)
>>> - goto out;
>>> - data_checked = 0;
>>> - /* verify every 4k data's checksum */
>>> - while (data_checked < read_len) {
>>> - csum = ~(u32)0;
>>> - tmp = offset + data_checked;
>>> -
>>> - csum = btrfs_csum_data((char *)data + tmp,
>>> - csum, fs_info->sectorsize);
>>> - btrfs_csum_final(csum, (u8 *)&csum);
>>> -
>>> - csum_offset = leaf_offset +
>>> - tmp / fs_info->sectorsize * csum_size;
>>> - read_extent_buffer(eb, (char *)&csum_expected,
>>> - csum_offset, csum_size);
>>> - /* try another mirror */
>>> - if (csum != csum_expected) {
>>> - fprintf(stderr, "mirror %d bytenr %llu csum %u
>>> expected csum %u\n",
>>> + for (mirror = 1; mirror <= num_copies; mirror++) {
>>
>> Got your point.
>> But what confuses me is that why mirror starts from 1 here.
>> The mirror influences btrfs_map_block() which is not related
>> to this patch though.
>
> mirror number has different meanings in fact.
>
> For 0, it means try to read *ANY* valid copy, it can be the copy of a
> duplication, or even rebuilt data from RAID5/6.
>
> For 1, it means the fist copy. Either the only copy of
> SINGLE/RAID0/RAID5/6, or the first copy of RAID1/10.
>
> For 2, it means the 2nd copy, it can be real copy for RAID1/RAID10, or
> rebuilt data from RAID5/6.
>
> And in fact Liu Bo is adding new copy numbers for RAID5/6 to allow us to
> specify how to build the missing data for RAID6.
>
> So here starting from mirror 1 is the correct behavior.
Shouldn't those be documented?
>
> Thanks,
> Qu
>>
>> Thanks,
>> Su
>>> + read_len = num_bytes - offset;
>>> + /* read as much space once a time */
>>> + ret = read_extent_data(fs_info, data + offset,
>>> + bytenr + offset, &read_len, mirror);
>>> + if (ret)
>>> + goto out;
>>> +
>>> + data_checked = 0;
>>> + /* verify every 4k data's checksum */
>>> + while (data_checked < read_len) {
>>> + csum = ~(u32)0;
>>> + tmp = offset + data_checked;
>>> +
>>> + csum = btrfs_csum_data((char *)data + tmp,
>>> + csum, fs_info->sectorsize);
>>> + btrfs_csum_final(csum, (u8 *)&csum);
>>> +
>>> + csum_offset = leaf_offset +
>>> + tmp / fs_info->sectorsize * csum_size;
>>> + read_extent_buffer(eb, (char *)&csum_expected,
>>> + csum_offset, csum_size);
>>> + if (csum != csum_expected)
>>> + fprintf(stderr,
>>> + "mirror %d bytenr %llu csum %u expected csum %u\n",
>>> mirror, bytenr + tmp,
>>> csum, csum_expected);
>>> - num_copies = btrfs_num_copies(root->fs_info,
>>> - bytenr, num_bytes);
>>> - if (mirror < num_copies - 1) {
>>> - mirror += 1;
>>> - goto again;
>>> - }
>>> + data_checked += fs_info->sectorsize;
>>> }
>>> - data_checked += fs_info->sectorsize;
>>> }
>>> offset += read_len;
>>> }
>>> @@ -5624,7 +5619,11 @@ static int check_csums(struct btrfs_root *root)
>>> leaf_offset = btrfs_item_ptr_offset(leaf, path.slots[0]);
>>> ret = check_extent_csums(root, key.offset, data_len,
>>> leaf_offset, leaf);
>>> - if (ret)
>>> + /*
>>> + * Only break for fatal errors, if mismatch is found,
>>> + * continue checking until all extents are checked.
>>> + */
>>> + if (ret < 0)
>>> break;
>>> skip_csum_check:
>>> if (!num_bytes) {
>>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
