[PATCH] md/raid1,raid10: don't broken array on failfast metadata write fails

It is not intended for the array to fail when a metadata write with
MD_FAILFAST fails.
After commit 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10"),
when md_error is called on the last device in RAID1/10,
the MD_BROKEN flag is set on the array.
Because of this, a failfast metadata write failure will
make the array "broken" state.

If rdev is not Faulty even after calling md_error,
the rdev is the last device, and there is nothing except
MD_BROKEN that prevents writes to the array.
Therefore, by clearing MD_BROKEN, the array will not become
"broken" after a failfast metadata write failure.

Fixes: 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10")
Signed-off-by: Kenta Akagi <redacted>
---
 drivers/md/md.c | 1 +
 drivers/md/md.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index ac85ec73a409..3ec4abf02fa0 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c

@@ -1002,6 +1002,7 @@ static void super_written(struct bio *bio)
 		md_error(mddev, rdev);
 		if (!test_bit(Faulty, &rdev->flags)
 		    && (bio->bi_opf & MD_FAILFAST)) {
+			clear_bit(MD_BROKEN, &mddev->flags);
 			set_bit(MD_SB_NEED_REWRITE, &mddev->sb_flags);
 			set_bit(LastDev, &rdev->flags);
 		}

diff --git a/drivers/md/md.h b/drivers/md/md.h
index 51af29a03079..2f87bcc5d834 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h

@@ -332,7 +332,7 @@ struct md_cluster_operations;
  *			       resync lock, need to release the lock.
  * @MD_FAILFAST_SUPPORTED: Using MD_FAILFAST on metadata writes is supported as
  *			    calls to md_error() will never cause the array to
- *			    become failed.
+ *			    become failed while fail_last_dev is not set.
  * @MD_HAS_PPL:  The raid array has PPL feature set.
  * @MD_HAS_MULTIPLE_PPLS: The raid array has multiple PPLs feature set.
  * @MD_NOT_READY: do_md_run() is active, so 'array_state', ust not report that

-- 
2.50.1

Hi,

在 2025/08/12 17:01, Kenta Akagi 写道:

It is not intended for the array to fail when a metadata write with
MD_FAILFAST fails.
After commit 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10"),
when md_error is called on the last device in RAID1/10,
the MD_BROKEN flag is set on the array.
Because of this, a failfast metadata write failure will
make the array "broken" state.

If rdev is not Faulty even after calling md_error,
the rdev is the last device, and there is nothing except
MD_BROKEN that prevents writes to the array.
Therefore, by clearing MD_BROKEN, the array will not become
"broken" after a failfast metadata write failure.

I don't understand here, I think MD_BROKEN is expected, the last
rdev has IO error while updating metadata, the array is now broken
and you can only read it afterwards. Allow using this broken array
read-write might causing more severe problem like data loss.

Thanks,
Kuai

On 2025/08/13 9:59, Yu Kuai wrote:

Hi,

在 2025/08/12 17:01, Kenta Akagi 写道:

quoted

It is not intended for the array to fail when a metadata write with
MD_FAILFAST fails.
After commit 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10"),
when md_error is called on the last device in RAID1/10,
the MD_BROKEN flag is set on the array.
Because of this, a failfast metadata write failure will
make the array "broken" state.

If rdev is not Faulty even after calling md_error,
the rdev is the last device, and there is nothing except
MD_BROKEN that prevents writes to the array.
Therefore, by clearing MD_BROKEN, the array will not become
"broken" after a failfast metadata write failure.

I don't understand here, I think MD_BROKEN is expected, the last
rdev has IO error while updating metadata, the array is now broken
and you can only read it afterwards. Allow using this broken array
read-write might causing more severe problem like data loss.

Thank you for reviewing.

I think that only when the bio has the MD_FAILFAST flag,
a metadata write failure to the last rdev should not make it
broken array at that point.

This is because a metadata write with MD_FAILFAST is retried after
failure as follows:

1. In super_written, MD_SB_NEED_REWRITE is set in sb_flags.

2. In md_super_wait, which is called by the function that
executed md_super_write and waits for completion,
-EAGAIN is returned because MD_SB_NEED_REWRITE is set.

3. The caller of md_super_wait (such as md_update_sb)
receives a negative return value and then retries md_super_write.

4. The md_super_write function, which is called to perform
the same metadata write, issues a write bio
without MD_FAILFAST this time, because the rdev has LastDev flag.

When a bio from super_written without MD_FAILFAST fails,
the array is truly broken, and MD_BROKEN should be set.

A failfast bio, for example in the case of nvme-tcp ,
will fail immediately if the connection to the target is
lost for a few seconds and the device enters a reconnecting
state - even though it would recover if given a few seconds.
This behavior is exactly as intended by the design of failfast.

However, md treats super_write operations fails with failfast as fatal.
For example, if an initiator - that is, a machine loading the md module -
loses all connections for a few seconds, the array becomes
broken and subsequent write is no longer possible.
This is the issue I am currently facing, and which this patch aims to fix.

Should I add more context to the commit message? Please advise.

Thanks,
AKAGI

Thanks,
Kuai

quoted

Fixes: 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10")
Signed-off-by: Kenta Akagi <redacted>
---
  drivers/md/md.c | 1 +
  drivers/md/md.h | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index ac85ec73a409..3ec4abf02fa0 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c

@@ -1002,6 +1002,7 @@ static void super_written(struct bio *bio)

          md_error(mddev, rdev);
          if (!test_bit(Faulty, &rdev->flags)
              && (bio->bi_opf & MD_FAILFAST)) {
+            clear_bit(MD_BROKEN, &mddev->flags);
              set_bit(MD_SB_NEED_REWRITE, &mddev->sb_flags);
              set_bit(LastDev, &rdev->flags);
          }

diff --git a/drivers/md/md.h b/drivers/md/md.h
index 51af29a03079..2f87bcc5d834 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h

@@ -332,7 +332,7 @@ struct md_cluster_operations;

   *                   resync lock, need to release the lock.
   * @MD_FAILFAST_SUPPORTED: Using MD_FAILFAST on metadata writes is supported as
   *                calls to md_error() will never cause the array to
- *                become failed.
+ *                become failed while fail_last_dev is not set.
   * @MD_HAS_PPL:  The raid array has PPL feature set.
   * @MD_HAS_MULTIPLE_PPLS: The raid array has multiple PPLs feature set.
   * @MD_NOT_READY: do_md_run() is active, so 'array_state', ust not report that

Hi,

在 2025/08/14 23:54, Kenta Akagi 写道:

On 2025/08/13 9:59, Yu Kuai wrote:

quoted

Hi,

在 2025/08/12 17:01, Kenta Akagi 写道:

quoted

It is not intended for the array to fail when a metadata write with
MD_FAILFAST fails.
After commit 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10"),
when md_error is called on the last device in RAID1/10,
the MD_BROKEN flag is set on the array.
Because of this, a failfast metadata write failure will
make the array "broken" state.

If rdev is not Faulty even after calling md_error,
the rdev is the last device, and there is nothing except
MD_BROKEN that prevents writes to the array.
Therefore, by clearing MD_BROKEN, the array will not become
"broken" after a failfast metadata write failure.

I don't understand here, I think MD_BROKEN is expected, the last
rdev has IO error while updating metadata, the array is now broken
and you can only read it afterwards. Allow using this broken array
read-write might causing more severe problem like data loss.

Thank you for reviewing.

I think that only when the bio has the MD_FAILFAST flag,
a metadata write failure to the last rdev should not make it
broken array at that point.

This is because a metadata write with MD_FAILFAST is retried after
failure as follows:

1. In super_written, MD_SB_NEED_REWRITE is set in sb_flags.

2. In md_super_wait, which is called by the function that
executed md_super_write and waits for completion,
-EAGAIN is returned because MD_SB_NEED_REWRITE is set.

3. The caller of md_super_wait (such as md_update_sb)
receives a negative return value and then retries md_super_write.

4. The md_super_write function, which is called to perform
the same metadata write, issues a write bio
without MD_FAILFAST this time, because the rdev has LastDev flag.

When a bio from super_written without MD_FAILFAST fails,
the array is truly broken, and MD_BROKEN should be set.

A failfast bio, for example in the case of nvme-tcp ,
will fail immediately if the connection to the target is
lost for a few seconds and the device enters a reconnecting
state - even though it would recover if given a few seconds.
This behavior is exactly as intended by the design of failfast.

However, md treats super_write operations fails with failfast as fatal.
For example, if an initiator - that is, a machine loading the md module -
loses all connections for a few seconds, the array becomes
broken and subsequent write is no longer possible.
This is the issue I am currently facing, and which this patch aims to fix.

Should I add more context to the commit message? Please advise.

Yes, please explain in detail in commit message.

Thanks,
AKAGI

quoted

Thanks,
Kuai

quoted

Fixes: 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10")
Signed-off-by: Kenta Akagi <redacted>
---
   drivers/md/md.c | 1 +
   drivers/md/md.h | 2 +-
   2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index ac85ec73a409..3ec4abf02fa0 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c

@@ -1002,6 +1002,7 @@ static void super_written(struct bio *bio)
           md_error(mddev, rdev);
           if (!test_bit(Faulty, &rdev->flags)
               && (bio->bi_opf & MD_FAILFAST)) {
+            clear_bit(MD_BROKEN, &mddev->flags);

And I feel a beeter way is to set MD_BROKEN only if the last rdev
failed, set it in middle and clear it is werid.

Thanks,
Kuai

quoted

quoted

               set_bit(MD_SB_NEED_REWRITE, &mddev->sb_flags);
               set_bit(LastDev, &rdev->flags);
           }

diff --git a/drivers/md/md.h b/drivers/md/md.h
index 51af29a03079..2f87bcc5d834 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h

@@ -332,7 +332,7 @@ struct md_cluster_operations;
    *                   resync lock, need to release the lock.
    * @MD_FAILFAST_SUPPORTED: Using MD_FAILFAST on metadata writes is supported as
    *                calls to md_error() will never cause the array to
- *                become failed.
+ *                become failed while fail_last_dev is not set.
    * @MD_HAS_PPL:  The raid array has PPL feature set.
    * @MD_HAS_MULTIPLE_PPLS: The raid array has multiple PPLs feature set.
    * @MD_NOT_READY: do_md_run() is active, so 'array_state', ust not report that

.

On 2025/08/15 10:26, Yu Kuai wrote:

Hi,

在 2025/08/14 23:54, Kenta Akagi 写道:

quoted

On 2025/08/13 9:59, Yu Kuai wrote:

quoted

Hi,

在 2025/08/12 17:01, Kenta Akagi 写道:

quoted

It is not intended for the array to fail when a metadata write with
MD_FAILFAST fails.
After commit 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10"),
when md_error is called on the last device in RAID1/10,
the MD_BROKEN flag is set on the array.
Because of this, a failfast metadata write failure will
make the array "broken" state.

If rdev is not Faulty even after calling md_error,
the rdev is the last device, and there is nothing except
MD_BROKEN that prevents writes to the array.
Therefore, by clearing MD_BROKEN, the array will not become
"broken" after a failfast metadata write failure.

I don't understand here, I think MD_BROKEN is expected, the last
rdev has IO error while updating metadata, the array is now broken
and you can only read it afterwards. Allow using this broken array
read-write might causing more severe problem like data loss.

Thank you for reviewing.

I think that only when the bio has the MD_FAILFAST flag,
a metadata write failure to the last rdev should not make it
broken array at that point.

This is because a metadata write with MD_FAILFAST is retried after
failure as follows:

1. In super_written, MD_SB_NEED_REWRITE is set in sb_flags.

2. In md_super_wait, which is called by the function that
executed md_super_write and waits for completion,
-EAGAIN is returned because MD_SB_NEED_REWRITE is set.

3. The caller of md_super_wait (such as md_update_sb)
receives a negative return value and then retries md_super_write.

4. The md_super_write function, which is called to perform
the same metadata write, issues a write bio
without MD_FAILFAST this time, because the rdev has LastDev flag.

When a bio from super_written without MD_FAILFAST fails,
the array is truly broken, and MD_BROKEN should be set.

A failfast bio, for example in the case of nvme-tcp ,
will fail immediately if the connection to the target is
lost for a few seconds and the device enters a reconnecting
state - even though it would recover if given a few seconds.
This behavior is exactly as intended by the design of failfast.

However, md treats super_write operations fails with failfast as fatal.
For example, if an initiator - that is, a machine loading the md module -
loses all connections for a few seconds, the array becomes
broken and subsequent write is no longer possible.
This is the issue I am currently facing, and which this patch aims to fix.

Should I add more context to the commit message? Please advise.

Yes, please explain in detail in commit message.

quoted

Thanks,
AKAGI

quoted

Thanks,
Kuai

quoted

Fixes: 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10")
Signed-off-by: Kenta Akagi <redacted>
---
   drivers/md/md.c | 1 +
   drivers/md/md.h | 2 +-
   2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index ac85ec73a409..3ec4abf02fa0 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c

@@ -1002,6 +1002,7 @@ static void super_written(struct bio *bio)

           md_error(mddev, rdev);
           if (!test_bit(Faulty, &rdev->flags)
               && (bio->bi_opf & MD_FAILFAST)) {
+            clear_bit(MD_BROKEN, &mddev->flags);

And I feel a beeter way is to set MD_BROKEN only if the last rdev
failed, set it in middle and clear it is werid.

Copy.
I'll modify logic and commit message, then send it out as v2.

Thanks, 
Akagi

Thanks,
Kuai

quoted

quoted

quoted

               set_bit(MD_SB_NEED_REWRITE, &mddev->sb_flags);
               set_bit(LastDev, &rdev->flags);
           }

diff --git a/drivers/md/md.h b/drivers/md/md.h
index 51af29a03079..2f87bcc5d834 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h

@@ -332,7 +332,7 @@ struct md_cluster_operations;

    *                   resync lock, need to release the lock.
    * @MD_FAILFAST_SUPPORTED: Using MD_FAILFAST on metadata writes is supported as
    *                calls to md_error() will never cause the array to
- *                become failed.
+ *                become failed while fail_last_dev is not set.
    * @MD_HAS_PPL:  The raid array has PPL feature set.
    * @MD_HAS_MULTIPLE_PPLS: The raid array has multiple PPLs feature set.
    * @MD_NOT_READY: do_md_run() is active, so 'array_state', ust not report that

.