Re: [PATCH 4/6] md/raid5: make the stripe batch size a module parameter

Hi,

=E5=9C=A8 2026/7/10 21:23, Hiroshi Nishida =E5=86=99=E9=81=93:
> handle_active_stripes() dequeues up to MAX_STRIPE_BATCH (8) stripes from =
a
> worker group under a single device_lock acquisition, and
> raid5_wakeup_stripe_thread() divides group->stripes_cnt by the same value
> to decide how many additional workers to wake.  The two uses must move
> together: the dequeue cap bounds how many stripes one worker drains, whil=
e
> the spawn divisor sizes the worker fan-out to match.
>
> Make the batch size a per-array value chosen by a new max_stripe_batch
> module parameter (1-32, default 8, so behaviour is unchanged out of the
> box).  Both the dequeue cap and the spawn divisor read the single
> conf->max_stripe_batch, so they cannot drift apart.  The on-stack batch[]
> array in handle_active_stripes() is sized to the STRIPE_BATCH_MAX (32)
> upper bound; only conf->max_stripe_batch entries are ever used.
>
> The value is resolved when an array is created.  On busy multi-threaded
> arrays a larger batch amortises the device_lock over more stripes at some
> latency cost; unlike the hash-lock count and the cache-size limit there i=
s
> no general hardware signal for the best batch size, so the default is lef=
t
> at the historical 8 and the value is simply exposed for tuning.
>
> Signed-off-by: Hiroshi Nishida <[email protected]>
> ---
>   drivers/md/raid5.c | 15 ++++++++++++---
>   drivers/md/raid5.h | 12 +++++++++++-
>   2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index e41d3fc92dd0..5f0825c5effe 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -78,6 +78,11 @@ module_param(stripe_cache_size_max, uint, 0644);
>   MODULE_PARM_DESC(stripe_cache_size_max,
>   		 "Maximum the per-array stripe_cache_size may be raised to.  0 (the d=
efault) derives the limit from system memory (never below the historical 32=
768), so large-memory hosts can grow the stripe cache without a recompile w=
hile small ones are not offered a limit above what RAM can back.  A non-zer=
o value sets a fixed limit");
>  =20
> +static unsigned int max_stripe_batch =3D STRIPE_BATCH_DEFAULT;
> +module_param(max_stripe_batch, uint, 0644);
> +MODULE_PARM_DESC(max_stripe_batch,
> +		 "Number of stripes a worker thread handles per device_lock acquisitio=
n, 1-32 (default 8).  Larger values amortise the lock over more stripes on =
busy multi-threaded arrays at some latency cost.  Read when an array is cre=
ated");
> +
>   static bool devices_handle_discard_safely =3D false;
>   module_param(devices_handle_discard_safely, bool, 0644);
>   MODULE_PARM_DESC(devices_handle_discard_safely,
> @@ -221,7 +226,7 @@ static void raid5_wakeup_stripe_thread(struct stripe_=
head *sh)
>   	/* at least one worker should run to avoid race */
>   	queue_work_on(sh->cpu, raid5_wq, &group->workers[0].work);
>  =20
> -	thread_cnt =3D group->stripes_cnt / MAX_STRIPE_BATCH - 1;
> +	thread_cnt =3D group->stripes_cnt / conf->max_stripe_batch - 1;
>   	/* wakeup more workers */
>   	for (i =3D 1; i < conf->worker_cnt_per_group && thread_cnt > 0; i++) {
>   		if (group->workers[i].working =3D=3D false) {
> @@ -6734,11 +6739,11 @@ static int handle_active_stripes(struct r5conf *c=
onf, int group,
>   				 struct list_head *temp_inactive_list)
>   		__must_hold(&conf->device_lock)
>   {
> -	struct stripe_head *batch[MAX_STRIPE_BATCH], *sh;
> +	struct stripe_head *batch[STRIPE_BATCH_MAX], *sh;
>   	int i, batch_size =3D 0, hash;
>   	bool release_inactive =3D false;
>  =20
> -	while (batch_size < MAX_STRIPE_BATCH &&
> +	while (batch_size < conf->max_stripe_batch &&
>   			(sh =3D __get_priority_stripe(conf, group)) !=3D NULL)
>   		batch[batch_size++] =3D sh;
>  =20
> @@ -7640,6 +7645,10 @@ static struct r5conf *setup_conf(struct mddev *mdd=
ev)
>   	    !conf->temp_inactive_list)
>   		goto abort;
>  =20
> +	/* Resolve the stripe batch size (see STRIPE_BATCH_* in raid5.h). */
> +	conf->max_stripe_batch =3D clamp_t(int, max_stripe_batch,
> +					 1, STRIPE_BATCH_MAX);
> +
>   #if PAGE_SIZE !=3D DEFAULT_STRIPE_SIZE
>   	conf->stripe_size =3D DEFAULT_STRIPE_SIZE;
>   	conf->stripe_shift =3D ilog2(DEFAULT_STRIPE_SIZE) - 9;
> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
> index 10c45fa22955..69a41c1310af 100644
> --- a/drivers/md/raid5.h
> +++ b/drivers/md/raid5.h
> @@ -490,7 +490,16 @@ struct disk_info {
>   #define BYPASS_THRESHOLD	1
>   #define NR_HASH			(PAGE_SIZE / sizeof(struct hlist_head))
>   #define HASH_MASK		(NR_HASH - 1)
> -#define MAX_STRIPE_BATCH	8
> +/*
> + * Stripe batch size: how many stripes a worker dequeues from its group =
per
> + * device_lock acquisition in handle_active_stripes(); the same value di=
vides
> + * group->stripes_cnt in raid5_wakeup_stripe_thread() to choose how many=
 extra
> + * workers to spawn, so the two stay coupled.  Selected per array by the
> + * max_stripe_batch module parameter: STRIPE_BATCH_DEFAULT is the histor=
ical
> + * value and STRIPE_BATCH_MAX bounds the on-stack batch[] array.
> + */
> +#define STRIPE_BATCH_DEFAULT	8
> +#define STRIPE_BATCH_MAX	32
>  =20
>   /*
>    * The stripe cache hash is striped across a power-of-two number of spi=
nlocks,
> @@ -690,6 +699,7 @@ struct r5conf {
>   	struct r5worker_group	*worker_groups;
>   	int			group_cnt;
>   	int			worker_cnt_per_group;
> +	int			max_stripe_batch; /* stripes/dequeue, 1..STRIPE_BATCH_MAX */

This is still weird to use a global module parameter for per-array setting.=
 Please consider
a new per-array sysfs API as well, and pass in while creating or assembling=
 the array.

>   	struct r5l_log		*log;
>   	void			*log_private;
>  =20

--=20
Thanks,
Kuai