Re: [PATCH v2 2/3] md/raid1,raid10: fix error-path detection with md_cloned_bio()

On Thu, May 21, 2026 at 9:25 AM Yu Kuai <[email protected]> wrote:
>
> Hi,
>
> 在 2026/5/1 19:46, Abd-Alrhman Masalkhi 写道:
> > Detect the error path using md_cloned_bio() instead of relying
> > on r1_bio in raid1 or r10_bio->read_slot in raid10, which may be
> > NULL or -1 after splitting and resubmitting a failed bio.
> >
> > As a result, the error path may not be recognized and memory
> > allocations can incorrectly use GFP_NOIO instead of
> > (GFP_NOIO | __GFP_HIGH), which can lead to a deadlock under
> > memory pressure.
>
> I don't think this patch will fix this problem. Because split bio
> is a new bio that will be resubmit by md_submit_bio(), such bio is
> not a md_cloned_bio yet until md_account_bio().

Hi Kuai

        bio = bio_submit_split_bioset(bio, max_sectors,
                          &conf->bio_split);
bio_submit_split_bioset returns the split bio and the remainder bio
will be submitted again which is already a md_cloned_bio.

Regards
Xiao
>
> >
> > Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().")
> > Fixes: 545250f24809 ("md/raid10: simplify handle_read_error()")
> > Signed-off-by: Abd-Alrhman Masalkhi <[email protected]>
> > ---
> > This patch depends on patch 1.
> >
> > Changes in v2:
> >   - New patch.
> > ---
> >   drivers/md/raid1.c  | 13 ++++++++++---
> >   drivers/md/raid10.c | 20 ++++++++++++++------
> >   2 files changed, 24 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> > index cc9914bd15c1..c52ecd38c163 100644
> > --- a/drivers/md/raid1.c
> > +++ b/drivers/md/raid1.c
> > @@ -1321,11 +1321,18 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> >       bool r1bio_existed = !!r1_bio;
> >
> >       /*
> > -      * If r1_bio is set, we are blocking the raid1d thread
> > -      * so there is a tiny risk of deadlock.  So ask for
> > +      * An md cloned bio indicates we are in the error path.
> > +      * This is more reliable than checking r1_bio, which might
> > +      * be NULL even in the error path if a failed bio was split.
> > +      */
> > +     bool err_path = md_cloned_bio(mddev, bio);
> > +
> > +     /*
> > +      * If we are in the error path, we are blocking the raid1d
> > +      * thread so there is a tiny risk of deadlock.  So ask for
> >        * emergency memory if needed.
> >        */
> > -     gfp_t gfp = r1_bio ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
> > +     gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
> >
> >       /*
> >        * Still need barrier for READ in case that whole
> > diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> > index 3a591e60a144..8c6fc398260e 100644
> > --- a/drivers/md/raid10.c
> > +++ b/drivers/md/raid10.c
> > @@ -1155,7 +1155,20 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
> >       char b[BDEVNAME_SIZE];
> >       int slot = r10_bio->read_slot;
> >       struct md_rdev *err_rdev = NULL;
> > -     gfp_t gfp = GFP_NOIO;
> > +
> > +     /*
> > +      * An md cloned bio indicates we are in the error path.
> > +      * This is more reliable than checking slot, which might
> > +      * be -1 even in the error path if a failed bio was split.
> > +      */
> > +     bool err_path = md_cloned_bio(mddev, bio);
> > +
> > +     /*
> > +      * If we are in the error path, we are blocking the raid10d
> > +      * thread so there is a tiny risk of deadlock.  So ask for
> > +      * emergency memory if needed.
> > +      */
> > +     gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
> >
> >       if (slot >= 0 && r10_bio->devs[slot].rdev) {
> >               /*
> > @@ -1166,11 +1179,6 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
> >                * we lose the device name in error messages.
> >                */
> >               int disk;
> > -             /*
> > -              * As we are blocking raid10, it is a little safer to
> > -              * use __GFP_HIGH.
> > -              */
> > -             gfp = GFP_NOIO | __GFP_HIGH;
> >
> >               disk = r10_bio->devs[slot].devnum;
> >               err_rdev = conf->mirrors[disk].rdev;
>
> --
> Thansk,
> Kuai
>