[PATCH v4 1/3] md/raid5: freeze and interrupt reshape when handling failed reshape

[PATCH v4 1/3] md/raid5: freeze and interrupt reshape when handling failed reshape

[Doug V Johnson]

Set the reshape freeze flag to prevent automatic reshape resumption
and interrupt the current reshape with md_sync_error() so that a
reshape always stops no matter how the failed reshape is triggered.

Previously, if there were no faulty or out of sync devices but there
was an unrecoverable read, the reshape continued and was not cancelled
after handle_failed_reshape(). This caused some data corruption as
block writes were cancelled but the reshape nonetheless continued.

Prior to Chen's fix, the kernel would deadlock with a failed stripe
read.  That case also caused some data corruption, so Chen's patch
did not introduce corruption in this case.

An example test case showing how this happened before this fix is
demonstrated by the following:

Simplified example (schematic logical sector numbers):

  1. Before reshape, destination D contains data for logical 20-27.
     The new geometry will use D for logical 100-107.

  2. Inject bad-block metadata on enough members that the old source
     stripe cannot be reconstructed. Logical sector 100 now returns
     a read error; sectors 101-107 are still readable.

  3. Reshape tries to read the source block for logical 100-107.
     The bad-block records prevent a successful read/reconstruction.
     Failure cleanup cancels the destination write:

       Source: logical 100-107 --X--> destination D
                                     still contains data for 20-27

  4. Without interruption, reshape continues and completes.
     The new mapping now sends reads of logical 100-107 to D.

  5. Reading logical 100-107 succeeds, but returns data for 20-27.
     Previously readable sectors 101-107 now return wrong data.

Code that injects bad blocks to demonstrate this problem before and
after Chen's fix as well as after this patch can be found here:

Link: https://github.com/dougvj/md-raid-reshape-badblock-corruption-test

Fixes: 788e4139463f ("md/raid5: fix reshape deadlock while failed devices more than max degraded")
Link: https://patch.msgid.link/20260624075824.2601110-1-chencheng@fnnas.com
Assisted-by: LLM
Signed-off-by: Doug V Johnson <me@dougvj.me>
---
 drivers/md/raid5.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index b91545ce090d..86ea65559576 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3773,6 +3773,10 @@ static void handle_failed_reshape(struct r5conf *conf, struct stripe_head *sh,
 	bool was_expanding = test_and_clear_bit(STRIPE_EXPANDING, &sh->state);
 	bool was_ready = test_and_clear_bit(STRIPE_EXPAND_READY, &sh->state);
 
+	/* Freeze reshaping to prevent data loss */
+	set_bit(MD_RECOVERY_FROZEN, &conf->mddev->recovery);
+	md_sync_error(conf->mddev);
+
 	if (was_expanding || was_ready) {
 		atomic_dec(&conf->reshape_stripes);
 		wake_up(&conf->wait_for_reshape);
-- 
2.54.0

[PATCH v4 2/3] md/raid5: do not reshape if overlapping rdev bad block ranges prevent it

[Doug V Johnson]

Ensure that a reshape is not started if there are more than max_degraded
devices in the array that are either out of sync, faulty, missing,
or have overlapping bad blocks such that a read is not recoverable.

Cheaply verify that the number of bad or unavailable devices exceeds
the max number of degraded drives and if so sweep the bad block lists
on each device for overlaps using a new function array_has_badblock().

Invoke this function in raid5_start_reshape() and if the array has
unreadable blocks, return -EIO to report it to userspace.

Leave in place the existing functionality to handle a failed reshape
in progress because it's possible for bad blocks to be discovered or
placed in metadata after a reshape has already been initiated.

Link: https://lore.kernel.org/r/21f250f9-04b5-76f6-84c9-1fd245b748c6@huaweicloud.com
Link: https://lore.kernel.org/r/a9119707-4fa1-58cb-377c-63df67f88957@huaweicloud.com
Suggested-by: Yu Kuai <yukuai1@huaweicloud.com>
Assisted-by: LLM
Signed-off-by: Doug V Johnson <me@dougvj.me>
---
 drivers/md/raid5.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 86ea65559576..23ff549fa363 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -8636,6 +8636,89 @@ static int check_reshape(struct mddev *mddev)
 				     + mddev->delta_disks));
 }
 
+static int rdev_is_unavailable(struct md_rdev *rdev)
+{
+	return !rdev || test_bit(Faulty, &rdev->flags) ||
+	       !test_bit(In_sync, &rdev->flags);
+}
+
+static int array_has_badblock(struct r5conf *conf)
+{
+	sector_t dev_sectors = conf->mddev->dev_sectors;
+	sector_t stripe_sectors = RAID5_STRIPE_SECTORS(conf);
+	sector_t max_sectors = round_down((sector_t)INT_MAX, stripe_sectors);
+	sector_t offset = 0;
+	int num_bad_devs = 0;
+	int i;
+
+	/* Check if there are even enough bad disks for a bad block condition */
+	for (i = 0; i < conf->raid_disks; i++) {
+		struct md_rdev *rdev = conf->disks[i].rdev;
+
+		if (rdev_is_unavailable(rdev) || rdev->badblocks.count)
+			num_bad_devs++;
+	}
+	if (num_bad_devs <= conf->max_degraded)
+		return 0;
+
+	/*
+	 * Sweep the sectors of the array for bad blocks in constituent devices.
+	 * Within a stripe if there are more bad disks than can be recoverable
+	 * (max degraded) then the array has bad blocks.
+	 *
+	 * The offset tracks the sweep. We advance to the first bad stripe
+	 * boundary found in any device.
+	 */
+	while (offset < dev_sectors) {
+		sector_t len = min(dev_sectors - offset, max_sectors);
+		int bad_disks = 0;
+
+		for (i = 0; i < conf->raid_disks; i++) {
+			struct md_rdev *rdev = conf->disks[i].rdev;
+			sector_t first_bad, bad_sectors, end;
+
+			if (rdev_is_unavailable(rdev)) {
+				bad_disks++;
+				continue;
+			}
+			if (!rdev->badblocks.count)
+				continue;
+			if (!badblocks_check(&rdev->badblocks,
+					     rdev->data_offset + offset, len,
+					     &first_bad, &bad_sectors))
+				continue;
+
+			/*
+			 * Rebase the bad blocks start and end to the device's data
+			 * offset for consistent comparision
+			 */
+			end = first_bad + bad_sectors - rdev->data_offset;
+			first_bad = max(first_bad, rdev->data_offset) -
+				    rdev->data_offset;
+
+			/* Snap the range to the stripe boundary extent */
+			first_bad = round_down(first_bad, stripe_sectors);
+			end = round_up(end, stripe_sectors);
+
+			/*
+			 * This device has a bad block at or overlapping the current
+			 * sweep position, so it's a bad disk
+			 */
+			if (first_bad <= offset) {
+				len = min(len, end - offset);
+				bad_disks++;
+			} else {
+				len = min(len, first_bad - offset);
+			}
+		}
+		if (bad_disks > conf->max_degraded)
+			return 1;
+		offset += len;
+		cond_resched();
+	}
+	return 0;
+}
+
 static int raid5_start_reshape(struct mddev *mddev)
 {
 	struct r5conf *conf = mddev->private;
@@ -8685,6 +8768,11 @@ static int raid5_start_reshape(struct mddev *mddev)
 			mdname(mddev));
 		return -EINVAL;
 	}
+	if (array_has_badblock(conf)) {
+		pr_warn("md/raid:%s: reshape not possible due to bad block list\n",
+			mdname(mddev));
+		return -EIO;
+	}
 	if (md_bitmap_enabled(mddev, false) &&
 	    mddev->bitmap_id == ID_LLBITMAP) {
 		i = mddev->bitmap_ops->resize(mddev, mddev->dev_sectors, 0);
-- 
2.54.0

[PATCH v4 3/3] md/raid5: warn when a read fails due in some part to bad-block metadata

[Doug V Johnson]

Report that bad block metadata contributed to a read failure on the
array so that it's clear that the failure may not be directly due to
an immediate underlying device read failure.

There has been a documented case of bad block metadata being erroneously
copied around contributing to mystery read failures with no underlying
device failure.  This warning will help users understand why the
failure exists and may lead to the discovery of this condition sooner.

Link: https://lore.kernel.org/r/20250224090209.2077-2-dougvj@dougvj.net
Link: https://lkml.iu.edu/2501.3/01088.html
Assisted-by: LLM
Signed-off-by: Doug V Johnson <me@dougvj.me>
---
 drivers/md/raid5.c | 16 ++++++++++++++++
 drivers/md/raid5.h |  2 +-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 23ff549fa363..f721eb97f655 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3672,6 +3672,16 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
 				wake_up_bit(&sh->dev[i].flags, R5_Overlap);
 			if (bi)
 				s->to_read--;
+			/*
+			 * If we recorded bad blocks from the metadata
+			 * on any of the devices then report this to
+			 * userspace in case anyone might suspect
+			 * something more fundamental instead
+			 */
+			if (bi && s->bad_blocks)
+				pr_warn_ratelimited("%s: read encountered block in device bad block list at %llu\n",
+						    mdname(conf->mddev),
+						    (unsigned long long)sh->sector);
 			while (bi && bi->bi_iter.bi_sector <
 			       sh->dev[i].sector + RAID5_STRIPE_SECTORS(conf)) {
 				struct bio *nextbi =
@@ -4764,6 +4774,12 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
 		if (rdev) {
 			is_bad = rdev_has_badblock(rdev, sh->sector,
 						   RAID5_STRIPE_SECTORS(conf));
+			if (is_bad) {
+				s->bad_blocks++;
+				pr_debug("bad blocks encountered dev %i sector %llu %lu\n",
+					 i, (unsigned long long)sh->sector,
+					 (unsigned long)RAID5_STRIPE_SECTORS(conf));
+			}
 			if (s->blocked_rdev == NULL) {
 				if (is_bad < 0)
 					set_bit(BlockedBadBlocks, &rdev->flags);
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index cb5feae04db2..6b90c9565ec0 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -282,7 +282,7 @@ struct stripe_head_state {
 	 * read all devices, just the replacement targets.
 	 */
 	int syncing, expanding, expanded, replacing;
-	int locked, uptodate, to_read, to_write, failed, written;
+	int locked, uptodate, to_read, to_write, failed, written, bad_blocks;
 	int to_fill, compute, req_compute, non_overwrite;
 	int injournal, just_cached;
 	int failed_num[2];
-- 
2.54.0

Re: [PATCH v4 1/3] md/raid5: freeze and interrupt reshape when handling failed reshape

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] md: stale did_freeze clears MD_RECOVERY_FROZEN requested by failed reshape
--

commit 0538f5adec4b4e67f586325e5d4064322224a7a9
Author: Doug V Johnson <me@dougvj.me>

md/raid5: freeze and interrupt reshape when handling failed reshape

This commit adds a freeze flag to stop reshape operations when an
unrecoverable read occurs during a failed reshape. It prevents automatic
reshaping resumption which otherwise leads to block writes being cancelled
while the reshape continues, corrupting data.

Link: https://github.com/dougvj/md-raid-reshape-badblock-corruption-test
Link: https://patch.msgid.link/20260624075824.2601110-1-chencheng@fnnas.com

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index b91545ce090de..86ea65559576a 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3773,6 +3773,10 @@ static void handle_failed_reshape(struct r5conf *conf, struct stripe_head *sh,
>  	bool was_expanding = test_and_clear_bit(STRIPE_EXPANDING, &sh->state);
>  	bool was_ready = test_and_clear_bit(STRIPE_EXPAND_READY, &sh->state);
>  
> +	/* Freeze reshaping to prevent data loss */
> +	set_bit(MD_RECOVERY_FROZEN, &conf->mddev->recovery);

[Severity: High]
Does setting the frozen state locklessly from the asynchronous worker thread
race with other configuration operations that clear it based on a stale
local variable?

If a user initiates an array teardown or read-only transition via sysfs
concurrently with a failing reshape:

1. do_md_stop() or md_set_readonly() observe MD_RECOVERY_FROZEN is clear,
   set it, and cache did_freeze = 1 locally.

2. They call stop_sync_thread() which intentionally drops
   mddev->reconfig_mutex while waiting.

3. Concurrently, the asynchronous raid5d worker executes this new patch
   logic in handle_failed_reshape() and sets MD_RECOVERY_FROZEN to prevent
   the reshape from resuming.

4. stop_sync_thread() returns and reacquires mddev->reconfig_mutex.

5. In error paths of do_md_stop() (or unconditionally in
   md_set_readonly()), the stale did_freeze variable is evaluated and
   MD_RECOVERY_FROZEN is cleared:

drivers/md/md.c:do_md_stop() {
    ...
    if (did_freeze) {
        clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
        set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
    }
    ...
}

Will this unconditionally clear the critical freeze requested by the worker
thread, causing MD_RECOVERY_NEEDED to be set and the array to automatically
resume the failed reshape, leading to data corruption?

> +	md_sync_error(conf->mddev);
> +
>  	if (was_expanding || was_ready) {
>  		atomic_dec(&conf->reshape_stripes);
>  		wake_up(&conf->wait_for_reshape);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/010101a142f57e37-cd14b78d-54cd-447b-9888-87f166a937ab-000000@us-west-2.amazonses.com?part=1