Re: [PATCH 1/4] md/raid10: prepare per-r10bio dev slot tracking

On Wed, Apr 22, 2026 at 08:40:42AM +0200, Paul Menzel wrote:

Hi Paul,

> Dear Cheng,
>
>
> Am 22.04.26 um 04:33 schrieb Chen Cheng:
> > From: Chen Cheng <[email protected]>
> >
> > raid10 reuses r10bio objects from both r10bio_pool and r10buf_pool. Track
> > the number of devs[] slots used by each request in the r10bio itself and
> > initialize it whenever one of these objects is reused.
> >
> > No functional change yet. A later patch will use this width when reshape
> > changes conf->geo.raid_disks.
>
> Your Signed-off-by: line is missing.

Yes, i missed it, thanks for point-out;

>
> > ---
> >   drivers/md/raid10.c | 4 ++++
> >   drivers/md/raid10.h | 1 +
> >   2 files changed, 5 insertions(+)
> >
> > diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> > index 0653b5d8545a..e93933632893 100644
> > --- a/drivers/md/raid10.c
> > +++ b/drivers/md/raid10.c
> > @@ -1540,6 +1540,7 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
> >     r10_bio->sector = bio->bi_iter.bi_sector;
> >     r10_bio->state = 0;
> >     r10_bio->read_slot = -1;
> > +   r10_bio->used_nr_devs = conf->geo.raid_disks;
> >     memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) *
> >                     conf->geo.raid_disks);
> > @@ -1727,6 +1728,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
> >     r10_bio->mddev = mddev;
> >     r10_bio->state = 0;
> >     r10_bio->sectors = 0;
> > +   r10_bio->used_nr_devs = geo->raid_disks;
> >     memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * geo->raid_disks);
> >     wait_blocked_dev(mddev, r10_bio);
> > @@ -3061,6 +3063,8 @@ static struct r10bio *raid10_alloc_init_r10buf(struct r10conf *conf)
> >     else
> >             nalloc = 2; /* recovery */
> > +   r10bio->used_nr_devs = nalloc;
> > +
> >     for (i = 0; i < nalloc; i++) {
> >             bio = r10bio->devs[i].bio;
> >             rp = bio->bi_private;
> > diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h
> > index ec79d87fb92f..92e8743023e6 100644
> > --- a/drivers/md/raid10.h
> > +++ b/drivers/md/raid10.h
> > @@ -127,6 +127,7 @@ struct r10bio {
> >      * if the IO is in READ direction, then this is where we read
> >      */
> >     int                     read_slot;
> > +   unsigned int            used_nr_devs;
>
> Most entries have a comment describing the use. Maybe add one too, or at
> least a blank line, so it’s clear that the existing comment is just for
> `read_slot`?

Agreed.

>
> >     struct list_head        retry_list;
> >     /*
>
> From a performance and resource usage point of view, will increasing the
> struct have a negative impact?

On 64-bit platform, doesn't have negative resource usage impact,
the new field fits into the existing padding after read_slot, so
offsetof(struct r10bio, devs) stays unchanged;

On 32-bit platform, may increase by 4 bytes per r10bio, but that's
negligible compared with the bios/pages allocated for each request;


No negative performance impact, cause bottleneck is IO, and
the IO path has no changed;

>
> The diff looks good.
>
> Reviewed-by: Paul Menzel <[email protected]>
>

Thanks for review;

>
> Kind regards,
>
> Paul


Thanks,
Cheng