* [PATCH] btrfs: scrub: expand scrub block size for data range scrub
@ 2022-11-13 15:35 Li Zhang
2022-11-13 15:37 ` li zhang
2022-11-13 22:58 ` Qu Wenruo
0 siblings, 2 replies; 8+ messages in thread
From: Li Zhang @ 2022-11-13 15:35 UTC (permalink / raw)
To: linux-btrfs; +Cc: Li Zhang
[implement]
1. Add the member checksum_error to the scrub_sector,
which indicates the checksum error of the sector
2. Use scrub_find_btrfs_ordered_sum to find the desired
btrfs_ordered_sum containing address logic, in
scrub_sectors_for_parity and scrub_sectors, call
Scrub_find_btrfs_ordered_sum finds the
btrfs_ordered_sum containing the current logical address, and
Calculate the exact checksum later.
3. In the scrub_checksum_data function,
we should check all sectors in the scrub_block.
4. The job of the scrub_handle_errored_block
function is to count the number of error and
repair sectors if they can be repaired.
The function enters the wrong scrub_block, and
the overall process is as follows
1) Check the scrub_block again, check again if the error is gone.
2) Check the corresponding mirror scrub_block, if there is no error,
Fix bad sblocks with mirror scrub_block.
3) If no error-free scrub_block is found, repair it sector by sector.
One difficulty with this function is rechecking the scrub_block.
Imagine this situation, if a sector is checked the first time
without errors, butthe recheck returns an error. What should
we do, this patch only fixes the bug that the sector first
appeared (As in the case where the scrub_block
contains only one scrub_sector).
Another reason to only handle the first error is,
If the device goes bad, the recheck function will report more and
more errors,if we want to deal with the errors in the recheck,
you need to recheck again and again, which may lead to
Stuck in scrub_handle_errored_block for a long time.
So rechecked bug reports will only be corrected in the
next scrub.
[test]
I designed two test scripts based on the fstest project,
the output is the same as the case where the scrub_block
contains only one scrub sector,
1. There are two situations in raid1,
raid1c3 and raid1c4. If an error occurs in
different sectors of the sblock, the error can be corrected.
An error is uncorrectable if all errors occur in the same
scrub_sector in the scrub_block.
2. For raid6, if more than 2 stripts are damaged and the error
cannot be repaired, a batch read error will also be reported.
Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
---
fs/btrfs/scrub.c | 385 ++++++++++++++++++++++++++++++-------------------------
1 file changed, 210 insertions(+), 175 deletions(-)
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 196c4c6..5ca9f43 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -72,6 +72,7 @@ struct scrub_sector {
atomic_t refs;
unsigned int have_csum:1;
unsigned int io_error:1;
+ unsigned int checksum_error:1;
u8 csum[BTRFS_CSUM_SIZE];
struct scrub_recover *recover;
@@ -252,6 +253,7 @@ static void detach_scrub_page_private(struct page *page)
#endif
}
+
static struct scrub_block *alloc_scrub_block(struct scrub_ctx *sctx,
struct btrfs_device *dev,
u64 logical, u64 physical,
@@ -404,7 +406,7 @@ static int scrub_write_sector_to_dev_replace(struct scrub_block *sblock,
static void scrub_parity_put(struct scrub_parity *sparity);
static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
u64 physical, struct btrfs_device *dev, u64 flags,
- u64 gen, int mirror_num, u8 *csum,
+ u64 gen, int mirror_num,
u64 physical_for_dev_replace);
static void scrub_bio_end_io(struct bio *bio);
static void scrub_bio_end_io_worker(struct work_struct *work);
@@ -420,6 +422,10 @@ static int scrub_add_sector_to_wr_bio(struct scrub_ctx *sctx,
static void scrub_wr_bio_end_io(struct bio *bio);
static void scrub_wr_bio_end_io_worker(struct work_struct *work);
static void scrub_put_ctx(struct scrub_ctx *sctx);
+static int scrub_find_btrfs_ordered_sum(struct scrub_ctx *sctx, u64 logical,
+ struct btrfs_ordered_sum **order_sum);
+static int scrub_get_sblock_checksum_error(struct scrub_block *sblock);
+
static inline int scrub_is_page_on_raid56(struct scrub_sector *sector)
{
@@ -991,19 +997,18 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
struct btrfs_fs_info *fs_info;
u64 logical;
unsigned int failed_mirror_index;
- unsigned int is_metadata;
- unsigned int have_csum;
/* One scrub_block for each mirror */
struct scrub_block *sblocks_for_recheck[BTRFS_MAX_MIRRORS] = { 0 };
struct scrub_block *sblock_bad;
int ret;
int mirror_index;
int sector_num;
- int success;
bool full_stripe_locked;
unsigned int nofs_flag;
static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
DEFAULT_RATELIMIT_BURST);
+ int correct_error = 0;
+ int uncorrect_error = 0;
BUG_ON(sblock_to_check->sector_count < 1);
fs_info = sctx->fs_info;
@@ -1023,9 +1028,6 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
logical = sblock_to_check->logical;
ASSERT(sblock_to_check->mirror_num);
failed_mirror_index = sblock_to_check->mirror_num - 1;
- is_metadata = !(sblock_to_check->sectors[0]->flags &
- BTRFS_EXTENT_FLAG_DATA);
- have_csum = sblock_to_check->sectors[0]->have_csum;
if (!sctx->is_dev_replace && btrfs_repair_one_zone(fs_info, logical))
return 0;
@@ -1054,7 +1056,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
if (ret == -ENOMEM)
sctx->stat.malloc_errors++;
sctx->stat.read_errors++;
- sctx->stat.uncorrectable_errors++;
+ sctx->stat.uncorrectable_errors += scrub_get_sblock_checksum_error(sblock_to_check);
+ sctx->stat.uncorrectable_errors += sblock_to_check->header_error;
spin_unlock(&sctx->stat_lock);
return ret;
}
@@ -1104,7 +1107,10 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
spin_lock(&sctx->stat_lock);
sctx->stat.malloc_errors++;
sctx->stat.read_errors++;
- sctx->stat.uncorrectable_errors++;
+ sctx->stat.uncorrectable_errors +=
+ scrub_get_sblock_checksum_error(sblock_to_check);
+ sctx->stat.uncorrectable_errors +=
+ sblock_to_check->header_error;
spin_unlock(&sctx->stat_lock);
btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
goto out;
@@ -1116,7 +1122,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
if (ret) {
spin_lock(&sctx->stat_lock);
sctx->stat.read_errors++;
- sctx->stat.uncorrectable_errors++;
+ sctx->stat.uncorrectable_errors += scrub_get_sblock_checksum_error(sblock_to_check);
+ sctx->stat.uncorrectable_errors += sblock_to_check->header_error;
spin_unlock(&sctx->stat_lock);
btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
goto out;
@@ -1138,7 +1145,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
* the cause)
*/
spin_lock(&sctx->stat_lock);
- sctx->stat.unverified_errors++;
+ sctx->stat.unverified_errors += scrub_get_sblock_checksum_error(sblock_to_check);
+ sctx->stat.unverified_errors += sblock_to_check->header_error;
sblock_to_check->data_corrected = 1;
spin_unlock(&sctx->stat_lock);
@@ -1147,22 +1155,7 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
goto out;
}
- if (!sblock_bad->no_io_error_seen) {
- spin_lock(&sctx->stat_lock);
- sctx->stat.read_errors++;
- spin_unlock(&sctx->stat_lock);
- if (__ratelimit(&rs))
- scrub_print_warning("i/o error", sblock_to_check);
- btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
- } else if (sblock_bad->checksum_error) {
- spin_lock(&sctx->stat_lock);
- sctx->stat.csum_errors++;
- spin_unlock(&sctx->stat_lock);
- if (__ratelimit(&rs))
- scrub_print_warning("checksum error", sblock_to_check);
- btrfs_dev_stat_inc_and_print(dev,
- BTRFS_DEV_STAT_CORRUPTION_ERRS);
- } else if (sblock_bad->header_error) {
+ if (sblock_to_check->header_error && sblock_bad->header_error) {
spin_lock(&sctx->stat_lock);
sctx->stat.verify_errors++;
spin_unlock(&sctx->stat_lock);
@@ -1175,8 +1168,48 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
else
btrfs_dev_stat_inc_and_print(dev,
BTRFS_DEV_STAT_CORRUPTION_ERRS);
+ } else if (sblock_to_check->header_error && !sblock_bad->header_error) {
+ spin_lock(&sctx->stat_lock);
+ sctx->stat.unverified_errors++;
+ spin_unlock(&sctx->stat_lock);
+ } else if (!sblock_to_check->header_error && sblock_bad->header_error) {
+ sblock_bad->header_error = 0;
+ } else {
+ for (sector_num = 0; sector_num < sblock_bad->sector_count; sector_num++) {
+ struct scrub_sector *bad_sector = sblock_bad->sectors[sector_num];
+ struct scrub_sector *check_sector = sblock_to_check->sectors[sector_num];
+
+ if (bad_sector->io_error || check_sector->io_error) {
+ spin_lock(&sctx->stat_lock);
+ sctx->stat.read_errors++;
+ spin_unlock(&sctx->stat_lock);
+ if (__ratelimit(&rs))
+ scrub_print_warning("i/o error", sblock_to_check);
+ btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
+ } else if (check_sector->checksum_error && bad_sector->checksum_error) {
+ spin_lock(&sctx->stat_lock);
+ sctx->stat.csum_errors++;
+ spin_unlock(&sctx->stat_lock);
+ if (__ratelimit(&rs))
+ scrub_print_warning("checksum error", sblock_to_check);
+ btrfs_dev_stat_inc_and_print(dev,
+ BTRFS_DEV_STAT_CORRUPTION_ERRS);
+
+ } else if (check_sector->checksum_error && !bad_sector->checksum_error) {
+ spin_lock(&sctx->stat_lock);
+ sctx->stat.unverified_errors++;
+ spin_unlock(&sctx->stat_lock);
+ } else if (!check_sector->checksum_error && bad_sector->checksum_error) {
+ struct scrub_sector *temp_sector = sblock_bad->sectors[sector_num];
+
+ sblock_bad->sectors[sector_num]
+ = sblock_to_check->sectors[sector_num];
+ sblock_to_check->sectors[sector_num] = temp_sector;
+ }
+ }
}
+
if (sctx->readonly) {
ASSERT(!sctx->is_dev_replace);
goto out;
@@ -1233,18 +1266,23 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
sblock_other->no_io_error_seen) {
if (sctx->is_dev_replace) {
scrub_write_block_to_dev_replace(sblock_other);
- goto corrected_error;
+ correct_error += scrub_get_sblock_checksum_error(sblock_bad);
+ correct_error += sblock_bad->header_error;
+ goto error_summary;
} else {
ret = scrub_repair_block_from_good_copy(
sblock_bad, sblock_other);
- if (!ret)
- goto corrected_error;
+ if (!ret) {
+ correct_error +=
+ scrub_get_sblock_checksum_error(sblock_bad);
+ correct_error += sblock_bad->header_error;
+ goto error_summary;
+ }
}
}
}
- if (sblock_bad->no_io_error_seen && !sctx->is_dev_replace)
- goto did_not_correct_error;
+
/*
* In case of I/O errors in the area that is supposed to be
@@ -1270,17 +1308,16 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
* mirror, even if other 512 byte sectors in the same sectorsize
* area are unreadable.
*/
- success = 1;
for (sector_num = 0; sector_num < sblock_bad->sector_count;
sector_num++) {
struct scrub_sector *sector_bad = sblock_bad->sectors[sector_num];
struct scrub_block *sblock_other = NULL;
/* Skip no-io-error sectors in scrub */
- if (!sector_bad->io_error && !sctx->is_dev_replace)
+ if (!(sector_bad->io_error || sector_bad->checksum_error) && !sctx->is_dev_replace)
continue;
- if (scrub_is_page_on_raid56(sblock_bad->sectors[0])) {
+ if (scrub_is_page_on_raid56(sector_bad)) {
/*
* In case of dev replace, if raid56 rebuild process
* didn't work out correct data, then copy the content
@@ -1289,6 +1326,7 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
* sblock_for_recheck array to target device.
*/
sblock_other = NULL;
+ uncorrect_error++;
} else if (sector_bad->io_error) {
/* Try to find no-io-error sector in mirrors */
for (mirror_index = 0;
@@ -1302,7 +1340,21 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
}
}
if (!sblock_other)
- success = 0;
+ uncorrect_error++;
+ } else if (sector_bad->checksum_error) {
+ for (mirror_index = 0;
+ mirror_index < BTRFS_MAX_MIRRORS &&
+ sblocks_for_recheck[mirror_index]->sector_count > 0;
+ mirror_index++) {
+ if (!sblocks_for_recheck[mirror_index]->sectors[sector_num]->io_error &&
+ !sblocks_for_recheck[mirror_index]->sectors[sector_num]->checksum_error) {
+ sblock_other = sblocks_for_recheck[mirror_index];
+ break;
+ }
+ }
+ if (!sblock_other) {
+ uncorrect_error++;
+ }
}
if (sctx->is_dev_replace) {
@@ -1319,56 +1371,28 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
sector_num) != 0) {
atomic64_inc(
&fs_info->dev_replace.num_write_errors);
- success = 0;
}
} else if (sblock_other) {
ret = scrub_repair_sector_from_good_copy(sblock_bad,
sblock_other,
sector_num, 0);
- if (0 == ret)
+ if (0 == ret && sector_bad->io_error) {
+ correct_error++;
sector_bad->io_error = 0;
- else
- success = 0;
+ } else if (0 == ret && sector_bad->checksum_error) {
+ correct_error++;
+ sector_bad->checksum_error = 0;
+ } else {
+ uncorrect_error++;
+ }
}
}
- if (success && !sctx->is_dev_replace) {
- if (is_metadata || have_csum) {
- /*
- * need to verify the checksum now that all
- * sectors on disk are repaired (the write
- * request for data to be repaired is on its way).
- * Just be lazy and use scrub_recheck_block()
- * which re-reads the data before the checksum
- * is verified, but most likely the data comes out
- * of the page cache.
- */
- scrub_recheck_block(fs_info, sblock_bad, 1);
- if (!sblock_bad->header_error &&
- !sblock_bad->checksum_error &&
- sblock_bad->no_io_error_seen)
- goto corrected_error;
- else
- goto did_not_correct_error;
- } else {
-corrected_error:
- spin_lock(&sctx->stat_lock);
- sctx->stat.corrected_errors++;
- sblock_to_check->data_corrected = 1;
- spin_unlock(&sctx->stat_lock);
- btrfs_err_rl_in_rcu(fs_info,
- "fixed up error at logical %llu on dev %s",
- logical, rcu_str_deref(dev->name));
- }
- } else {
-did_not_correct_error:
- spin_lock(&sctx->stat_lock);
- sctx->stat.uncorrectable_errors++;
- spin_unlock(&sctx->stat_lock);
- btrfs_err_rl_in_rcu(fs_info,
- "unable to fixup (regular) error at logical %llu on dev %s",
- logical, rcu_str_deref(dev->name));
- }
+error_summary:
+ spin_lock(&sctx->stat_lock);
+ sctx->stat.uncorrectable_errors += uncorrect_error;
+ sctx->stat.corrected_errors += correct_error;
+ spin_unlock(&sctx->stat_lock);
out:
for (mirror_index = 0; mirror_index < BTRFS_MAX_MIRRORS; mirror_index++) {
@@ -1513,10 +1537,10 @@ static int scrub_setup_recheck_block(struct scrub_block *original_sblock,
}
sector->flags = flags;
sector->generation = generation;
- sector->have_csum = have_csum;
+ sector->have_csum = original_sblock->sectors[sector_index]->have_csum;
if (have_csum)
memcpy(sector->csum,
- original_sblock->sectors[0]->csum,
+ original_sblock->sectors[sector_index]->csum,
sctx->fs_info->csum_size);
scrub_stripe_index_and_offset(logical,
@@ -1688,11 +1712,15 @@ static int scrub_repair_block_from_good_copy(struct scrub_block *sblock_bad,
for (i = 0; i < sblock_bad->sector_count; i++) {
int ret_sub;
-
- ret_sub = scrub_repair_sector_from_good_copy(sblock_bad,
+ if (sblock_bad->sectors[i]->checksum_error == 1
+ && sblock_good->sectors[i]->checksum_error == 0){
+ ret_sub = scrub_repair_sector_from_good_copy(sblock_bad,
sblock_good, i, 1);
- if (ret_sub)
- ret = ret_sub;
+ if (ret_sub)
+ ret = ret_sub;
+ } else if (sblock_bad->sectors[i]->checksum_error == 1) {
+ ret = 1;
+ }
}
return ret;
@@ -1984,22 +2012,46 @@ static int scrub_checksum_data(struct scrub_block *sblock)
u8 csum[BTRFS_CSUM_SIZE];
struct scrub_sector *sector;
char *kaddr;
+ int i;
+ int io_error = 0;
BUG_ON(sblock->sector_count < 1);
- sector = sblock->sectors[0];
- if (!sector->have_csum)
- return 0;
-
- kaddr = scrub_sector_get_kaddr(sector);
shash->tfm = fs_info->csum_shash;
crypto_shash_init(shash);
+ for (i = 0; i < sblock->sector_count; i++) {
+ sector = sblock->sectors[i];
+ if (sector->io_error == 1) {
+ io_error = 1;
+ continue;
+ }
+ if (!sector->have_csum)
+ continue;
- crypto_shash_digest(shash, kaddr, fs_info->sectorsize, csum);
+ kaddr = scrub_sector_get_kaddr(sector);
+ crypto_shash_digest(shash, kaddr, fs_info->sectorsize, csum);
+ if (memcmp(csum, sector->csum, fs_info->csum_size)) {
+ sector->checksum_error = 1;
+ sblock->checksum_error = 1;
+ } else {
+ sector->checksum_error = 0;
+ }
+ }
+ return sblock->checksum_error | io_error;
+}
- if (memcmp(csum, sector->csum, fs_info->csum_size))
- sblock->checksum_error = 1;
- return sblock->checksum_error;
+static int scrub_get_sblock_checksum_error(struct scrub_block *sblock)
+{
+ int count = 0;
+ int i;
+
+ if (sblock == NULL)
+ return count;
+ for (i = 0; i < sblock->sector_count; i++) {
+ if (sblock->sectors[i]->checksum_error == 1)
+ count++;
+ }
+ return count;
}
static int scrub_checksum_tree_block(struct scrub_block *sblock)
@@ -2062,8 +2114,12 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
}
crypto_shash_final(shash, calculated_csum);
- if (memcmp(calculated_csum, on_disk_csum, sctx->fs_info->csum_size))
+ if (memcmp(calculated_csum, on_disk_csum, sctx->fs_info->csum_size)) {
+ sblock->sectors[0]->checksum_error = 1;
sblock->checksum_error = 1;
+ } else {
+ sblock->sectors[0]->checksum_error = 0;
+ }
return sblock->header_error || sblock->checksum_error;
}
@@ -2400,12 +2456,14 @@ static void scrub_missing_raid56_pages(struct scrub_block *sblock)
static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
u64 physical, struct btrfs_device *dev, u64 flags,
- u64 gen, int mirror_num, u8 *csum,
+ u64 gen, int mirror_num,
u64 physical_for_dev_replace)
{
struct scrub_block *sblock;
const u32 sectorsize = sctx->fs_info->sectorsize;
int index;
+ int have_csum;
+ struct btrfs_ordered_sum *order_sum = NULL;
sblock = alloc_scrub_block(sctx, dev, logical, physical,
physical_for_dev_replace, mirror_num);
@@ -2415,7 +2473,6 @@ static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
spin_unlock(&sctx->stat_lock);
return -ENOMEM;
}
-
for (index = 0; len > 0; index++) {
struct scrub_sector *sector;
/*
@@ -2424,7 +2481,6 @@ static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
* more memory for PAGE_SIZE > sectorsize case.
*/
u32 l = min(sectorsize, len);
-
sector = alloc_scrub_sector(sblock, logical, GFP_KERNEL);
if (!sector) {
spin_lock(&sctx->stat_lock);
@@ -2435,11 +2491,25 @@ static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
}
sector->flags = flags;
sector->generation = gen;
- if (csum) {
- sector->have_csum = 1;
- memcpy(sector->csum, csum, sctx->fs_info->csum_size);
- } else {
- sector->have_csum = 0;
+ if (flags & BTRFS_EXTENT_FLAG_DATA) {
+ if (order_sum == NULL ||
+ (order_sum->bytenr + order_sum->len <= logical)) {
+ order_sum = NULL;
+ have_csum = scrub_find_btrfs_ordered_sum(sctx, logical, &order_sum);
+ }
+ if (have_csum == 0) {
+ ++sctx->stat.no_csum;
+ sector->have_csum = 0;
+ } else {
+ int order_csum_index;
+
+ sector->have_csum = 1;
+ order_csum_index = (logical-order_sum->bytenr)
+ >> sctx->fs_info->sectorsize_bits;
+ memcpy(sector->csum,
+ order_sum->sums + order_csum_index * sctx->fs_info->csum_size,
+ sctx->fs_info->csum_size);
+ }
}
len -= l;
logical += l;
@@ -2571,7 +2641,8 @@ static void scrub_block_complete(struct scrub_block *sblock)
{
int corrupted = 0;
- if (!sblock->no_io_error_seen) {
+ if (!sblock->no_io_error_seen && !(sblock->sector_count > 0
+ && (sblock->sectors[0]->flags & BTRFS_EXTENT_FLAG_DATA))) {
corrupted = 1;
scrub_handle_errored_block(sblock);
} else {
@@ -2597,61 +2668,30 @@ static void scrub_block_complete(struct scrub_block *sblock)
}
}
-static void drop_csum_range(struct scrub_ctx *sctx, struct btrfs_ordered_sum *sum)
-{
- sctx->stat.csum_discards += sum->len >> sctx->fs_info->sectorsize_bits;
- list_del(&sum->list);
- kfree(sum);
-}
-
/*
- * Find the desired csum for range [logical, logical + sectorsize), and store
- * the csum into @csum.
+ * Find the desired btrfs_ordered_sum contain address logical, and store
+ * the result into @order_sum.
*
* The search source is sctx->csum_list, which is a pre-populated list
- * storing bytenr ordered csum ranges. We're responsible to cleanup any range
- * that is before @logical.
+ * storing bytenr ordered csum ranges.
*
- * Return 0 if there is no csum for the range.
- * Return 1 if there is csum for the range and copied to @csum.
+ * Return 0 if there is no btrfs_ordered_sum contain the address logical.
+ * Return 1 if there is btrfs_order_sum contain the address logincal and copied to @order_sum.
*/
-static int scrub_find_csum(struct scrub_ctx *sctx, u64 logical, u8 *csum)
+static int scrub_find_btrfs_ordered_sum(struct scrub_ctx *sctx, u64 logical,
+ struct btrfs_ordered_sum **order_sum)
{
bool found = false;
+ struct btrfs_ordered_sum *sum;
- while (!list_empty(&sctx->csum_list)) {
- struct btrfs_ordered_sum *sum = NULL;
- unsigned long index;
- unsigned long num_sectors;
-
- sum = list_first_entry(&sctx->csum_list,
- struct btrfs_ordered_sum, list);
- /* The current csum range is beyond our range, no csum found */
+ list_for_each_entry(sum, &sctx->csum_list, list) {
+ /* no btrfs_ordered_sum found */
if (sum->bytenr > logical)
break;
-
- /*
- * The current sum is before our bytenr, since scrub is always
- * done in bytenr order, the csum will never be used anymore,
- * clean it up so that later calls won't bother with the range,
- * and continue search the next range.
- */
- if (sum->bytenr + sum->len <= logical) {
- drop_csum_range(sctx, sum);
+ if (sum->bytenr + sum->len <= logical)
continue;
- }
-
- /* Now the csum range covers our bytenr, copy the csum */
found = true;
- index = (logical - sum->bytenr) >> sctx->fs_info->sectorsize_bits;
- num_sectors = sum->len >> sctx->fs_info->sectorsize_bits;
-
- memcpy(csum, sum->sums + index * sctx->fs_info->csum_size,
- sctx->fs_info->csum_size);
-
- /* Cleanup the range if we're at the end of the csum range */
- if (index == num_sectors - 1)
- drop_csum_range(sctx, sum);
+ *order_sum = sum;
break;
}
if (!found)
@@ -2669,7 +2709,6 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
u64 src_physical = physical;
int src_mirror = mirror_num;
int ret;
- u8 csum[BTRFS_CSUM_SIZE];
u32 blocksize;
if (flags & BTRFS_EXTENT_FLAG_DATA) {
@@ -2685,7 +2724,7 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK)
blocksize = map->stripe_len;
else
- blocksize = sctx->fs_info->nodesize;
+ blocksize = BTRFS_STRIPE_LEN;
spin_lock(&sctx->stat_lock);
sctx->stat.tree_extents_scrubbed++;
sctx->stat.tree_bytes_scrubbed += len;
@@ -2709,17 +2748,9 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
&src_dev, &src_mirror);
while (len) {
u32 l = min(len, blocksize);
- int have_csum = 0;
-
- if (flags & BTRFS_EXTENT_FLAG_DATA) {
- /* push csums to sbio */
- have_csum = scrub_find_csum(sctx, logical, csum);
- if (have_csum == 0)
- ++sctx->stat.no_csum;
- }
ret = scrub_sectors(sctx, logical, l, src_physical, src_dev,
flags, gen, src_mirror,
- have_csum ? csum : NULL, physical);
+ physical);
if (ret)
return ret;
len -= l;
@@ -2733,12 +2764,14 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
static int scrub_sectors_for_parity(struct scrub_parity *sparity,
u64 logical, u32 len,
u64 physical, struct btrfs_device *dev,
- u64 flags, u64 gen, int mirror_num, u8 *csum)
+ u64 flags, u64 gen, int mirror_num)
{
struct scrub_ctx *sctx = sparity->sctx;
struct scrub_block *sblock;
const u32 sectorsize = sctx->fs_info->sectorsize;
int index;
+ struct btrfs_ordered_sum *order_sum = NULL;
+ int have_csum;
ASSERT(IS_ALIGNED(len, sectorsize));
@@ -2770,11 +2803,24 @@ static int scrub_sectors_for_parity(struct scrub_parity *sparity,
list_add_tail(§or->list, &sparity->sectors_list);
sector->flags = flags;
sector->generation = gen;
- if (csum) {
- sector->have_csum = 1;
- memcpy(sector->csum, csum, sctx->fs_info->csum_size);
- } else {
- sector->have_csum = 0;
+ if (flags & BTRFS_EXTENT_FLAG_DATA) {
+ if (order_sum == NULL
+ || (order_sum->bytenr + order_sum->len <= logical)) {
+ order_sum = NULL;
+ have_csum = scrub_find_btrfs_ordered_sum(sctx, logical, &order_sum);
+ }
+ if (have_csum == 0) {
+ sector->have_csum = 0;
+ } else {
+ int order_csum_index;
+
+ sector->have_csum = 1;
+ order_csum_index = (logical-order_sum->bytenr)
+ >> sctx->fs_info->sectorsize_bits;
+ memcpy(sector->csum,
+ order_sum->sums + order_csum_index * sctx->fs_info->csum_size,
+ sctx->fs_info->csum_size);
+ }
}
/* Iterate over the stripe range in sectorsize steps */
@@ -2807,7 +2853,6 @@ static int scrub_extent_for_parity(struct scrub_parity *sparity,
{
struct scrub_ctx *sctx = sparity->sctx;
int ret;
- u8 csum[BTRFS_CSUM_SIZE];
u32 blocksize;
if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state)) {
@@ -2826,20 +2871,10 @@ static int scrub_extent_for_parity(struct scrub_parity *sparity,
while (len) {
u32 l = min(len, blocksize);
- int have_csum = 0;
-
- if (flags & BTRFS_EXTENT_FLAG_DATA) {
- /* push csums to sbio */
- have_csum = scrub_find_csum(sctx, logical, csum);
- if (have_csum == 0)
- goto skip;
- }
ret = scrub_sectors_for_parity(sparity, logical, l, physical, dev,
- flags, gen, mirror_num,
- have_csum ? csum : NULL);
+ flags, gen, mirror_num);
if (ret)
return ret;
-skip:
len -= l;
logical += l;
physical += l;
@@ -4148,7 +4183,7 @@ static noinline_for_stack int scrub_supers(struct scrub_ctx *sctx,
ret = scrub_sectors(sctx, bytenr, BTRFS_SUPER_INFO_SIZE, bytenr,
scrub_dev, BTRFS_EXTENT_FLAG_SUPER, gen, i,
- NULL, bytenr);
+ bytenr);
if (ret)
return ret;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: scrub: expand scrub block size for data range scrub
2022-11-13 15:35 [PATCH] btrfs: scrub: expand scrub block size for data range scrub Li Zhang
@ 2022-11-13 15:37 ` li zhang
2022-11-13 22:37 ` Qu Wenruo
2022-11-13 22:58 ` Qu Wenruo
1 sibling, 1 reply; 8+ messages in thread
From: li zhang @ 2022-11-13 15:37 UTC (permalink / raw)
To: linux-btrfs
[-- Attachment #1: Type: text/plain, Size: 38223 bytes --]
Here is my test script.
Li Zhang <zhanglikernel@gmail.com> 于2022年11月13日周日 23:36写道：
>
> [implement]
> 1. Add the member checksum_error to the scrub_sector,
> which indicates the checksum error of the sector
>
> 2. Use scrub_find_btrfs_ordered_sum to find the desired
> btrfs_ordered_sum containing address logic, in
> scrub_sectors_for_parity and scrub_sectors, call
> Scrub_find_btrfs_ordered_sum finds the
> btrfs_ordered_sum containing the current logical address, and
> Calculate the exact checksum later.
>
> 3. In the scrub_checksum_data function,
> we should check all sectors in the scrub_block.
>
> 4. The job of the scrub_handle_errored_block
> function is to count the number of error and
> repair sectors if they can be repaired.
>
> The function enters the wrong scrub_block, and
> the overall process is as follows
>
> 1) Check the scrub_block again, check again if the error is gone.
>
> 2) Check the corresponding mirror scrub_block, if there is no error,
> Fix bad sblocks with mirror scrub_block.
>
> 3) If no error-free scrub_block is found, repair it sector by sector.
>
> One difficulty with this function is rechecking the scrub_block.
>
> Imagine this situation, if a sector is checked the first time
> without errors, butthe recheck returns an error. What should
> we do, this patch only fixes the bug that the sector first
> appeared (As in the case where the scrub_block
> contains only one scrub_sector).
>
> Another reason to only handle the first error is,
> If the device goes bad, the recheck function will report more and
> more errors,if we want to deal with the errors in the recheck,
> you need to recheck again and again, which may lead to
> Stuck in scrub_handle_errored_block for a long time.
>
> So rechecked bug reports will only be corrected in the
> next scrub.
>
> [test]
> I designed two test scripts based on the fstest project,
> the output is the same as the case where the scrub_block
> contains only one scrub sector,
>
> 1. There are two situations in raid1,
> raid1c3 and raid1c4. If an error occurs in
> different sectors of the sblock, the error can be corrected.
>
> An error is uncorrectable if all errors occur in the same
> scrub_sector in the scrub_block.
>
> 2. For raid6, if more than 2 stripts are damaged and the error
> cannot be repaired, a batch read error will also be reported.
>
> Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
> ---
> fs/btrfs/scrub.c | 385 ++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 210 insertions(+), 175 deletions(-)
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 196c4c6..5ca9f43 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -72,6 +72,7 @@ struct scrub_sector {
> atomic_t refs;
> unsigned int have_csum:1;
> unsigned int io_error:1;
> + unsigned int checksum_error:1;
> u8 csum[BTRFS_CSUM_SIZE];
>
> struct scrub_recover *recover;
> @@ -252,6 +253,7 @@ static void detach_scrub_page_private(struct page *page)
> #endif
> }
>
> +
> static struct scrub_block *alloc_scrub_block(struct scrub_ctx *sctx,
> struct btrfs_device *dev,
> u64 logical, u64 physical,
> @@ -404,7 +406,7 @@ static int scrub_write_sector_to_dev_replace(struct scrub_block *sblock,
> static void scrub_parity_put(struct scrub_parity *sparity);
> static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
> u64 physical, struct btrfs_device *dev, u64 flags,
> - u64 gen, int mirror_num, u8 *csum,
> + u64 gen, int mirror_num,
> u64 physical_for_dev_replace);
> static void scrub_bio_end_io(struct bio *bio);
> static void scrub_bio_end_io_worker(struct work_struct *work);
> @@ -420,6 +422,10 @@ static int scrub_add_sector_to_wr_bio(struct scrub_ctx *sctx,
> static void scrub_wr_bio_end_io(struct bio *bio);
> static void scrub_wr_bio_end_io_worker(struct work_struct *work);
> static void scrub_put_ctx(struct scrub_ctx *sctx);
> +static int scrub_find_btrfs_ordered_sum(struct scrub_ctx *sctx, u64 logical,
> + struct btrfs_ordered_sum **order_sum);
> +static int scrub_get_sblock_checksum_error(struct scrub_block *sblock);
> +
>
> static inline int scrub_is_page_on_raid56(struct scrub_sector *sector)
> {
> @@ -991,19 +997,18 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> struct btrfs_fs_info *fs_info;
> u64 logical;
> unsigned int failed_mirror_index;
> - unsigned int is_metadata;
> - unsigned int have_csum;
> /* One scrub_block for each mirror */
> struct scrub_block *sblocks_for_recheck[BTRFS_MAX_MIRRORS] = { 0 };
> struct scrub_block *sblock_bad;
> int ret;
> int mirror_index;
> int sector_num;
> - int success;
> bool full_stripe_locked;
> unsigned int nofs_flag;
> static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
> DEFAULT_RATELIMIT_BURST);
> + int correct_error = 0;
> + int uncorrect_error = 0;
>
> BUG_ON(sblock_to_check->sector_count < 1);
> fs_info = sctx->fs_info;
> @@ -1023,9 +1028,6 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> logical = sblock_to_check->logical;
> ASSERT(sblock_to_check->mirror_num);
> failed_mirror_index = sblock_to_check->mirror_num - 1;
> - is_metadata = !(sblock_to_check->sectors[0]->flags &
> - BTRFS_EXTENT_FLAG_DATA);
> - have_csum = sblock_to_check->sectors[0]->have_csum;
>
> if (!sctx->is_dev_replace && btrfs_repair_one_zone(fs_info, logical))
> return 0;
> @@ -1054,7 +1056,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> if (ret == -ENOMEM)
> sctx->stat.malloc_errors++;
> sctx->stat.read_errors++;
> - sctx->stat.uncorrectable_errors++;
> + sctx->stat.uncorrectable_errors += scrub_get_sblock_checksum_error(sblock_to_check);
> + sctx->stat.uncorrectable_errors += sblock_to_check->header_error;
> spin_unlock(&sctx->stat_lock);
> return ret;
> }
> @@ -1104,7 +1107,10 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> spin_lock(&sctx->stat_lock);
> sctx->stat.malloc_errors++;
> sctx->stat.read_errors++;
> - sctx->stat.uncorrectable_errors++;
> + sctx->stat.uncorrectable_errors +=
> + scrub_get_sblock_checksum_error(sblock_to_check);
> + sctx->stat.uncorrectable_errors +=
> + sblock_to_check->header_error;
> spin_unlock(&sctx->stat_lock);
> btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
> goto out;
> @@ -1116,7 +1122,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> if (ret) {
> spin_lock(&sctx->stat_lock);
> sctx->stat.read_errors++;
> - sctx->stat.uncorrectable_errors++;
> + sctx->stat.uncorrectable_errors += scrub_get_sblock_checksum_error(sblock_to_check);
> + sctx->stat.uncorrectable_errors += sblock_to_check->header_error;
> spin_unlock(&sctx->stat_lock);
> btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
> goto out;
> @@ -1138,7 +1145,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> * the cause)
> */
> spin_lock(&sctx->stat_lock);
> - sctx->stat.unverified_errors++;
> + sctx->stat.unverified_errors += scrub_get_sblock_checksum_error(sblock_to_check);
> + sctx->stat.unverified_errors += sblock_to_check->header_error;
> sblock_to_check->data_corrected = 1;
> spin_unlock(&sctx->stat_lock);
>
> @@ -1147,22 +1155,7 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> goto out;
> }
>
> - if (!sblock_bad->no_io_error_seen) {
> - spin_lock(&sctx->stat_lock);
> - sctx->stat.read_errors++;
> - spin_unlock(&sctx->stat_lock);
> - if (__ratelimit(&rs))
> - scrub_print_warning("i/o error", sblock_to_check);
> - btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
> - } else if (sblock_bad->checksum_error) {
> - spin_lock(&sctx->stat_lock);
> - sctx->stat.csum_errors++;
> - spin_unlock(&sctx->stat_lock);
> - if (__ratelimit(&rs))
> - scrub_print_warning("checksum error", sblock_to_check);
> - btrfs_dev_stat_inc_and_print(dev,
> - BTRFS_DEV_STAT_CORRUPTION_ERRS);
> - } else if (sblock_bad->header_error) {
> + if (sblock_to_check->header_error && sblock_bad->header_error) {
> spin_lock(&sctx->stat_lock);
> sctx->stat.verify_errors++;
> spin_unlock(&sctx->stat_lock);
> @@ -1175,8 +1168,48 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> else
> btrfs_dev_stat_inc_and_print(dev,
> BTRFS_DEV_STAT_CORRUPTION_ERRS);
> + } else if (sblock_to_check->header_error && !sblock_bad->header_error) {
> + spin_lock(&sctx->stat_lock);
> + sctx->stat.unverified_errors++;
> + spin_unlock(&sctx->stat_lock);
> + } else if (!sblock_to_check->header_error && sblock_bad->header_error) {
> + sblock_bad->header_error = 0;
> + } else {
> + for (sector_num = 0; sector_num < sblock_bad->sector_count; sector_num++) {
> + struct scrub_sector *bad_sector = sblock_bad->sectors[sector_num];
> + struct scrub_sector *check_sector = sblock_to_check->sectors[sector_num];
> +
> + if (bad_sector->io_error || check_sector->io_error) {
> + spin_lock(&sctx->stat_lock);
> + sctx->stat.read_errors++;
> + spin_unlock(&sctx->stat_lock);
> + if (__ratelimit(&rs))
> + scrub_print_warning("i/o error", sblock_to_check);
> + btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
> + } else if (check_sector->checksum_error && bad_sector->checksum_error) {
> + spin_lock(&sctx->stat_lock);
> + sctx->stat.csum_errors++;
> + spin_unlock(&sctx->stat_lock);
> + if (__ratelimit(&rs))
> + scrub_print_warning("checksum error", sblock_to_check);
> + btrfs_dev_stat_inc_and_print(dev,
> + BTRFS_DEV_STAT_CORRUPTION_ERRS);
> +
> + } else if (check_sector->checksum_error && !bad_sector->checksum_error) {
> + spin_lock(&sctx->stat_lock);
> + sctx->stat.unverified_errors++;
> + spin_unlock(&sctx->stat_lock);
> + } else if (!check_sector->checksum_error && bad_sector->checksum_error) {
> + struct scrub_sector *temp_sector = sblock_bad->sectors[sector_num];
> +
> + sblock_bad->sectors[sector_num]
> + = sblock_to_check->sectors[sector_num];
> + sblock_to_check->sectors[sector_num] = temp_sector;
> + }
> + }
> }
>
> +
> if (sctx->readonly) {
> ASSERT(!sctx->is_dev_replace);
> goto out;
> @@ -1233,18 +1266,23 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> sblock_other->no_io_error_seen) {
> if (sctx->is_dev_replace) {
> scrub_write_block_to_dev_replace(sblock_other);
> - goto corrected_error;
> + correct_error += scrub_get_sblock_checksum_error(sblock_bad);
> + correct_error += sblock_bad->header_error;
> + goto error_summary;
> } else {
> ret = scrub_repair_block_from_good_copy(
> sblock_bad, sblock_other);
> - if (!ret)
> - goto corrected_error;
> + if (!ret) {
> + correct_error +=
> + scrub_get_sblock_checksum_error(sblock_bad);
> + correct_error += sblock_bad->header_error;
> + goto error_summary;
> + }
> }
> }
> }
>
> - if (sblock_bad->no_io_error_seen && !sctx->is_dev_replace)
> - goto did_not_correct_error;
> +
>
> /*
> * In case of I/O errors in the area that is supposed to be
> @@ -1270,17 +1308,16 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> * mirror, even if other 512 byte sectors in the same sectorsize
> * area are unreadable.
> */
> - success = 1;
> for (sector_num = 0; sector_num < sblock_bad->sector_count;
> sector_num++) {
> struct scrub_sector *sector_bad = sblock_bad->sectors[sector_num];
> struct scrub_block *sblock_other = NULL;
>
> /* Skip no-io-error sectors in scrub */
> - if (!sector_bad->io_error && !sctx->is_dev_replace)
> + if (!(sector_bad->io_error || sector_bad->checksum_error) && !sctx->is_dev_replace)
> continue;
>
> - if (scrub_is_page_on_raid56(sblock_bad->sectors[0])) {
> + if (scrub_is_page_on_raid56(sector_bad)) {
> /*
> * In case of dev replace, if raid56 rebuild process
> * didn't work out correct data, then copy the content
> @@ -1289,6 +1326,7 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> * sblock_for_recheck array to target device.
> */
> sblock_other = NULL;
> + uncorrect_error++;
> } else if (sector_bad->io_error) {
> /* Try to find no-io-error sector in mirrors */
> for (mirror_index = 0;
> @@ -1302,7 +1340,21 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> }
> }
> if (!sblock_other)
> - success = 0;
> + uncorrect_error++;
> + } else if (sector_bad->checksum_error) {
> + for (mirror_index = 0;
> + mirror_index < BTRFS_MAX_MIRRORS &&
> + sblocks_for_recheck[mirror_index]->sector_count > 0;
> + mirror_index++) {
> + if (!sblocks_for_recheck[mirror_index]->sectors[sector_num]->io_error &&
> + !sblocks_for_recheck[mirror_index]->sectors[sector_num]->checksum_error) {
> + sblock_other = sblocks_for_recheck[mirror_index];
> + break;
> + }
> + }
> + if (!sblock_other) {
> + uncorrect_error++;
> + }
> }
>
> if (sctx->is_dev_replace) {
> @@ -1319,56 +1371,28 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> sector_num) != 0) {
> atomic64_inc(
> &fs_info->dev_replace.num_write_errors);
> - success = 0;
> }
> } else if (sblock_other) {
> ret = scrub_repair_sector_from_good_copy(sblock_bad,
> sblock_other,
> sector_num, 0);
> - if (0 == ret)
> + if (0 == ret && sector_bad->io_error) {
> + correct_error++;
> sector_bad->io_error = 0;
> - else
> - success = 0;
> + } else if (0 == ret && sector_bad->checksum_error) {
> + correct_error++;
> + sector_bad->checksum_error = 0;
> + } else {
> + uncorrect_error++;
> + }
> }
> }
>
> - if (success && !sctx->is_dev_replace) {
> - if (is_metadata || have_csum) {
> - /*
> - * need to verify the checksum now that all
> - * sectors on disk are repaired (the write
> - * request for data to be repaired is on its way).
> - * Just be lazy and use scrub_recheck_block()
> - * which re-reads the data before the checksum
> - * is verified, but most likely the data comes out
> - * of the page cache.
> - */
> - scrub_recheck_block(fs_info, sblock_bad, 1);
> - if (!sblock_bad->header_error &&
> - !sblock_bad->checksum_error &&
> - sblock_bad->no_io_error_seen)
> - goto corrected_error;
> - else
> - goto did_not_correct_error;
> - } else {
> -corrected_error:
> - spin_lock(&sctx->stat_lock);
> - sctx->stat.corrected_errors++;
> - sblock_to_check->data_corrected = 1;
> - spin_unlock(&sctx->stat_lock);
> - btrfs_err_rl_in_rcu(fs_info,
> - "fixed up error at logical %llu on dev %s",
> - logical, rcu_str_deref(dev->name));
> - }
> - } else {
> -did_not_correct_error:
> - spin_lock(&sctx->stat_lock);
> - sctx->stat.uncorrectable_errors++;
> - spin_unlock(&sctx->stat_lock);
> - btrfs_err_rl_in_rcu(fs_info,
> - "unable to fixup (regular) error at logical %llu on dev %s",
> - logical, rcu_str_deref(dev->name));
> - }
> +error_summary:
> + spin_lock(&sctx->stat_lock);
> + sctx->stat.uncorrectable_errors += uncorrect_error;
> + sctx->stat.corrected_errors += correct_error;
> + spin_unlock(&sctx->stat_lock);
>
> out:
> for (mirror_index = 0; mirror_index < BTRFS_MAX_MIRRORS; mirror_index++) {
> @@ -1513,10 +1537,10 @@ static int scrub_setup_recheck_block(struct scrub_block *original_sblock,
> }
> sector->flags = flags;
> sector->generation = generation;
> - sector->have_csum = have_csum;
> + sector->have_csum = original_sblock->sectors[sector_index]->have_csum;
> if (have_csum)
> memcpy(sector->csum,
> - original_sblock->sectors[0]->csum,
> + original_sblock->sectors[sector_index]->csum,
> sctx->fs_info->csum_size);
>
> scrub_stripe_index_and_offset(logical,
> @@ -1688,11 +1712,15 @@ static int scrub_repair_block_from_good_copy(struct scrub_block *sblock_bad,
>
> for (i = 0; i < sblock_bad->sector_count; i++) {
> int ret_sub;
> -
> - ret_sub = scrub_repair_sector_from_good_copy(sblock_bad,
> + if (sblock_bad->sectors[i]->checksum_error == 1
> + && sblock_good->sectors[i]->checksum_error == 0){
> + ret_sub = scrub_repair_sector_from_good_copy(sblock_bad,
> sblock_good, i, 1);
> - if (ret_sub)
> - ret = ret_sub;
> + if (ret_sub)
> + ret = ret_sub;
> + } else if (sblock_bad->sectors[i]->checksum_error == 1) {
> + ret = 1;
> + }
> }
>
> return ret;
> @@ -1984,22 +2012,46 @@ static int scrub_checksum_data(struct scrub_block *sblock)
> u8 csum[BTRFS_CSUM_SIZE];
> struct scrub_sector *sector;
> char *kaddr;
> + int i;
> + int io_error = 0;
>
> BUG_ON(sblock->sector_count < 1);
> - sector = sblock->sectors[0];
> - if (!sector->have_csum)
> - return 0;
> -
> - kaddr = scrub_sector_get_kaddr(sector);
>
> shash->tfm = fs_info->csum_shash;
> crypto_shash_init(shash);
> + for (i = 0; i < sblock->sector_count; i++) {
> + sector = sblock->sectors[i];
> + if (sector->io_error == 1) {
> + io_error = 1;
> + continue;
> + }
> + if (!sector->have_csum)
> + continue;
>
> - crypto_shash_digest(shash, kaddr, fs_info->sectorsize, csum);
> + kaddr = scrub_sector_get_kaddr(sector);
> + crypto_shash_digest(shash, kaddr, fs_info->sectorsize, csum);
> + if (memcmp(csum, sector->csum, fs_info->csum_size)) {
> + sector->checksum_error = 1;
> + sblock->checksum_error = 1;
> + } else {
> + sector->checksum_error = 0;
> + }
> + }
> + return sblock->checksum_error | io_error;
> +}
>
> - if (memcmp(csum, sector->csum, fs_info->csum_size))
> - sblock->checksum_error = 1;
> - return sblock->checksum_error;
> +static int scrub_get_sblock_checksum_error(struct scrub_block *sblock)
> +{
> + int count = 0;
> + int i;
> +
> + if (sblock == NULL)
> + return count;
> + for (i = 0; i < sblock->sector_count; i++) {
> + if (sblock->sectors[i]->checksum_error == 1)
> + count++;
> + }
> + return count;
> }
>
> static int scrub_checksum_tree_block(struct scrub_block *sblock)
> @@ -2062,8 +2114,12 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
> }
>
> crypto_shash_final(shash, calculated_csum);
> - if (memcmp(calculated_csum, on_disk_csum, sctx->fs_info->csum_size))
> + if (memcmp(calculated_csum, on_disk_csum, sctx->fs_info->csum_size)) {
> + sblock->sectors[0]->checksum_error = 1;
> sblock->checksum_error = 1;
> + } else {
> + sblock->sectors[0]->checksum_error = 0;
> + }
>
> return sblock->header_error || sblock->checksum_error;
> }
> @@ -2400,12 +2456,14 @@ static void scrub_missing_raid56_pages(struct scrub_block *sblock)
>
> static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
> u64 physical, struct btrfs_device *dev, u64 flags,
> - u64 gen, int mirror_num, u8 *csum,
> + u64 gen, int mirror_num,
> u64 physical_for_dev_replace)
> {
> struct scrub_block *sblock;
> const u32 sectorsize = sctx->fs_info->sectorsize;
> int index;
> + int have_csum;
> + struct btrfs_ordered_sum *order_sum = NULL;
>
> sblock = alloc_scrub_block(sctx, dev, logical, physical,
> physical_for_dev_replace, mirror_num);
> @@ -2415,7 +2473,6 @@ static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
> spin_unlock(&sctx->stat_lock);
> return -ENOMEM;
> }
> -
> for (index = 0; len > 0; index++) {
> struct scrub_sector *sector;
> /*
> @@ -2424,7 +2481,6 @@ static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
> * more memory for PAGE_SIZE > sectorsize case.
> */
> u32 l = min(sectorsize, len);
> -
> sector = alloc_scrub_sector(sblock, logical, GFP_KERNEL);
> if (!sector) {
> spin_lock(&sctx->stat_lock);
> @@ -2435,11 +2491,25 @@ static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
> }
> sector->flags = flags;
> sector->generation = gen;
> - if (csum) {
> - sector->have_csum = 1;
> - memcpy(sector->csum, csum, sctx->fs_info->csum_size);
> - } else {
> - sector->have_csum = 0;
> + if (flags & BTRFS_EXTENT_FLAG_DATA) {
> + if (order_sum == NULL ||
> + (order_sum->bytenr + order_sum->len <= logical)) {
> + order_sum = NULL;
> + have_csum = scrub_find_btrfs_ordered_sum(sctx, logical, &order_sum);
> + }
> + if (have_csum == 0) {
> + ++sctx->stat.no_csum;
> + sector->have_csum = 0;
> + } else {
> + int order_csum_index;
> +
> + sector->have_csum = 1;
> + order_csum_index = (logical-order_sum->bytenr)
> + >> sctx->fs_info->sectorsize_bits;
> + memcpy(sector->csum,
> + order_sum->sums + order_csum_index * sctx->fs_info->csum_size,
> + sctx->fs_info->csum_size);
> + }
> }
> len -= l;
> logical += l;
> @@ -2571,7 +2641,8 @@ static void scrub_block_complete(struct scrub_block *sblock)
> {
> int corrupted = 0;
>
> - if (!sblock->no_io_error_seen) {
> + if (!sblock->no_io_error_seen && !(sblock->sector_count > 0
> + && (sblock->sectors[0]->flags & BTRFS_EXTENT_FLAG_DATA))) {
> corrupted = 1;
> scrub_handle_errored_block(sblock);
> } else {
> @@ -2597,61 +2668,30 @@ static void scrub_block_complete(struct scrub_block *sblock)
> }
> }
>
> -static void drop_csum_range(struct scrub_ctx *sctx, struct btrfs_ordered_sum *sum)
> -{
> - sctx->stat.csum_discards += sum->len >> sctx->fs_info->sectorsize_bits;
> - list_del(&sum->list);
> - kfree(sum);
> -}
> -
> /*
> - * Find the desired csum for range [logical, logical + sectorsize), and store
> - * the csum into @csum.
> + * Find the desired btrfs_ordered_sum contain address logical, and store
> + * the result into @order_sum.
> *
> * The search source is sctx->csum_list, which is a pre-populated list
> - * storing bytenr ordered csum ranges. We're responsible to cleanup any range
> - * that is before @logical.
> + * storing bytenr ordered csum ranges.
> *
> - * Return 0 if there is no csum for the range.
> - * Return 1 if there is csum for the range and copied to @csum.
> + * Return 0 if there is no btrfs_ordered_sum contain the address logical.
> + * Return 1 if there is btrfs_order_sum contain the address logincal and copied to @order_sum.
> */
> -static int scrub_find_csum(struct scrub_ctx *sctx, u64 logical, u8 *csum)
> +static int scrub_find_btrfs_ordered_sum(struct scrub_ctx *sctx, u64 logical,
> + struct btrfs_ordered_sum **order_sum)
> {
> bool found = false;
> + struct btrfs_ordered_sum *sum;
>
> - while (!list_empty(&sctx->csum_list)) {
> - struct btrfs_ordered_sum *sum = NULL;
> - unsigned long index;
> - unsigned long num_sectors;
> -
> - sum = list_first_entry(&sctx->csum_list,
> - struct btrfs_ordered_sum, list);
> - /* The current csum range is beyond our range, no csum found */
> + list_for_each_entry(sum, &sctx->csum_list, list) {
> + /* no btrfs_ordered_sum found */
> if (sum->bytenr > logical)
> break;
> -
> - /*
> - * The current sum is before our bytenr, since scrub is always
> - * done in bytenr order, the csum will never be used anymore,
> - * clean it up so that later calls won't bother with the range,
> - * and continue search the next range.
> - */
> - if (sum->bytenr + sum->len <= logical) {
> - drop_csum_range(sctx, sum);
> + if (sum->bytenr + sum->len <= logical)
> continue;
> - }
> -
> - /* Now the csum range covers our bytenr, copy the csum */
> found = true;
> - index = (logical - sum->bytenr) >> sctx->fs_info->sectorsize_bits;
> - num_sectors = sum->len >> sctx->fs_info->sectorsize_bits;
> -
> - memcpy(csum, sum->sums + index * sctx->fs_info->csum_size,
> - sctx->fs_info->csum_size);
> -
> - /* Cleanup the range if we're at the end of the csum range */
> - if (index == num_sectors - 1)
> - drop_csum_range(sctx, sum);
> + *order_sum = sum;
> break;
> }
> if (!found)
> @@ -2669,7 +2709,6 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
> u64 src_physical = physical;
> int src_mirror = mirror_num;
> int ret;
> - u8 csum[BTRFS_CSUM_SIZE];
> u32 blocksize;
>
> if (flags & BTRFS_EXTENT_FLAG_DATA) {
> @@ -2685,7 +2724,7 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
> if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK)
> blocksize = map->stripe_len;
> else
> - blocksize = sctx->fs_info->nodesize;
> + blocksize = BTRFS_STRIPE_LEN;
> spin_lock(&sctx->stat_lock);
> sctx->stat.tree_extents_scrubbed++;
> sctx->stat.tree_bytes_scrubbed += len;
> @@ -2709,17 +2748,9 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
> &src_dev, &src_mirror);
> while (len) {
> u32 l = min(len, blocksize);
> - int have_csum = 0;
> -
> - if (flags & BTRFS_EXTENT_FLAG_DATA) {
> - /* push csums to sbio */
> - have_csum = scrub_find_csum(sctx, logical, csum);
> - if (have_csum == 0)
> - ++sctx->stat.no_csum;
> - }
> ret = scrub_sectors(sctx, logical, l, src_physical, src_dev,
> flags, gen, src_mirror,
> - have_csum ? csum : NULL, physical);
> + physical);
> if (ret)
> return ret;
> len -= l;
> @@ -2733,12 +2764,14 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
> static int scrub_sectors_for_parity(struct scrub_parity *sparity,
> u64 logical, u32 len,
> u64 physical, struct btrfs_device *dev,
> - u64 flags, u64 gen, int mirror_num, u8 *csum)
> + u64 flags, u64 gen, int mirror_num)
> {
> struct scrub_ctx *sctx = sparity->sctx;
> struct scrub_block *sblock;
> const u32 sectorsize = sctx->fs_info->sectorsize;
> int index;
> + struct btrfs_ordered_sum *order_sum = NULL;
> + int have_csum;
>
> ASSERT(IS_ALIGNED(len, sectorsize));
>
> @@ -2770,11 +2803,24 @@ static int scrub_sectors_for_parity(struct scrub_parity *sparity,
> list_add_tail(§or->list, &sparity->sectors_list);
> sector->flags = flags;
> sector->generation = gen;
> - if (csum) {
> - sector->have_csum = 1;
> - memcpy(sector->csum, csum, sctx->fs_info->csum_size);
> - } else {
> - sector->have_csum = 0;
> + if (flags & BTRFS_EXTENT_FLAG_DATA) {
> + if (order_sum == NULL
> + || (order_sum->bytenr + order_sum->len <= logical)) {
> + order_sum = NULL;
> + have_csum = scrub_find_btrfs_ordered_sum(sctx, logical, &order_sum);
> + }
> + if (have_csum == 0) {
> + sector->have_csum = 0;
> + } else {
> + int order_csum_index;
> +
> + sector->have_csum = 1;
> + order_csum_index = (logical-order_sum->bytenr)
> + >> sctx->fs_info->sectorsize_bits;
> + memcpy(sector->csum,
> + order_sum->sums + order_csum_index * sctx->fs_info->csum_size,
> + sctx->fs_info->csum_size);
> + }
> }
>
> /* Iterate over the stripe range in sectorsize steps */
> @@ -2807,7 +2853,6 @@ static int scrub_extent_for_parity(struct scrub_parity *sparity,
> {
> struct scrub_ctx *sctx = sparity->sctx;
> int ret;
> - u8 csum[BTRFS_CSUM_SIZE];
> u32 blocksize;
>
> if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state)) {
> @@ -2826,20 +2871,10 @@ static int scrub_extent_for_parity(struct scrub_parity *sparity,
>
> while (len) {
> u32 l = min(len, blocksize);
> - int have_csum = 0;
> -
> - if (flags & BTRFS_EXTENT_FLAG_DATA) {
> - /* push csums to sbio */
> - have_csum = scrub_find_csum(sctx, logical, csum);
> - if (have_csum == 0)
> - goto skip;
> - }
> ret = scrub_sectors_for_parity(sparity, logical, l, physical, dev,
> - flags, gen, mirror_num,
> - have_csum ? csum : NULL);
> + flags, gen, mirror_num);
> if (ret)
> return ret;
> -skip:
> len -= l;
> logical += l;
> physical += l;
> @@ -4148,7 +4183,7 @@ static noinline_for_stack int scrub_supers(struct scrub_ctx *sctx,
>
> ret = scrub_sectors(sctx, bytenr, BTRFS_SUPER_INFO_SIZE, bytenr,
> scrub_dev, BTRFS_EXTENT_FLAG_SUPER, gen, i,
> - NULL, bytenr);
> + bytenr);
> if (ret)
> return ret;
> }
> --
> 1.8.3.1
>
[-- Attachment #2: raid1test --]
[-- Type: application/octet-stream, Size: 3786 bytes --]
#! /bin/bash
# SPDX-License-Identifier: GPL-2.0
# Copyright (c) 2022 YOUR NAME HERE. All Rights Reserved.
#
# FS QA Test 278-scrub
#
# what am I here for?
#
. ./common/preamble
_begin_fstest auto quick scrub
. ./common/filter
_supported_fs btrfs
_require_scratch_dev_pool 4
_require_btrfs_support_sectorsize 4096
_require_non_zoned_device "${SCRATCH_DEV}"
# real QA test starts here
# Modify as appropriate.
declare -a TEST_VECTORS_WITHMIRROR=(
"2:raid1:raid1"
"3:raid1c3:raid1c3"
"4:raid1c4:raid1c4"
)
run_correct_testcase() {
IFS=':' read -ra args <<< $1
num_disks=${args[0]}
metadata_type=${args[1]}
data_type=${args[2]}
_scratch_dev_pool_get $num_disks
_scratch_pool_mkfs "-m$metadata_type -d$data_type -s 4k" >> $seqres.full 2>&1
_scratch_mount
# Create a data extent with 2 sectors
$XFS_IO_PROG -fc "pwrite -S 0xaa 0 8k" $SCRATCH_MNT/foobar >> $seqres.full 2>&1
sync
first_logical=$(_btrfs_get_first_logical $SCRATCH_MNT/foobar)
echo "logical of the first sector: $first_logical" >> $seqres.full
second_logical=$(( $first_logical + 4096 ))
echo "logical of the second sector: $second_logical" >> $seqres.full
first_physical=$(_btrfs_get_physical $first_logical 2)
echo "physical of the second sector: $first_physical" >> $seqres.full
second_physical=$(_btrfs_get_physical $second_logical 1)
echo "physical of the second sector: $second_physical" >> $seqres.full
second_dev=$(_btrfs_get_device_path $second_logical 1)
first_dev=$(_btrfs_get_device_path $first_logical 2)
echo $first_dev>>$seqres.full
echo $second_dev>>$seqres.full
_scratch_unmount
$XFS_IO_PROG -c "pwrite -S 0x00 $second_physical 4k" $second_dev >> $seqres.full
$XFS_IO_PROG -c "pwrite -S 0x00 $first_physical 4k" $first_dev >> $seqres.full
_scratch_mount
$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT &> $tmp.output
cat $tmp.output >> $seqres.full
_scratch_unmount
grep "Corrected:" $tmp.output | grep "2" >> $seqres.full
if ! grep -q "csum=2" $tmp.output ; then
echo "Scrub failed to detect corruption"
fi
if ! grep -q "Corrected:.*2" $tmp.output ; then
echo "Scrub failed to correct corruption"
fi
_scratch_dev_pool_put
}
run_uncorrect_testcase() {
IFS=':' read -ra args <<< $1
num_disks=${args[0]}
metadata_type=${args[1]}
data_type=${args[2]}
_scratch_dev_pool_get $num_disks
_scratch_pool_mkfs "-m$metadata_type -d$data_type -s 4k" >> $seqres.full 2>&1
_scratch_mount
# Create a data extent with 2 sectors
$XFS_IO_PROG -fc "pwrite -S 0xaa 0 8k" $SCRATCH_MNT/foobar >> $seqres.full 2>&1
sync
first_logical=$(_btrfs_get_first_logical $SCRATCH_MNT/foobar)
echo "logical of the first sector: $first_logical" >> $seqres.full
second_logical=$(( $first_logical + 4096 ))
echo "logical of the second sector: $second_logical" >> $seqres.full
disk_num=`expr 0+$num_disks`
for ((i=0; i<$disk_num; i ++))
do
physical=$(_btrfs_get_physical $second_logical $[$i+1])
echo "physical of the second sector: $physical" >> $seqres.full
dev=$(_btrfs_get_device_path $second_logical $[$i+1])
echo $dev>>$seqres.full
_scratch_unmount
$XFS_IO_PROG -c "pwrite -S 0x00 $physical 4k" $dev >> $seqres.full
_scratch_mount
done
$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT &> $tmp.output
cat $tmp.output >> $seqres.full
_scratch_unmount
if ! grep -q "csum=$num_disks" $tmp.output ; then
echo "Scrub failed to detect corruption"
fi
if ! grep -q "Uncorrectable:.*$num_disks" $tmp.output ; then
echo "Scrub failed on uncorrect corruption"
fi
_scratch_dev_pool_put
}
#correctable checksum error raid1
for i in "${TEST_VECTORS_WITHMIRROR[@]}"; do
run_correct_testcase $i
done
#uncorrectable checksum error raid1
for i in "${TEST_VECTORS_WITHMIRROR[@]}"; do
run_uncorrect_testcase $i
done
echo "Silence is golden"
status=0
exit
[-- Attachment #3: raid6test --]
[-- Type: application/octet-stream, Size: 3211 bytes --]
#! /bin/bash
# SPDX-License-Identifier: GPL-2.0
# Copyright (c) 2017 Oracle. All Rights Reserved.
#
# FS QA Test 158
#
# The test case is check if scrub is able fix raid6 data corruption,
# ie. if there is data corruption on two disks in the same horizontal
# stripe, e.g. due to bitrot.
#
# The kernel fixes are
# Btrfs: make raid6 rebuild retry more
# Btrfs: fix scrub to repair raid6 corruption
#
. ./common/preamble
_begin_fstest auto quick raid scrub
# Import common functions.
. ./common/filter
# real QA test starts here
# Modify as appropriate.
_supported_fs btrfs
_require_scratch_dev_pool 4
_require_btrfs_command inspect-internal dump-tree
_require_btrfs_fs_feature raid56
get_physical()
{
local stripe=$1
$BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
grep " DATA\|RAID6" -A 10 | \
$AWK_PROG "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$6 }"
}
get_devid()
{
local stripe=$1
$BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
grep " DATA\|RAID6" -A 10 | \
$AWK_PROG "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$4 }"
}
get_device_path()
{
local devid=$1
echo "$SCRATCH_DEV_POOL" | $AWK_PROG "{print \$$devid}"
}
_scratch_dev_pool_get 4
# step 1: create a raid6 btrfs and create a 128K file
echo "step 1......mkfs.btrfs" >>$seqres.full
_check_minimal_fs_size $(( 1024 * 1024 * 1024 ))
mkfs_opts="-d raid6 -b 1G"
_scratch_pool_mkfs $mkfs_opts >>$seqres.full 2>&1
# make sure data is written to the start position of the data chunk
_scratch_mount $(_btrfs_no_v1_cache_opt)
# [0,64K) is written to stripe 0 and [64K, 128K) is written to stripe 1
$XFS_IO_PROG -f -d -c "pwrite -S 0xaa 0 128K" -c "fsync" \
"$SCRATCH_MNT/foobar" >> $seqres.full
_scratch_unmount
phy0=$(get_physical 0)
devid0=$(get_devid 0)
devpath0=$(get_device_path $devid0)
phy1=$(get_physical 1)
devid1=$(get_devid 1)
devpath1=$(get_device_path $devid1)
phy2=$(get_physical 2)
devid2=$(get_devid 2)
devpath2=$(get_device_path $devid2)
# step 2: corrupt the 1st and 2nd stripe (stripe 0 and 1)
echo "step 2......simulate bitrot at:" >>$seqres.full
echo " ......stripe #0: devid $devid0 devpath $devpath0 phy $phy0" \
>>$seqres.full
echo " ......stripe #1: devid $devid1 devpath $devpath1 phy $phy1" \
>>$seqres.full
echo " ......stripe #2: devid $devid2 devpath $devpath2 phy $phy2" \
>>$seqres.full
$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $phy0 64K" $devpath0 > /dev/null
$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $phy1 64K" $devpath1 > /dev/null
$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $phy2 64K" $devpath2 > /dev/null
# step 3: scrub filesystem to repair the bitrot
echo "step 3......repair the bitrot" >> $seqres.full
_scratch_mount $(_btrfs_no_v1_cache_opt)
$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT > $tmp.output 2>&1
cat $tmp.output >> $seqres.full
if ! grep -q "csum=96" $tmp.output ; then
echo "Scrub failed to detect corruption"
fi
if ! grep -q "read=32" $tmp.output ; then
echo "Scrub failed to detect corruption"
fi
if ! grep -q "Uncorrectable:.*128" $tmp.output ; then
echo "Scrub failed on uncorrect corruption"
fi
_scratch_dev_pool_put
# success, all done
echo "Silence is golden"
status=0
exit
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: scrub: expand scrub block size for data range scrub
2022-11-13 15:37 ` li zhang
@ 2022-11-13 22:37 ` Qu Wenruo
0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2022-11-13 22:37 UTC (permalink / raw)
To: li zhang, linux-btrfs
On 2022/11/13 23:37, li zhang wrote:
> Here is my test script.
Top posting is not recommended for most Linux community IIRC.
And if you're resuing some fstests, just mention the test case naming.
If you're enhancing the existing test case, please submit a proper patch.
Thanks,
Qu
>
> Li Zhang <zhanglikernel@gmail.com> 于2022年11月13日周日 23:36写道：
>>
>> [implement]
>> 1. Add the member checksum_error to the scrub_sector,
>> which indicates the checksum error of the sector
>>
>> 2. Use scrub_find_btrfs_ordered_sum to find the desired
>> btrfs_ordered_sum containing address logic, in
>> scrub_sectors_for_parity and scrub_sectors, call
>> Scrub_find_btrfs_ordered_sum finds the
>> btrfs_ordered_sum containing the current logical address, and
>> Calculate the exact checksum later.
>>
>> 3. In the scrub_checksum_data function,
>> we should check all sectors in the scrub_block.
>>
>> 4. The job of the scrub_handle_errored_block
>> function is to count the number of error and
>> repair sectors if they can be repaired.
>>
>> The function enters the wrong scrub_block, and
>> the overall process is as follows
>>
>> 1) Check the scrub_block again, check again if the error is gone.
>>
>> 2) Check the corresponding mirror scrub_block, if there is no error,
>> Fix bad sblocks with mirror scrub_block.
>>
>> 3) If no error-free scrub_block is found, repair it sector by sector.
>>
>> One difficulty with this function is rechecking the scrub_block.
>>
>> Imagine this situation, if a sector is checked the first time
>> without errors, butthe recheck returns an error. What should
>> we do, this patch only fixes the bug that the sector first
>> appeared (As in the case where the scrub_block
>> contains only one scrub_sector).
>>
>> Another reason to only handle the first error is,
>> If the device goes bad, the recheck function will report more and
>> more errors,if we want to deal with the errors in the recheck,
>> you need to recheck again and again, which may lead to
>> Stuck in scrub_handle_errored_block for a long time.
>>
>> So rechecked bug reports will only be corrected in the
>> next scrub.
>>
>> [test]
>> I designed two test scripts based on the fstest project,
>> the output is the same as the case where the scrub_block
>> contains only one scrub sector,
>>
>> 1. There are two situations in raid1,
>> raid1c3 and raid1c4. If an error occurs in
>> different sectors of the sblock, the error can be corrected.
>>
>> An error is uncorrectable if all errors occur in the same
>> scrub_sector in the scrub_block.
>>
>> 2. For raid6, if more than 2 stripts are damaged and the error
>> cannot be repaired, a batch read error will also be reported.
>>
>> Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
>> ---
>> fs/btrfs/scrub.c | 385 ++++++++++++++++++++++++++++++-------------------------
>> 1 file changed, 210 insertions(+), 175 deletions(-)
>>
>> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
>> index 196c4c6..5ca9f43 100644
>> --- a/fs/btrfs/scrub.c
>> +++ b/fs/btrfs/scrub.c
>> @@ -72,6 +72,7 @@ struct scrub_sector {
>> atomic_t refs;
>> unsigned int have_csum:1;
>> unsigned int io_error:1;
>> + unsigned int checksum_error:1;
>> u8 csum[BTRFS_CSUM_SIZE];
>>
>> struct scrub_recover *recover;
>> @@ -252,6 +253,7 @@ static void detach_scrub_page_private(struct page *page)
>> #endif
>> }
>>
>> +
>> static struct scrub_block *alloc_scrub_block(struct scrub_ctx *sctx,
>> struct btrfs_device *dev,
>> u64 logical, u64 physical,
>> @@ -404,7 +406,7 @@ static int scrub_write_sector_to_dev_replace(struct scrub_block *sblock,
>> static void scrub_parity_put(struct scrub_parity *sparity);
>> static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
>> u64 physical, struct btrfs_device *dev, u64 flags,
>> - u64 gen, int mirror_num, u8 *csum,
>> + u64 gen, int mirror_num,
>> u64 physical_for_dev_replace);
>> static void scrub_bio_end_io(struct bio *bio);
>> static void scrub_bio_end_io_worker(struct work_struct *work);
>> @@ -420,6 +422,10 @@ static int scrub_add_sector_to_wr_bio(struct scrub_ctx *sctx,
>> static void scrub_wr_bio_end_io(struct bio *bio);
>> static void scrub_wr_bio_end_io_worker(struct work_struct *work);
>> static void scrub_put_ctx(struct scrub_ctx *sctx);
>> +static int scrub_find_btrfs_ordered_sum(struct scrub_ctx *sctx, u64 logical,
>> + struct btrfs_ordered_sum **order_sum);
>> +static int scrub_get_sblock_checksum_error(struct scrub_block *sblock);
>> +
>>
>> static inline int scrub_is_page_on_raid56(struct scrub_sector *sector)
>> {
>> @@ -991,19 +997,18 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> struct btrfs_fs_info *fs_info;
>> u64 logical;
>> unsigned int failed_mirror_index;
>> - unsigned int is_metadata;
>> - unsigned int have_csum;
>> /* One scrub_block for each mirror */
>> struct scrub_block *sblocks_for_recheck[BTRFS_MAX_MIRRORS] = { 0 };
>> struct scrub_block *sblock_bad;
>> int ret;
>> int mirror_index;
>> int sector_num;
>> - int success;
>> bool full_stripe_locked;
>> unsigned int nofs_flag;
>> static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
>> DEFAULT_RATELIMIT_BURST);
>> + int correct_error = 0;
>> + int uncorrect_error = 0;
>>
>> BUG_ON(sblock_to_check->sector_count < 1);
>> fs_info = sctx->fs_info;
>> @@ -1023,9 +1028,6 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> logical = sblock_to_check->logical;
>> ASSERT(sblock_to_check->mirror_num);
>> failed_mirror_index = sblock_to_check->mirror_num - 1;
>> - is_metadata = !(sblock_to_check->sectors[0]->flags &
>> - BTRFS_EXTENT_FLAG_DATA);
>> - have_csum = sblock_to_check->sectors[0]->have_csum;
>>
>> if (!sctx->is_dev_replace && btrfs_repair_one_zone(fs_info, logical))
>> return 0;
>> @@ -1054,7 +1056,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> if (ret == -ENOMEM)
>> sctx->stat.malloc_errors++;
>> sctx->stat.read_errors++;
>> - sctx->stat.uncorrectable_errors++;
>> + sctx->stat.uncorrectable_errors += scrub_get_sblock_checksum_error(sblock_to_check);
>> + sctx->stat.uncorrectable_errors += sblock_to_check->header_error;
>> spin_unlock(&sctx->stat_lock);
>> return ret;
>> }
>> @@ -1104,7 +1107,10 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> spin_lock(&sctx->stat_lock);
>> sctx->stat.malloc_errors++;
>> sctx->stat.read_errors++;
>> - sctx->stat.uncorrectable_errors++;
>> + sctx->stat.uncorrectable_errors +=
>> + scrub_get_sblock_checksum_error(sblock_to_check);
>> + sctx->stat.uncorrectable_errors +=
>> + sblock_to_check->header_error;
>> spin_unlock(&sctx->stat_lock);
>> btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
>> goto out;
>> @@ -1116,7 +1122,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> if (ret) {
>> spin_lock(&sctx->stat_lock);
>> sctx->stat.read_errors++;
>> - sctx->stat.uncorrectable_errors++;
>> + sctx->stat.uncorrectable_errors += scrub_get_sblock_checksum_error(sblock_to_check);
>> + sctx->stat.uncorrectable_errors += sblock_to_check->header_error;
>> spin_unlock(&sctx->stat_lock);
>> btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
>> goto out;
>> @@ -1138,7 +1145,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> * the cause)
>> */
>> spin_lock(&sctx->stat_lock);
>> - sctx->stat.unverified_errors++;
>> + sctx->stat.unverified_errors += scrub_get_sblock_checksum_error(sblock_to_check);
>> + sctx->stat.unverified_errors += sblock_to_check->header_error;
>> sblock_to_check->data_corrected = 1;
>> spin_unlock(&sctx->stat_lock);
>>
>> @@ -1147,22 +1155,7 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> goto out;
>> }
>>
>> - if (!sblock_bad->no_io_error_seen) {
>> - spin_lock(&sctx->stat_lock);
>> - sctx->stat.read_errors++;
>> - spin_unlock(&sctx->stat_lock);
>> - if (__ratelimit(&rs))
>> - scrub_print_warning("i/o error", sblock_to_check);
>> - btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
>> - } else if (sblock_bad->checksum_error) {
>> - spin_lock(&sctx->stat_lock);
>> - sctx->stat.csum_errors++;
>> - spin_unlock(&sctx->stat_lock);
>> - if (__ratelimit(&rs))
>> - scrub_print_warning("checksum error", sblock_to_check);
>> - btrfs_dev_stat_inc_and_print(dev,
>> - BTRFS_DEV_STAT_CORRUPTION_ERRS);
>> - } else if (sblock_bad->header_error) {
>> + if (sblock_to_check->header_error && sblock_bad->header_error) {
>> spin_lock(&sctx->stat_lock);
>> sctx->stat.verify_errors++;
>> spin_unlock(&sctx->stat_lock);
>> @@ -1175,8 +1168,48 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> else
>> btrfs_dev_stat_inc_and_print(dev,
>> BTRFS_DEV_STAT_CORRUPTION_ERRS);
>> + } else if (sblock_to_check->header_error && !sblock_bad->header_error) {
>> + spin_lock(&sctx->stat_lock);
>> + sctx->stat.unverified_errors++;
>> + spin_unlock(&sctx->stat_lock);
>> + } else if (!sblock_to_check->header_error && sblock_bad->header_error) {
>> + sblock_bad->header_error = 0;
>> + } else {
>> + for (sector_num = 0; sector_num < sblock_bad->sector_count; sector_num++) {
>> + struct scrub_sector *bad_sector = sblock_bad->sectors[sector_num];
>> + struct scrub_sector *check_sector = sblock_to_check->sectors[sector_num];
>> +
>> + if (bad_sector->io_error || check_sector->io_error) {
>> + spin_lock(&sctx->stat_lock);
>> + sctx->stat.read_errors++;
>> + spin_unlock(&sctx->stat_lock);
>> + if (__ratelimit(&rs))
>> + scrub_print_warning("i/o error", sblock_to_check);
>> + btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_READ_ERRS);
>> + } else if (check_sector->checksum_error && bad_sector->checksum_error) {
>> + spin_lock(&sctx->stat_lock);
>> + sctx->stat.csum_errors++;
>> + spin_unlock(&sctx->stat_lock);
>> + if (__ratelimit(&rs))
>> + scrub_print_warning("checksum error", sblock_to_check);
>> + btrfs_dev_stat_inc_and_print(dev,
>> + BTRFS_DEV_STAT_CORRUPTION_ERRS);
>> +
>> + } else if (check_sector->checksum_error && !bad_sector->checksum_error) {
>> + spin_lock(&sctx->stat_lock);
>> + sctx->stat.unverified_errors++;
>> + spin_unlock(&sctx->stat_lock);
>> + } else if (!check_sector->checksum_error && bad_sector->checksum_error) {
>> + struct scrub_sector *temp_sector = sblock_bad->sectors[sector_num];
>> +
>> + sblock_bad->sectors[sector_num]
>> + = sblock_to_check->sectors[sector_num];
>> + sblock_to_check->sectors[sector_num] = temp_sector;
>> + }
>> + }
>> }
>>
>> +
>> if (sctx->readonly) {
>> ASSERT(!sctx->is_dev_replace);
>> goto out;
>> @@ -1233,18 +1266,23 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> sblock_other->no_io_error_seen) {
>> if (sctx->is_dev_replace) {
>> scrub_write_block_to_dev_replace(sblock_other);
>> - goto corrected_error;
>> + correct_error += scrub_get_sblock_checksum_error(sblock_bad);
>> + correct_error += sblock_bad->header_error;
>> + goto error_summary;
>> } else {
>> ret = scrub_repair_block_from_good_copy(
>> sblock_bad, sblock_other);
>> - if (!ret)
>> - goto corrected_error;
>> + if (!ret) {
>> + correct_error +=
>> + scrub_get_sblock_checksum_error(sblock_bad);
>> + correct_error += sblock_bad->header_error;
>> + goto error_summary;
>> + }
>> }
>> }
>> }
>>
>> - if (sblock_bad->no_io_error_seen && !sctx->is_dev_replace)
>> - goto did_not_correct_error;
>> +
>>
>> /*
>> * In case of I/O errors in the area that is supposed to be
>> @@ -1270,17 +1308,16 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> * mirror, even if other 512 byte sectors in the same sectorsize
>> * area are unreadable.
>> */
>> - success = 1;
>> for (sector_num = 0; sector_num < sblock_bad->sector_count;
>> sector_num++) {
>> struct scrub_sector *sector_bad = sblock_bad->sectors[sector_num];
>> struct scrub_block *sblock_other = NULL;
>>
>> /* Skip no-io-error sectors in scrub */
>> - if (!sector_bad->io_error && !sctx->is_dev_replace)
>> + if (!(sector_bad->io_error || sector_bad->checksum_error) && !sctx->is_dev_replace)
>> continue;
>>
>> - if (scrub_is_page_on_raid56(sblock_bad->sectors[0])) {
>> + if (scrub_is_page_on_raid56(sector_bad)) {
>> /*
>> * In case of dev replace, if raid56 rebuild process
>> * didn't work out correct data, then copy the content
>> @@ -1289,6 +1326,7 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> * sblock_for_recheck array to target device.
>> */
>> sblock_other = NULL;
>> + uncorrect_error++;
>> } else if (sector_bad->io_error) {
>> /* Try to find no-io-error sector in mirrors */
>> for (mirror_index = 0;
>> @@ -1302,7 +1340,21 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> }
>> }
>> if (!sblock_other)
>> - success = 0;
>> + uncorrect_error++;
>> + } else if (sector_bad->checksum_error) {
>> + for (mirror_index = 0;
>> + mirror_index < BTRFS_MAX_MIRRORS &&
>> + sblocks_for_recheck[mirror_index]->sector_count > 0;
>> + mirror_index++) {
>> + if (!sblocks_for_recheck[mirror_index]->sectors[sector_num]->io_error &&
>> + !sblocks_for_recheck[mirror_index]->sectors[sector_num]->checksum_error) {
>> + sblock_other = sblocks_for_recheck[mirror_index];
>> + break;
>> + }
>> + }
>> + if (!sblock_other) {
>> + uncorrect_error++;
>> + }
>> }
>>
>> if (sctx->is_dev_replace) {
>> @@ -1319,56 +1371,28 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
>> sector_num) != 0) {
>> atomic64_inc(
>> &fs_info->dev_replace.num_write_errors);
>> - success = 0;
>> }
>> } else if (sblock_other) {
>> ret = scrub_repair_sector_from_good_copy(sblock_bad,
>> sblock_other,
>> sector_num, 0);
>> - if (0 == ret)
>> + if (0 == ret && sector_bad->io_error) {
>> + correct_error++;
>> sector_bad->io_error = 0;
>> - else
>> - success = 0;
>> + } else if (0 == ret && sector_bad->checksum_error) {
>> + correct_error++;
>> + sector_bad->checksum_error = 0;
>> + } else {
>> + uncorrect_error++;
>> + }
>> }
>> }
>>
>> - if (success && !sctx->is_dev_replace) {
>> - if (is_metadata || have_csum) {
>> - /*
>> - * need to verify the checksum now that all
>> - * sectors on disk are repaired (the write
>> - * request for data to be repaired is on its way).
>> - * Just be lazy and use scrub_recheck_block()
>> - * which re-reads the data before the checksum
>> - * is verified, but most likely the data comes out
>> - * of the page cache.
>> - */
>> - scrub_recheck_block(fs_info, sblock_bad, 1);
>> - if (!sblock_bad->header_error &&
>> - !sblock_bad->checksum_error &&
>> - sblock_bad->no_io_error_seen)
>> - goto corrected_error;
>> - else
>> - goto did_not_correct_error;
>> - } else {
>> -corrected_error:
>> - spin_lock(&sctx->stat_lock);
>> - sctx->stat.corrected_errors++;
>> - sblock_to_check->data_corrected = 1;
>> - spin_unlock(&sctx->stat_lock);
>> - btrfs_err_rl_in_rcu(fs_info,
>> - "fixed up error at logical %llu on dev %s",
>> - logical, rcu_str_deref(dev->name));
>> - }
>> - } else {
>> -did_not_correct_error:
>> - spin_lock(&sctx->stat_lock);
>> - sctx->stat.uncorrectable_errors++;
>> - spin_unlock(&sctx->stat_lock);
>> - btrfs_err_rl_in_rcu(fs_info,
>> - "unable to fixup (regular) error at logical %llu on dev %s",
>> - logical, rcu_str_deref(dev->name));
>> - }
>> +error_summary:
>> + spin_lock(&sctx->stat_lock);
>> + sctx->stat.uncorrectable_errors += uncorrect_error;
>> + sctx->stat.corrected_errors += correct_error;
>> + spin_unlock(&sctx->stat_lock);
>>
>> out:
>> for (mirror_index = 0; mirror_index < BTRFS_MAX_MIRRORS; mirror_index++) {
>> @@ -1513,10 +1537,10 @@ static int scrub_setup_recheck_block(struct scrub_block *original_sblock,
>> }
>> sector->flags = flags;
>> sector->generation = generation;
>> - sector->have_csum = have_csum;
>> + sector->have_csum = original_sblock->sectors[sector_index]->have_csum;
>> if (have_csum)
>> memcpy(sector->csum,
>> - original_sblock->sectors[0]->csum,
>> + original_sblock->sectors[sector_index]->csum,
>> sctx->fs_info->csum_size);
>>
>> scrub_stripe_index_and_offset(logical,
>> @@ -1688,11 +1712,15 @@ static int scrub_repair_block_from_good_copy(struct scrub_block *sblock_bad,
>>
>> for (i = 0; i < sblock_bad->sector_count; i++) {
>> int ret_sub;
>> -
>> - ret_sub = scrub_repair_sector_from_good_copy(sblock_bad,
>> + if (sblock_bad->sectors[i]->checksum_error == 1
>> + && sblock_good->sectors[i]->checksum_error == 0){
>> + ret_sub = scrub_repair_sector_from_good_copy(sblock_bad,
>> sblock_good, i, 1);
>> - if (ret_sub)
>> - ret = ret_sub;
>> + if (ret_sub)
>> + ret = ret_sub;
>> + } else if (sblock_bad->sectors[i]->checksum_error == 1) {
>> + ret = 1;
>> + }
>> }
>>
>> return ret;
>> @@ -1984,22 +2012,46 @@ static int scrub_checksum_data(struct scrub_block *sblock)
>> u8 csum[BTRFS_CSUM_SIZE];
>> struct scrub_sector *sector;
>> char *kaddr;
>> + int i;
>> + int io_error = 0;
>>
>> BUG_ON(sblock->sector_count < 1);
>> - sector = sblock->sectors[0];
>> - if (!sector->have_csum)
>> - return 0;
>> -
>> - kaddr = scrub_sector_get_kaddr(sector);
>>
>> shash->tfm = fs_info->csum_shash;
>> crypto_shash_init(shash);
>> + for (i = 0; i < sblock->sector_count; i++) {
>> + sector = sblock->sectors[i];
>> + if (sector->io_error == 1) {
>> + io_error = 1;
>> + continue;
>> + }
>> + if (!sector->have_csum)
>> + continue;
>>
>> - crypto_shash_digest(shash, kaddr, fs_info->sectorsize, csum);
>> + kaddr = scrub_sector_get_kaddr(sector);
>> + crypto_shash_digest(shash, kaddr, fs_info->sectorsize, csum);
>> + if (memcmp(csum, sector->csum, fs_info->csum_size)) {
>> + sector->checksum_error = 1;
>> + sblock->checksum_error = 1;
>> + } else {
>> + sector->checksum_error = 0;
>> + }
>> + }
>> + return sblock->checksum_error | io_error;
>> +}
>>
>> - if (memcmp(csum, sector->csum, fs_info->csum_size))
>> - sblock->checksum_error = 1;
>> - return sblock->checksum_error;
>> +static int scrub_get_sblock_checksum_error(struct scrub_block *sblock)
>> +{
>> + int count = 0;
>> + int i;
>> +
>> + if (sblock == NULL)
>> + return count;
>> + for (i = 0; i < sblock->sector_count; i++) {
>> + if (sblock->sectors[i]->checksum_error == 1)
>> + count++;
>> + }
>> + return count;
>> }
>>
>> static int scrub_checksum_tree_block(struct scrub_block *sblock)
>> @@ -2062,8 +2114,12 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
>> }
>>
>> crypto_shash_final(shash, calculated_csum);
>> - if (memcmp(calculated_csum, on_disk_csum, sctx->fs_info->csum_size))
>> + if (memcmp(calculated_csum, on_disk_csum, sctx->fs_info->csum_size)) {
>> + sblock->sectors[0]->checksum_error = 1;
>> sblock->checksum_error = 1;
>> + } else {
>> + sblock->sectors[0]->checksum_error = 0;
>> + }
>>
>> return sblock->header_error || sblock->checksum_error;
>> }
>> @@ -2400,12 +2456,14 @@ static void scrub_missing_raid56_pages(struct scrub_block *sblock)
>>
>> static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
>> u64 physical, struct btrfs_device *dev, u64 flags,
>> - u64 gen, int mirror_num, u8 *csum,
>> + u64 gen, int mirror_num,
>> u64 physical_for_dev_replace)
>> {
>> struct scrub_block *sblock;
>> const u32 sectorsize = sctx->fs_info->sectorsize;
>> int index;
>> + int have_csum;
>> + struct btrfs_ordered_sum *order_sum = NULL;
>>
>> sblock = alloc_scrub_block(sctx, dev, logical, physical,
>> physical_for_dev_replace, mirror_num);
>> @@ -2415,7 +2473,6 @@ static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
>> spin_unlock(&sctx->stat_lock);
>> return -ENOMEM;
>> }
>> -
>> for (index = 0; len > 0; index++) {
>> struct scrub_sector *sector;
>> /*
>> @@ -2424,7 +2481,6 @@ static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
>> * more memory for PAGE_SIZE > sectorsize case.
>> */
>> u32 l = min(sectorsize, len);
>> -
>> sector = alloc_scrub_sector(sblock, logical, GFP_KERNEL);
>> if (!sector) {
>> spin_lock(&sctx->stat_lock);
>> @@ -2435,11 +2491,25 @@ static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
>> }
>> sector->flags = flags;
>> sector->generation = gen;
>> - if (csum) {
>> - sector->have_csum = 1;
>> - memcpy(sector->csum, csum, sctx->fs_info->csum_size);
>> - } else {
>> - sector->have_csum = 0;
>> + if (flags & BTRFS_EXTENT_FLAG_DATA) {
>> + if (order_sum == NULL ||
>> + (order_sum->bytenr + order_sum->len <= logical)) {
>> + order_sum = NULL;
>> + have_csum = scrub_find_btrfs_ordered_sum(sctx, logical, &order_sum);
>> + }
>> + if (have_csum == 0) {
>> + ++sctx->stat.no_csum;
>> + sector->have_csum = 0;
>> + } else {
>> + int order_csum_index;
>> +
>> + sector->have_csum = 1;
>> + order_csum_index = (logical-order_sum->bytenr)
>> + >> sctx->fs_info->sectorsize_bits;
>> + memcpy(sector->csum,
>> + order_sum->sums + order_csum_index * sctx->fs_info->csum_size,
>> + sctx->fs_info->csum_size);
>> + }
>> }
>> len -= l;
>> logical += l;
>> @@ -2571,7 +2641,8 @@ static void scrub_block_complete(struct scrub_block *sblock)
>> {
>> int corrupted = 0;
>>
>> - if (!sblock->no_io_error_seen) {
>> + if (!sblock->no_io_error_seen && !(sblock->sector_count > 0
>> + && (sblock->sectors[0]->flags & BTRFS_EXTENT_FLAG_DATA))) {
>> corrupted = 1;
>> scrub_handle_errored_block(sblock);
>> } else {
>> @@ -2597,61 +2668,30 @@ static void scrub_block_complete(struct scrub_block *sblock)
>> }
>> }
>>
>> -static void drop_csum_range(struct scrub_ctx *sctx, struct btrfs_ordered_sum *sum)
>> -{
>> - sctx->stat.csum_discards += sum->len >> sctx->fs_info->sectorsize_bits;
>> - list_del(&sum->list);
>> - kfree(sum);
>> -}
>> -
>> /*
>> - * Find the desired csum for range [logical, logical + sectorsize), and store
>> - * the csum into @csum.
>> + * Find the desired btrfs_ordered_sum contain address logical, and store
>> + * the result into @order_sum.
>> *
>> * The search source is sctx->csum_list, which is a pre-populated list
>> - * storing bytenr ordered csum ranges. We're responsible to cleanup any range
>> - * that is before @logical.
>> + * storing bytenr ordered csum ranges.
>> *
>> - * Return 0 if there is no csum for the range.
>> - * Return 1 if there is csum for the range and copied to @csum.
>> + * Return 0 if there is no btrfs_ordered_sum contain the address logical.
>> + * Return 1 if there is btrfs_order_sum contain the address logincal and copied to @order_sum.
>> */
>> -static int scrub_find_csum(struct scrub_ctx *sctx, u64 logical, u8 *csum)
>> +static int scrub_find_btrfs_ordered_sum(struct scrub_ctx *sctx, u64 logical,
>> + struct btrfs_ordered_sum **order_sum)
>> {
>> bool found = false;
>> + struct btrfs_ordered_sum *sum;
>>
>> - while (!list_empty(&sctx->csum_list)) {
>> - struct btrfs_ordered_sum *sum = NULL;
>> - unsigned long index;
>> - unsigned long num_sectors;
>> -
>> - sum = list_first_entry(&sctx->csum_list,
>> - struct btrfs_ordered_sum, list);
>> - /* The current csum range is beyond our range, no csum found */
>> + list_for_each_entry(sum, &sctx->csum_list, list) {
>> + /* no btrfs_ordered_sum found */
>> if (sum->bytenr > logical)
>> break;
>> -
>> - /*
>> - * The current sum is before our bytenr, since scrub is always
>> - * done in bytenr order, the csum will never be used anymore,
>> - * clean it up so that later calls won't bother with the range,
>> - * and continue search the next range.
>> - */
>> - if (sum->bytenr + sum->len <= logical) {
>> - drop_csum_range(sctx, sum);
>> + if (sum->bytenr + sum->len <= logical)
>> continue;
>> - }
>> -
>> - /* Now the csum range covers our bytenr, copy the csum */
>> found = true;
>> - index = (logical - sum->bytenr) >> sctx->fs_info->sectorsize_bits;
>> - num_sectors = sum->len >> sctx->fs_info->sectorsize_bits;
>> -
>> - memcpy(csum, sum->sums + index * sctx->fs_info->csum_size,
>> - sctx->fs_info->csum_size);
>> -
>> - /* Cleanup the range if we're at the end of the csum range */
>> - if (index == num_sectors - 1)
>> - drop_csum_range(sctx, sum);
>> + *order_sum = sum;
>> break;
>> }
>> if (!found)
>> @@ -2669,7 +2709,6 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
>> u64 src_physical = physical;
>> int src_mirror = mirror_num;
>> int ret;
>> - u8 csum[BTRFS_CSUM_SIZE];
>> u32 blocksize;
>>
>> if (flags & BTRFS_EXTENT_FLAG_DATA) {
>> @@ -2685,7 +2724,7 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
>> if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK)
>> blocksize = map->stripe_len;
>> else
>> - blocksize = sctx->fs_info->nodesize;
>> + blocksize = BTRFS_STRIPE_LEN;
>> spin_lock(&sctx->stat_lock);
>> sctx->stat.tree_extents_scrubbed++;
>> sctx->stat.tree_bytes_scrubbed += len;
>> @@ -2709,17 +2748,9 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
>> &src_dev, &src_mirror);
>> while (len) {
>> u32 l = min(len, blocksize);
>> - int have_csum = 0;
>> -
>> - if (flags & BTRFS_EXTENT_FLAG_DATA) {
>> - /* push csums to sbio */
>> - have_csum = scrub_find_csum(sctx, logical, csum);
>> - if (have_csum == 0)
>> - ++sctx->stat.no_csum;
>> - }
>> ret = scrub_sectors(sctx, logical, l, src_physical, src_dev,
>> flags, gen, src_mirror,
>> - have_csum ? csum : NULL, physical);
>> + physical);
>> if (ret)
>> return ret;
>> len -= l;
>> @@ -2733,12 +2764,14 @@ static int scrub_extent(struct scrub_ctx *sctx, struct map_lookup *map,
>> static int scrub_sectors_for_parity(struct scrub_parity *sparity,
>> u64 logical, u32 len,
>> u64 physical, struct btrfs_device *dev,
>> - u64 flags, u64 gen, int mirror_num, u8 *csum)
>> + u64 flags, u64 gen, int mirror_num)
>> {
>> struct scrub_ctx *sctx = sparity->sctx;
>> struct scrub_block *sblock;
>> const u32 sectorsize = sctx->fs_info->sectorsize;
>> int index;
>> + struct btrfs_ordered_sum *order_sum = NULL;
>> + int have_csum;
>>
>> ASSERT(IS_ALIGNED(len, sectorsize));
>>
>> @@ -2770,11 +2803,24 @@ static int scrub_sectors_for_parity(struct scrub_parity *sparity,
>> list_add_tail(§or->list, &sparity->sectors_list);
>> sector->flags = flags;
>> sector->generation = gen;
>> - if (csum) {
>> - sector->have_csum = 1;
>> - memcpy(sector->csum, csum, sctx->fs_info->csum_size);
>> - } else {
>> - sector->have_csum = 0;
>> + if (flags & BTRFS_EXTENT_FLAG_DATA) {
>> + if (order_sum == NULL
>> + || (order_sum->bytenr + order_sum->len <= logical)) {
>> + order_sum = NULL;
>> + have_csum = scrub_find_btrfs_ordered_sum(sctx, logical, &order_sum);
>> + }
>> + if (have_csum == 0) {
>> + sector->have_csum = 0;
>> + } else {
>> + int order_csum_index;
>> +
>> + sector->have_csum = 1;
>> + order_csum_index = (logical-order_sum->bytenr)
>> + >> sctx->fs_info->sectorsize_bits;
>> + memcpy(sector->csum,
>> + order_sum->sums + order_csum_index * sctx->fs_info->csum_size,
>> + sctx->fs_info->csum_size);
>> + }
>> }
>>
>> /* Iterate over the stripe range in sectorsize steps */
>> @@ -2807,7 +2853,6 @@ static int scrub_extent_for_parity(struct scrub_parity *sparity,
>> {
>> struct scrub_ctx *sctx = sparity->sctx;
>> int ret;
>> - u8 csum[BTRFS_CSUM_SIZE];
>> u32 blocksize;
>>
>> if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state)) {
>> @@ -2826,20 +2871,10 @@ static int scrub_extent_for_parity(struct scrub_parity *sparity,
>>
>> while (len) {
>> u32 l = min(len, blocksize);
>> - int have_csum = 0;
>> -
>> - if (flags & BTRFS_EXTENT_FLAG_DATA) {
>> - /* push csums to sbio */
>> - have_csum = scrub_find_csum(sctx, logical, csum);
>> - if (have_csum == 0)
>> - goto skip;
>> - }
>> ret = scrub_sectors_for_parity(sparity, logical, l, physical, dev,
>> - flags, gen, mirror_num,
>> - have_csum ? csum : NULL);
>> + flags, gen, mirror_num);
>> if (ret)
>> return ret;
>> -skip:
>> len -= l;
>> logical += l;
>> physical += l;
>> @@ -4148,7 +4183,7 @@ static noinline_for_stack int scrub_supers(struct scrub_ctx *sctx,
>>
>> ret = scrub_sectors(sctx, bytenr, BTRFS_SUPER_INFO_SIZE, bytenr,
>> scrub_dev, BTRFS_EXTENT_FLAG_SUPER, gen, i,
>> - NULL, bytenr);
>> + bytenr);
>> if (ret)
>> return ret;
>> }
>> --
>> 1.8.3.1
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: scrub: expand scrub block size for data range scrub
2022-11-13 15:35 [PATCH] btrfs: scrub: expand scrub block size for data range scrub Li Zhang
2022-11-13 15:37 ` li zhang
@ 2022-11-13 22:58 ` Qu Wenruo
2022-11-14 14:52 ` li zhang
[not found] ` <CAAa-AGmQpL34eG8yx3bg8FYcbbOOjb3o8fb5YEocRbRPH1=NBw@mail.gmail.com>
1 sibling, 2 replies; 8+ messages in thread
From: Qu Wenruo @ 2022-11-13 22:58 UTC (permalink / raw)
To: Li Zhang, linux-btrfs
On 2022/11/13 23:35, Li Zhang wrote:
A small description here on the problem would help on the context.
> [implement]
> 1. Add the member checksum_error to the scrub_sector,
> which indicates the checksum error of the sector
>
> 2. Use scrub_find_btrfs_ordered_sum to find the desired
Please don't put "_btrfs_" in the middle.
Just "scrub_find_ordered_sum()" is good enough.
> btrfs_ordered_sum containing address logic, in
> scrub_sectors_for_parity and scrub_sectors, call
> Scrub_find_btrfs_ordered_sum finds the
> btrfs_ordered_sum containing the current logical address, and
> Calculate the exact checksum later.
>
> 3. In the scrub_checksum_data function,
> we should check all sectors in the scrub_block.
>
> 4. The job of the scrub_handle_errored_block
> function is to count the number of error and
> repair sectors if they can be repaired.
The idea is good.
And I hope it can also unify the error accounting of data and metadata.
Currently for metadata csum mismatch, we only report one csum error even
if the metadata is 4 sectors.
While for data, we always report the number of corrupted sectors.
>
> The function enters the wrong scrub_block, and
> the overall process is as follows
>
> 1) Check the scrub_block again, check again if the error is gone.
>
> 2) Check the corresponding mirror scrub_block, if there is no error,
> Fix bad sblocks with mirror scrub_block.
>
> 3) If no error-free scrub_block is found, repair it sector by sector.
>
> One difficulty with this function is rechecking the scrub_block.
>
> Imagine this situation, if a sector is checked the first time
> without errors, butthe recheck returns an error.
If you mean the interleaved corrupted sectors like the following:
0 1 2 3
Mirror 1: |X| |X| |
Mirror 2: | |X| |X|
Then it's pretty simple, when doing re-check, we should only bother the
one with errors.
You do not always treat the scrub_block all together.
Thus if you're handling mirror 1, then you should only need to fix
sector 0 and sector 2, not bothering fixing the good sectors (1 and 3).
> What should
> we do, this patch only fixes the bug that the sector first
> appeared (As in the case where the scrub_block
> contains only one scrub_sector).
>
> Another reason to only handle the first error is,
> If the device goes bad, the recheck function will report more and
> more errors,if we want to deal with the errors in the recheck,
> you need to recheck again and again, which may lead to
> Stuck in scrub_handle_errored_block for a long time.
Taking longer time is not a problem, compared to corrupted data.
Although I totally understand that the existing scrub is complex in its
design, that's exactly why I'm trying to implement a better scrub_fs
interface:
https://lwn.net/Articles/907058/
RAID56 has a similiar problem until recent big refactor, changing it to
a more streamlined flow.
But the per-sector repair is still there, you can not avoid that, no
matter if scrub_block contains single or multiple sectors.
(Although single sector scrub_block make is much easier)
[...]
> @@ -1054,7 +1056,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> if (ret == -ENOMEM)
> sctx->stat.malloc_errors++;
> sctx->stat.read_errors++;
> - sctx->stat.uncorrectable_errors++;
> + sctx->stat.uncorrectable_errors += scrub_get_sblock_checksum_error(sblock_to_check);
> + sctx->stat.uncorrectable_errors += sblock_to_check->header_error;
Do we double accout the header_error for metadata?
Thanks,
Qu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: scrub: expand scrub block size for data range scrub
2022-11-13 22:58 ` Qu Wenruo
@ 2022-11-14 14:52 ` li zhang
[not found] ` <CAAa-AGmQpL34eG8yx3bg8FYcbbOOjb3o8fb5YEocRbRPH1=NBw@mail.gmail.com>
1 sibling, 0 replies; 8+ messages in thread
From: li zhang @ 2022-11-14 14:52 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
Qu Wenruo <quwenruo.btrfs@gmx.com> 于2022年11月14日周一 06:58写道：
>
>
>
> On 2022/11/13 23:35, Li Zhang wrote:
>
> A small description here on the problem would help on the context.
>
> > [implement]
> > 1. Add the member checksum_error to the scrub_sector,
> > which indicates the checksum error of the sector
> >
> > 2. Use scrub_find_btrfs_ordered_sum to find the desired
>
> Please don't put "_btrfs_" in the middle.
>
> Just "scrub_find_ordered_sum()" is good enough.
>
> > btrfs_ordered_sum containing address logic, in
> > scrub_sectors_for_parity and scrub_sectors, call
> > Scrub_find_btrfs_ordered_sum finds the
> > btrfs_ordered_sum containing the current logical address, and
> > Calculate the exact checksum later.
> >
> > 3. In the scrub_checksum_data function,
> > we should check all sectors in the scrub_block.
> >
> > 4. The job of the scrub_handle_errored_block
> > function is to count the number of error and
> > repair sectors if they can be repaired.
>
> The idea is good.
>
> And I hope it can also unify the error accounting of data and metadata.
>
> Currently for metadata csum mismatch, we only report one csum error even
> if the metadata is 4 sectors.
> While for data, we always report the number of corrupted sectors.
>
Because only one checksum is calculated for all sectors of the
metadata block.
In this case the checksum count is ambiguous if
checksum_error means sector error, it should count all sectors, but if
checksum_error indicates that the checksum does not match and can only be
incremented by 1.
Also, since the metadata data block is a complete block, it cannot be
Fix sector by sector, the patch doesn't take this case into account.
So the patch still has bugs.
> >
> > The function enters the wrong scrub_block, and
> > the overall process is as follows
> >
> > 1) Check the scrub_block again, check again if the error is gone.
> >
> > 2) Check the corresponding mirror scrub_block, if there is no error,
> > Fix bad sblocks with mirror scrub_block.
> >
> > 3) If no error-free scrub_block is found, repair it sector by sector.
> >
> > One difficulty with this function is rechecking the scrub_block.
> >
> > Imagine this situation, if a sector is checked the first time
> > without errors, butthe recheck returns an error.
>
> If you mean the interleaved corrupted sectors like the following:
> 0 1 2 3
> Mirror 1: |X| |X| |
> Mirror 2: | |X| |X|
>
> Then it's pretty simple, when doing re-check, we should only bother the
> one with errors.
> You do not always treat the scrub_block all together.
>
> Thus if you're handling mirror 1, then you should only need to fix
> sector 0 and sector 2, not bothering fixing the good sectors (1 and 3).
>
Consider the following
The results of the first checking are as follows:
0 1 2 3
Mirror 1: |X| |X| |
Mirror 2: | |X| | |
The result of the recheck is:
0 1 2 3
Mirror 1: |X| |X|X|
Mirror 2: | |X| | |
An additional error was reported, what should we do,
recheck (which means it would recheck twice or more)?
Or just check only bad sectors during recheck.
>
> > What should
> > we do, this patch only fixes the bug that the sector first
> > appeared (As in the case where the scrub_block
> > contains only one scrub_sector).
> >
> > Another reason to only handle the first error is,
> > If the device goes bad, the recheck function will report more and
> > more errors,if we want to deal with the errors in the recheck,
> > you need to recheck again and again, which may lead to
> > Stuck in scrub_handle_errored_block for a long time.
>
> Taking longer time is not a problem, compared to corrupted data.
>
> Although I totally understand that the existing scrub is complex in its
> design, that's exactly why I'm trying to implement a better scrub_fs
> interface:
>
> https://lwn.net/Articles/907058/
>
> RAID56 has a similiar problem until recent big refactor, changing it to
> a more streamlined flow.
>
> But the per-sector repair is still there, you can not avoid that, no
> matter if scrub_block contains single or multiple sectors.
> (Although single sector scrub_block make is much easier)
>
> [...]
> > @@ -1054,7 +1056,8 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> > if (ret == -ENOMEM)
> > sctx->stat.malloc_errors++;
> > sctx->stat.read_errors++;
> > - sctx->stat.uncorrectable_errors++;
> > + sctx->stat.uncorrectable_errors += scrub_get_sblock_checksum_error(sblock_to_check);
> > + sctx->stat.uncorrectable_errors += sblock_to_check->header_error;
>
> Do we double accout the header_error for metadata?
>
> Thanks,
> Qu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: scrub: expand scrub block size for data range scrub
[not found] ` <11a71790-de79-3c2f-97f3-b97305b99378@gmx.com>
@ 2022-11-15 10:37 ` Qu Wenruo
2022-11-15 18:29 ` li zhang
0 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2022-11-15 10:37 UTC (permalink / raw)
To: li zhang, linux-btrfs
[Adding the mailing list, as the reply is to the first mail I got, which
lacks the ML]
On 2022/11/15 06:39, Qu Wenruo wrote:
[...]
>>>
>>> And I hope it can also unify the error accounting of data and metadata.
>>>
>>
>> Because only one checksum is calculated for all sectors of the
>> metadata block.
>>
>> In this case the checksum count is ambiguous if
>> checksum_error means sector error, it should count all sectors, but if
>> checksum_error indicates that the checksum does not match and can only be
>> incremented by 1.
In fact, this is the problem of the existing scrub interface.
The scrub_process only reports an very ambiguous csum mismatch count.
If scrub reports 4 csum errors, you can not distinguish if it's 4 tree
blocks csum mismatch or 4 data sectors.
Thus in my scrub_fs proposal, I do the error reports separately.
Furthermore unify the error report to use bytes.
That's to say, if a metadata (16K by default) has csum mismatch, it will
report 16K bytes has csum mismatch for metadata.
>>
>> Also, since the metadata data block is a complete block, it cannot be
>> Fix sector by sector, the patch doesn't take this case into account.
>> So the patch still has bugs.
That doesn't matter. What end users really want is, how many bytes are
affected, which is generic between data and metadata.
Not some ambiguous standard which changes according to if it's a
metadata or data.
>>
>>> Currently for metadata csum mismatch, we only report one csum error even
>>> if the metadata is 4 sectors.
>>> While for data, we always report the number of corrupted sectors.
>>>
>>>>
>>>> The function enters the wrong scrub_block, and
>>>> the overall process is as follows
>>>>
>>>> 1) Check the scrub_block again, check again if the error is gone.
>>>>
>>>> 2) Check the corresponding mirror scrub_block, if there is no error,
>>>> Fix bad sblocks with mirror scrub_block.
>>>>
>>>> 3) If no error-free scrub_block is found, repair it sector by sector.
>>>>
>>>> One difficulty with this function is rechecking the scrub_block.
>>>>
>>>> Imagine this situation, if a sector is checked the first time
>>>> without errors, butthe recheck returns an error.
>>>
>>> If you mean the interleaved corrupted sectors like the following:
>>> 0 1 2 3
>>> Mirror 1: |X| |X| |
>>> Mirror 2: | |X| |X|
>>>
>>> Then it's pretty simple, when doing re-check, we should only bother the
>>> one with errors.
>>> You do not always treat the scrub_block all together.
>>>
>>> Thus if you're handling mirror 1, then you should only need to fix
>>> sector 0 and sector 2, not bothering fixing the good sectors (1 and 3).
>>>
>>
>> Consider the following
>>
>> The results of the first checking are as follows:
>
> We're talking about "X" = corrupted sector right?
>
>> 0 1 2 3
>> Mirror 1: |X| |X| |
>> Mirror 2: | |X| | |
>>
>> The result of the recheck is:
>> 0 1 2 3
>> Mirror 1: |X| |X|X|
>> Mirror 2: | |X| | |
>> An additional error was reported, what should we do,
>> recheck (which means it would recheck twice or more)?
>> Or just check only bad sectors during recheck.
>
> Of course we should only repair the corrupted sectors.
> Why bother the already good sectors?
>
> The csum error report should on cover the initial mirror we're checking.
> The re-check thing is only to make sure after repair, the repaired
> sectors are good.
>
> So the csum error accounting should be the original corrupted sector
> number.
In fact, since my RAID56 work almost finished, I guess I should spend
more time on integrating the scrub_fs ideas into the existing scrub code.
(Other than just introduce a new interface from scratch)
I'll try to craft a PoC patchset to a stripe by stripe verification (to
get rid of the complex bio form shaping code), and a proper bitmap based
verification and repair (to only repair the corrupted sectors).
But as I mentioned above, the bad csum error reporting can not be easily
fixed without a behavior change on btrfs_scrub_progress results.
Thanks,
Qu
>
>>
>>>
>>>> What should
>>>> we do, this patch only fixes the bug that the sector first
>>>> appeared (As in the case where the scrub_block
>>>> contains only one scrub_sector).
>>>>
>>>> Another reason to only handle the first error is,
>>>> If the device goes bad, the recheck function will report more and
>>>> more errors,if we want to deal with the errors in the recheck,
>>>> you need to recheck again and again, which may lead to
>>>> Stuck in scrub_handle_errored_block for a long time.
>>>
>>> Taking longer time is not a problem, compared to corrupted data.
>>>
>>> Although I totally understand that the existing scrub is complex in its
>>> design, that's exactly why I'm trying to implement a better scrub_fs
>>> interface:
>>>
>>> https://lwn.net/Articles/907058/
>
> Again, all of the behavior I mentioned can be found in above patchset.
>
> But unfortunately I don't have a good way to apply them all to the
> existing scrub infrastructure without such a full rework...
>
> Thanks,
> Qu
>
>>>
>>> RAID56 has a similiar problem until recent big refactor, changing it to
>>> a more streamlined flow.
>>>
>>> But the per-sector repair is still there, you can not avoid that, no
>>> matter if scrub_block contains single or multiple sectors.
>>> (Although single sector scrub_block make is much easier)
>>>
>>> [...]
>>>> @@ -1054,7 +1056,8 @@ static int scrub_handle_errored_block(struct
>>>> scrub_block *sblock_to_check)
>>>> if (ret == -ENOMEM)
>>>> sctx->stat.malloc_errors++;
>>>> sctx->stat.read_errors++;
>>>> - sctx->stat.uncorrectable_errors++;
>>>> + sctx->stat.uncorrectable_errors +=
>>>> scrub_get_sblock_checksum_error(sblock_to_check);
>>>> + sctx->stat.uncorrectable_errors +=
>>>> sblock_to_check->header_error;
>>>
>>> Do we double accout the header_error for metadata?
>>>
>>> Thanks,
>>> Qu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: scrub: expand scrub block size for data range scrub
2022-11-15 10:37 ` Qu Wenruo
@ 2022-11-15 18:29 ` li zhang
2022-11-15 22:57 ` Qu Wenruo
0 siblings, 1 reply; 8+ messages in thread
From: li zhang @ 2022-11-15 18:29 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
Qu Wenruo <quwenruo.btrfs@gmx.com> 于2022年11月15日周二 18:37写道：
>
> [Adding the mailing list, as the reply is to the first mail I got, which
> lacks the ML]
>
> On 2022/11/15 06:39, Qu Wenruo wrote:
> [...]
> I'll try to craft a PoC patchset to a stripe by stripe verification (to
> get rid of the complex bio form shaping code), and a proper bitmap based
> verification and repair (to only repair the corrupted sectors).
>
> But as I mentioned above, the bad csum error reporting can not be easily
> fixed without a behavior change on btrfs_scrub_progress results.
>
Actually, I also considered refactoring the functions
scrub_recheck_block and scrub_recheck_block_checksum to import a
bitmap to represent bad sectors, but this seems too complicated and
may affect many things, so I choose
Handle newly discovered errors by recheck in
scrub_handle_errored_block, ignoring recheck errors by exchanging the
result of the first check and the result of the recheck, as follows:
else if (!check_sector->checksum_error && bad_sector->checksum_error) {
struct scrub_sector *temp_sector = sblock_bad->sectors[sector_num];
sblock_bad->sectors[sector_num]
= sblock_to_check->sectors[sector_num];
sblock_to_check->sectors[sector_num] = temp_sector;
Anyway, I'll take a hard look at your scrub_fs idea
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: scrub: expand scrub block size for data range scrub
2022-11-15 18:29 ` li zhang
@ 2022-11-15 22:57 ` Qu Wenruo
0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2022-11-15 22:57 UTC (permalink / raw)
To: li zhang, Qu Wenruo, linux-btrfs
On 2022/11/16 02:29, li zhang wrote:
> Qu Wenruo <quwenruo.btrfs@gmx.com> 于2022年11月15日周二 18:37写道：
>>
>> [Adding the mailing list, as the reply is to the first mail I got, which
>> lacks the ML]
>>
>> On 2022/11/15 06:39, Qu Wenruo wrote:
>> [...]
>
>> I'll try to craft a PoC patchset to a stripe by stripe verification (to
>> get rid of the complex bio form shaping code), and a proper bitmap based
>> verification and repair (to only repair the corrupted sectors).
>>
>> But as I mentioned above, the bad csum error reporting can not be easily
>> fixed without a behavior change on btrfs_scrub_progress results.
>>
>
> Actually, I also considered refactoring the functions
> scrub_recheck_block and scrub_recheck_block_checksum to import a
> bitmap to represent bad sectors, but this seems too complicated and
> may affect many things,
Another thing I don't like is the various jumps using different end_io
functions.
I'm a super big fan of submit-and-wait, and already converted RAID56 to
this way.
Thus I believe I can also handle the situation in scrub, and hopefully
get it easier to read.
> so I choose
> Handle newly discovered errors by recheck in
> scrub_handle_errored_block, ignoring recheck errors by exchanging the
> result of the first check and the result of the recheck, as follows:
> else if (!check_sector->checksum_error && bad_sector->checksum_error) {
> struct scrub_sector *temp_sector = sblock_bad->sectors[sector_num];
>
> sblock_bad->sectors[sector_num]
> = sblock_to_check->sectors[sector_num];
> sblock_to_check->sectors[sector_num] = temp_sector;
>
> Anyway, I'll take a hard look at your scrub_fs idea
Feel free to do any comments, I'm pretty eager to get some feedback on it.
Thanks,
Qu
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-15 22:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-13 15:35 [PATCH] btrfs: scrub: expand scrub block size for data range scrub Li Zhang
2022-11-13 15:37 ` li zhang
2022-11-13 22:37 ` Qu Wenruo
2022-11-13 22:58 ` Qu Wenruo
2022-11-14 14:52 ` li zhang
[not found] ` <CAAa-AGmQpL34eG8yx3bg8FYcbbOOjb3o8fb5YEocRbRPH1=NBw@mail.gmail.com>
[not found] ` <11a71790-de79-3c2f-97f3-b97305b99378@gmx.com>
2022-11-15 10:37 ` Qu Wenruo
2022-11-15 18:29 ` li zhang
2022-11-15 22:57 ` Qu Wenruo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.