From 7435b73f05fbb40c07b087fefd3d40bfd759519c Mon Sep 17 00:00:00 2001 From: Li Nan Date: Mon, 5 Jan 2026 19:02:59 +0800 Subject: [PATCH] md/raid10: cleanup skip handling in raid10_sync_request Skip a sector in raid10_sync_request() when it needs no syncing or no readable device exists. Current skip handling is unnecessary: - Use 'skip' label to reissue the next sector instead of return directly - Complete sync and return 'max_sectors' when multiple sectors are skipped due to badblocks The first is error-prone. For example, commit bc49694a9e8f ("md: pass in max_sectors for pers->sync_request()") removed redundant max_sector assignments. Since skip modifies max_sectors, `goto skip` leaves max_sectors equal to sector_nr after the jump, which is incorrect. The second causes sync to complete erroneously when no actual sync occurs. For recovery, recording badblocks and continue syncing subsequent sectors is more suitable. For resync, just skip bad sectors and syncing subsequent sectors. Clean up complex and unnecessary skip code. Return immediately when a sector should be skipped. Reduce code paths and lower regression risk. Link: https://lore.kernel.org/linux-raid/20260105110300.1442509-12-linan666@huaweicloud.com Fixes: bc49694a9e8f ("md: pass in max_sectors for pers->sync_request()") Signed-off-by: Li Nan Reviewed-by: Yu Kuai Signed-off-by: Yu Kuai --- drivers/md/raid10.c | 96 +++++++++++---------------------------------- 1 file changed, 22 insertions(+), 74 deletions(-) diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index b258ed8b4e3af..6f5a4aefb4e57 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -3161,11 +3161,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, int i; int max_sync; sector_t sync_blocks; - sector_t sectors_skipped = 0; - int chunks_skipped = 0; sector_t chunk_mask = conf->geo.chunk_mask; int page_idx = 0; - int error_disk = -1; /* * Allow skipping a full rebuild for incremental assembly @@ -3186,7 +3183,6 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, if (init_resync(conf)) return 0; - skipped: if (sector_nr >= max_sector) { conf->cluster_sync_low = 0; conf->cluster_sync_high = 0; @@ -3238,33 +3234,12 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, mddev->bitmap_ops->close_sync(mddev); close_sync(conf); *skipped = 1; - return sectors_skipped; + return 0; } if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery)) return reshape_request(mddev, sector_nr, skipped); - if (chunks_skipped >= conf->geo.raid_disks) { - pr_err("md/raid10:%s: %s fails\n", mdname(mddev), - test_bit(MD_RECOVERY_SYNC, &mddev->recovery) ? "resync" : "recovery"); - if (error_disk >= 0 && - !test_bit(MD_RECOVERY_SYNC, &mddev->recovery)) { - /* - * recovery fails, set mirrors.recovery_disabled, - * device shouldn't be added to there. - */ - conf->mirrors[error_disk].recovery_disabled = - mddev->recovery_disabled; - return 0; - } - /* - * if there has been nothing to do on any drive, - * then there is nothing to do at all. - */ - *skipped = 1; - return (max_sector - sector_nr) + sectors_skipped; - } - if (max_sector > mddev->resync_max) max_sector = mddev->resync_max; /* Don't do IO beyond here */ @@ -3347,7 +3322,6 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, /* yep, skip the sync_blocks here, but don't assume * that there will never be anything to do here */ - chunks_skipped = -1; continue; } if (mrdev) @@ -3478,29 +3452,19 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, for (k = 0; k < conf->copies; k++) if (r10_bio->devs[k].devnum == i) break; - if (mrdev && !test_bit(In_sync, - &mrdev->flags) - && !rdev_set_badblocks( - mrdev, - r10_bio->devs[k].addr, - max_sync, 0)) - any_working = 0; - if (mreplace && - !rdev_set_badblocks( - mreplace, - r10_bio->devs[k].addr, - max_sync, 0)) - any_working = 0; - } - if (!any_working) { - if (!test_and_set_bit(MD_RECOVERY_INTR, - &mddev->recovery)) - pr_warn("md/raid10:%s: insufficient working devices for recovery.\n", - mdname(mddev)); - mirror->recovery_disabled - = mddev->recovery_disabled; - } else { - error_disk = i; + if (mrdev && + !test_bit(In_sync, &mrdev->flags)) + rdev_set_badblocks( + mrdev, + r10_bio->devs[k].addr, + max_sync, 0); + if (mreplace) + rdev_set_badblocks( + mreplace, + r10_bio->devs[k].addr, + max_sync, 0); + pr_warn("md/raid10:%s: cannot recovery sector %llu + %d.\n", + mdname(mddev), r10_bio->devs[k].addr, max_sync); } put_buf(r10_bio); if (rb2) @@ -3541,7 +3505,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, rb2->master_bio = NULL; put_buf(rb2); } - goto giveup; + *skipped = 1; + return max_sync; } } else { /* resync. Schedule a read for every block at this virt offset */ @@ -3565,7 +3530,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, &mddev->recovery)) { /* We can skip this block */ *skipped = 1; - return sync_blocks + sectors_skipped; + return sync_blocks; } if (sync_blocks < max_sync) max_sync = sync_blocks; @@ -3657,8 +3622,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, mddev); } put_buf(r10_bio); - biolist = NULL; - goto giveup; + *skipped = 1; + return max_sync; } } @@ -3678,7 +3643,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, if (WARN_ON(!bio_add_page(bio, page, len, 0))) { bio->bi_status = BLK_STS_RESOURCE; bio_endio(bio); - goto giveup; + *skipped = 1; + return max_sync; } } nr_sectors += len>>9; @@ -3746,25 +3712,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, } } - if (sectors_skipped) - /* pretend they weren't skipped, it makes - * no important difference in this case - */ - md_done_sync(mddev, sectors_skipped); - - return sectors_skipped + nr_sectors; - giveup: - /* There is nowhere to write, so all non-sync - * drives must be failed or in resync, all drives - * have a bad block, so try the next chunk... - */ - if (sector_nr + max_sync < max_sector) - max_sector = sector_nr + max_sync; - - sectors_skipped += (max_sector - sector_nr); - chunks_skipped ++; - sector_nr = max_sector; - goto skipped; + return nr_sectors; } static sector_t -- 2.47.3