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

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.

> ---
>   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`?

>   
>   	struct list_head	retry_list;
>   	/*

 From a performance and resource usage point of view, will increasing 
the struct have a negative impact?

The diff looks good.

Reviewed-by: Paul Menzel <[email protected]>


Kind regards,

Paul