Re: [PATCH v3 1/3] md/raid1,raid10: Do not set MD_BROKEN on failfast io failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




在 2025/8/29 0:32, Kenta Akagi 写道:
This commit ensures that an MD_FAILFAST IO failure does not put
the array into a broken state.

When failfast is enabled on rdev in RAID1 or RAID10,
the array may be flagged MD_BROKEN in the following cases.
- If MD_FAILFAST IOs to multiple rdevs fail simultaneously
- If an MD_FAILFAST metadata write to the 'last' rdev fails

[...]

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 408c26398321..8a61fd93b3ff 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -470,6 +470,7 @@ static void raid1_end_write_request(struct bio *bio)
  		    (bio->bi_opf & MD_FAILFAST) &&
  		    /* We never try FailFast to WriteMostly devices */
  		    !test_bit(WriteMostly, &rdev->flags)) {
+			set_bit(FailfastIOFailure, &rdev->flags);
  			md_error(r1_bio->mddev, rdev);
  		}
@@ -1746,8 +1747,12 @@ static void raid1_status(struct seq_file *seq, struct mddev *mddev)
   *	- recovery is interrupted.
   *	- &mddev->degraded is bumped.
   *
- * @rdev is marked as &Faulty excluding case when array is failed and
- * &mddev->fail_last_dev is off.
+ * If @rdev has &FailfastIOFailure and it is the 'last' rdev,
+ * then @mddev and @rdev will not be marked as failed.
+ *
+ * @rdev is marked as &Faulty excluding any cases:
+ *	- when @mddev is failed and &mddev->fail_last_dev is off
+ *	- when @rdev is last device and &FailfastIOFailure flag is set
   */
  static void raid1_error(struct mddev *mddev, struct md_rdev *rdev)
  {
@@ -1758,6 +1763,13 @@ static void raid1_error(struct mddev *mddev, struct md_rdev *rdev)
if (test_bit(In_sync, &rdev->flags) &&
  	    (conf->raid_disks - mddev->degraded) == 1) {
+		if (test_and_clear_bit(FailfastIOFailure, &rdev->flags)) {
+			spin_unlock_irqrestore(&conf->device_lock, flags);
+			pr_warn_ratelimited("md/raid1:%s: Failfast IO failure on %pg, "
+				"last device but ignoring it\n",
+				mdname(mddev), rdev->bdev);
+			return;
+		}
  		set_bit(MD_BROKEN, &mddev->flags);
if (!mddev->fail_last_dev) {
@@ -2148,6 +2160,7 @@ static int fix_sync_read_error(struct r1bio *r1_bio)
  	if (test_bit(FailFast, &rdev->flags)) {
  		/* Don't try recovering from here - just fail it
  		 * ... unless it is the last working device of course */
+		set_bit(FailfastIOFailure, &rdev->flags);
  		md_error(mddev, rdev);
  		if (test_bit(Faulty, &rdev->flags))
  			/* Don't try to read from here, but make sure
@@ -2652,6 +2665,7 @@ static void handle_read_error(struct r1conf *conf, struct r1bio *r1_bio)
  		fix_read_error(conf, r1_bio);
  		unfreeze_array(conf);
  	} else if (mddev->ro == 0 && test_bit(FailFast, &rdev->flags)) {
+		set_bit(FailfastIOFailure, &rdev->flags);
  		md_error(mddev, rdev);
  	} else {
  		r1_bio->bios[r1_bio->read_disk] = IO_BLOCKED;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index b60c30bfb6c7..530ad6503189 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -488,6 +488,7 @@ static void raid10_end_write_request(struct bio *bio)
  			dec_rdev = 0;
  			if (test_bit(FailFast, &rdev->flags) &&
  			    (bio->bi_opf & MD_FAILFAST)) {
+				set_bit(FailfastIOFailure, &rdev->flags);
  				md_error(rdev->mddev, rdev);
  			}

Thank you for the patch. There may be an issue with 'test_and_clear'.

If two write IO go to the same rdev, MD_BROKEN may be set as below:

IO1					IO2
set FailfastIOFailure
					set FailfastIOFailure		
 md_error
  raid1_error
   test_and_clear FailfastIOFailur
  					 md_error
					  raid1_error
					   //FailfastIOFailur is cleared
					   set MD_BROKEN

Maybe we should check whether FailfastIOFailure is already set before
setting it. It also needs to be considered in metadata writes.

@@ -1995,8 +1996,12 @@ static int enough(struct r10conf *conf, int ignore)
   *	- recovery is interrupted.
   *	- &mddev->degraded is bumped.
   *
- * @rdev is marked as &Faulty excluding case when array is failed and
- * &mddev->fail_last_dev is off.
+ * If @rdev has &FailfastIOFailure and it is the 'last' rdev,
+ * then @mddev and @rdev will not be marked as failed.
+ *
+ * @rdev is marked as &Faulty excluding any cases:
+ *	- when @mddev is failed and &mddev->fail_last_dev is off
+ *	- when @rdev is last device and &FailfastIOFailure flag is set
   */
  static void raid10_error(struct mddev *mddev, struct md_rdev *rdev)
  {
@@ -2006,6 +2011,13 @@ static void raid10_error(struct mddev *mddev, struct md_rdev *rdev)
  	spin_lock_irqsave(&conf->device_lock, flags);
if (test_bit(In_sync, &rdev->flags) && !enough(conf, rdev->raid_disk)) {
+		if (test_and_clear_bit(FailfastIOFailure, &rdev->flags)) {
+			spin_unlock_irqrestore(&conf->device_lock, flags);
+			pr_warn_ratelimited("md/raid10:%s: Failfast IO failure on %pg, "
+				"last device but ignoring it\n",
+				mdname(mddev), rdev->bdev);
+			return;
+ >   		set_bit(MD_BROKEN, &mddev->flags);
if (!mddev->fail_last_dev) {
@@ -2413,6 +2425,7 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
  				continue;
  		} else if (test_bit(FailFast, &rdev->flags)) {
  			/* Just give up on this device */
+			set_bit(FailfastIOFailure, &rdev->flags);
  			md_error(rdev->mddev, rdev);
  			continue;
  		}
@@ -2865,8 +2878,10 @@ static void handle_read_error(struct mddev *mddev, struct r10bio *r10_bio)
  		freeze_array(conf, 1);
  		fix_read_error(conf, mddev, r10_bio);
  		unfreeze_array(conf);
-	} else
+	} else {
+		set_bit(FailfastIOFailure, &rdev->flags);
  		md_error(mddev, rdev);
+	}
rdev_dec_pending(rdev, mddev);
  	r10_bio->state = 0;

--
Thanks,
Nan





[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux