[PATCH] md/raid5: fix inverted condition in rdev_set_badblocks call

[PATCH] md/raid5: fix inverted condition in rdev_set_badblocks call

[zhanghao]

Commit fd4d44c14ff6 ("md: mark rdev Faulty when badblocks setting fails")
attempted to unify badblock handling but introduced a logic inversion in
raid5_end_read_request().

The original check was:

    if (!(set_bad && In_sync && rdev_set_badblocks(...)))
            md_error(...);

which records badblocks when `set_bad && In_sync`, and calls md_error()
either when recording fails or when the rdev is not In_sync. The new
condition `if (!(set_bad && In_sync))` skips rdev_set_badblocks() exactly
during recovery/reshape (In_sync is set), so failed badblock updates no
longer trigger md_error() and the rdev is not marked Faulty. This allows
reconstruction to continue on a disk with a saturated badblock table,
potentially propagating corrupted data to the replacement disk.

Fix the condition to match the intended behavior: call rdev_set_badblocks()
when `set_bad && In_sync`, and let the callee handle md_error() on failure.

Fixes: fd4d44c14ff6 ("md: mark rdev Faulty when badblocks setting fails")
Cc: stable@vger.kernel.org
Signed-off-by: zhanghao <zh_exit@163.com>
---
 drivers/md/raid5.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index d52adf81065a..dcfb1bfa9ebf 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2866,7 +2866,7 @@ static void raid5_end_read_request(struct bio * bi)
 		else {
 			clear_bit(R5_ReadError, &sh->dev[i].flags);
 			clear_bit(R5_ReWrite, &sh->dev[i].flags);
-			if (!(set_bad && test_bit(In_sync, &rdev->flags)))
+			if (set_bad && test_bit(In_sync, &rdev->flags))
 				rdev_set_badblocks(rdev, sh->sector,
 						   RAID5_STRIPE_SECTORS(conf), 0);
 		}
-- 
2.43.0

[PATCH v2] md/raid5: fix inverted condition in rdev_set_badblocks call

[zhanghao]

Commit fd4d44c14ff6 ("md: mark rdev Faulty when badblocks setting fails")
attempted to unify badblock handling but introduced a logic inversion in
raid5_end_read_request().

The original check was:

    if (!(set_bad && In_sync && rdev_set_badblocks(...)))
            md_error(...);

which records badblocks when `set_bad && In_sync`, and calls md_error()
either when recording fails or when the rdev is not In_sync. The new
condition `if (!(set_bad && In_sync))` skips rdev_set_badblocks() exactly
during recovery/reshape (In_sync is set), so failed badblock updates no
longer trigger md_error() and the rdev is not marked Faulty. This allows
reconstruction to continue on a disk with a saturated badblock table,
potentially propagating corrupted data to the replacement disk.

Restore the intended behavior by recording badblocks when `set_bad &&
In_sync`, and calling md_error() in the else branch so that non-In_sync
rdevs, replacement devices, and the too-many-read-errors path are still
marked Faulty.

Fixes: fd4d44c14ff6 ("md: mark rdev Faulty when badblocks setting fails")
Cc: stable@vger.kernel.org
Signed-off-by: zhanghao <zh_exit@163.com>
---
v2:
- Add the else branch calling md_error() for the !(set_bad && In_sync)
  cases, as pointed out by Sashiko: replacement device read errors,
  the too-many-read-errors path, and non-In_sync uncorrectable reads
  would otherwise no longer mark the rdev Faulty.

 drivers/md/raid5.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index d52adf81065a..25395a9c996a 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2866,9 +2866,11 @@ static void raid5_end_read_request(struct bio * bi)
 		else {
 			clear_bit(R5_ReadError, &sh->dev[i].flags);
 			clear_bit(R5_ReWrite, &sh->dev[i].flags);
-			if (!(set_bad && test_bit(In_sync, &rdev->flags)))
+			if (set_bad && test_bit(In_sync, &rdev->flags))
 				rdev_set_badblocks(rdev, sh->sector,
 						   RAID5_STRIPE_SECTORS(conf), 0);
+			else
+				md_error(conf->mddev, rdev);
 		}
 	}
 	rdev_dec_pending(rdev, conf->mddev);
-- 
2.43.0