[PATCH 0/6] md/raid5: size stripe-cache and worker tuning from the hardware

[PATCH 0/6] md/raid5: size stripe-cache and worker tuning from the hardware

[Hiroshi Nishida]

md/raid5 has several tuning values that are compile-time constants chosen
for the modest systems of years ago:

  - NR_STRIPE_HASH_LOCKS   (8)    stripe-cache hash lock striping
  - the 32768 stripe_cache_size ceiling
  - NR_STRIPES             (256)  initial stripe cache size
  - MAX_STRIPE_BATCH       (8)    stripes handled per device_lock
  - worker_cnt_per_group   (0)    raid5 worker threads (group_thread_cnt)

On a large-memory, many-core host backing a wide array these are all too
small, and the only remedy today is to retune each one by hand after every
assembly, or to recompile.  On a small NAS a fixed larger value would just
waste memory.

This series makes each value derive a default from the hardware -- system
memory for the cache sizes, CPU count for the lock and worker counts --
while keeping every one overridable:

  1/6 (prerequisite) size the worker_groups[] array by nr_node_ids
  2/6 size the stripe-cache hash locks from the CPU count (8..32)
  3/6 scale the stripe_cache_size limit with memory (never below 32768)
  4/6 make the stripe batch size a module parameter
  5/6 scale the default stripe cache size with memory (256..4096)
  6/6 derive the default group_thread_cnt from the CPU count

Patch 1 is a prerequisite correctness fix that patch 6 depends on:
alloc_thread_groups() sizes the worker_groups[] array by
num_possible_nodes() (a node count) but indexes it by cpu_to_node() (a
node id), so a sparse NUMA node map can index out of bounds.  That is only
reachable once worker groups are enabled, which patch 6 does by default --
so the fix goes first.  It stands alone (Fixes: 851c30c9badf) and can be
taken independently.

Each tunable default only rises on hardware that can back it: the lock and
worker counts (2, 6) scale with the CPU count, the cache sizes (3, 5) with
RAM.  A genuinely small system -- a few cores and a few GB -- therefore
keeps today's values and today's memory footprint.  Patches 2-6 each add a
module parameter to override or pin the value (patch 6 also keeps the
existing per-array group_thread_cnt sysfs attribute), so an administrator
can force any of them, including back to the historical behaviour.

Patches 2-5 are sizing/capability changes with no throughput claim -- the
intent is simply that the out-of-the-box configuration tracks the machine
instead of a fixed constant.  Patch 6 does move throughput: enabling md's
worker groups by default measures 2.1-3.2x on a 16-disk NVMe array (a
32-vCPU / 16-core host, steady state), with no regression on small
machines -- a 4-CPU box gets 2 workers and is never slower, a box with two
or fewer CPUs gets 0 and is unchanged.

Tested on a KASAN + lockdep + DEBUG_LIST kernel (RAID5 on loop devices):
create, 180MB write + sha256 verify, disk fail + spare rebuild (data
verified), scrub (mismatch_cnt=0), and bitmap add/remove -- which drives
raid5_quiesce()'s lock_all_device_hash_locks_irq() path -- with the
parameters both left at their hardware-derived defaults and pinned to
explicit values, including nr_stripe_hash_locks=32 (the 33-lock quiesce
path).  No KASAN, lockdep, or list-debug reports.

Hiroshi Nishida (6):
  md/raid5: size the worker group array by nr_node_ids
  md/raid5: size stripe-cache hash locks from the CPU count
  md/raid5: scale the stripe_cache_size limit with system memory
  md/raid5: make the stripe batch size a module parameter
  md/raid5: scale the default stripe cache size with system memory
  md/raid5: derive the default group_thread_cnt from the hardware

 drivers/md/raid5-cache.c |   2 +-
 drivers/md/raid5.c       | 233 +++++++++++++++++++++++++++++++++------
 drivers/md/raid5.h       |  43 ++++++--
 3 files changed, 233 insertions(+), 45 deletions(-)


base-commit: 55b77337bdd088c77461588e5ec094421b89911b
--
2.43.0

[PATCH 1/6] md/raid5: size the worker group array by nr_node_ids

[Hiroshi Nishida]

alloc_thread_groups() sizes conf->worker_groups[] by num_possible_nodes()
-- the number of possible NUMA nodes -- but raid5_wakeup_stripe_thread()
indexes it by cpu_to_group(cpu), i.e. cpu_to_node(cpu), which is a node
id.  When the node map is sparse (for example possible nodes 0 and 2) the
node count is 2 while the largest node id is 2, so the index reaches
worker_groups[2] on a two-element array -- an out-of-bounds access.

This has stayed latent because worker groups are only allocated when
group_thread_cnt is non-zero, and the historical default is 0.  Size the
array by nr_node_ids -- one past the largest possible node id -- so that
indexing by cpu_to_node() is always in bounds.  On a dense node map
nr_node_ids equals num_possible_nodes() and nothing changes; on a sparse
map the array just gains the unused id slots it needs.

Fixes: 851c30c9badf ("raid5: offload stripe handle to workqueue")
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
 drivers/md/raid5.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 0c5c9fb0606e..d8807114a693 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -7322,7 +7322,13 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt, int *group_cnt,
 		*worker_groups = NULL;
 		return 0;
 	}
-	*group_cnt = num_possible_nodes();
+	/*
+	 * worker_groups is indexed by cpu_to_group() == cpu_to_node(), a node
+	 * id, so it must have room for the largest possible id.  Size it by
+	 * nr_node_ids (one past that id), not num_possible_nodes(), which is
+	 * only the node count and is smaller on a sparse node map.
+	 */
+	*group_cnt = nr_node_ids;
 	size = sizeof(struct r5worker) * cnt;
 	workers = kcalloc(size, *group_cnt, GFP_NOIO);
 	*worker_groups = kzalloc_objs(struct r5worker_group, *group_cnt,
-- 
2.43.0

[PATCH 2/6] md/raid5: size stripe-cache hash locks from the CPU count

[Hiroshi Nishida]

The stripe cache hash is striped across NR_STRIPE_HASH_LOCKS spinlocks
(see stripe_hash_locks_hash()).  The count has been a fixed 8 since the
per-hash locking was introduced.  On many-core servers 8 buckets is
small: stripe cache lookup and allocation contend on the same few locks
once the CPU count greatly exceeds the bucket count.  Simply raising the
compile-time constant, however, would grow three per-array arrays in
struct r5conf (hash_locks, inactive_list, temp_inactive_list) for every
array unconditionally, spending memory on small systems that see no
benefit from the extra buckets.

Make the count a per-array value, sized when the array is created and
selected by a new nr_stripe_hash_locks module parameter:

  - 0 (the default) auto-sizes the count from the online CPU count,
    rounded up to a power of two and clamped to the range
    [NR_STRIPE_HASH_LOCKS_DEFAULT (8), NR_STRIPE_HASH_LOCKS_MAX (32)].
    A machine with 8 or fewer CPUs therefore keeps the historical 8 and
    the historical memory footprint; wider machines stripe the locks
    further with no user action and no recompile.

  - A non-zero value overrides the heuristic (still rounded up to a
    power of two and capped at 32), for administrators who want to pin
    it.

  - The maximum is 32 because raid5_quiesce() takes every hash lock plus
    device_lock at once via lock_all_device_hash_locks_irq(); that holds
    nr_hash_locks + 1 locks simultaneously, which must stay below
    MAX_LOCK_DEPTH (48) so the held-lock array does not overflow when
    lockdep is enabled.

  - The three per-array arrays are now sized with kcalloc(nr_hash_locks)
    when the array is created, so a system that ends up with 8 locks uses
    no more memory than before.  The embedded and on-stack
    temp_inactive_list arrays (r5worker, raid5_plug_cb) are sized to the
    NR_STRIPE_HASH_LOCKS_MAX upper bound; only nr_hash_locks entries are
    ever initialised or used.

The value is resolved when an array is created, so existing arrays keep
their lock count and a value written later applies only to arrays
created afterwards.

Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
 drivers/md/raid5-cache.c |  2 +-
 drivers/md/raid5.c       | 84 +++++++++++++++++++++++++++++-----------
 drivers/md/raid5.h       | 31 ++++++++++-----
 3 files changed, 84 insertions(+), 33 deletions(-)

diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 7b7546bfa21f..9cf58a13250a 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -38,7 +38,7 @@
 /* start flush with these full stripes */
 #define R5C_FULL_STRIPE_FLUSH_BATCH(conf) (conf->max_nr_stripes / 4)
 /* reclaim stripes in groups */
-#define R5C_RECLAIM_STRIPE_GROUP (NR_STRIPE_HASH_LOCKS * 2)
+#define R5C_RECLAIM_STRIPE_GROUP (NR_STRIPE_HASH_LOCKS_DEFAULT * 2)
 
 /*
  * We only need 2 bios per I/O unit to make progress, but ensure we
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index d8807114a693..7f72981121fd 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -67,6 +67,11 @@
 
 #define RAID5_MAX_REQ_STRIPES 256
 
+static unsigned int nr_stripe_hash_locks;
+module_param(nr_stripe_hash_locks, uint, 0644);
+MODULE_PARM_DESC(nr_stripe_hash_locks,
+		 "Number of spinlocks the stripe cache hash is striped across, rounded up to a power of two and capped at 32.  0 (the default) auto-sizes it from the online CPU count (never below 8); a non-zero value overrides that.  Larger values reduce lock contention on many-core systems at a small per-array memory cost.  Read when an array is created");
+
 static bool devices_handle_discard_safely = false;
 module_param(devices_handle_discard_safely, bool, 0644);
 MODULE_PARM_DESC(devices_handle_discard_safely,
@@ -83,7 +88,7 @@ static inline struct hlist_head *stripe_hash(struct r5conf *conf, sector_t sect)
 
 static inline int stripe_hash_locks_hash(struct r5conf *conf, sector_t sect)
 {
-	return (sect >> RAID5_STRIPE_SHIFT(conf)) & STRIPE_HASH_LOCKS_MASK;
+	return (sect >> RAID5_STRIPE_SHIFT(conf)) & conf->hash_locks_mask;
 }
 
 static inline void lock_device_hash_lock(struct r5conf *conf, int hash)
@@ -105,7 +110,7 @@ static inline void lock_all_device_hash_locks_irq(struct r5conf *conf)
 {
 	int i;
 	spin_lock_irq(conf->hash_locks);
-	for (i = 1; i < NR_STRIPE_HASH_LOCKS; i++)
+	for (i = 1; i < conf->nr_hash_locks; i++)
 		spin_lock_nest_lock(conf->hash_locks + i, conf->hash_locks);
 	spin_lock(&conf->device_lock);
 }
@@ -115,7 +120,7 @@ static inline void unlock_all_device_hash_locks_irq(struct r5conf *conf)
 {
 	int i;
 	spin_unlock(&conf->device_lock);
-	for (i = NR_STRIPE_HASH_LOCKS - 1; i; i--)
+	for (i = conf->nr_hash_locks - 1; i; i--)
 		spin_unlock(conf->hash_locks + i);
 	spin_unlock_irq(conf->hash_locks);
 }
@@ -317,7 +322,7 @@ static void __release_stripe(struct r5conf *conf, struct stripe_head *sh,
 }
 
 /*
- * @hash could be NR_STRIPE_HASH_LOCKS, then we have a list of inactive_list
+ * @hash could be conf->nr_hash_locks, then we have a list of inactive_list
  *
  * Be careful: Only one task can add/delete stripes from temp_inactive_list at
  * given time. Adding stripes only takes device lock, while deleting stripes
@@ -331,9 +336,9 @@ static void release_inactive_stripe_list(struct r5conf *conf,
 	bool do_wakeup = false;
 	unsigned long flags;
 
-	if (hash == NR_STRIPE_HASH_LOCKS) {
-		size = NR_STRIPE_HASH_LOCKS;
-		hash = NR_STRIPE_HASH_LOCKS - 1;
+	if (hash == conf->nr_hash_locks) {
+		size = conf->nr_hash_locks;
+		hash = conf->nr_hash_locks - 1;
 	} else
 		size = 1;
 	while (size) {
@@ -2408,7 +2413,7 @@ static int grow_one_stripe(struct r5conf *conf, gfp_t gfp)
 		return 0;
 	}
 	sh->hash_lock_index =
-		conf->max_nr_stripes % NR_STRIPE_HASH_LOCKS;
+		conf->max_nr_stripes % conf->nr_hash_locks;
 	/* we just created an active stripe so... */
 	atomic_inc(&conf->active_stripes);
 
@@ -2612,8 +2617,8 @@ static int resize_stripes(struct r5conf *conf, int newsize)
 		nsh->hash_lock_index = hash;
 		free_stripe(conf->slab_cache, osh);
 		cnt++;
-		if (cnt >= conf->max_nr_stripes / NR_STRIPE_HASH_LOCKS +
-		    !!((conf->max_nr_stripes % NR_STRIPE_HASH_LOCKS) > hash)) {
+		if (cnt >= conf->max_nr_stripes / conf->nr_hash_locks +
+		    !!((conf->max_nr_stripes % conf->nr_hash_locks) > hash)) {
 			hash++;
 			cnt = 0;
 		}
@@ -2697,7 +2702,7 @@ static int resize_stripes(struct r5conf *conf, int newsize)
 static int drop_one_stripe(struct r5conf *conf)
 {
 	struct stripe_head *sh;
-	int hash = (conf->max_nr_stripes - 1) & STRIPE_HASH_LOCKS_MASK;
+	int hash = (conf->max_nr_stripes - 1) & conf->hash_locks_mask;
 
 	spin_lock_irq(conf->hash_locks + hash);
 	sh = get_free_stripe(conf, hash);
@@ -5638,7 +5643,7 @@ static struct stripe_head *__get_priority_stripe(struct r5conf *conf, int group)
 struct raid5_plug_cb {
 	struct blk_plug_cb	cb;
 	struct list_head	list;
-	struct list_head	temp_inactive_list[NR_STRIPE_HASH_LOCKS];
+	struct list_head	temp_inactive_list[NR_STRIPE_HASH_LOCKS_MAX];
 };
 
 static void raid5_unplug(struct blk_plug_cb *blk_cb, bool from_schedule)
@@ -5674,7 +5679,7 @@ static void raid5_unplug(struct blk_plug_cb *blk_cb, bool from_schedule)
 		spin_unlock_irq(&conf->device_lock);
 	}
 	release_inactive_stripe_list(conf, cb->temp_inactive_list,
-				     NR_STRIPE_HASH_LOCKS);
+				     conf->nr_hash_locks);
 	if (!mddev_is_dm(mddev))
 		trace_block_unplug(mddev->gendisk->queue, cnt, !from_schedule);
 	kfree(cb);
@@ -5698,7 +5703,7 @@ static void release_stripe_plug(struct mddev *mddev,
 	if (cb->list.next == NULL) {
 		int i;
 		INIT_LIST_HEAD(&cb->list);
-		for (i = 0; i < NR_STRIPE_HASH_LOCKS; i++)
+		for (i = 0; i < NR_STRIPE_HASH_LOCKS_MAX; i++)
 			INIT_LIST_HEAD(cb->temp_inactive_list + i);
 	}
 
@@ -6732,10 +6737,10 @@ static int handle_active_stripes(struct r5conf *conf, int group,
 		batch[batch_size++] = sh;
 
 	if (batch_size == 0) {
-		for (i = 0; i < NR_STRIPE_HASH_LOCKS; i++)
+		for (i = 0; i < conf->nr_hash_locks; i++)
 			if (!list_empty(temp_inactive_list + i))
 				break;
-		if (i == NR_STRIPE_HASH_LOCKS) {
+		if (i == conf->nr_hash_locks) {
 			spin_unlock_irq(&conf->device_lock);
 			log_flush_stripe_to_raid(conf);
 			spin_lock_irq(&conf->device_lock);
@@ -6746,7 +6751,7 @@ static int handle_active_stripes(struct r5conf *conf, int group,
 	spin_unlock_irq(&conf->device_lock);
 
 	release_inactive_stripe_list(conf, temp_inactive_list,
-				     NR_STRIPE_HASH_LOCKS);
+				     conf->nr_hash_locks);
 
 	r5l_flush_stripe_to_raid(conf->log);
 	if (release_inactive) {
@@ -7353,7 +7358,7 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt, int *group_cnt,
 			worker->group = group;
 			INIT_WORK(&worker->work, raid5_do_work);
 
-			for (k = 0; k < NR_STRIPE_HASH_LOCKS; k++)
+			for (k = 0; k < conf->nr_hash_locks; k++)
 				INIT_LIST_HEAD(worker->temp_inactive_list + k);
 		}
 	}
@@ -7448,6 +7453,9 @@ static void free_conf(struct r5conf *conf)
 	kfree(conf->disks);
 	bioset_exit(&conf->bio_split);
 	kfree(conf->stripe_hashtbl);
+	kfree(conf->hash_locks);
+	kfree(conf->inactive_list);
+	kfree(conf->temp_inactive_list);
 	kfree(conf->pending_data);
 
 	mempool_destroy(conf->ctx_pool);
@@ -7565,6 +7573,38 @@ static struct r5conf *setup_conf(struct mddev *mddev)
 	if (conf == NULL)
 		goto abort;
 
+	/*
+	 * Size the stripe cache hash-lock striping per array.  The
+	 * nr_stripe_hash_locks module parameter selects the count: 0 (the
+	 * default) auto-sizes it from the online CPU count so busier machines
+	 * stripe the locks wider, while a non-zero value overrides that.  The
+	 * result is rounded up to a power of two (the hash uses it as a
+	 * bitmask) and capped at NR_STRIPE_HASH_LOCKS_MAX; the auto path also
+	 * keeps at least the historical NR_STRIPE_HASH_LOCKS_DEFAULT so small
+	 * systems are unchanged.  Allocating per array means an untuned system
+	 * uses no more memory than before.
+	 */
+	if (nr_stripe_hash_locks == 0)
+		conf->nr_hash_locks = clamp_t(unsigned int,
+				roundup_pow_of_two(num_online_cpus()),
+				NR_STRIPE_HASH_LOCKS_DEFAULT,
+				NR_STRIPE_HASH_LOCKS_MAX);
+	else
+		conf->nr_hash_locks = roundup_pow_of_two(
+				min_t(unsigned int, nr_stripe_hash_locks,
+				      NR_STRIPE_HASH_LOCKS_MAX));
+	conf->hash_locks_mask = conf->nr_hash_locks - 1;
+	conf->hash_locks = kcalloc(conf->nr_hash_locks,
+				   sizeof(*conf->hash_locks), GFP_KERNEL);
+	conf->inactive_list = kcalloc(conf->nr_hash_locks,
+				      sizeof(*conf->inactive_list), GFP_KERNEL);
+	conf->temp_inactive_list = kcalloc(conf->nr_hash_locks,
+					   sizeof(*conf->temp_inactive_list),
+					   GFP_KERNEL);
+	if (!conf->hash_locks || !conf->inactive_list ||
+	    !conf->temp_inactive_list)
+		goto abort;
+
 #if PAGE_SIZE != DEFAULT_STRIPE_SIZE
 	conf->stripe_size = DEFAULT_STRIPE_SIZE;
 	conf->stripe_shift = ilog2(DEFAULT_STRIPE_SIZE) - 9;
@@ -7646,13 +7686,13 @@ static struct r5conf *setup_conf(struct mddev *mddev)
 	 * lockdep that we know what we are doing.
 	 */
 	spin_lock_init(conf->hash_locks);
-	for (i = 1; i < NR_STRIPE_HASH_LOCKS; i++)
+	for (i = 1; i < conf->nr_hash_locks; i++)
 		spin_lock_init(conf->hash_locks + i);
 
-	for (i = 0; i < NR_STRIPE_HASH_LOCKS; i++)
+	for (i = 0; i < conf->nr_hash_locks; i++)
 		INIT_LIST_HEAD(conf->inactive_list + i);
 
-	for (i = 0; i < NR_STRIPE_HASH_LOCKS; i++)
+	for (i = 0; i < conf->nr_hash_locks; i++)
 		INIT_LIST_HEAD(conf->temp_inactive_list + i);
 
 	atomic_set(&conf->r5c_cached_full_stripes, 0);
@@ -7729,7 +7769,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
 	}
 	memory = conf->min_nr_stripes * (sizeof(struct stripe_head) +
 		 max_disks * ((sizeof(struct bio) + PAGE_SIZE))) / 1024;
-	atomic_set(&conf->empty_inactive_list_nr, NR_STRIPE_HASH_LOCKS);
+	atomic_set(&conf->empty_inactive_list_nr, conf->nr_hash_locks);
 	if (grow_stripes(conf, conf->min_nr_stripes)) {
 		pr_warn("md/raid:%s: couldn't allocate %dkB for buffers\n",
 			mdname(mddev), memory);
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index cb5feae04db2..10c45fa22955 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -492,18 +492,27 @@ struct disk_info {
 #define HASH_MASK		(NR_HASH - 1)
 #define MAX_STRIPE_BATCH	8
 
-/* NOTE NR_STRIPE_HASH_LOCKS must remain below 64.
- * This is because we sometimes take all the spinlocks
- * and creating that much locking depth can cause
- * problems.
+/*
+ * The stripe cache hash is striped across a power-of-two number of spinlocks,
+ * chosen per array from the nr_stripe_hash_locks module parameter and stored
+ * in r5conf->nr_hash_locks (with the mask in r5conf->hash_locks_mask).  Sizing
+ * the locks per array means systems that do not tune it pay no extra memory
+ * beyond the historical default.
+ *
+ * NR_STRIPE_HASH_LOCKS_DEFAULT is that historical value, used when the module
+ * parameter is left alone.  NR_STRIPE_HASH_LOCKS_MAX bounds the count: taking
+ * all the hash locks at once in lock_all_device_hash_locks_irq(), plus
+ * device_lock, must keep the held lock count below MAX_LOCK_DEPTH (48) with
+ * lockdep enabled, and it also sizes the embedded/on-stack temp_inactive_list
+ * arrays.
  */
-#define NR_STRIPE_HASH_LOCKS 8
-#define STRIPE_HASH_LOCKS_MASK (NR_STRIPE_HASH_LOCKS - 1)
+#define NR_STRIPE_HASH_LOCKS_DEFAULT	8
+#define NR_STRIPE_HASH_LOCKS_MAX	32
 
 struct r5worker {
 	struct work_struct work;
 	struct r5worker_group *group;
-	struct list_head temp_inactive_list[NR_STRIPE_HASH_LOCKS];
+	struct list_head temp_inactive_list[NR_STRIPE_HASH_LOCKS_MAX];
 	bool working;
 };
 
@@ -570,7 +579,9 @@ struct raid5_percpu {
 struct r5conf {
 	struct hlist_head	*stripe_hashtbl;
 	/* only protect corresponding hash list and inactive_list */
-	spinlock_t		hash_locks[NR_STRIPE_HASH_LOCKS];
+	spinlock_t		*hash_locks;
+	int			nr_hash_locks;	 /* power of two, <= NR_STRIPE_HASH_LOCKS_MAX */
+	int			hash_locks_mask; /* nr_hash_locks - 1 */
 	struct mddev		*mddev;
 	int			chunk_sectors;
 	int			level, algorithm, rmw_level;
@@ -650,7 +661,7 @@ struct r5conf {
 	 * Free stripes pool
 	 */
 	atomic_t		active_stripes;
-	struct list_head	inactive_list[NR_STRIPE_HASH_LOCKS];
+	struct list_head	*inactive_list;
 
 	atomic_t		r5c_cached_full_stripes;
 	struct list_head	r5c_full_stripe_list;
@@ -675,7 +686,7 @@ struct r5conf {
 	 * the new thread here until we fully activate the array.
 	 */
 	struct md_thread __rcu	*thread;
-	struct list_head	temp_inactive_list[NR_STRIPE_HASH_LOCKS];
+	struct list_head	*temp_inactive_list;
 	struct r5worker_group	*worker_groups;
 	int			group_cnt;
 	int			worker_cnt_per_group;
-- 
2.43.0

[PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory

[Hiroshi Nishida]

raid5_set_cache_size() caps how far the per-array stripe_cache_size sysfs
knob may be raised at a fixed 32768 stripes.  The stripe cache costs
roughly max_nr_stripes * (sizeof(stripe_head) + pool_size * (sizeof(bio) +
PAGE_SIZE)), so 32768 stripes is about 1.5GB on a 12-disk array.  That
fixed limit is wrong at both ends: a host with hundreds of GB of RAM
backing a wide array cannot grow the cache past ~1.5GB even though it has
the memory to spare, while on a small box 32768 stripes can already exceed
total RAM.

Derive the limit from memory instead.  When the new stripe_cache_size_max
module parameter is 0 (the default), the ceiling is the stripe count that
fits in at most 1/8 of RAM for this array's width, but never less than the
historical 32768 -- so the limit only ever grows relative to today and
small systems are unchanged.  A non-zero stripe_cache_size_max pins a fixed
ceiling for administrators who want one.

The default stripe count (256), and thus the default memory footprint, is
unchanged; this only changes how high an administrator may raise
stripe_cache_size.

Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
 drivers/md/raid5.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 7f72981121fd..e41d3fc92dd0 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -44,6 +44,7 @@
 #include <linux/seq_file.h>
 #include <linux/cpu.h>
 #include <linux/slab.h>
+#include <linux/mm.h>
 #include <linux/ratelimit.h>
 #include <linux/nodemask.h>
 
@@ -72,6 +73,11 @@ module_param(nr_stripe_hash_locks, uint, 0644);
 MODULE_PARM_DESC(nr_stripe_hash_locks,
 		 "Number of spinlocks the stripe cache hash is striped across, rounded up to a power of two and capped at 32.  0 (the default) auto-sizes it from the online CPU count (never below 8); a non-zero value overrides that.  Larger values reduce lock contention on many-core systems at a small per-array memory cost.  Read when an array is created");
 
+static unsigned int stripe_cache_size_max;
+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 default) derives the limit from system memory (never below the historical 32768), so large-memory hosts can grow the stripe cache without a recompile while small ones are not offered a limit above what RAM can back.  A non-zero value sets a fixed limit");
+
 static bool devices_handle_discard_safely = false;
 module_param(devices_handle_discard_safely, bool, 0644);
 MODULE_PARM_DESC(devices_handle_discard_safely,
@@ -6922,13 +6928,42 @@ raid5_show_stripe_cache_size(struct mddev *mddev, char *page)
 	return ret;
 }
 
+/*
+ * Upper bound that the per-array stripe_cache_size may be raised to.  The
+ * stripe cache costs roughly max_nr_stripes * (sizeof(stripe_head) +
+ * pool_size * (sizeof(bio) + PAGE_SIZE)).  The limit was historically a fixed
+ * 32768 stripes, which both under-serves large-memory hosts backing wide
+ * arrays and, on a small box, still permits a cache larger than RAM.  Derive
+ * it from memory instead -- at most 1/8 of RAM -- but never below the
+ * historical 32768, so the limit only ever grows relative to today.  A
+ * non-zero stripe_cache_size_max module parameter overrides the heuristic.
+ */
+#define RAID5_CACHE_SIZE_FLOOR		32768
+#define RAID5_CACHE_SIZE_RAM_SHIFT	3	/* cap the cache at 1/8 of RAM */
+
+static unsigned long raid5_max_cache_size(struct r5conf *conf)
+{
+	unsigned long per_stripe, limit;
+
+	if (stripe_cache_size_max) {
+		limit = stripe_cache_size_max;
+	} else {
+		per_stripe = sizeof(struct stripe_head) +
+			     conf->pool_size * (sizeof(struct bio) + PAGE_SIZE);
+		limit = ((totalram_pages() << PAGE_SHIFT) >>
+			 RAID5_CACHE_SIZE_RAM_SHIFT) / per_stripe;
+		limit = max_t(unsigned long, limit, RAID5_CACHE_SIZE_FLOOR);
+	}
+	return min_t(unsigned long, limit, INT_MAX);
+}
+
 int
 raid5_set_cache_size(struct mddev *mddev, int size)
 {
 	int result = 0;
 	struct r5conf *conf = mddev->private;
 
-	if (size <= 16 || size > 32768)
+	if (size <= 16 || size > raid5_max_cache_size(conf))
 		return -EINVAL;
 
 	WRITE_ONCE(conf->min_nr_stripes, size);
-- 
2.43.0

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

[Hiroshi Nishida]

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, while
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 is
no general hardware signal for the best batch size, so the default is left
at the historical 8 and the value is simply exposed for tuning.

Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
 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 default) derives the limit from system memory (never below the historical 32768), so large-memory hosts can grow the stripe cache without a recompile while small ones are not offered a limit above what RAM can back.  A non-zero value sets a fixed limit");
 
+static unsigned int max_stripe_batch = 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 acquisition, 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 created");
+
 static bool devices_handle_discard_safely = 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);
 
-	thread_cnt = group->stripes_cnt / MAX_STRIPE_BATCH - 1;
+	thread_cnt = group->stripes_cnt / conf->max_stripe_batch - 1;
 	/* wakeup more workers */
 	for (i = 1; i < conf->worker_cnt_per_group && thread_cnt > 0; i++) {
 		if (group->workers[i].working == false) {
@@ -6734,11 +6739,11 @@ static int handle_active_stripes(struct r5conf *conf, 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 = 0, hash;
 	bool release_inactive = false;
 
-	while (batch_size < MAX_STRIPE_BATCH &&
+	while (batch_size < conf->max_stripe_batch &&
 			(sh = __get_priority_stripe(conf, group)) != NULL)
 		batch[batch_size++] = sh;
 
@@ -7640,6 +7645,10 @@ static struct r5conf *setup_conf(struct mddev *mddev)
 	    !conf->temp_inactive_list)
 		goto abort;
 
+	/* Resolve the stripe batch size (see STRIPE_BATCH_* in raid5.h). */
+	conf->max_stripe_batch = clamp_t(int, max_stripe_batch,
+					 1, STRIPE_BATCH_MAX);
+
 #if PAGE_SIZE != DEFAULT_STRIPE_SIZE
 	conf->stripe_size = DEFAULT_STRIPE_SIZE;
 	conf->stripe_shift = 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 divides
+ * 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 historical
+ * value and STRIPE_BATCH_MAX bounds the on-stack batch[] array.
+ */
+#define STRIPE_BATCH_DEFAULT	8
+#define STRIPE_BATCH_MAX	32
 
 /*
  * The stripe cache hash is striped across a power-of-two number of spinlocks,
@@ -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 */
 	struct r5l_log		*log;
 	void			*log_private;
 
-- 
2.43.0

[PATCH 5/6] md/raid5: scale the default stripe cache size with system memory

[Hiroshi Nishida]

setup_conf() starts every array with min_nr_stripes = NR_STRIPES (256),
about 12MB of stripe cache on a 12-disk array.  That fixed default was
chosen for small systems and is never revisited: a server with hundreds
of GB of RAM backing a wide array still creates its arrays with the same
256 stripes, and only benefits from more after an administrator writes
stripe_cache_size by hand.

Auto-size the initial count from memory when the new
stripe_cache_size_default module parameter is 0 (the default): keep the
historical NR_STRIPES up to RAID5_CACHE_DEFAULT_BASE_GB (8GB) of RAM, then
grow the count using about 1/512 of the RAM above that baseline, capped at
RAID5_CACHE_DEFAULT_MAX (4096).  A system with 8GB or less is therefore
unchanged -- same count, same footprint -- while larger ones scale up
smoothly (no jump at the baseline) to a bounded maximum.  A non-zero
stripe_cache_size_default sets a fixed initial size for administrators who
want one; existing arrays are unaffected, and a reshape still forces at
least its window's worth of stripes.

Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
 drivers/md/raid5.c | 44 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 5f0825c5effe..8e1c2eba4241 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -83,6 +83,11 @@ module_param(max_stripe_batch, uint, 0644);
 MODULE_PARM_DESC(max_stripe_batch,
 		 "Number of stripes a worker thread handles per device_lock acquisition, 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 created");
 
+static unsigned int stripe_cache_size_default;
+module_param(stripe_cache_size_default, uint, 0644);
+MODULE_PARM_DESC(stripe_cache_size_default,
+		 "Initial stripe_cache_size for newly created arrays.  0 (the default) auto-sizes it from system memory: the historical 256 on small hosts, scaling up with RAM to a capped maximum on larger ones.  A non-zero value sets a fixed initial size.  Existing arrays are unaffected");
+
 static bool devices_handle_discard_safely = false;
 module_param(devices_handle_discard_safely, bool, 0644);
 MODULE_PARM_DESC(devices_handle_discard_safely,
@@ -7568,6 +7573,17 @@ static unsigned long raid5_cache_count(struct shrinker *shrink,
 	return max_stripes - min_stripes;
 }
 
+/*
+ * Auto-sizing of the initial stripe cache (stripe_cache_size_default == 0):
+ * stay at the historical NR_STRIPES up to RAID5_CACHE_DEFAULT_BASE_GB of RAM,
+ * then grow the count using about 1/512 of the RAM above that base, capped at
+ * RAID5_CACHE_DEFAULT_MAX.  So a system at or below the base is unchanged and
+ * keeps the historical footprint, while larger ones scale.
+ */
+#define RAID5_CACHE_DEFAULT_BASE_GB	8	/* unchanged at or below this much RAM */
+#define RAID5_CACHE_DEFAULT_RAM_SHIFT	9	/* above it: ~1/512 of the extra RAM */
+#define RAID5_CACHE_DEFAULT_MAX		4096
+
 static struct r5conf *setup_conf(struct mddev *mddev)
 {
 	struct r5conf *conf;
@@ -7801,15 +7817,37 @@ static struct r5conf *setup_conf(struct mddev *mddev)
 		conf->prev_algo = conf->algorithm;
 	}
 
-	conf->min_nr_stripes = NR_STRIPES;
+	/*
+	 * Choose the initial stripe cache size.  stripe_cache_size_default
+	 * selects it: 0 (the default) auto-sizes from memory -- the historical
+	 * NR_STRIPES up to RAID5_CACHE_DEFAULT_BASE_GB of RAM, then scaling up
+	 * to at most RAID5_CACHE_DEFAULT_MAX -- and a non-zero value sets it
+	 * directly.  A reshape still forces at least enough stripes for its
+	 * window, below.
+	 */
+	if (stripe_cache_size_default) {
+		conf->min_nr_stripes = clamp_t(unsigned long,
+					stripe_cache_size_default, 16, INT_MAX);
+	} else {
+		unsigned long per_stripe = sizeof(struct stripe_head) +
+			max_disks * (sizeof(struct bio) + PAGE_SIZE);
+		unsigned long ram = totalram_pages() << PAGE_SHIFT;
+		unsigned long base = (unsigned long)RAID5_CACHE_DEFAULT_BASE_GB << 30;
+		unsigned long extra = ram > base ?
+			((ram - base) >> RAID5_CACHE_DEFAULT_RAM_SHIFT) / per_stripe : 0;
+
+		conf->min_nr_stripes = clamp_t(unsigned long, NR_STRIPES + extra,
+					       NR_STRIPES, RAID5_CACHE_DEFAULT_MAX);
+	}
 	if (mddev->reshape_position != MaxSector) {
 		int stripes = max_t(int,
 			((mddev->chunk_sectors << 9) / RAID5_STRIPE_SIZE(conf)) * 4,
 			((mddev->new_chunk_sectors << 9) / RAID5_STRIPE_SIZE(conf)) * 4);
-		conf->min_nr_stripes = max(NR_STRIPES, stripes);
-		if (conf->min_nr_stripes != NR_STRIPES)
+		if (stripes > conf->min_nr_stripes) {
+			conf->min_nr_stripes = stripes;
 			pr_info("md/raid:%s: force stripe size %d for reshape\n",
 				mdname(mddev), conf->min_nr_stripes);
+		}
 	}
 	memory = conf->min_nr_stripes * (sizeof(struct stripe_head) +
 		 max_disks * ((sizeof(struct bio) + PAGE_SIZE))) / 1024;
-- 
2.43.0

[PATCH 6/6] md/raid5: derive the default group_thread_cnt from the hardware

[Hiroshi Nishida]

setup_conf() starts every array with worker_cnt_per_group = 0, i.e. a
single raid5d thread and no worker groups.  On a many-core host backing a
fast, wide array that single thread is the bottleneck; the multi-threading
that group_thread_cnt enables has to be turned on by hand, per array,
after every assembly.

Pick a starting value from the CPU count instead.  When the new
group_thread_cnt_default module parameter is left at -1 (the default),
raid5_default_group_thread_cnt() uses half the online CPU count spread
across the NUMA nodes, capped at 256:

	gtc = num_online_cpus() / (2 * nr_nodes)

The count is per NUMA node, matching alloc_thread_groups(), so dividing by
the node count keeps the total near half the CPUs regardless of socket
count.  It is only a ceiling: raid5_wakeup_stripe_thread() wakes workers
in proportion to the queued stripe count, so a lightly loaded array uses
far fewer than the maximum.  A lone worker is not worth its overhead over
raid5d, so a result of 1 collapses back to 0, which keeps boxes with two
or fewer CPUs single-threaded.

Measured on a 16-disk raid6 array of NVMe SSDs (a 32-vCPU host: 16 cores,
two NUMA nodes; steady state, interleaved runs): the derived count of 8
raises throughput 2.1-3.2x over the single-threaded default -- 4K random
write from ~39k to ~100k IOPS (2.6x), mixed database/OLTP/high-concurrency
2.1-2.5x, partial stripe write 3.2x.  There is no regression on smaller
machines: a 4-CPU box gets 2 workers and is faster or equal on every
workload, and a box with two or fewer CPUs gets 0 and is byte-for-byte
unchanged.

group_thread_cnt_default overrides the heuristic (0 forces the historical
single-threaded behaviour; a positive value pins a count, capped at 256),
and the existing per-array group_thread_cnt sysfs attribute continues to
override it at runtime and still accepts up to its own 8192 limit.

Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
 drivers/md/raid5.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 8e1c2eba4241..6e91eb0ad575 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -88,6 +88,11 @@ module_param(stripe_cache_size_default, uint, 0644);
 MODULE_PARM_DESC(stripe_cache_size_default,
 		 "Initial stripe_cache_size for newly created arrays.  0 (the default) auto-sizes it from system memory: the historical 256 on small hosts, scaling up with RAM to a capped maximum on larger ones.  A non-zero value sets a fixed initial size.  Existing arrays are unaffected");
 
+static int group_thread_cnt_default = -1;
+module_param(group_thread_cnt_default, int, 0644);
+MODULE_PARM_DESC(group_thread_cnt_default,
+		 "Initial group_thread_cnt (raid5 worker threads per NUMA node) for newly created arrays.  A negative value (the default, -1) auto-sizes it from the CPU count; 0 forces the single-threaded raid5d; a positive value sets a fixed count (capped at 256).  The per-array group_thread_cnt sysfs attribute overrides this and allows larger values");
+
 static bool devices_handle_discard_safely = false;
 module_param(devices_handle_discard_safely, bool, 0644);
 MODULE_PARM_DESC(devices_handle_discard_safely,
@@ -7584,6 +7589,31 @@ static unsigned long raid5_cache_count(struct shrinker *shrink,
 #define RAID5_CACHE_DEFAULT_RAM_SHIFT	9	/* above it: ~1/512 of the extra RAM */
 #define RAID5_CACHE_DEFAULT_MAX		4096
 
+/*
+ * Default group_thread_cnt (worker_cnt_per_group) for a new array when the
+ * group_thread_cnt_default module parameter is left at -1.  The historical
+ * default is 0 -- a single raid5d thread -- which cannot keep a fast, wide
+ * array busy on a many-core host.  Derive a starting point from the CPU
+ * count: half the online CPUs divided across the NUMA nodes (this is a
+ * per-node count -- see alloc_thread_groups() -- so the total lands near
+ * half the online CPUs regardless of socket count), capped at
+ * RAID5_AUTO_GROUP_THREAD_MAX.  This is only a ceiling:
+ * raid5_wakeup_stripe_thread() wakes workers in proportion to the queued
+ * stripe count, so a lightly loaded array uses far fewer.  A lone worker is
+ * not worth its overhead, so 1 collapses back to 0.  The group_thread_cnt
+ * sysfs attribute overrides this per array.
+ */
+#define RAID5_AUTO_GROUP_THREAD_MAX	256
+
+static int raid5_default_group_thread_cnt(void)
+{
+	unsigned int gtc = num_online_cpus() / (2 * num_possible_nodes());
+
+	if (gtc > RAID5_AUTO_GROUP_THREAD_MAX)
+		gtc = RAID5_AUTO_GROUP_THREAD_MAX;
+	return gtc == 1 ? 0 : gtc;
+}
+
 static struct r5conf *setup_conf(struct mddev *mddev)
 {
 	struct r5conf *conf;
@@ -7594,6 +7624,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
 	int i;
 	int group_cnt;
 	struct r5worker_group *new_group;
+	int def_threads;
 	int ret = -ENOMEM;
 
 	if (mddev->new_level != 5
@@ -7677,10 +7708,18 @@ static struct r5conf *setup_conf(struct mddev *mddev)
 		goto abort;
 	for (i = 0; i < PENDING_IO_MAX; i++)
 		list_add(&conf->pending_data[i].sibling, &conf->free_list);
-	/* Don't enable multi-threading by default*/
-	if (!alloc_thread_groups(conf, 0, &group_cnt, &new_group)) {
+	/*
+	 * Multi-threading defaults to a hardware-derived worker count (see
+	 * raid5_default_group_thread_cnt()); group_thread_cnt_default overrides
+	 * the choice, and the group_thread_cnt sysfs attribute overrides it per
+	 * array.
+	 */
+	def_threads = group_thread_cnt_default < 0 ?
+		raid5_default_group_thread_cnt() :
+		min(group_thread_cnt_default, RAID5_AUTO_GROUP_THREAD_MAX);
+	if (!alloc_thread_groups(conf, def_threads, &group_cnt, &new_group)) {
 		conf->group_cnt = group_cnt;
-		conf->worker_cnt_per_group = 0;
+		conf->worker_cnt_per_group = def_threads;
 		conf->worker_groups = new_group;
 	} else
 		goto abort;
-- 
2.43.0

Re: [PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Integer overflow in `raid5_max_cache_size` on 32-bit platforms with 4GB+ RAM.
--

--- Patch [3]: [PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory ---
commit 9086a4a6d9e318381885104d84e3f0ec9bbeec13
Author: Hiroshi Nishida <nishidafmly@gmail.com>

md/raid5: scale the stripe_cache_size limit with system memory

This commit derives the upper bound for the per-array stripe_cache_size from
system memory rather than a fixed limit. It scales the maximum cache size to
1/8 of RAM, allowing large-memory hosts to grow the cache while preventing
small systems from exceeding total RAM.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 7f72981121fd5..e41d3fc92dd05 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c

[ ... ]

> @@ -6922,13 +6928,42 @@ raid5_show_stripe_cache_size(struct mddev *mddev, char *page)
>  	return ret;
>  }
>  
> +/*
> + * Upper bound that the per-array stripe_cache_size may be raised to.  The
> + * stripe cache costs roughly max_nr_stripes * (sizeof(stripe_head) +
> + * pool_size * (sizeof(bio) + PAGE_SIZE)).  The limit was historically a fixed
> + * 32768 stripes, which both under-serves large-memory hosts backing wide
> + * arrays and, on a small box, still permits a cache larger than RAM.  Derive
> + * it from memory instead -- at most 1/8 of RAM -- but never below the
> + * historical 32768, so the limit only ever grows relative to today.  A
> + * non-zero stripe_cache_size_max module parameter overrides the heuristic.
> + */
> +#define RAID5_CACHE_SIZE_FLOOR		32768
> +#define RAID5_CACHE_SIZE_RAM_SHIFT	3	/* cap the cache at 1/8 of RAM */
> +
> +static unsigned long raid5_max_cache_size(struct r5conf *conf)
> +{
> +	unsigned long per_stripe, limit;
> +
> +	if (stripe_cache_size_max) {
> +		limit = stripe_cache_size_max;
> +	} else {
> +		per_stripe = sizeof(struct stripe_head) +
> +			     conf->pool_size * (sizeof(struct bio) + PAGE_SIZE);
> +		limit = ((totalram_pages() << PAGE_SHIFT) >>
> +			 RAID5_CACHE_SIZE_RAM_SHIFT) / per_stripe;

[Severity: Medium]
Will this calculation overflow on 32-bit platforms with 4GB or more of RAM?

Since totalram_pages() returns an unsigned long, on a 32-bit system with high
memory (e.g., using PAE), totalram_pages() can be 1,048,576 or more.
Left-shifting this by PAGE_SHIFT (12) will overflow the 32-bit integer,
yielding a severely truncated value or 0.

This seems like it would cause the dynamic limit to inappropriately clamp to
the RAID5_CACHE_SIZE_FLOOR on such systems, breaking the intended memory-based
scaling.

> +		limit = max_t(unsigned long, limit, RAID5_CACHE_SIZE_FLOOR);
> +	}
> +	return min_t(unsigned long, limit, INT_MAX);
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Large unsigned values of `max_stripe_batch` are incorrectly clamped to 1 instead of 32.
--

--- Patch [4]: [PATCH 4/6] md/raid5: make the stripe batch size a module parameter ---
commit 5fef4dd203fcdfd7a223e9904fbea20f876f265b
Author: Hiroshi Nishida <nishidafmly@gmail.com>

md/raid5: make the stripe batch size a module parameter

This commit makes the stripe batch size a configurable module parameter to
allow tuning for busy multi-threaded arrays. It synchronizes the dequeue
cap and the spawn divisor by tying both to a single per-array variable.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c

[ ... ]

> @@ -7640,6 +7645,10 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>  	    !conf->temp_inactive_list)
>  		goto abort;
>  
> +	/* Resolve the stripe batch size (see STRIPE_BATCH_* in raid5.h). */
> +	conf->max_stripe_batch = clamp_t(int, max_stripe_batch,
> +					 1, STRIPE_BATCH_MAX);

[Severity: Low]
Does this code incorrectly clamp large unsigned values to 1 instead of 32?

Since max_stripe_batch is defined as an unsigned int, if an administrator
provides a value greater than INT_MAX, the clamp_t(int, ...) cast will treat
it as a negative number.

This will cause it to be clamped to the minimum bound of 1, rather than the
intended maximum bound of STRIPE_BATCH_MAX.

> +
>  #if PAGE_SIZE != DEFAULT_STRIPE_SIZE
>  	conf->stripe_size = DEFAULT_STRIPE_SIZE;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=4

Re: [PATCH 5/6] md/raid5: scale the default stripe cache size with system memory

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] 32-bit arithmetic overflow in `setup_conf` causes incorrect auto-sizing of the RAID5 stripe cache on 32-bit systems.
- [Medium] TOCTOU race condition when reading the module parameter `stripe_cache_size_default` allows bypassing intended bounds.
- [Medium] The commit message incorrectly claims that 'existing arrays are unaffected' by the new auto-sizing default.
--

--- Patch [5]: [PATCH 5/6] md/raid5: scale the default stripe cache size with system memory ---
commit 879380beda117fd029123c90fae16a214ae91020
Author: Hiroshi Nishida <nishidafmly@gmail.com>

md/raid5: scale the default stripe cache size with system memory

setup_conf() starts every array with min_nr_stripes = NR_STRIPES (256),
about 12MB of stripe cache on a 12-disk array. That fixed default was
chosen for small systems and is never revisited. Auto-size the initial
count from memory when the new stripe_cache_size_default module parameter
is 0.

> Auto-size the initial count from memory when the new
> stripe_cache_size_default module parameter is 0 (the default): keep the
> historical NR_STRIPES up to RAID5_CACHE_DEFAULT_BASE_GB (8GB) of RAM, then
> grow the count using about 1/512 of the RAM above that baseline, capped at
> RAID5_CACHE_DEFAULT_MAX (4096).  A system with 8GB or less is therefore
> unchanged -- same count, same footprint -- while larger ones scale up
> smoothly (no jump at the baseline) to a bounded maximum.  A non-zero
> stripe_cache_size_default sets a fixed initial size for administrators who
> want one; existing arrays are unaffected, and a reshape still forces at
> least its window's worth of stripes.

[Severity: Medium]
Is the claim that existing arrays are unaffected accurate?

Because setup_conf() is invoked during array assembly and the
stripe_cache_size is not persisted in the superblock, it appears an
existing array will boot with the dynamically scaled cache size instead
of the historical 256 stripes.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 5f0825c5effec..8e1c2eba42417 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c

[ ... ]

> @@ -7801,15 +7817,37 @@ static struct r5conf *setup_conf(struct mddev *mddev)

[ ... ]

> +	if (stripe_cache_size_default) {
> +		conf->min_nr_stripes = clamp_t(unsigned long,
> +					stripe_cache_size_default, 16, INT_MAX);

[Severity: Medium]
Could this read of stripe_cache_size_default race with a concurrent sysfs
modification?

If stripe_cache_size_default is changed to 0 exactly between the if
condition and the clamp_t evaluation, conf->min_nr_stripes would be clamped
to 16 rather than the historical 256.

Would it be safer to read this module parameter once into a local variable?

> +	} else {
> +		unsigned long per_stripe = sizeof(struct stripe_head) +
> +			max_disks * (sizeof(struct bio) + PAGE_SIZE);
> +		unsigned long ram = totalram_pages() << PAGE_SHIFT;
> +		unsigned long base = (unsigned long)RAID5_CACHE_DEFAULT_BASE_GB << 30;

[Severity: High]
Do these calculations overflow on 32-bit systems?

Since unsigned long is 32 bits on those platforms, calculating base as
8 << 30 will wrap to 0. 

Similarly, calculating ram by shifting totalram_pages() by PAGE_SHIFT will
wrap for systems with 4GB or more of memory, potentially breaking the
scaling logic for higher memory systems.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=5

Re: [PATCH 1/6] md/raid5: size the worker group array by nr_node_ids

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Holding reconfig_mutex while waiting for raid5_wq workers to finish can cause an ABBA deadlock if workers wait for MD_SB_CHANGE_PENDING to be cleared.
--

--- Patch [1]: [PATCH 1/6] md/raid5: size the worker group array by nr_node_ids ---
commit 4f7875645a221b114b23eee045960c40b057b078
Author: Hiroshi Nishida <nishidafmly@gmail.com>

md/raid5: size the worker group array by nr_node_ids

This patch fixes an out-of-bounds array access when the NUMA node map is
sparse by sizing the worker_groups array using nr_node_ids rather than
num_possible_nodes().

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 0c5c9fb0606ee..d8807114a6932 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7322,7 +7322,13 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt, int *group_cnt,
>  		*worker_groups = NULL;
>  		return 0;
>  	}
> -	*group_cnt = num_possible_nodes();
> +	/*
> +	 * worker_groups is indexed by cpu_to_group() == cpu_to_node(), a node
> +	 * id, so it must have room for the largest possible id.  Size it by
> +	 * nr_node_ids (one past that id), not num_possible_nodes(), which is
> +	 * only the node count and is smaller on a sparse node map.
> +	 */
> +	*group_cnt = nr_node_ids;

[Severity: High]
This is a pre-existing issue, but while reviewing this allocation change,
I noticed a potential deadlock risk in the caller,
raid5_store_group_thread_cnt(), when configuring these worker groups.

When writing to the group_thread_cnt sysfs attribute,
raid5_store_group_thread_cnt() acquires reconfig_mutex via
mddev_suspend_and_lock() and then blocks waiting for workers in
raid5_wq to finish:

drivers/md/raid5.c:raid5_store_group_thread_cnt() {
    err = mddev_suspend_and_lock(mddev);
    ...
    raid5_quiesce(mddev, true);

    if (new != conf->worker_cnt_per_group) {
        old_groups = conf->worker_groups;
        if (old_groups)
            flush_workqueue(raid5_wq);

        err = alloc_thread_groups(conf, new, &group_cnt, &new_groups);
}

However, the raid5 workers can block waiting for the md_thread to clear
MD_SB_CHANGE_PENDING:

drivers/md/raid5.c:raid5_do_work() {
    ...
    wait_event_lock_irq(mddev->sb_wait,
        !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags),
        conf->device_lock);
}

The md_thread requires the reconfig_mutex to clear this flag:

drivers/md/md.c:md_check_recovery() {
    ...
    if (mddev_trylock(mddev)) {
        ...
        if (mddev->sb_flags)
            md_update_sb(mddev, 0);
}

Does this sequence create a deadlock where the sysfs thread holds
reconfig_mutex and waits for raid5_wq workers, while the workers wait for
the md_thread to clear MD_SB_CHANGE_PENDING, but the md_thread fails to
acquire reconfig_mutex via mddev_trylock()?

>  	size = sizeof(struct r5worker) * cnt;
>  	workers = kcalloc(size, *group_cnt, GFP_NOIO);
>  	*worker_groups = kzalloc_objs(struct r5worker_group, *group_cnt,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=1

Re: [PATCH 2/6] md/raid5: size stripe-cache hash locks from the CPU count

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Time-of-check to time-of-use (TOCTOU) race condition in `nr_stripe_hash_locks` module parameter evaluation can lead to undefined behavior and a potential kernel panic during array creation.

Pre-existing issues:
- [High] The worker threads (`r5worker->work`) are not explicitly canceled or flushed during array teardown, which can lead to a Use-After-Free.
--

--- Patch [2]: [PATCH 2/6] md/raid5: size stripe-cache hash locks from the CPU count ---
commit bae2376a7fe52c6b3de7d64514b554d3101957fe
Author: Hiroshi Nishida <nishidafmly@gmail.com>

md/raid5: size stripe-cache hash locks from the CPU count

This commit modifies the stripe cache hash to use a per-array dynamic count
of spinlocks instead of a fixed 8, automatically sizing it based on the
online CPU count to reduce lock contention on many-core systems.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index d8807114a6932..7f72981121fd5 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -7448,6 +7453,9 @@ static void free_conf(struct r5conf *conf)
>  	kfree(conf->disks);
>  	bioset_exit(&conf->bio_split);
>  	kfree(conf->stripe_hashtbl);
> +	kfree(conf->hash_locks);
> +	kfree(conf->inactive_list);
> +	kfree(conf->temp_inactive_list);
>  	kfree(conf->pending_data);

[Severity: High]
This is a pre-existing issue, but looking at the teardown sequence, could
the worker threads access freed memory here?

During array teardown, md_stop() eventually causes a wait for
active_stripes to reach 0. In a worker thread executing raid5_do_work(),
active_stripes drops to 0 when handle_active_stripes() finishes releasing
the final stripes.

At this point, raid5_quiesce() wakes up and teardown proceeds to call
free_thread_groups() which frees the worker structures without calling
flush_workqueue() or cancel_work_sync().

The still-executing worker thread could then drop its lock and continue
to execute flush_deferred_bios() and r5l_flush_stripe_to_raid() using the
freed structures.

Should the worker threads be explicitly canceled or flushed before the
structures are freed?

[ ... ]
> @@ -7565,6 +7573,38 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>  	if (conf == NULL)
>  		goto abort;
>  
> +	/*
> +	 * Size the stripe cache hash-lock striping per array.  The
> +	 * nr_stripe_hash_locks module parameter selects the count: 0 (the
> +	 * default) auto-sizes it from the online CPU count so busier machines
> +	 * stripe the locks wider, while a non-zero value overrides that.  The
> +	 * result is rounded up to a power of two (the hash uses it as a
> +	 * bitmask) and capped at NR_STRIPE_HASH_LOCKS_MAX; the auto path also
> +	 * keeps at least the historical NR_STRIPE_HASH_LOCKS_DEFAULT so small
> +	 * systems are unchanged.  Allocating per array means an untuned system
> +	 * uses no more memory than before.
> +	 */
> +	if (nr_stripe_hash_locks == 0)
> +		conf->nr_hash_locks = clamp_t(unsigned int,
> +				roundup_pow_of_two(num_online_cpus()),
> +				NR_STRIPE_HASH_LOCKS_DEFAULT,
> +				NR_STRIPE_HASH_LOCKS_MAX);
> +	else
> +		conf->nr_hash_locks = roundup_pow_of_two(
> +				min_t(unsigned int, nr_stripe_hash_locks,
> +				      NR_STRIPE_HASH_LOCKS_MAX));

[Severity: High]
Could this be vulnerable to a time-of-check to time-of-use race if the
sysfs parameter is modified concurrently?

If the module parameter is changed from a non-zero value to 0 right after
the check:

Thread 1 in setup_conf():
    if (nr_stripe_hash_locks == 0)
        // check fails, proceeds to else block

Thread 2 writing to sysfs:
    nr_stripe_hash_locks = 0;

Thread 1 inside the else block:
    conf->nr_hash_locks = roundup_pow_of_two(min_t(..., 0, ...));

In this scenario, roundup_pow_of_two() would evaluate to 0. When kcalloc()
is subsequently called, it would return the zero size pointer placeholder
value (16).

The safety check testing for a null pointer would fail to catch this since
16 is non-null, and the kernel could panic when spin_lock_init()
dereferences it.

Would it be safer to read the parameter into a local variable first?

    unsigned int locks = READ_ONCE(nr_stripe_hash_locks);
    if (locks == 0)
        conf->nr_hash_locks = clamp_t(...);
    else
        conf->nr_hash_locks = roundup_pow_of_two(...);

> +	conf->hash_locks_mask = conf->nr_hash_locks - 1;
> +	conf->hash_locks = kcalloc(conf->nr_hash_locks,
> +				   sizeof(*conf->hash_locks), GFP_KERNEL);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=2

Re: [PATCH 6/6] md/raid5: derive the default group_thread_cnt from the hardware

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Double-read of the sysfs-modifiable `group_thread_cnt_default` module parameter can cause an array initialization failure due to a large negative worker count.
- [Medium] Massive overallocation of worker threads and worker groups on systems with sparse NUMA node maps.

Pre-existing issues:
- [High] A failure to register the CPU hotplug instance in `raid5_alloc_percpu()` causes a NULL pointer dereference (Oops) during error cleanup.
--

--- Patch [6]: [PATCH 6/6] md/raid5: derive the default group_thread_cnt from the hardware ---
commit 9c6dbf46ca41e580ada80950d4adaba43e7b0100
Author: Hiroshi Nishida <nishidafmly@gmail.com>

md/raid5: derive the default group_thread_cnt from the hardware

This commit updates the RAID5 subsystem to automatically configure the number of
worker threads per NUMA node based on the hardware CPU count, rather than
defaulting to a single raid5d thread. It calculates a default value bounded by
a cap, and allows overriding this via a new module parameter or the existing
sysfs attribute.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 8e1c2eba42417..6e91eb0ad5759 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c

[ ... ]

> @@ -7584,6 +7589,31 @@ static unsigned long raid5_cache_count(struct shrinker *shrink,
>  #define RAID5_CACHE_DEFAULT_RAM_SHIFT	9	/* above it: ~1/512 of the extra RAM */
>  #define RAID5_CACHE_DEFAULT_MAX		4096
>  
> +/*
> + * Default group_thread_cnt (worker_cnt_per_group) for a new array when the
> + * group_thread_cnt_default module parameter is left at -1.  The historical
> + * default is 0 -- a single raid5d thread -- which cannot keep a fast, wide
> + * array busy on a many-core host.  Derive a starting point from the CPU
> + * count: half the online CPUs divided across the NUMA nodes (this is a
> + * per-node count -- see alloc_thread_groups() -- so the total lands near
> + * half the online CPUs regardless of socket count), capped at
> + * RAID5_AUTO_GROUP_THREAD_MAX.  This is only a ceiling:
> + * raid5_wakeup_stripe_thread() wakes workers in proportion to the queued
> + * stripe count, so a lightly loaded array uses far fewer.  A lone worker is
> + * not worth its overhead, so 1 collapses back to 0.  The group_thread_cnt
> + * sysfs attribute overrides this per array.
> + */
> +#define RAID5_AUTO_GROUP_THREAD_MAX	256
> +
> +static int raid5_default_group_thread_cnt(void)
> +{
> +	unsigned int gtc = num_online_cpus() / (2 * num_possible_nodes());

[Severity: Medium]
Could this lead to massive overallocation of worker threads on systems with
sparse NUMA node maps?

Here in raid5_default_group_thread_cnt() in drivers/md/raid5.c, gtc is
calculated using num_possible_nodes(), which returns the actual count of
possible nodes. However, alloc_thread_groups() allocates threads based on
nr_node_ids:

drivers/md/raid5.c:alloc_thread_groups() {
	...
	*group_cnt = nr_node_ids;
	size = sizeof(struct r5worker) * cnt;
	workers = kcalloc(size, *group_cnt, GFP_NOIO);
	...
}

If a system has sparse NUMA node IDs (e.g., nodes 0 and 64, making
num_possible_nodes() == 2 but nr_node_ids == 65), would multiplying the
large gtc value by the highest node ID cause hundreds of unused struct
r5worker and struct r5worker_group instances to be allocated?

> +
> +	if (gtc > RAID5_AUTO_GROUP_THREAD_MAX)
> +		gtc = RAID5_AUTO_GROUP_THREAD_MAX;
> +	return gtc == 1 ? 0 : gtc;
> +}
> +
>  static struct r5conf *setup_conf(struct mddev *mddev)
>  {
>  	struct r5conf *conf;

[ ... ]

> @@ -7677,10 +7708,18 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>  		goto abort;
>  	for (i = 0; i < PENDING_IO_MAX; i++)
>  		list_add(&conf->pending_data[i].sibling, &conf->free_list);
> -	/* Don't enable multi-threading by default*/
> -	if (!alloc_thread_groups(conf, 0, &group_cnt, &new_group)) {
> +	/*
> +	 * Multi-threading defaults to a hardware-derived worker count (see
> +	 * raid5_default_group_thread_cnt()); group_thread_cnt_default overrides
> +	 * the choice, and the group_thread_cnt sysfs attribute overrides it per
> +	 * array.
> +	 */
> +	def_threads = group_thread_cnt_default < 0 ?
> +		raid5_default_group_thread_cnt() :
> +		min(group_thread_cnt_default, RAID5_AUTO_GROUP_THREAD_MAX);

[Severity: Medium]
Is it possible for a concurrent sysfs write to cause an array initialization
failure due to a large negative worker count?

The module parameter group_thread_cnt_default is evaluated twice without
READ_ONCE() (once in the ternary condition, and once in the min() macro).

Could an administrator writing to the sysfs parameter concurrently with array
assembly cause the ternary condition to read a positive value, and the min()
macro to read a negative value, resulting in def_threads becoming -1?

> +	if (!alloc_thread_groups(conf, def_threads, &group_cnt, &new_group)) {
>  		conf->group_cnt = group_cnt;
> -		conf->worker_cnt_per_group = 0;
> +		conf->worker_cnt_per_group = def_threads;
>  		conf->worker_groups = new_group;
>  	} else
>  		goto abort;

[Severity: High]
This isn't a bug introduced by this patch, but does the cleanup path for
setup failures lead to a NULL pointer dereference?

If cpuhp_state_add_instance() in raid5_alloc_percpu() previously returned
an error (like -ENOMEM) and failed to register the hotplug node, the error
bubbles up and causes a jump to the abort label.

In the cleanup path, raid5_free_percpu() unconditionally calls
cpuhp_state_remove_instance():

drivers/md/raid5.c:raid5_free_percpu() {
	if (!conf->percpu)
		return;
	cpuhp_state_remove_instance(CPUHP_MD_RAID5_PREPARE, &conf->node);
	...
}

Because conf->node is completely zero-initialized and was never successfully
hashed, its pprev pointer is NULL. Won't the subsequent hlist_del() in the
teardown path dereference this NULL pointer and crash the kernel? Should we
check hlist_unhashed(&conf->node) before removing the instance?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=6