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 <w...@suse.com> >> --- >> 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. 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 majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
signature.asc
Description: OpenPGP digital signature