Re: [PATCH 1/3] raid0, linear, md: add error_handlers for raid0 and linear

On 12/19/21 11:26 AM, Xiao Ni wrote:

On Fri, Dec 17, 2021 at 10:00 AM Guoqing Jiang [off-list ref] wrote:

quoted

On 12/16/21 10:52 PM, Mariusz Tkaczyk wrote:

quoted

Patch 62f7b1989c0 ("md raid0/linear: Mark array as 'broken' and fail BIOs
if a member is gone") allowed to finish writes earlier (before level
dependent actions) for non-redundant arrays.

To achieve that MD_BROKEN is added to mddev->flags if drive disappearance
is detected. This is done in is_mddev_broken() which is confusing and not
consistent with other levels where error_handler() is used.
This patch adds appropriate error_handler for raid0 and linear.

It also adopts md_error(), we only want to call .error_handler for those
levels. mddev->pers->sync_request is additionally checked, its existence
implies a level with redundancy.

Usage of error_handler causes that disk failure can be requested from
userspace. User can fail the array via #mdadm --set-faulty command. This
is not safe and will be fixed in mdadm. It is correctable because failed
state is not recorded in the metadata. After next assembly array will be
read-write again. For safety reason is better to keep MD_BROKEN in
runtime only.

Signed-off-by: Mariusz Tkaczyk <redacted>
---
   drivers/md/md-linear.c | 15 ++++++++++++++-
   drivers/md/md.c        |  6 +++++-
   drivers/md/md.h        | 10 ++--------
   drivers/md/raid0.c     | 15 ++++++++++++++-
   4 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/md/md-linear.c b/drivers/md/md-linear.c
index 1ff51647a682..415d2615d17a 100644
--- a/drivers/md/md-linear.c
+++ b/drivers/md/md-linear.c

@@ -233,7 +233,8 @@ static bool linear_make_request(struct mddev *mddev, struct bio *bio)
                    bio_sector < start_sector))
               goto out_of_bounds;

-     if (unlikely(is_mddev_broken(tmp_dev->rdev, "linear"))) {
+     if (unlikely(is_rdev_broken(tmp_dev->rdev))) {
+             md_error(mddev, tmp_dev->rdev);
               bio_io_error(bio);
               return true;
       }

@@ -281,6 +282,17 @@ static void linear_status (struct seq_file *seq, struct mddev *mddev)
       seq_printf(seq, " %dk rounding", mddev->chunk_sectors / 2);
   }

+static void linear_error(struct mddev *mddev, struct md_rdev *rdev)
+{
+     char b[BDEVNAME_SIZE];
+
+     if (!test_and_set_bit(MD_BROKEN, &rdev->mddev->flags))
+             pr_crit("md/linear%s: Disk failure on %s detected.\n"
+                     "md/linear:%s: Cannot continue, failing array.\n",
+                     mdname(mddev), bdevname(rdev->bdev, b),
+                     mdname(mddev));
+}
+

Do you consider to use %pg to print block device?

quoted

   static void linear_quiesce(struct mddev *mddev, int state)
   {
   }

@@ -297,6 +309,7 @@ static struct md_personality linear_personality =
       .hot_add_disk   = linear_add,
       .size           = linear_size,
       .quiesce        = linear_quiesce,
+     .error_handler  = linear_error,
   };

   static int __init linear_init (void)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index e8666bdc0d28..f888ef197765 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c

@@ -7982,7 +7982,11 @@ void md_error(struct mddev *mddev, struct md_rdev *rdev)

       if (!mddev->pers || !mddev->pers->error_handler)
               return;
-     mddev->pers->error_handler(mddev,rdev);
+     mddev->pers->error_handler(mddev, rdev);
+
+     if (!mddev->pers->sync_request)
+             return;
+

What is the reason of the above change? I suppose dm event can be missed.

Hi Guoqing

What's the dm event here?

Pls see mddev->event_work after above line, also commit 768e587e18 for dm
which might be relevant.

Thanks,
Guoqing