Skip to content

Commit 8eb3dd1

Browse files
adam900710kdave
authored andcommitted
btrfs: dev-replace: error out if we have unrepaired metadata error during
[BUG] Even before the scrub rework, if we have some corrupted metadata failed to be repaired during replace, we still continue replacing and let it finish just as there is nothing wrong: BTRFS info (device dm-4): dev_replace from /dev/mapper/test-scratch1 (devid 1) to /dev/mapper/test-scratch2 started BTRFS warning (device dm-4): tree block 5578752 mirror 1 has bad csum, has 0x00000000 want 0xade80ca1 BTRFS warning (device dm-4): tree block 5578752 mirror 0 has bad csum, has 0x00000000 want 0xade80ca1 BTRFS warning (device dm-4): checksum error at logical 5578752 on dev /dev/mapper/test-scratch1, physical 5578752: metadata leaf (level 0) in tree 5 BTRFS warning (device dm-4): checksum error at logical 5578752 on dev /dev/mapper/test-scratch1, physical 5578752: metadata leaf (level 0) in tree 5 BTRFS error (device dm-4): bdev /dev/mapper/test-scratch1 errs: wr 0, rd 0, flush 0, corrupt 1, gen 0 BTRFS warning (device dm-4): tree block 5578752 mirror 1 has bad bytenr, has 0 want 5578752 BTRFS error (device dm-4): unable to fixup (regular) error at logical 5578752 on dev /dev/mapper/test-scratch1 BTRFS info (device dm-4): dev_replace from /dev/mapper/test-scratch1 (devid 1) to /dev/mapper/test-scratch2 finished This can lead to unexpected problems for the resulting filesystem. [CAUSE] Btrfs reuses scrub code path for dev-replace to iterate all dev extents. But unlike scrub, dev-replace doesn't really bother to check the scrub progress, which records all the errors found during replace. And even if we check the progress, we cannot really determine which errors are minor, which are critical just by the plain numbers. (remember we don't treat metadata/data checksum error differently). This behavior is there from the very beginning. [FIX] Instead of continuing the replace, just error out if we hit an unrepaired metadata sector. Now the dev-replace would be rejected with -EIO, to let the user know. Although it also means, the filesystem has some metadata error which cannot be repaired, the user would be upset anyway. The new dmesg would look like this: BTRFS info (device dm-4): dev_replace from /dev/mapper/test-scratch1 (devid 1) to /dev/mapper/test-scratch2 started BTRFS warning (device dm-4): tree block 5578752 mirror 1 has bad csum, has 0x00000000 want 0xade80ca1 BTRFS warning (device dm-4): tree block 5578752 mirror 1 has bad csum, has 0x00000000 want 0xade80ca1 BTRFS error (device dm-4): unable to fixup (regular) error at logical 5570560 on dev /dev/mapper/test-scratch1 physical 5570560 BTRFS warning (device dm-4): header error at logical 5570560 on dev /dev/mapper/test-scratch1, physical 5570560: metadata leaf (level 0) in tree 5 BTRFS warning (device dm-4): header error at logical 5570560 on dev /dev/mapper/test-scratch1, physical 5570560: metadata leaf (level 0) in tree 5 BTRFS error (device dm-4): stripe 5570560 has unrepaired metadata sector at 5578752 BTRFS error (device dm-4): btrfs_scrub_dev(/dev/mapper/test-scratch1, 1, /dev/mapper/test-scratch2) failed -5 Signed-off-by: Qu Wenruo <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 524f14b commit 8eb3dd1

File tree

1 file changed

+42
-5
lines changed

1 file changed

+42
-5
lines changed

fs/btrfs/scrub.c

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1656,14 +1656,33 @@ static void scrub_submit_initial_read(struct scrub_ctx *sctx,
16561656
btrfs_submit_bio(bbio, mirror);
16571657
}
16581658

1659-
static void flush_scrub_stripes(struct scrub_ctx *sctx)
1659+
static bool stripe_has_metadata_error(struct scrub_stripe *stripe)
1660+
{
1661+
int i;
1662+
1663+
for_each_set_bit(i, &stripe->error_bitmap, stripe->nr_sectors) {
1664+
if (stripe->sectors[i].is_metadata) {
1665+
struct btrfs_fs_info *fs_info = stripe->bg->fs_info;
1666+
1667+
btrfs_err(fs_info,
1668+
"stripe %llu has unrepaired metadata sector at %llu",
1669+
stripe->logical,
1670+
stripe->logical + (i << fs_info->sectorsize_bits));
1671+
return true;
1672+
}
1673+
}
1674+
return false;
1675+
}
1676+
1677+
static int flush_scrub_stripes(struct scrub_ctx *sctx)
16601678
{
16611679
struct btrfs_fs_info *fs_info = sctx->fs_info;
16621680
struct scrub_stripe *stripe;
16631681
const int nr_stripes = sctx->cur_stripe;
1682+
int ret = 0;
16641683

16651684
if (!nr_stripes)
1666-
return;
1685+
return 0;
16671686

16681687
ASSERT(test_bit(SCRUB_STRIPE_FLAG_INITIALIZED, &sctx->stripes[0].state));
16691688

@@ -1709,6 +1728,16 @@ static void flush_scrub_stripes(struct scrub_ctx *sctx)
17091728

17101729
/* Submit for dev-replace. */
17111730
if (sctx->is_dev_replace) {
1731+
/*
1732+
* For dev-replace, if we know there is something wrong with
1733+
* metadata, we should immedately abort.
1734+
*/
1735+
for (int i = 0; i < nr_stripes; i++) {
1736+
if (stripe_has_metadata_error(&sctx->stripes[i])) {
1737+
ret = -EIO;
1738+
goto out;
1739+
}
1740+
}
17121741
for (int i = 0; i < nr_stripes; i++) {
17131742
unsigned long good;
17141743

@@ -1729,7 +1758,9 @@ static void flush_scrub_stripes(struct scrub_ctx *sctx)
17291758
wait_scrub_stripe_io(stripe);
17301759
scrub_reset_stripe(stripe);
17311760
}
1761+
out:
17321762
sctx->cur_stripe = 0;
1763+
return ret;
17331764
}
17341765

17351766
static void raid56_scrub_wait_endio(struct bio *bio)
@@ -1745,8 +1776,11 @@ static int queue_scrub_stripe(struct scrub_ctx *sctx, struct btrfs_block_group *
17451776
int ret;
17461777

17471778
/* No available slot, submit all stripes and wait for them. */
1748-
if (sctx->cur_stripe >= SCRUB_STRIPES_PER_SCTX)
1749-
flush_scrub_stripes(sctx);
1779+
if (sctx->cur_stripe >= SCRUB_STRIPES_PER_SCTX) {
1780+
ret = flush_scrub_stripes(sctx);
1781+
if (ret < 0)
1782+
return ret;
1783+
}
17501784

17511785
stripe = &sctx->stripes[sctx->cur_stripe];
17521786

@@ -2075,6 +2109,7 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
20752109
const u64 profile = map->type & BTRFS_BLOCK_GROUP_PROFILE_MASK;
20762110
const u64 chunk_logical = bg->start;
20772111
int ret;
2112+
int ret2;
20782113
u64 physical = map->stripes[stripe_index].physical;
20792114
const u64 dev_stripe_len = btrfs_calc_stripe_length(em);
20802115
const u64 physical_end = physical + dev_stripe_len;
@@ -2202,7 +2237,9 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
22022237
break;
22032238
}
22042239
out:
2205-
flush_scrub_stripes(sctx);
2240+
ret2 = flush_scrub_stripes(sctx);
2241+
if (!ret2)
2242+
ret = ret2;
22062243
if (sctx->raid56_data_stripes) {
22072244
for (int i = 0; i < nr_data_stripes(map); i++)
22082245
release_scrub_stripe(&sctx->raid56_data_stripes[i]);

0 commit comments

Comments
 (0)