* [PATCH 1/2] md: change chunk_sectors and stripe cache counts to unsigned int
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132329.7273-1-nishidafmly@gmail.com>
chunk_sectors, new_chunk_sectors, prev_chunk_sectors, max_nr_stripes,
and min_nr_stripes are never negative. Using signed int is semantically
wrong and prevents the compiler from optimizing division/modulo by
power-of-two chunk sizes to right shifts in the hot I/O path.
Change all struct fields and derived local variables to unsigned int:
mddev->chunk_sectors
mddev->new_chunk_sectors
r5conf->chunk_sectors
r5conf->prev_chunk_sectors
r5conf->max_nr_stripes
r5conf->min_nr_stripes
Local: sectors_per_chunk, new_chunk, chunk_sectors
The min() in r5c_check_cached_full_stripe() required both operands to
match signedness; this is now satisfied with max_nr_stripes unsigned.
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/md.h | 4 ++--
drivers/md/raid5.c | 14 +++++++-------
drivers/md/raid5.h | 8 ++++----
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/md/md.h b/drivers/md/md.h
index d8daf0f75cbb..b9ad26844799 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -437,7 +437,7 @@ struct mddev {
int external; /* metadata is
* managed externally */
char metadata_type[17]; /* externally set*/
- int chunk_sectors;
+ unsigned int chunk_sectors;
time64_t ctime, utime;
int level, layout;
char clevel[16];
@@ -466,7 +466,7 @@ struct mddev {
*/
sector_t reshape_position;
int delta_disks, new_level, new_layout;
- int new_chunk_sectors;
+ unsigned int new_chunk_sectors;
int reshape_backwards;
struct md_thread __rcu *thread; /* management thread */
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 0c5c9fb0606e..28828e083c2b 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2970,7 +2970,7 @@ sector_t raid5_compute_sector(struct r5conf *conf, sector_t r_sector,
sector_t new_sector;
int algorithm = previous ? conf->prev_algo
: conf->algorithm;
- int sectors_per_chunk = previous ? conf->prev_chunk_sectors
+ unsigned int sectors_per_chunk = previous ? conf->prev_chunk_sectors
: conf->chunk_sectors;
int raid_disks = previous ? conf->previous_raid_disks
: conf->raid_disks;
@@ -3166,7 +3166,7 @@ sector_t raid5_compute_blocknr(struct stripe_head *sh, int i, int previous)
int raid_disks = sh->disks;
int data_disks = raid_disks - conf->max_degraded;
sector_t new_sector = sh->sector, check;
- int sectors_per_chunk = previous ? conf->prev_chunk_sectors
+ unsigned int sectors_per_chunk = previous ? conf->prev_chunk_sectors
: conf->chunk_sectors;
int algorithm = previous ? conf->prev_algo
: conf->algorithm;
@@ -3584,7 +3584,7 @@ static void end_reshape(struct r5conf *conf);
static void stripe_set_idx(sector_t stripe, struct r5conf *conf, int previous,
struct stripe_head *sh)
{
- int sectors_per_chunk =
+ unsigned int sectors_per_chunk =
previous ? conf->prev_chunk_sectors : conf->chunk_sectors;
int dd_idx;
int chunk_offset = sector_div(stripe, sectors_per_chunk);
@@ -6103,7 +6103,7 @@ static enum stripe_result make_stripe_request(struct mddev *mddev,
static sector_t raid5_bio_lowest_chunk_sector(struct r5conf *conf,
struct bio *bi)
{
- int sectors_per_chunk = conf->chunk_sectors;
+ unsigned int sectors_per_chunk = conf->chunk_sectors;
int raid_disks = conf->raid_disks;
int dd_idx;
struct stripe_head sh;
@@ -7930,7 +7930,7 @@ static int raid5_run(struct mddev *mddev)
sector_t here_new, here_old;
int old_disks;
int max_degraded = (mddev->level == 6 ? 2 : 1);
- int chunk_sectors;
+ unsigned int chunk_sectors;
int new_data_disks;
if (journal_dev) {
@@ -8832,7 +8832,7 @@ static int raid5_check_reshape(struct mddev *mddev)
* to be used by a reshape pass.
*/
struct r5conf *conf = mddev->private;
- int new_chunk = mddev->new_chunk_sectors;
+ unsigned int new_chunk = mddev->new_chunk_sectors;
if (mddev->new_layout >= 0 && !algorithm_valid_raid5(mddev->new_layout))
return -EINVAL;
@@ -8866,7 +8866,7 @@ static int raid5_check_reshape(struct mddev *mddev)
static int raid6_check_reshape(struct mddev *mddev)
{
- int new_chunk = mddev->new_chunk_sectors;
+ unsigned int new_chunk = mddev->new_chunk_sectors;
if (mddev->new_layout >= 0 && !algorithm_valid_raid6(mddev->new_layout))
return -EINVAL;
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index cb5feae04db2..5cd9d0f36b6e 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -572,12 +572,12 @@ struct r5conf {
/* only protect corresponding hash list and inactive_list */
spinlock_t hash_locks[NR_STRIPE_HASH_LOCKS];
struct mddev *mddev;
- int chunk_sectors;
+ unsigned int chunk_sectors;
int level, algorithm, rmw_level;
int max_degraded;
int raid_disks;
- int max_nr_stripes;
- int min_nr_stripes;
+ unsigned int max_nr_stripes;
+ unsigned int min_nr_stripes;
#if PAGE_SIZE != DEFAULT_STRIPE_SIZE
unsigned long stripe_size;
unsigned int stripe_shift;
@@ -595,7 +595,7 @@ struct r5conf {
*/
sector_t reshape_safe;
int previous_raid_disks;
- int prev_chunk_sectors;
+ unsigned int prev_chunk_sectors;
int prev_algo;
short generation; /* increments with every reshape */
seqcount_spinlock_t gen_lock; /* lock against generation changes */
--
2.43.0
^ permalink raw reply related
* [PATCH 2/2] md: widen badblock sectors param from int to sector_t
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132329.7273-1-nishidafmly@gmail.com>
The badblocks core API -- badblocks_set(), badblocks_clear() and
badblocks_check() -- and the is_badblock() helper all take the range
length as sector_t. The md wrappers rdev_set_badblocks(),
rdev_clear_badblocks() and rdev_has_badblock(), however, declared the
same length as int, narrowing sector_t to int and back again in the
middle of an otherwise 64-bit clean path.
Change the sectors parameter to sector_t in these three wrappers so it
matches the core API and is_badblock(). No functional change: current
callers pass per-I/O or per-resync-chunk lengths well within int range.
This just removes a gratuitous truncation point and keeps the type
consistent end to end.
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/md.c | 4 ++--
drivers/md/md.h | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..61f40fa41e78 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -10553,7 +10553,7 @@ EXPORT_SYMBOL(md_finish_reshape);
/* Bad block management */
/* Returns true on success, false on failure */
-bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
+bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, sector_t sectors,
int is_new)
{
struct mddev *mddev = rdev->mddev;
@@ -10593,7 +10593,7 @@ bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
}
EXPORT_SYMBOL_GPL(rdev_set_badblocks);
-void rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
+void rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, sector_t sectors,
int is_new)
{
if (is_new)
diff --git a/drivers/md/md.h b/drivers/md/md.h
index b9ad26844799..95835a3286aa 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -311,7 +311,7 @@ static inline int is_badblock(struct md_rdev *rdev, sector_t s, sector_t sectors
}
static inline int rdev_has_badblock(struct md_rdev *rdev, sector_t s,
- int sectors)
+ sector_t sectors)
{
sector_t first_bad;
sector_t bad_sectors;
@@ -319,9 +319,9 @@ static inline int rdev_has_badblock(struct md_rdev *rdev, sector_t s,
return is_badblock(rdev, s, sectors, &first_bad, &bad_sectors);
}
-extern bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
+extern bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, sector_t sectors,
int is_new);
-extern void rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
+extern void rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, sector_t sectors,
int is_new);
struct md_cluster_info;
struct md_cluster_operations;
--
2.43.0
^ permalink raw reply related
* [PATCH 0/2] md/raid5: reduce resync/recovery dispatch overhead
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
Two changes to how resync/recovery submits work, reducing per-stripe
overhead on the rebuild path. Neither adds memory.
1/2 submits a window of stripes per raid5_sync_request() call instead
of one at a time. On a rebuild this reduces the number of
raid5_sync_request() invocations by ~30x (a call-count measurement,
not a throughput figure) and bounds the per-stripe jiffy-sleep that
otherwise throttles progress when the stripe cache is under
pressure.
2/2 reserves a small amount of stripe cache for user I/O during a
rebuild, so foreground I/O is not starved by the resync stream. A
pure rebuild with no competing I/O is unchanged.
Both touch only the resync/recovery path and are independent of the other
md patches I'm sending separately.
Hiroshi Nishida (2):
md/raid5: submit a window of stripes during resync/recovery
md/raid5: reserve stripe cache for user I/O during rebuild
drivers/md/raid5.c | 68 +++++++++++++++++++++++++++++++++++++---------
drivers/md/raid5.h | 2 ++
2 files changed, 57 insertions(+), 13 deletions(-)
base-commit: 55b77337bdd088c77461588e5ec094421b89911b
--
2.43.0
^ permalink raw reply
* [PATCH 1/2] md/raid5: submit a window of stripes during resync/recovery
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132339.7284-1-nishidafmly@gmail.com>
raid5_sync_request() dispatches one stripe per call: it fetches a single
stripe head, marks it for sync, and returns one stripe's worth of
sectors. When the stripe cache is full the NOBLOCK fetch fails and it
re-enters a one-jiffy throttle sleep (schedule_timeout_uninterruptible(1))
before retrying. Because that sleep is taken per stripe, sustained cache
pressure bounds sync progress to roughly HZ stripes/second regardless of
how fast the member devices are.
Dispatch up to RAID5_SYNC_WINDOW (32) stripes per call instead. Only the
first stripe of the window keeps the original behaviour (block, then the
one-jiffy throttle if the cache was full); the remaining stripes are
requested with R5_GAS_NOBLOCK and the loop stops as soon as the cache is
full. So at most one throttle sleep is taken per window rather than per
stripe, and when the cache has free slots a single call can queue a batch
instead of one stripe at a time.
With a warm cache the window stays near full: counting raid5_sync_request()
invocations across a rebuild showed it averaging ~30 of the 32 stripes per
call, i.e. roughly 30x fewer calls into the sync path for the same resync.
The return value reports the number of stripes actually submitted, so
md_do_sync()'s recovery_active accounting stays balanced, and the window
is bounded by both the end of the sync region (max_sector) and
mddev->resync_max, so a user- or cluster-imposed sync ceiling is not
overshot.
This does not change which data is read or written during resync or
recovery.
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 47 +++++++++++++++++++++++++++++++++-------------
drivers/md/raid5.h | 1 +
2 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 0c5c9fb0606e..574880e4f23f 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6563,7 +6563,8 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
struct stripe_head *sh;
sector_t sync_blocks;
bool still_degraded = false;
- int i;
+ int i, submitted;
+ sector_t win_sector;
if (sector_nr >= max_sector) {
/* just being told to finish up .. nothing much to do */
@@ -6620,16 +6621,7 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
if (md_bitmap_enabled(mddev, false))
mddev->bitmap_ops->cond_end_sync(mddev, sector_nr, false);
- sh = raid5_get_active_stripe(conf, NULL, sector_nr,
- R5_GAS_NOBLOCK);
- if (sh == NULL) {
- sh = raid5_get_active_stripe(conf, NULL, sector_nr, 0);
- /* make sure we don't swamp the stripe cache if someone else
- * is trying to get access
- */
- schedule_timeout_uninterruptible(1);
- }
- /* Need to check if array will still be degraded after recovery/resync
+ /* Check once whether array will still be degraded after recovery/resync.
* Note in case of > 1 drive failures it's possible we're rebuilding
* one drive while leaving another faulty drive in array.
*/
@@ -6640,13 +6632,42 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
still_degraded = true;
}
+ /* First stripe: block if stripe cache is full, then throttle. */
+ sh = raid5_get_active_stripe(conf, NULL, sector_nr, R5_GAS_NOBLOCK);
+ if (sh == NULL) {
+ sh = raid5_get_active_stripe(conf, NULL, sector_nr, 0);
+ /* make sure we don't swamp the stripe cache if someone else
+ * is trying to get access
+ */
+ schedule_timeout_uninterruptible(1);
+ }
md_bitmap_start_sync(mddev, sector_nr, &sync_blocks, still_degraded);
set_bit(STRIPE_SYNC_REQUESTED, &sh->state);
set_bit(STRIPE_HANDLE, &sh->state);
-
raid5_release_stripe(sh);
- return RAID5_STRIPE_SECTORS(conf);
+ /* Submit remaining stripes in the window non-blocking. Stop early
+ * if the stripe cache is full: the disk queue is already saturated.
+ * Bound by resync_max so a user- or cluster-imposed sync ceiling is
+ * not overshot.
+ */
+ win_sector = sector_nr + RAID5_STRIPE_SECTORS(conf);
+ for (submitted = 1;
+ submitted < RAID5_SYNC_WINDOW && win_sector < max_sector &&
+ win_sector < mddev->resync_max;
+ submitted++, win_sector += RAID5_STRIPE_SECTORS(conf)) {
+ sh = raid5_get_active_stripe(conf, NULL, win_sector,
+ R5_GAS_NOBLOCK);
+ if (!sh)
+ break;
+ md_bitmap_start_sync(mddev, win_sector, &sync_blocks,
+ still_degraded);
+ set_bit(STRIPE_SYNC_REQUESTED, &sh->state);
+ set_bit(STRIPE_HANDLE, &sh->state);
+ raid5_release_stripe(sh);
+ }
+
+ return submitted * RAID5_STRIPE_SECTORS(conf);
}
static int retry_aligned_read(struct r5conf *conf, struct bio *raid_bio,
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index cb5feae04db2..63b630118782 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -491,6 +491,7 @@ struct disk_info {
#define NR_HASH (PAGE_SIZE / sizeof(struct hlist_head))
#define HASH_MASK (NR_HASH - 1)
#define MAX_STRIPE_BATCH 8
+#define RAID5_SYNC_WINDOW 32 /* stripes to pre-submit per sync_request call */
/* NOTE NR_STRIPE_HASH_LOCKS must remain below 64.
* This is because we sometimes take all the spinlocks
--
2.43.0
^ permalink raw reply related
* [PATCH 2/2] md/raid5: reserve stripe cache for user I/O during rebuild
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132339.7284-1-nishidafmly@gmail.com>
The resync read-ahead window (RAID5_SYNC_WINDOW) can fill the stripe
cache with rebuild stripes and starve concurrent user I/O, producing a
burst-starvation flip-flop between rebuild and application throughput.
Add two yield points to the window-submission loop:
- stop the window immediately if any thread is waiting for a stripe
(waitqueue_active(&conf->wait_for_stripe)); the check is intentionally
racy -- a waiter appearing just after is serviced by the next
sync_request call, so no barrier is needed.
- stop expanding once active_stripes reaches half the cache
(max_nr_stripes / RAID5_SYNC_HWMARK), but only when
preread_active_stripes > 0, i.e. user write I/O is actually competing.
Sync stripes never set STRIPE_PREREAD_ACTIVE, so during a pure rebuild
the counter stays zero and the window fills freely; rebuild-only
throughput is unchanged.
This bounds the share of the stripe cache a rebuild may hold while user
I/O is present, so application latency no longer collapses during the
read-ahead bursts, without throttling a rebuild that has the array to
itself.
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 21 +++++++++++++++++++++
drivers/md/raid5.h | 1 +
2 files changed, 22 insertions(+)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 574880e4f23f..6575bdb84bb2 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6656,6 +6656,27 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
submitted < RAID5_SYNC_WINDOW && win_sector < max_sector &&
win_sector < mddev->resync_max;
submitted++, win_sector += RAID5_STRIPE_SECTORS(conf)) {
+ /*
+ * Yield to user I/O: stop the read-ahead if anyone is waiting
+ * for a stripe. The check is intentionally racy -- a waiter
+ * appearing just after is serviced by the next sync_request
+ * call, so no barrier is needed.
+ */
+ if (waitqueue_active(&conf->wait_for_stripe))
+ break;
+ /*
+ * Reserve cache for user I/O only when it is actually competing.
+ * preread_active_stripes counts stripes queued for write I/O
+ * (including the read phase of RMW); sync stripes never set
+ * STRIPE_PREREAD_ACTIVE, so during a pure rebuild it stays zero
+ * and the window fills freely. Competing user reads do not bump
+ * the counter but are caught by the waitqueue_active() check
+ * above.
+ */
+ if (atomic_read(&conf->preread_active_stripes) > 0 &&
+ atomic_read(&conf->active_stripes) >=
+ conf->max_nr_stripes / RAID5_SYNC_HWMARK)
+ break;
sh = raid5_get_active_stripe(conf, NULL, win_sector,
R5_GAS_NOBLOCK);
if (!sh)
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 63b630118782..263330af4e7d 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -492,6 +492,7 @@ struct disk_info {
#define HASH_MASK (NR_HASH - 1)
#define MAX_STRIPE_BATCH 8
#define RAID5_SYNC_WINDOW 32 /* stripes to pre-submit per sync_request call */
+#define RAID5_SYNC_HWMARK 2 /* rebuild uses at most 1/N of stripe cache */
/* NOTE NR_STRIPE_HASH_LOCKS must remain below 64.
* This is because we sometimes take all the spinlocks
--
2.43.0
^ permalink raw reply related
* [PATCH 0/6] md/raid5: size stripe-cache and worker tuning from the hardware
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, 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
^ permalink raw reply
* [PATCH 1/6] md/raid5: size the worker group array by nr_node_ids
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132346.7295-1-nishidafmly@gmail.com>
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
^ permalink raw reply related
* [PATCH 2/6] md/raid5: size stripe-cache hash locks from the CPU count
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132346.7295-1-nishidafmly@gmail.com>
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
^ permalink raw reply related
* [PATCH 4/6] md/raid5: make the stripe batch size a module parameter
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132346.7295-1-nishidafmly@gmail.com>
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
^ permalink raw reply related
* [PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132346.7295-1-nishidafmly@gmail.com>
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
^ permalink raw reply related
* [PATCH 5/6] md/raid5: scale the default stripe cache size with system memory
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132346.7295-1-nishidafmly@gmail.com>
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
^ permalink raw reply related
* [PATCH 6/6] md/raid5: derive the default group_thread_cnt from the hardware
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132346.7295-1-nishidafmly@gmail.com>
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
^ permalink raw reply related
* Re: [PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory
From: sashiko-bot @ 2026-07-10 13:34 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132346.7295-4-nishidafmly@gmail.com>
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
^ permalink raw reply
* Re: [PATCH 2/2] md/raid5: reserve stripe cache for user I/O during rebuild
From: sashiko-bot @ 2026-07-10 13:34 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: yukuai, linux-raid
In-Reply-To: <20260710132339.7284-3-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added stripe cache reservation logic is completely ineffective because it fails to throttle the calling loop in md_do_sync().
--
--- Patch [2]: [PATCH 2/2] md/raid5: reserve stripe cache for user I/O during rebuild ---
commit fb8a80692f7a386d69a8bee1d9b364db1d33380f
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: reserve stripe cache for user I/O during rebuild
This commit attempts to prevent burst-starvation of user I/O during a RAID5
rebuild by reserving a portion of the stripe cache. It adds checks in the
read-ahead loop of raid5_sync_request() to yield when there is competing
user I/O.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 574880e4f23fe..6575bdb84bb23 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -6656,6 +6656,27 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
> submitted < RAID5_SYNC_WINDOW && win_sector < max_sector &&
> win_sector < mddev->resync_max;
> submitted++, win_sector += RAID5_STRIPE_SECTORS(conf)) {
> + /*
> + * Yield to user I/O: stop the read-ahead if anyone is waiting
> + * for a stripe. The check is intentionally racy -- a waiter
> + * appearing just after is serviced by the next sync_request
> + * call, so no barrier is needed.
> + */
> + if (waitqueue_active(&conf->wait_for_stripe))
> + break;
> + /*
> + * Reserve cache for user I/O only when it is actually competing.
> + * preread_active_stripes counts stripes queued for write I/O
> + * (including the read phase of RMW); sync stripes never set
> + * STRIPE_PREREAD_ACTIVE, so during a pure rebuild it stays zero
> + * and the window fills freely. Competing user reads do not bump
> + * the counter but are caught by the waitqueue_active() check
> + * above.
> + */
> + if (atomic_read(&conf->preread_active_stripes) > 0 &&
> + atomic_read(&conf->active_stripes) >=
> + conf->max_nr_stripes / RAID5_SYNC_HWMARK)
> + break;
[Severity: High]
Does this actually throttle the rebuild as intended?
When we break out of this loop here, raid5_sync_request() returns the number
of sectors processed so far back to md_do_sync(). Since md_do_sync()
continuously loops calling the sync_request handler until the rate-limit window
is met, it will immediately call raid5_sync_request() again.
When raid5_sync_request() is re-entered, the very first stripe is allocated
unconditionally without checking the high-water mark:
drivers/md/raid5.c:raid5_sync_request() {
...
sh = raid5_get_active_stripe(conf, NULL, sector_nr, R5_GAS_NOBLOCK);
...
}
Will the sync thread just rapidly consume stripes one-by-one, completely
bypassing the 50% cache reservation and filling the cache 100% anyway?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132339.7284-1-nishidafmly@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 1/2] md/raid5: submit a window of stripes during resync/recovery
From: sashiko-bot @ 2026-07-10 13:36 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132339.7284-2-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The batched sync loop can greedily drain the stripe cache without yielding, causing severe latency spikes and potentially starving user I/O.
--
--- Patch [1]: [PATCH 1/2] md/raid5: submit a window of stripes during resync/recovery ---
commit 2b3cdb45d82bdea6112cba5dad14b59ec3acb23a
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: submit a window of stripes during resync/recovery
This patch updates the RAID5 sync path to dispatch a batch of up to 32
stripes per call instead of just one. It throttles only on the first stripe,
allowing subsequent stripes to be submitted without blocking to improve
resync performance.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 0c5c9fb0606ee..574880e4f23fe 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -6640,13 +6632,42 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
[ ... ]
> + /* Submit remaining stripes in the window non-blocking. Stop early
> + * if the stripe cache is full: the disk queue is already saturated.
> + * Bound by resync_max so a user- or cluster-imposed sync ceiling is
> + * not overshot.
> + */
> + win_sector = sector_nr + RAID5_STRIPE_SECTORS(conf);
> + for (submitted = 1;
> + submitted < RAID5_SYNC_WINDOW && win_sector < max_sector &&
> + win_sector < mddev->resync_max;
> + submitted++, win_sector += RAID5_STRIPE_SECTORS(conf)) {
> + sh = raid5_get_active_stripe(conf, NULL, win_sector,
> + R5_GAS_NOBLOCK);
> + if (!sh)
> + break;
[Severity: High]
Can this batched sync loop greedily drain the stripe cache without yielding to
user I/O?
During array resync or recovery, this loop acquires up to 32 free stripes
sequentially. Could this starve user I/O threads that are blocked on the
conf->wait_for_stripe waitqueue, since it doesn't check if user I/O is waiting
(e.g., via waitqueue_active(&conf->wait_for_stripe)) before getting the next
stripe?
> + md_bitmap_start_sync(mddev, win_sector, &sync_blocks,
> + still_degraded);
> + set_bit(STRIPE_SYNC_REQUESTED, &sh->state);
> + set_bit(STRIPE_HANDLE, &sh->state);
> + raid5_release_stripe(sh);
> + }
> +
> + return submitted * RAID5_STRIPE_SECTORS(conf);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132339.7284-1-nishidafmly@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 4/6] md/raid5: make the stripe batch size a module parameter
From: sashiko-bot @ 2026-07-10 13:36 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: yukuai, linux-raid
In-Reply-To: <20260710132346.7295-5-nishidafmly@gmail.com>
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
^ permalink raw reply
* Re: [PATCH 5/6] md/raid5: scale the default stripe cache size with system memory
From: sashiko-bot @ 2026-07-10 13:37 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132346.7295-6-nishidafmly@gmail.com>
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
^ permalink raw reply
* Re: [PATCH 1/6] md/raid5: size the worker group array by nr_node_ids
From: sashiko-bot @ 2026-07-10 13:39 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: yukuai, linux-raid
In-Reply-To: <20260710132346.7295-2-nishidafmly@gmail.com>
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
^ permalink raw reply
* Re: [PATCH 2/2] md: widen badblock sectors param from int to sector_t
From: sashiko-bot @ 2026-07-10 13:40 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132329.7273-3-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Arithmetic underflow in `is_badblock()` when a physical bad block overlaps `data_offset`.
- [High] Discarded macro results in `block/badblocks.c` make block alignment a no-op.
--
--- Patch [2]: [PATCH 2/2] md: widen badblock sectors param from int to sector_t ---
commit 19521e4f1c32c01e7a6122a185436691618d94c1
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md: widen badblock sectors param from int to sector_t
This commit changes the sectors parameter in md wrappers (rdev_set_badblocks,
rdev_clear_badblocks, and rdev_has_badblock) from int to sector_t. This
matches the core API and is_badblock() to prevent a truncation point.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c81..61f40fa41e781 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> @@ -10553,7 +10553,7 @@ EXPORT_SYMBOL(md_finish_reshape);
> /* Bad block management */
>
> /* Returns true on success, false on failure */
> -bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
> +bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, sector_t sectors,
> int is_new)
> {
[Severity: High]
This isn't a bug introduced by this patch, but since this code interacts
with the core badblocks API, I noticed a pre-existing bug in how block
alignment is handled in block/badblocks.c.
In _badblocks_set(), the code attempts to align sectors:
block/badblocks.c:_badblocks_set() {
...
rounddown(s, 1 << bb->shift);
roundup(next, 1 << bb->shift);
sectors = next - s;
...
}
Similar logic exists in _badblocks_clear() and badblocks_check().
Because rounddown() and roundup() are macros that evaluate to the rounded
value but do not modify their arguments in-place, aren't the alignment
results being ignored here, leaving s and next unaligned?
If bb->shift > 0, wouldn't this break bad block alignment, fragment the
bad block table, and risk premature table exhaustion?
[ ... ]
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index b9ad26844799d..95835a3286aac 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
[ ... ]
> @@ -311,7 +311,7 @@ static inline int is_badblock(struct md_rdev *rdev, sector_t s, sector_t sectors
> }
>
> static inline int rdev_has_badblock(struct md_rdev *rdev, sector_t s,
> - int sectors)
> + sector_t sectors)
> {
> sector_t first_bad;
[Severity: High]
This is a pre-existing issue, but while reviewing the badblock sector types,
I noticed a logic error in is_badblock() located just above this function.
drivers/md/md.h:is_badblock() {
...
int rv = badblocks_check(&rdev->badblocks, rdev->data_offset + s,
sectors, first_bad, bad_sectors);
if (rv)
*first_bad -= rdev->data_offset;
...
}
If userspace tools (e.g., mdadm via sysfs bb_store) write a physical bad
block that starts before rdev->data_offset but extends into the array's data
area, badblocks_check() returns a *first_bad physical sector less than
data_offset.
Could the unconditional subtraction (*first_bad -= rdev->data_offset)
underflow here?
Would this create a massive false first_bad sector value, causing callers
like raid1 to mistakenly issue I/O to a known bad sector?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132329.7273-1-nishidafmly@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 2/6] md/raid5: size stripe-cache hash locks from the CPU count
From: sashiko-bot @ 2026-07-10 13:41 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132346.7295-3-nishidafmly@gmail.com>
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
^ permalink raw reply
* Re: [PATCH 6/6] md/raid5: derive the default group_thread_cnt from the hardware
From: sashiko-bot @ 2026-07-10 13:42 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: yukuai, linux-raid
In-Reply-To: <20260710132346.7295-7-nishidafmly@gmail.com>
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
^ permalink raw reply
* Re: [PATCH 0/8] md/raid5: scalability and rebuild-path improvements
From: Hiroshi Nishida @ 2026-07-10 13:45 UTC (permalink / raw)
To: yukuai; +Cc: Song Liu, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <CAE1ksKTLcamf2jX6s6vaV5keXwS1pmJa_cV7nPZ84CNUSQRZxw@mail.gmail.com>
The cover quotes the 2.1-3.2x headline; here is the full per-workload data
behind it, in case it helps review.
The same raid456.ko is used in both arms -- only the module parameters
differ at array-create -- so this isolates the *defaults*, not code changes.
Real NVMe SSDs, not a ramdisk.
Setup: GCP n2-standard-32 -- 32 vCPUs = 16 physical cores (a vCPU is one
hyperthread), 2 NUMA nodes, 125 GB RAM, 16x 375 GB local NVMe. Kernel: full
build of the 6-patch series (base 55b77337), non-debug. raid6, 64k chunk,
bitmap=none, --assume-clean. Workloads: a 5-test fio suite (direct=1,
libaio, 30 s each): 4K random write (RMW), DB-mixed 75/25 8K,
high-concurrency 70/30 4K, OLTP 70/30 16K, partial-stripe 8K.
AUTO = all defaults left auto (the hardware-derived values); STOCK = the
historical fixed values (hash 8 / ceiling 32768 / batch 8 / cache 256 /
group_thread_cnt 0). On this host AUTO resolves to group_thread_cnt=8,
stripe_cache_size~3886, 32 hash locks.
1) Wide 16-disk raid6 (14+2), steady state, interleaved N=5 (STOCK and AUTO
alternate each round so SSD drift cancels; cv 1-8%):
Workload STOCK IOPS AUTO IOPS AUTO/STOCK
4K random write 39,147 100,069 2.56x
DB mixed 75/25 8K 88,040 208,284 2.37x
High-conc 70/30 4K 122,765 257,952 2.10x
OLTP 70/30 16K 39,597 100,033 2.53x
Partial-stripe 8K 20,491 64,511 3.15x
2) Low-end 4-CPU / 8 GB, 6-disk raid6 (4+2), N=3 -- no-regression control.
4 online CPUs + 8 GB, single node (num_online_cpus()=4 -> gtc=2); the
cache stays 256 and the hash count stays 8, so AUTO differs from STOCK
only in group_thread_cnt (2 vs 0):
Workload STOCK IOPS AUTO IOPS AUTO/STOCK
4K random write 52,017 81,132 1.56x
DB mixed 125,043 126,188 1.01x
High-conc 180,570 217,074 1.20x
OLTP 54,308 55,840 1.03x
Partial-stripe 27,060 42,751 1.58x
No regression on any workload; modest write gains even here. A
slower-disk NAS is more device-bound (the gains flatten) but still shows
no regression, and a box with <=2 online CPUs derives gtc=0 =
byte-for-byte stock.
The gain is essentially patch 6 (worker groups on by default); patches 2-5
(cache and lock sizing) are throughput-neutral on real SSD and earn their
place as configurability plus sane defaults, not a speed claim. Patch 1 is
the NUMA-sizing prerequisite and does not affect throughput.
Thanks,
2026年7月10日(金) 6:09 Hiroshi Nishida <nishidafmly@gmail.com>:
>
> Hi Yu Kuai,
>
> Thanks again for the review, and in particular for:
>
> > I can accept make those values configurable, but not direct
> > modifications.
>
> I've reworked the tunables along exactly those lines and will send them as
> a separate, self-contained series, "md/raid5: size stripe-cache and worker
> tuning from the hardware".
>
> Rather than raising any fixed constant, each value now derives a default
> from the hardware and stays overridable:
>
> - the stripe-cache hash lock count (was a fixed 8) is sized from the CPU
> count, clamped to 8..32;
> - the stripe_cache_size ceiling (was a fixed 32768) scales with memory,
> but never drops below 32768;
> - the initial stripe_cache_size (was a fixed 256) scales gently with
> memory, 256..4096;
> - the default group_thread_cnt (was 0, i.e. single-threaded) is derived
> from the CPU count (half the online CPUs per NUMA node, capped);
> - the stripe batch size (was a fixed 8) is exposed as a parameter.
>
> Ahead of those, the series opens with a one-line prerequisite fix:
> alloc_thread_groups() sizes the per-node worker_groups[] array by
> num_possible_nodes() but indexes it by cpu_to_node(), so a sparse NUMA node
> map can index off the end -- reachable once the worker-group default (the
> last patch) is on. It is Fixes:-tagged and can be taken on its own.
>
> The key point for the consumer-NAS concern you raised: each default only
> rises on hardware that can back it -- the lock and worker counts scale with
> the core count, the cache sizes with RAM -- so a genuinely small system (a
> few cores and a few GB) keeps today's values and footprint. And each one is
> overridable via a module parameter (group_thread_cnt also via its existing
> sysfs attribute), including all the way back to today's behaviour. So nothing is
> imposed; the default simply tracks the machine instead of a constant, and
> a wide array on a large host no longer needs a recompile or manual
> per-array tuning to use the memory and cores it has.
>
> And on performance -- on real NVMe this time, not a ramdisk: on a 16-disk
> raid6 array (a 32-vCPU / 16-core host, steady state, interleaved runs) the
> hardware-derived defaults run 2.1-3.2x the stock defaults (4K random write
> ~39k -> ~100k IOPS), essentially all of it from patch 6 turning on the
> worker groups. Patches 2-5 (the cache and lock sizing) are
> throughput-neutral on real SSD, as you'd expect -- they earn their place as
> configurability and sane defaults, not a speed claim. And there is no
> regression at the small end: a 4-CPU box derives 2 workers and is never
> slower on any workload, and a box with two or fewer CPUs derives 0 and is
> byte-for-byte unchanged.
>
> It has been through KASAN + lockdep + DEBUG_LIST on RAID5
> (create/rebuild/scrub, plus the bitmap add/remove that drives the
> lock-all-hash-locks quiesce path), at both the derived defaults and pinned
> values including nr_stripe_hash_locks=32.
>
> The two unrelated parts of the original series -- the type-widening /
> correctness fixes and the resync/recovery dispatch changes -- I'll send as
> their own small series, as discussed.
>
> Thanks,
>
> 2026年7月5日(日) 19:56 yu kuai <yukuai@fygo.io>:
> >
> > Hi,
> >
> > 在 2026/6/24 23:54, Hiroshi Nishida 写道:
> > > This series collects small, individually low-risk md/raid5 changes for
> > > large, many-core, many-disk arrays. Their common theme is reducing
> > > per-stripe and stripe-cache contention, so the benefit appears mainly
> > > when the raid5 stripe-handling worker threads are in use
> > > (group_thread_cnt > 0); at the default group_thread_cnt = 0 (a single
> > > handling thread) the series is essentially neutral.
> > >
> > > - patches 1-3 remove signed arithmetic from a hot-path divisor, lift an
> > > arbitrary stripe-cache size cap, and widen a badblock length argument
> > > that currently truncates large ranges;
> > > - patch 4 raises NR_STRIPE_HASH_LOCKS (8 -> 32) to spread stripe-hash
> > > contention on high core-count systems;
> > > - patches 5 and 8 reduce per-stripe overhead in the resync/recovery
> > > path and bound the share of the stripe cache a rebuild may hold while
> > > user I/O is competing;
> > > - patch 6 allocates each worker group's array on its own NUMA node;
> > > - patch 7 raises MAX_STRIPE_BATCH (8 -> 32).
> > >
> > > Measured effect, treatment vs baseline, % change in mean IOPS (N=3),
> > > swept over group_thread_cnt (RAID6 4+2, 22-core host, ramdisk members):
> >
> > Testing with ramdisk does serve as a useful reference, but it does not reflect
> > real world usage.
> >
> > >
> > > workload gtc=0 gtc=2 gtc=4 gtc=8
> > > random 4K write (RMW) +4.2% +8.1% +17.4% +6.5%
> > > DB mixed 75/25 8K +0.4% +4.2% +10.3% +4.7%
> > > high-concurrency 70/30 4K +3.9% +1.2% +10.0% +0.2%
> > > OLTP 70/30 16K -0.3% +4.7% +10.1% +9.3%
> > > partial-stripe write 8K +1.1% +4.8% +11.2% +14.2%
> >
> > With a quick review I saw many static configurations is changed, I agree
> > these changes can improve arrays with ssd/nvme and a system with large
> > memory available. However, we already tested with hdd and about 8G memory
> > available, these changes will not improve performance at all, with the
> > extra memory overhead.
> >
> > I can accept make those values configurable, but not direct modifications.
> > As validation is required for numerous scenarios. Memory resources are precious
> > especially for most consumer NAS devices.
> >
> > >
> > > At the default single handling thread (group_thread_cnt = 0) the series is
> > > neutral (no regression). As worker threads are added the gain grows,
> > > peaking broadly around group_thread_cnt = 4 at roughly +10-17% across the
> > > whole mix; at gtc = 8 the write-heavy workloads keep gaining while the
> > > read-heavy high-concurrency case has saturated. (Per-run cv was <1%
> > > except the random-write test, ~5-9%, from a cold first run.)
> > >
> > > These numbers are on a ramdisk, which removes device latency and so
> > > overstates the CPU-side contention effect relative to a real device;
> > > they show the direction and the group_thread_cnt dependence, not an
> > > absolute speedup. The stripe-hash/batch patches (4, 7) and the cache cap
> > > (2) drive this; patch 6 only matters on multi-socket systems (not
> > > exercised above) and patches 5/8 act on the resync/recovery path rather
> > > than this steady-state workload.
> > >
> > > Reproduction (stock mdadm + fio):
> > > mdadm --create /dev/md0 --level=6 --raid-devices=6 --chunk=512 \
> > > --assume-clean <6 members>
> > > echo 16384 > /sys/block/md0/md/stripe_cache_size
> > > echo N > /sys/block/md0/md/group_thread_cnt # N = 0,2,4,8
> > > fio --filename=/dev/md0 --direct=1 --ioengine=libaio --group_reporting \
> > > --time_based --runtime=15 --name=w <per-workload opts>:
> > > random write : --rw=randwrite --bs=4k --numjobs=4 --iodepth=32
> > > DB mixed : --rw=randrw --rwmixread=75 --bs=8k --numjobs=8 --iodepth=16
> > > high-concur. : --rw=randrw --rwmixread=70 --bs=4k --numjobs=16 --iodepth=8
> > > OLTP : --rw=randrw --rwmixread=70 --bs=16k --numjobs=6 --iodepth=16
> > > partial-stripe : --rw=randwrite --bs=8k --numjobs=4 --iodepth=32
> > >
> > > Each patch stands on its own; I am happy to drop or defer any that is not
> > > justified on its own merit.
> > >
> > > Functional testing on RAID5 and RAID6: create, fail a member, rebuild
> > > onto a spare / re-add, full data read-back verified, and scrub
> > > ("check") reporting mismatch_cnt == 0. The series was also exercised
> > > with KASAN and lockdep enabled -- including heavy group_thread_cnt
> > > churn on a multi-node setup to stress the per-NUMA-node worker
> > > allocation and the raid5_quiesce hash-lock-all path -- with no reports.
> > >
> > > Hiroshi Nishida (8):
> > > md: change chunk_sectors and stripe cache counts to unsigned int
> > > md/raid5: raise stripe cache limit from 32768 to 262144
> > > md: widen badblock sectors param from int to sector_t
> > > md/raid5: raise NR_STRIPE_HASH_LOCKS from 8 to 32
> > > md/raid5: submit a window of stripes during resync/recovery
> > > md/raid5: allocate worker groups per NUMA node
> > > md/raid5: raise MAX_STRIPE_BATCH from 8 to 32
> > > md/raid5: reserve stripe cache for user I/O during rebuild
> > >
> > > drivers/md/md.c | 4 +-
> > > drivers/md/md.h | 10 ++--
> > > drivers/md/raid5.c | 129 ++++++++++++++++++++++++++++++++-------------
> > > drivers/md/raid5.h | 33 ++++++++----
> > > 4 files changed, 121 insertions(+), 55 deletions(-)
> > >
> > > base-commit: 55b77337bdd088c77461588e5ec094421b89911b
> > >
> > --
> > Thanks,
> > Kuai
>
>
>
> --
> Hiroshi Nishida
> nishidafmly@gmail.com
--
Hiroshi Nishida
nishidafmly@gmail.com
^ permalink raw reply
* Re: [PATCH 1/2] md: change chunk_sectors and stripe cache counts to unsigned int
From: sashiko-bot @ 2026-07-10 15:44 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132329.7273-2-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] Lockless read race on total_cached evaluates to a negative value, triggering unsigned promotion and false cache pressure signals.
Pre-existing issues:
- [Critical] The calculation of total sectors per chunk stripe (chunk size multiplied by data disks) is performed using 32-bit arithmetic, which can overflow. This leads to a divide-by-zero kernel panic or incorrect boundary math if a maliciously large chunk size is provided.
--
--- Patch [1]: [PATCH 1/2] md: change chunk_sectors and stripe cache counts to unsigned int ---
commit 6fa5b599e890f5ca04407ab1005af1cc096d709e
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md: change chunk_sectors and stripe cache counts to unsigned int
This commit changes various chunk and stripe counts from signed to
unsigned int in the md/raid5 subsystem to prevent negative values.
It allows the compiler to optimize division and modulo operations by
power-of-two chunk sizes to right shifts in the hot I/O path.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 0c5c9fb0606ee..28828e083c2b3 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -7929,7 +7929,7 @@ static int raid5_run(struct mddev *mddev)
> sector_t here_new, here_old;
> int old_disks;
> int max_degraded = (mddev->level == 6 ? 2 : 1);
> - int chunk_sectors;
> + unsigned int chunk_sectors;
> int new_data_disks;
[Severity: Critical]
This isn't a bug introduced by this patch, but changing the types to
unsigned int fails to resolve an underlying 32-bit wrapping regression.
Does this code overflow when calculating total sectors per chunk stripe?
In raid5_run(), if a maliciously crafted RAID superblock sets chunksize
to 0x08000000 (64 GiB) and raid_disks to 33, the multiplication
(chunk_sectors * new_data_disks) evaluates to 0x08000000 * 32.
This wraps to 0 in 32-bit arithmetic, which is then passed to sector_div():
if (sector_div(here_new, chunk_sectors * new_data_disks)) {
This triggers a divide-by-zero panic. A similar issue occurs in
make_discard_request() where DIV_ROUND_UP_SECTOR_T() is called with a
wrapped stripe_sectors value.
> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
> index cb5feae04db27..5cd9d0f36b6e0 100644
> --- a/drivers/md/raid5.h
> +++ b/drivers/md/raid5.h
[ ... ]
> @@ -572,12 +572,12 @@ struct r5conf {
> /* only protect corresponding hash list and inactive_list */
> spinlock_t hash_locks[NR_STRIPE_HASH_LOCKS];
> struct mddev *mddev;
> - int chunk_sectors;
> + unsigned int chunk_sectors;
> int level, algorithm, rmw_level;
> int max_degraded;
> int raid_disks;
> - int max_nr_stripes;
> - int min_nr_stripes;
> + unsigned int max_nr_stripes;
> + unsigned int min_nr_stripes;
[Severity: Medium]
Does changing min_nr_stripes to unsigned int introduce a spurious cache
flush regression due to type promotion?
In drivers/md/raid5-cache.c:r5c_do_reclaim(), total_cached can evaluate
to -1 if r5c_finish_stripe_write_out() concurrently decrements
r5c_flushing_partial_stripes and r5c_cached_partial_stripes during a
lockless read.
Since min_nr_stripes is now unsigned, the -1 is promoted to UINT_MAX in
this condition:
if (total_cached > conf->min_nr_stripes * 3 / 4 ||
This makes the cache pressure check spuriously evaluate to true, leading
to unnecessary CPU wakeups and a performance regression during writeback
workloads.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132329.7273-1-nishidafmly@gmail.com?part=1
^ permalink raw reply
* Re: LBS setup
From: Piergiorgio Sartor @ 2026-07-10 16:20 UTC (permalink / raw)
To: Piergiorgio Sartor; +Cc: linux-raid
In-Reply-To: <ahLSj1Ua5WJ0GTgY@lazy.lzy>
Hi again,
not sure, but has anybody comments
on the topic?
Thanks,
bye,
pg
On Sun, May 24, 2026 at 12:27:27PM +0200, Piergiorgio Sartor wrote:
> Hi all,
>
> I've some questions about some recent
> report from kernel.
> I assume from 7.x.y, from what I understood.
> Not sure about the <= 6.18 reported.
>
> In the boot log there is:
>
> md0: echo current LBS to md/logical_block_size to prevent data loss issues from LBS changes.
> Note: After setting, array will not be assembled in old kernels (<= 6.18)
>
> I found the docs stating:
>
> logical_block_size
> ... Write the value before starting array. ...
>
> Now, should we really copy the value from:
>
> /sys/class/block/md0/queue/logical_block_size
>
> to:
>
> /sys/class/block/md0/md/logical_block_size
>
> ???
>
> Note, the first in my system is 512, the
> second is 0.
>
> Second, the docs says "before starting array".
> In case this is due (copy the value), how to do
> for a system array?
> I mean, the array where the whole OS lies.
>
> If needed, is there any way to do this automatically?
> Using mdadm or something?
>
> Thanks,
>
> bye,
>
> --
>
> piergiorgio
>
--
piergiorgio
^ permalink raw reply
* Re: LBS setup
From: Piergiorgio Sartor @ 2026-07-10 17:54 UTC (permalink / raw)
To: Wol; +Cc: Piergiorgio Sartor, linux-raid
In-Reply-To: <811c24a5-0c72-4b83-af19-609e42e12613@youngman.org.uk>
Hi Wol
thanks for the answer.
My question is about "what to do?".
Not much what happened.
The log says we should copy some size
from somewhere to somewhere else.
The docs says this should be done
with array at rest.
Is there any documentation about this?
Should we just ignore the log?
What should be done, if any?
The log:
md1: echo current LBS to md/logical_block_size to prevent data loss issues from LBS changes.
Note: After setting, array will not be assembled in old kernels (<= 6.18)
The docs:
logical_block_size
Configure the array’s logical block size in bytes.
This attribute is only supported for 1.x meta.
*Write the value before starting array.* (emphasis mine)
The final array LBS uses the maximum between this
configuration and LBS of all combined devices.
Note that LBS cannot exceed PAGE_SIZE before RAID
supports folio.
WARNING: Arrays created on new kernel cannot be
assembled at old kernel due to padding check,
Set module parameter ‘check_new_feature’ to
false to bypass, but data loss may occur.
The kernel is pretty new, actually 7.1.3,
from Fedora.
Old kernels is for me no problem, it would
be a problem losing the array.
There is no procedure explained, documentation
is, for me, quite dry.
It would be good to have some comments from
whom wrote it (and the code as well).
Thanks,
bye,
pg
On Fri, Jul 10, 2026 at 06:10:47PM +0100, Wol wrote:
> On 10/07/2026 17:20, Piergiorgio Sartor wrote:
> > Hi again,
> >
> > not sure, but has anybody comments
> > on the topic?
>
> I don't understand what you're asking, but I do think I remember the events
> you're talking about.
>
> Basically, some change accidentally slipped into the wild with a kernel,
> probably a 6.x, and people started creating kernels with the new kernel.
>
> Then, as people started upgrading kernels, they suddenly realised that this
> change affected the raid format, such that new kernels couldn't read old
> arrays, and vice versa. More importantly, and more dangerously, it led to
> arrays being trashed!
>
> What to do? They couldn't pull old kernels obviously, there were too many
> arrays out there. They couldn't pull new kernels, there were already some
> arrays out there.
>
> So they rushed out a fix, such that if a new kernel created an array, it
> flagged it as being a new array. If you booted a new kernel with an array
> that didn't have the flag IT WOULDN'T LOAD THE ARRAY. And they trusted to
> luck that people wouldn't downgrade an array and try to load a new array on
> an old kernel.
>
> So this now meant that all of a sudden new kernels wouldn't boot on old
> systems, until the user explicitly set the flag on the array to say "this is
> an old-style array". If they happened to be upgrading from one of the few
> early new-style kernels, they could set the flag to "this is a new-style
> array".
>
> Point is, this code was meant to make it very hard to upgrade your kernel
> without being prompted what sort of array you had, so that the kernel devs
> could try to ensure you didn't lose your array.
>
> A pretty awful way of doing it, but nobody could think of anything better.
>
> Hopefully that explains what's going on there, and you'll be able to work
> out your own answer from this information.
>
> Cheers,
> Wol
--
piergiorgio
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox