* [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 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 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 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 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 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 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 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 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 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 0/2] md: widen size/count types for large arrays
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 small type-widening fixes for large arrays. Neither changes
behaviour or memory use on any current configuration.
1/2 changes chunk_sectors and the stripe-cache count fields to unsigned
int, matching how they are used and avoiding implicit sign
extension in the cache shrinker's subtraction.
2/2 widens the badblocks sector parameters from int to sector_t so the
helpers do not narrow a sector_t argument on very large devices.
These are independent of the other md/raid5 patches I'm sending and can
be applied on their own.
Hiroshi Nishida (2):
md: change chunk_sectors and stripe cache counts to unsigned int
md: widen badblock sectors param from int to sector_t
drivers/md/md.c | 4 ++--
drivers/md/md.h | 10 +++++-----
drivers/md/raid5.c | 14 +++++++-------
drivers/md/raid5.h | 8 ++++----
4 files changed, 18 insertions(+), 18 deletions(-)
base-commit: 55b77337bdd088c77461588e5ec094421b89911b
--
2.43.0
^ permalink raw reply
* Re: [PATCH 0/8] md/raid5: scalability and rebuild-path improvements
From: Hiroshi Nishida @ 2026-07-10 13:09 UTC (permalink / raw)
To: yukuai; +Cc: Song Liu, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <aa24dd05-f231-4657-8fef-8201138da65f@fygo.io>
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
^ permalink raw reply
* Re: [PATCH v4 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths
From: sashiko-bot @ 2026-07-10 10:31 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: yukuai, linux-raid
In-Reply-To: <20260710101521.1714-2-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Allocation of `r10_bio` with `GFP_NOIO` can sleep, violating `REQ_NOWAIT` semantics.
- [High] Functions in the IO path (`wait_blocked_dev()` and `wait_barrier()`) sleep unconditionally, violating `REQ_NOWAIT` semantics.
--
--- Patch [1]: [PATCH v4 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths ---
commit d87114f7731cb53cc142eafbf2338e5b721f84ad
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md/raid10: fix r10bio leak in raid10_write_request() error paths
When raid10_write_request() fails because REQ_NOWAIT is set, the
allocated r10_bio is not freed before returning, resulting in a memory
leak. Free r10_bio before returning from the REQ_NOWAIT error paths.
Closes: https://lore.kernel.org/linux-raid/20260613184042.BCEC01F000E9@smtp.kernel.org/
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0a3cfdd3f5df8..bd322eccdc3ff 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1365,6 +1365,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
> /* Bail out if REQ_NOWAIT is set for the bio */
> if (bio->bi_opf & REQ_NOWAIT) {
> bio_wouldblock_error(bio);
> + free_r10bio(r10_bio);
> return false;
> }
[Severity: High]
This isn't a bug introduced by this patch, but while looking at REQ_NOWAIT
handling, it appears the initial allocation of r10_bio in __make_request()
and raid10_handle_discard() ignores the REQ_NOWAIT flag.
Both functions call:
r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
Since GFP_NOIO includes __GFP_DIRECT_RECLAIM, this can sleep if the mempool
is depleted.
Could this violate the strict non-blocking guarantees of REQ_NOWAIT IO and
potentially starve async workers?
> for (;;) {
[ ... ]
> @@ -1398,6 +1399,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
> if (bio->bi_opf & REQ_NOWAIT) {
> allow_barrier(conf);
> bio_wouldblock_error(bio);
> + free_r10bio(r10_bio);
> return false;
> }
[Severity: High]
This is a pre-existing issue, but further down in the IO path, several
functions seem to sleep unconditionally even for REQ_NOWAIT bios.
For example, wait_blocked_dev() does:
md_wait_for_blocked_rdev(blocked_rdev, mddev);
wait_barrier(conf, false);
goto retry_wait;
Also, in raid10_write_request() and raid10_read_request(), when a bio spans a
chunk boundary and is split, the code unconditionally blocks:
allow_barrier(conf);
bio = bio_submit_split_bioset(bio, max_sectors, &conf->bio_split);
wait_barrier(conf, false);
Could these calls to wait_barrier(conf, false) and md_wait_for_blocked_rdev()
deadlock async submitters that rely on the non-blocking guarantee?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710101521.1714-1-abd.masalkhi@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v4 3/7] md/raid10: consistently fail atomic writes that require splitting
From: sashiko-bot @ 2026-07-10 10:28 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: yukuai, linux-raid
In-Reply-To: <20260710101521.1714-4-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent disk removal during a failed or split write causes a NULL pointer dereference in the error handling path.
--
--- Patch [3]: [PATCH v4 3/7] md/raid10: consistently fail atomic writes that require splitting ---
commit a682adb077534fe2522eacd5e431dab764a60d3f
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md/raid10: consistently fail atomic writes that require splitting
This patch addresses atomic writes in RAID10 by checking if they require
splitting. If an atomic write would be split due to badblocks or other
limits, it now correctly fails the bio with EIO rather than allowing the
implicit EINVAL from bio_submit_split_bioset().
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index bd322eccdc3ff..3480fc7907f01 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
[ ... ]
> @@ -1493,6 +1484,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
> r10_bio->sectors = max_sectors;
>
> if (r10_bio->sectors < bio_sectors(bio)) {
> + if (atomic)
> + goto err_handle;
[Severity: High]
This is a pre-existing issue, but can this jump to the error handling path
trigger a NULL pointer dereference if a disk removal happens concurrently?
If raid10_write_request() initiates an IO and skips a Faulty primary rdev,
it uses the replacement (setting repl_bio and incrementing rrdev->nr_pending).
If md_check_recovery() concurrently calls raid10_remove_disk() for the Faulty
primary, it can promote the replacement to primary and set p->replacement
to NULL.
When this atomic check fails, it jumps to the error unwinding loop which
does:
struct md_rdev *rrdev = conf->mirrors[d].replacement;
...
if (r10_bio->devs[k].repl_bio) {
rdev_dec_pending(rrdev, mddev);
r10_bio->devs[k].repl_bio = NULL;
}
Since replacement is now NULL, rrdev is NULL, and rdev_dec_pending() will
dereference it.
Looking at raid10_end_write_request(), it implements a fallback to handle
concurrent promotion by falling back to conf->mirrors[d].rdev when
replacement is NULL.
Does the error unwinding loop in raid10_write_request() need a similar
fallback?
> +
> allow_barrier(conf);
> bio = bio_submit_split_bioset(bio, r10_bio->sectors,
> &conf->bio_split);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710101521.1714-1-abd.masalkhi@gmail.com?part=3
^ permalink raw reply
* [PATCH v4 7/7] md/raid10: simplify read request error handling
From: Abd-Alrhman Masalkhi @ 2026-07-10 10:15 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260710101521.1714-1-abd.masalkhi@gmail.com>
raid10_read_request() currently handles bio completion, barrier
handling, and r10_bio lifetime management in several different error
paths. This results in duplicated cleanup logic and increases the risk
of introducing bugs in future modifications.
Make raid10_read_request() return a status to its callers, consolidate
the read error paths, and free r10_bio from a single location in the
callers. Since the callers allocate r10_bio, they should also be
responsible for freeing it when the request fails.
This makes the read path follow the same ownership model as the write
path and simplifies the error handling flow.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v4:
- No changes.
- Link to v3: https://lore.kernel.org/linux-raid/20260708101341.473750-8-abd.masalkhi@gmail.com/
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-8-abd.masalkhi@gmail.com/
Changes in v2:
- Fix a compilation error (bi -> bio).
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-8-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 45 +++++++++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index d94c1f28a6f6..01162c483644 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1143,7 +1143,7 @@ static bool regular_request_wait(struct mddev *mddev, struct r10conf *conf,
return true;
}
-static void raid10_read_request(struct mddev *mddev, struct bio *bio,
+static bool raid10_read_request(struct mddev *mddev, struct bio *bio,
struct r10bio *r10_bio)
{
struct r10conf *conf = mddev->private;
@@ -1191,8 +1191,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
if (!regular_request_wait(mddev, conf, bio, r10_bio->sectors)) {
bio_wouldblock_error(bio);
- free_r10bio(r10_bio);
- return;
+ return false;
}
rdev = read_balance(conf, r10_bio, &max_sectors);
@@ -1202,8 +1201,8 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
mdname(mddev), b,
(unsigned long long)r10_bio->sector);
}
- raid_end_bio_io(r10_bio);
- return;
+ bio_io_error(bio);
+ goto err_allow_barrier;
}
if (err_rdev)
pr_err_ratelimited("md/raid10:%s: %pg: redirecting sector %llu to another mirror\n",
@@ -1215,10 +1214,8 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
wait_barrier(conf, false);
- if (!bio) {
- set_bit(R10BIO_Returned, &r10_bio->state);
- goto err_handle;
- }
+ if (!bio)
+ goto err_dec_pending;
r10_bio->master_bio = bio;
r10_bio->sectors = max_sectors;
@@ -1244,10 +1241,16 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
read_bio->bi_private = r10_bio;
mddev_trace_remap(mddev, read_bio, r10_bio->sector);
submit_bio_noacct(read_bio);
- return;
-err_handle:
+
+ return true;
+
+err_dec_pending:
atomic_dec(&rdev->nr_pending);
- raid_end_bio_io(r10_bio);
+
+err_allow_barrier:
+ allow_barrier(conf);
+
+ return false;
}
static void raid10_write_one_disk(struct mddev *mddev, struct r10bio *r10_bio,
@@ -1538,14 +1541,13 @@ static bool __make_request(struct mddev *mddev, struct bio *bio, int sectors)
memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) *
conf->geo.raid_disks);
- ret = true;
if (bio_data_dir(bio) == READ)
- raid10_read_request(mddev, bio, r10_bio);
- else {
+ ret = raid10_read_request(mddev, bio, r10_bio);
+ else
ret = raid10_write_request(mddev, bio, r10_bio);
- if (!ret)
- free_r10bio(r10_bio);
- }
+
+ if (!ret)
+ free_r10bio(r10_bio);
return ret;
}
@@ -1875,6 +1877,7 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
sector_t chunk_mask = (conf->geo.chunk_mask & conf->prev.chunk_mask);
int chunk_sects = chunk_mask + 1;
int sectors = bio_sectors(bio);
+ bool write = bio_data_dir(bio) == WRITE;
if (unlikely(bio->bi_opf & REQ_PREFLUSH)
&& md_flush_request(mddev, bio))
@@ -1898,7 +1901,7 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
sectors = chunk_sects -
(bio->bi_iter.bi_sector &
(chunk_sects - 1));
- if (!__make_request(mddev, bio, sectors))
+ if (!__make_request(mddev, bio, sectors) && write)
md_write_end(mddev);
/* In case raid10d snuck in to freeze_array */
@@ -2866,7 +2869,9 @@ static void handle_read_error(struct mddev *mddev, struct r10bio *r10_bio)
rdev_dec_pending(rdev, mddev);
r10_bio->state = 0;
- raid10_read_request(mddev, r10_bio->master_bio, r10_bio);
+ if (!raid10_read_request(mddev, r10_bio->master_bio, r10_bio))
+ free_r10bio(r10_bio);
+
/*
* allow_barrier after re-submit to ensure no sync io
* can be issued while regular io pending.
--
2.43.0
^ permalink raw reply related
* [PATCH v4 6/7] md/raid10: simplify write request error handling
From: Abd-Alrhman Masalkhi @ 2026-07-10 10:15 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260710101521.1714-1-abd.masalkhi@gmail.com>
raid10_write_request() currently handles bio completion, barrier
handling, and r10_bio lifetime management in several different error
paths. This results in duplicated cleanup logic and increases the risk
of introducing bugs in future modifications.
Move bio_wouldblock_error() handling to the callers of
regular_request_wait(), consolidate the write error paths, and free
r10_bio from a single location in __make_request() when
raid10_write_request() fails.
It remove redundant local copies of r10_bio->sectors and use a single
max_sectors variable throughout the function.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v4:
- No changes.
- Link to v3: https://lore.kernel.org/linux-raid/20260708101341.473750-7-abd.masalkhi@gmail.com/
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-7-abd.masalkhi@gmail.com/
Changes in v2:
- No changes.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-7-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 58 ++++++++++++++++++++++-----------------------
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 57813f249578..d94c1f28a6f6 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1123,18 +1123,16 @@ static bool regular_request_wait(struct mddev *mddev, struct r10conf *conf,
struct bio *bio, sector_t sectors)
{
/* Bail out if REQ_NOWAIT is set for the bio */
- if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT)) {
- bio_wouldblock_error(bio);
+ if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT))
return false;
- }
+
while (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
bio->bi_iter.bi_sector < conf->reshape_progress &&
bio->bi_iter.bi_sector + sectors > conf->reshape_progress) {
allow_barrier(conf);
- if (bio->bi_opf & REQ_NOWAIT) {
- bio_wouldblock_error(bio);
+ if (bio->bi_opf & REQ_NOWAIT)
return false;
- }
+
mddev_add_trace_msg(conf->mddev, "raid10 wait reshape");
wait_event(conf->wait_barrier,
conf->reshape_progress <= bio->bi_iter.bi_sector ||
@@ -1192,6 +1190,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
}
if (!regular_request_wait(mddev, conf, bio, r10_bio->sectors)) {
+ bio_wouldblock_error(bio);
free_r10bio(r10_bio);
return;
}
@@ -1354,8 +1353,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
{
struct r10conf *conf = mddev->private;
int i, k;
- sector_t sectors;
- int max_sectors;
+ int max_sectors = r10_bio->sectors;
+ bool nowait = bio->bi_opf & REQ_NOWAIT;
bool atomic = bio->bi_opf & REQ_ATOMIC;
if ((mddev_is_clustered(mddev) &&
@@ -1363,9 +1362,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
bio->bi_iter.bi_sector,
bio_end_sector(bio)))) {
/* Bail out if REQ_NOWAIT is set for the bio */
- if (bio->bi_opf & REQ_NOWAIT) {
+ if (nowait) {
bio_wouldblock_error(bio);
- free_r10bio(r10_bio);
return false;
}
@@ -1375,28 +1373,25 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
bio_end_sector(bio)));
}
- sectors = r10_bio->sectors;
- if (!regular_request_wait(mddev, conf, bio, sectors)) {
- free_r10bio(r10_bio);
+ if (!regular_request_wait(mddev, conf, bio, max_sectors)) {
+ bio_wouldblock_error(bio);
return false;
}
if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
(mddev->reshape_backwards
? (bio->bi_iter.bi_sector < conf->reshape_safe &&
- bio->bi_iter.bi_sector + sectors > conf->reshape_progress)
- : (bio->bi_iter.bi_sector + sectors > conf->reshape_safe &&
+ bio->bi_iter.bi_sector + max_sectors > conf->reshape_progress)
+ : (bio->bi_iter.bi_sector + max_sectors > conf->reshape_safe &&
bio->bi_iter.bi_sector < conf->reshape_progress))) {
/* Need to update reshape_position in metadata */
mddev->reshape_position = conf->reshape_progress;
set_mask_bits(&mddev->sb_flags, 0,
BIT(MD_SB_CHANGE_DEVS) | BIT(MD_SB_CHANGE_PENDING));
md_wakeup_thread(mddev->thread);
- if (bio->bi_opf & REQ_NOWAIT) {
- allow_barrier(conf);
+ if (nowait) {
bio_wouldblock_error(bio);
- free_r10bio(r10_bio);
- return false;
+ goto err_allow_barrier;
}
mddev_add_trace_msg(conf->mddev,
"raid10 wait reshape metadata");
@@ -1421,8 +1416,6 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
wait_blocked_dev(mddev, r10_bio);
- max_sectors = r10_bio->sectors;
-
for (i = 0; i < conf->copies; i++) {
int d = r10_bio->devs[i].devnum;
struct md_rdev *rdev, *rrdev;
@@ -1479,15 +1472,15 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
r10_bio->sectors = max_sectors;
if (r10_bio->sectors < bio_sectors(bio)) {
- if (atomic)
- goto err_handle;
+ if (atomic) {
+ bio_io_error(bio);
+ goto err_dec_pending;
+ }
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
- if (!bio) {
- set_bit(R10BIO_Returned, &r10_bio->state);
- goto err_handle;
- }
+ if (!bio)
+ goto err_dec_pending;
r10_bio->master_bio = bio;
}
@@ -1505,7 +1498,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
one_write_done(r10_bio);
return true;
-err_handle:
+err_dec_pending:
for (k = 0; k < i; k++) {
int d = r10_bio->devs[k].devnum;
struct md_rdev *rdev = conf->mirrors[d].rdev;
@@ -1521,7 +1514,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
}
}
- raid_end_bio_io(r10_bio);
+err_allow_barrier:
+ allow_barrier(conf);
+
return false;
}
@@ -1546,8 +1541,11 @@ static bool __make_request(struct mddev *mddev, struct bio *bio, int sectors)
ret = true;
if (bio_data_dir(bio) == READ)
raid10_read_request(mddev, bio, r10_bio);
- else
+ else {
ret = raid10_write_request(mddev, bio, r10_bio);
+ if (!ret)
+ free_r10bio(r10_bio);
+ }
return ret;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v4 5/7] md/raid10: replace wait loop with wait_event_idle()
From: Abd-Alrhman Masalkhi @ 2026-07-10 10:15 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260710101521.1714-1-abd.masalkhi@gmail.com>
The wait loop is equivalent to wait_event_idle() and can be simplified
by usaing it for improving readability.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v4:
- No changes.
- Link to v3: https://lore.kernel.org/linux-raid/20260708101341.473750-6-abd.masalkhi@gmail.com/
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-6-abd.masalkhi@gmail.com/
Changes in v2:
- No changes.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-6-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 2574f60dd771..57813f249578 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1362,22 +1362,17 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
mddev->cluster_ops->area_resyncing(mddev, WRITE,
bio->bi_iter.bi_sector,
bio_end_sector(bio)))) {
- DEFINE_WAIT(w);
/* Bail out if REQ_NOWAIT is set for the bio */
if (bio->bi_opf & REQ_NOWAIT) {
bio_wouldblock_error(bio);
free_r10bio(r10_bio);
return false;
}
- for (;;) {
- prepare_to_wait(&conf->wait_barrier,
- &w, TASK_IDLE);
- if (!mddev->cluster_ops->area_resyncing(mddev, WRITE,
- bio->bi_iter.bi_sector, bio_end_sector(bio)))
- break;
- schedule();
- }
- finish_wait(&conf->wait_barrier, &w);
+
+ wait_event_idle(conf->wait_barrier,
+ !mddev->cluster_ops->area_resyncing(mddev, WRITE,
+ bio->bi_iter.bi_sector,
+ bio_end_sector(bio)));
}
sectors = r10_bio->sectors;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 4/7] md/raid10: remove unnecessary barrier around bio_submit_split_bioset()
From: Abd-Alrhman Masalkhi @ 2026-07-10 10:15 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260710101521.1714-1-abd.masalkhi@gmail.com>
raid10_write_request() drops the barrier before calling
bio_submit_split_bioset() and reacquires it afterwards. This is no
longer necessary because the split bio cannot re-enter
raid10_write_request() while the barrier is held.
The allow_barrier()/wait_barrier() pair was introduced by commit
e820d55cb99d ("md: fix raid10 hang issue caused by barrier") when
submit_flushes() called md_handle_request() directly, allowing re-entry
into raid10_write_request(). Since v5.2, submit_flushes() has instead
gone through submit_bio(), eliminating that recursion. submit_flushes()
was later removed entirely by commit b75197e86e6d ("md: Remove flush
handling").
Currently, raid10_write_request() is only entered from the bio
submission path, so the split bio submitted by bio_submit_split_bioset()
cannot recurse back into wait_barrier().
Remove the redundant allow_barrier()/wait_barrier() pair around
bio_submit_split_bioset().
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v4:
- No changes.
- Link to v3: https://lore.kernel.org/linux-raid/20260708101341.473750-5-abd.masalkhi@gmail.com/
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-5-abd.masalkhi@gmail.com/
Changes in v2:
- Expand the commit message to explain why the
allow_barrier()/wait_barrier() pair is no longer needed.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-5-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 3480fc7907f0..2574f60dd771 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1487,10 +1487,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (atomic)
goto err_handle;
- allow_barrier(conf);
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
- wait_barrier(conf, false);
if (!bio) {
set_bit(R10BIO_Returned, &r10_bio->state);
goto err_handle;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 3/7] md/raid10: consistently fail atomic writes that require splitting
From: Abd-Alrhman Masalkhi @ 2026-07-10 10:15 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260710101521.1714-1-abd.masalkhi@gmail.com>
RAID10 currently handles one badblock path explicitly by failing atomic
writes with EIO. However, another badblock path can also reduce the
writable range and force the bio through bio_submit_split_bioset(),
which implicitly completes the bio with EINVAL.
Fix this by handling atomic writes in the common split check. If RAID10
determines that an atomic write would require splitting, complete the
bio with EIO.
Fixes: a1d9b4fd42d9 ("md/raid10: Atomic write support")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v4:
- No changes.
- Link to v3: https://lore.kernel.org/linux-raid/20260708101341.473750-4-abd.masalkhi@gmail.com/
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-4-abd.masalkhi@gmail.com/
Changes in v2:
- Drop the early atomic write split check from raid10_write_request()
and rely on queue limits instead.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-4-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index bd322eccdc3f..3480fc7907f0 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1356,6 +1356,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
int i, k;
sector_t sectors;
int max_sectors;
+ bool atomic = bio->bi_opf & REQ_ATOMIC;
if ((mddev_is_clustered(mddev) &&
mddev->cluster_ops->area_resyncing(mddev, WRITE,
@@ -1464,16 +1465,6 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (is_bad) {
int good_sectors;
- /*
- * We cannot atomically write this, so just
- * error in that case. It could be possible to
- * atomically write other mirrors, but the
- * complexity of supporting that is not worth
- * the benefit.
- */
- if (bio->bi_opf & REQ_ATOMIC)
- goto err_handle;
-
good_sectors = first_bad - dev_sector;
if (good_sectors < max_sectors)
max_sectors = good_sectors;
@@ -1493,6 +1484,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
r10_bio->sectors = max_sectors;
if (r10_bio->sectors < bio_sectors(bio)) {
+ if (atomic)
+ goto err_handle;
+
allow_barrier(conf);
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
--
2.43.0
^ permalink raw reply related
* [PATCH v4 2/7] md/raid1: restrict atomic write limits and handle runtime constraints
From: Abd-Alrhman Masalkhi @ 2026-07-10 10:15 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260710101521.1714-1-abd.masalkhi@gmail.com>
Restrict the RAID1 atomic write limits by setting chunk_sectors to
BARRIER_UNIT_SECTOR_SIZE so that atomic writes never straddle a barrier
unit.
A bio that passes block-layer validation may still become unserviceable
within RAID1 due to bad blocks or write-behind constraints. In the former
case, complete the bio with EIO. In the latter case, disable
write-behind rather than failing the bio with EIO.
Fixes: f2a38abf5f1c ("md/raid1: Atomic write support")
Fixes: a4c55c902670 ("md/raid1: simplify raid1_write_request() error handling")
Reviewed-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v4:
- Improve the commit message.
- Add Reviewed-by tag from John Garry.
- Link to v3: https://lore.kernel.org/linux-raid/20260708101341.473750-3-abd.masalkhi@gmail.com/
Changes in v3:
- Set chunk_sectors to BARRIER_UNIT_SECTOR_SIZE instead of setting
atomic_write_hw_unit_max.
- Avoid enabling write-behind when the atomic write exceeds the
write-behind limit.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-3-abd.masalkhi@gmail.com/
Changes in v2:
- Drop the early atomic write split check from raid1_write_request().
- Advertise the atomic write size limit via queue limits.
- Disable write-behind instead of failing atomic writes when the
BIO_MAX_VECS limit is encountered.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-3-abd.masalkhi@gmail.com/
---
drivers/md/raid1.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index afe2ca96ad8c..6c8beca995e6 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1522,6 +1522,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
int first_clone;
bool write_behind = false;
bool nowait = bio->bi_opf & REQ_NOWAIT;
+ bool atomic = bio->bi_opf & REQ_ATOMIC;
bool is_discard = op_is_discard(bio->bi_opf);
sector_t sector = bio->bi_iter.bi_sector;
@@ -1577,7 +1578,8 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
* write-mostly, which means we could allocate write behind
* bio later.
*/
- if (!is_discard && rdev && test_bit(WriteMostly, &rdev->flags))
+ if (!is_discard && rdev && test_bit(WriteMostly, &rdev->flags) &&
+ (!atomic || max_sectors <= BIO_MAX_VECS * (PAGE_SIZE >> 9)))
write_behind = true;
r1_bio->bios[i] = NULL;
@@ -1603,20 +1605,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
}
if (is_bad) {
int good_sectors;
-
- /*
- * We cannot atomically write this, so just
- * error in that case. It could be possible to
- * atomically write other mirrors, but the
- * complexity of supporting that is not worth
- * the benefit.
- */
- if (bio->bi_opf & REQ_ATOMIC) {
- bio->bi_status = BLK_STS_NOTSUPP;
- bio_endio(bio);
- goto err_dec_pending;
- }
-
good_sectors = first_bad - sector;
if (good_sectors < max_sectors)
max_sectors = good_sectors;
@@ -1636,7 +1624,13 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
if (write_behind && mddev->bitmap)
max_sectors = min_t(int, max_sectors,
BIO_MAX_VECS * (PAGE_SIZE >> 9));
+
if (max_sectors < bio_sectors(bio)) {
+ if (atomic) {
+ bio_io_error(bio);
+ goto err_dec_pending;
+ }
+
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
if (!bio)
@@ -3228,6 +3222,7 @@ static int raid1_set_limits(struct mddev *mddev)
md_init_stacking_limits(&lim);
lim.max_write_zeroes_sectors = 0;
lim.max_hw_wzeroes_unmap_sectors = 0;
+ lim.chunk_sectors = BARRIER_UNIT_SECTOR_SIZE;
lim.logical_block_size = mddev->logical_block_size;
lim.features |= BLK_FEAT_ATOMIC_WRITES;
lim.features |= BLK_FEAT_PCI_P2PDMA;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths
From: Abd-Alrhman Masalkhi @ 2026-07-10 10:15 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid, sashiko-bot
In-Reply-To: <20260710101521.1714-1-abd.masalkhi@gmail.com>
When raid10_write_request() fails because REQ_NOWAIT is set, the
allocated r10_bio is not freed before returning, resulting in a memory
leak. Free r10_bio before returning from the REQ_NOWAIT error paths.
Fixes: c9aa889b035f ("md: raid10 add nowait support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-raid/20260613184042.BCEC01F000E9@smtp.kernel.org/
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v4:
- No changes.
- Link to v3: https://lore.kernel.org/linux-raid/20260708101341.473750-2-abd.masalkhi@gmail.com/
Changes in v3:
- No changes.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-2-abd.masalkhi@gmail.com/
Changes in v2:
- No changes.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-2-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0a3cfdd3f5df..bd322eccdc3f 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1365,6 +1365,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
/* Bail out if REQ_NOWAIT is set for the bio */
if (bio->bi_opf & REQ_NOWAIT) {
bio_wouldblock_error(bio);
+ free_r10bio(r10_bio);
return false;
}
for (;;) {
@@ -1398,6 +1399,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (bio->bi_opf & REQ_NOWAIT) {
allow_barrier(conf);
bio_wouldblock_error(bio);
+ free_r10bio(r10_bio);
return false;
}
mddev_add_trace_msg(conf->mddev,
--
2.43.0
^ permalink raw reply related
* [PATCH v4 0/7] md/raid10: fixes, atomic write handling, and error-path cleanup
From: Abd-Alrhman Masalkhi @ 2026-07-10 10:15 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
Hi,
This v4 of series contains a mix of bug fixes and cleanups for RAID10,
along with a related atomic write fix for RAID1.
Changes in v4:
- Improve the commit message of patch 2.
- Add Reviewed-by tag from John Garry to patch 2.
- Link to v3: https://lore.kernel.org/linux-raid/20260708101341.473750-1-abd.masalkhi@gmail.com/
Changes in v3:
- Set chunk_sectors to BARRIER_UNIT_SECTOR_SIZE instead of setting
atomic_write_hw_unit_max.
- Avoid enabling write-behind when the atomic write exceeds the
write-behind limit.
- Link to v2: https://lore.kernel.org/linux-raid/20260628142420.1051027-1-abd.masalkhi@gmail.com/
Changes in v2:
- Expand the commit message to explain why the
allow_barrier()/wait_barrier() pair is no longer needed.
- Drop the early atomic write split check from raid1_write_request().
- Advertise the atomic write size limit via queue limits.
- Disable write-behind instead of failing atomic writes when the
BIO_MAX_VECS limit is encountered.
- Drop the early atomic write split check from raid10_write_request()
and rely on queue limits instead.
- Fix a compilation error (bi -> bio).
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-1-abd.masalkhi@gmail.com/
Thanks,
Abd-alrhman,
Abd-Alrhman Masalkhi (7):
md/raid10: fix r10bio leak in raid10_write_request() error paths
md/raid1: restrict atomic write limits and handle runtime constraints
md/raid10: consistently fail atomic writes that require splitting
md/raid10: remove unnecessary barrier around bio_submit_split_bioset()
md/raid10: replace wait loop with wait_event_idle()
md/raid10: simplify write request error handling
md/raid10: simplify read request error handling
drivers/md/raid1.c | 25 ++++------
drivers/md/raid10.c | 118 +++++++++++++++++++++-----------------------
2 files changed, 65 insertions(+), 78 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v3 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: Abd-Alrhman Masalkhi @ 2026-07-10 9:42 UTC (permalink / raw)
To: John Garry, song, yukuai, magiclinan, xiao, axboe, vverma,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <350a2662-7de0-45e6-a905-76dbd0a99e98@oracle.com>
On Thu, Jul 09, 2026 at 13:11 +0100, John Garry wrote:
> On 08/07/2026 11:13, Abd-Alrhman Masalkhi wrote:
>> Atomic writes in RAID1 must fit within a single barrier unit. Set
>> chunk_sectors to BARRIER_UNIT_SECTOR_SIZE so that bios which would cross
>> a barrier-unit boundary are rejected by the block layer before reaching
>> MD.
>
> More accurate would be to say that we restrict the atomic write limits
> so that we never produce a write which straddles BARRIER_UNIT_SECTOR_SIZE.
>
Yes, that is more accurate. I'll update the commit message accordingly.
Thanks for the suggestion.
>>
>> A bio that passes block-layer validation may still become unserviceable
>> within RAID1 due to bad blocks or write-behind constraints. In the former
>> case, complete the bio with EIO. In the latter case, disable
>> write-behind rather than failing the bio with EIO.
>>
>> Fixes: f2a38abf5f1c ("md/raid1: Atomic write support")
>> Fixes: a4c55c902670 ("md/raid1: simplify raid1_write_request() error handling")
>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>
> Reviewed-by: John Garry <john.g.garry@oracle.com>
>
>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH v2] md: recheck spare changes before starting sync
From: yu kuai @ 2026-07-10 7:50 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, magiclinan, xiao, abd.masalkhi
Cc: linux-raid, linux-kernel, yu kuai
In-Reply-To: <20260708112003.474537-1-abd.masalkhi@gmail.com>
在 2026/7/8 19:20, Abd-Alrhman Masalkhi 写道:
> remove_spares() and remove_and_add_spares() modify the array's rdev
> configuration. These operations are only safe after the array has been
> suspended.
>
> md_start_sync() checks whether spare configuration changes are needed
> before taking reconfig_mutex. However, the rdev state can change before
> the mutex is acquired, so the initial check can become stale. In that
> case, md_choose_sync_action() may remove or replace rdevs while normal
> I/O is still accessing them.
>
> The race can occur as follows:
>
> raid10d Worker Normal IO
> ____________ _______________________ ______________________
>
> raid10_write_request()
> wait_blocked_dev()
> set Blocked
> set Faulty
> Skip Faulty rdev
> rrdev->nr_pending++
> .repl_bio = bio
> removeable_rdev = false .
> array not suspended .
> lock mddev goto err_handle
> lock mddev (wait)
> .
> update sb .
> clear Blocked .
> .
> unlock mddev .
> lock mddev (acquires)
> remove_spares()
> removeable_rdev = true
>
> raid10_remove_disk()
> rdev = replacement
> replacement = NULL
> rdev_dec_pending(NULL)
> unlock mddev (NULL)->nr_pending--
>
> In this case, rdev_dec_pending() is called with a NULL pointer,
> resulting in a NULL pointer dereference when attempting to decrement
> nr_pending.
>
> Fix this by suspending the array when spare configuration changes are
> needed, including for non-read-write arrays, and checking again after
> taking reconfig_mutex. If the array was not already suspended and a
> change is now needed, release the mutex, suspend the array, and
> reacquire the mutex before continuing.
>
> Fixes: bc08041b32ab ("md: suspend array in md_start_sync() if array need reconfiguration")
> Reported-by: sashiko-bot<sashiko-bot@kernel.org>
> Closes:https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3
> Signed-off-by: Abd-Alrhman Masalkhi<abd.masalkhi@gmail.com>
> ---
> Changes in v2:
> - Recheck whether spare configuration changes are needed after taking
> reconfig_mutex, and suspend the array before continuing if necessary.
> - Account for read only arrays, where reshape_position may not be
> MaxSector even though md_start_sync() can still modify the spare
> configuration.
> - Link to v1:https://lore.kernel.org/linux-raid/20260630075640.1081634-1-abd.masalkhi@gmail.com/
> ---
> drivers/md/md.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: validate journal checksum slots during recovery
From: yu kuai @ 2026-07-10 7:37 UTC (permalink / raw)
To: Guangshuo Li, Song Liu, Li Nan, Xiao Ni, Junrui Luo, linux-raid,
linux-kernel, yu kuai
In-Reply-To: <20260708133534.770770-1-lgs201920130244@gmail.com>
Hi,
在 2026/7/8 21:35, Guangshuo Li 写道:
> The change referenced by the Fixes tag added payload length validation
> before accessing journal metadata during raid5-cache recovery.
>
> However, the DATA and PARITY payload length is computed from the on-disk
> size field without ensuring that the checksum slots read later are
> actually present. struct r5l_payload_data_parity ends with a flexible
> checksum array, so sizeof(struct r5l_payload_data_parity) does not cover
> any checksum entries.
>
> A corrupted journal can set a DATA payload size smaller than one page.
> The computed checksum count then becomes zero and the payload length only
> covers the fixed header, but recovery still reads checksum[0]. For RAID6
> PARITY payloads, recovery also reads checksum[1], so the payload must
> cover two checksum entries.
>
> Make the validated DATA and PARITY payload length include the checksum
> entries that recovery may read. Also make sure fixed payload headers are
> present before reading their fields.
>
> Fixes: b0cc3ae97e89 ("md/raid5: validate payload size before accessing journal metadata")
Does the problem this patch fixes introduced by above fix tag? Or it's just a
new validation, if so please replace with a real fix tag.
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> drivers/md/raid5-cache.c | 108 +++++++++++++++++++++++++++++----------
> 1 file changed, 82 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> index 7b7546bfa21f..4aef692182b4 100644
> --- a/drivers/md/raid5-cache.c
> +++ b/drivers/md/raid5-cache.c
> @@ -1980,6 +1980,29 @@ r5l_recovery_verify_data_checksum(struct r5l_log *log,
> return (le32_to_cpu(log_checksum) == checksum) ? 0 : -EINVAL;
> }
>
> +static sector_t r5l_recovery_payload_data_parity_len(struct r5conf *conf,
> + const struct r5l_payload_data_parity *payload, bool parity)
> +{
> + unsigned int nr_csum;
> + unsigned int min_csum = 1;
> +
> + /*
> + * The payload size determines how many checksum entries are stored,
> + * but recovery always reads checksum[0]. For RAID6 parity payloads
> + * it also reads checksum[1] for Q. Make the validated payload length
> + * cover every checksum entry that will be read below.
> + */
> + nr_csum = le32_to_cpu(payload->size) >> (PAGE_SHIFT - 9);
> +
> + if (parity && conf->max_degraded == 2)
> + min_csum = 2;
> + if (nr_csum < min_csum)
> + nr_csum = min_csum;
> +
> + return sizeof(*payload) + (sector_t)sizeof(__le32) * nr_csum;
Please use struct_size()
> +}
> +
> +
Two blank line here. Please run checkpatch before submission.
> /*
> * before loading data to stripe cache, we need verify checksum for all data,
> * if there is mismatch for any data page, we drop all data in the mata block
> @@ -1992,6 +2015,7 @@ r5l_recovery_verify_data_checksum_for_mb(struct r5l_log *log,
> struct r5conf *conf = mddev->private;
> struct r5l_meta_block *mb = page_address(ctx->meta_page);
> sector_t mb_offset = sizeof(struct r5l_meta_block);
> + sector_t meta_size = le32_to_cpu(mb->meta_size);
> sector_t log_offset = r5l_ring_add(log, ctx->pos, BLOCK_SECTORS);
> struct page *page;
> struct r5l_payload_data_parity *payload;
> @@ -2001,28 +2025,42 @@ r5l_recovery_verify_data_checksum_for_mb(struct r5l_log *log,
> if (!page)
> return -ENOMEM;
>
> - while (mb_offset < le32_to_cpu(mb->meta_size)) {
> + while (mb_offset < meta_size) {
> sector_t payload_len;
> + u16 type;
>
> payload = (void *)mb + mb_offset;
> payload_flush = (void *)mb + mb_offset;
>
> - if (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_DATA) {
> - payload_len = sizeof(struct r5l_payload_data_parity) +
> - (sector_t)sizeof(__le32) *
> - (le32_to_cpu(payload->size) >> (PAGE_SHIFT - 9));
> - if (mb_offset + payload_len > le32_to_cpu(mb->meta_size))
> + if (mb_offset + sizeof(payload->header) > meta_size)
> + goto mismatch;
> +
> + type = le16_to_cpu(payload->header.type);
> +
> + if (type == R5LOG_PAYLOAD_DATA) {
> + if (mb_offset + sizeof(*payload) > meta_size)
> goto mismatch;
> +
> + payload_len = r5l_recovery_payload_data_parity_len(conf,
> + payload,
> + false);
> + if (payload_len > meta_size - mb_offset)
> + goto mismatch;
> +
> if (r5l_recovery_verify_data_checksum(
> log, ctx, page, log_offset,
> payload->checksum[0]) < 0)
> goto mismatch;
> - } else if (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_PARITY) {
> - payload_len = sizeof(struct r5l_payload_data_parity) +
> - (sector_t)sizeof(__le32) *
> - (le32_to_cpu(payload->size) >> (PAGE_SHIFT - 9));
> - if (mb_offset + payload_len > le32_to_cpu(mb->meta_size))
> + } else if (type == R5LOG_PAYLOAD_PARITY) {
> + if (mb_offset + sizeof(*payload) > meta_size)
> goto mismatch;
> +
> + payload_len = r5l_recovery_payload_data_parity_len(conf,
> + payload,
> + true);
> + if (payload_len > meta_size - mb_offset)
> + goto mismatch;
> +
> if (r5l_recovery_verify_data_checksum(
> log, ctx, page, log_offset,
> payload->checksum[0]) < 0)
> @@ -2034,15 +2072,18 @@ r5l_recovery_verify_data_checksum_for_mb(struct r5l_log *log,
> BLOCK_SECTORS),
> payload->checksum[1]) < 0)
> goto mismatch;
> - } else if (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_FLUSH) {
> + } else if (type == R5LOG_PAYLOAD_FLUSH) {
> + if (mb_offset + sizeof(*payload_flush) > meta_size)
> + goto mismatch;
> +
> payload_len = sizeof(struct r5l_payload_flush) +
> (sector_t)le32_to_cpu(payload_flush->size);
> - if (mb_offset + payload_len > le32_to_cpu(mb->meta_size))
> + if (payload_len > meta_size - mb_offset)
> goto mismatch;
> } else /* not R5LOG_PAYLOAD_DATA/PARITY/FLUSH */
> goto mismatch;
>
> - if (le16_to_cpu(payload->header.type) != R5LOG_PAYLOAD_FLUSH) {
> + if (type != R5LOG_PAYLOAD_FLUSH) {
> log_offset = r5l_ring_add(log, log_offset,
> le32_to_cpu(payload->size));
> }
> @@ -2075,7 +2116,8 @@ r5c_recovery_analyze_meta_block(struct r5l_log *log,
> struct r5l_meta_block *mb;
> struct r5l_payload_data_parity *payload;
> struct r5l_payload_flush *payload_flush;
> - int mb_offset;
> + sector_t mb_offset;
> + sector_t meta_size;
> sector_t log_offset;
> sector_t stripe_sect;
> struct stripe_head *sh;
> @@ -2094,22 +2136,31 @@ r5c_recovery_analyze_meta_block(struct r5l_log *log,
>
> mb = page_address(ctx->meta_page);
> mb_offset = sizeof(struct r5l_meta_block);
> + meta_size = le32_to_cpu(mb->meta_size);
> log_offset = r5l_ring_add(log, ctx->pos, BLOCK_SECTORS);
>
> - while (mb_offset < le32_to_cpu(mb->meta_size)) {
> + while (mb_offset < meta_size) {
> sector_t payload_len;
> + u16 type;
> int dd;
>
> payload = (void *)mb + mb_offset;
> payload_flush = (void *)mb + mb_offset;
>
> - if (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_FLUSH) {
> + if (mb_offset + sizeof(payload->header) > meta_size)
> + return -EINVAL;
> +
> + type = le16_to_cpu(payload->header.type);
> +
> + if (type == R5LOG_PAYLOAD_FLUSH) {
> int i, count;
>
> + if (mb_offset + sizeof(*payload_flush) > meta_size)
> + return -EINVAL;
> +
> payload_len = sizeof(struct r5l_payload_flush) +
> (sector_t)le32_to_cpu(payload_flush->size);
> - if (mb_offset + payload_len >
> - le32_to_cpu(mb->meta_size))
> + if (payload_len > meta_size - mb_offset)
> return -EINVAL;
>
> count = le32_to_cpu(payload_flush->size) / sizeof(__le64);
> @@ -2130,13 +2181,18 @@ r5c_recovery_analyze_meta_block(struct r5l_log *log,
> }
>
> /* DATA or PARITY payload */
> - payload_len = sizeof(struct r5l_payload_data_parity) +
> - (sector_t)sizeof(__le32) *
> - (le32_to_cpu(payload->size) >> (PAGE_SHIFT - 9));
> - if (mb_offset + payload_len > le32_to_cpu(mb->meta_size))
> + if (type != R5LOG_PAYLOAD_DATA && type != R5LOG_PAYLOAD_PARITY)
> + return -EINVAL;
> +
> + if (mb_offset + sizeof(*payload) > meta_size)
> + return -EINVAL;
> +
> + payload_len = r5l_recovery_payload_data_parity_len(conf, payload,
> + type == R5LOG_PAYLOAD_PARITY);
> + if (payload_len > meta_size - mb_offset)
> return -EINVAL;
>
> - stripe_sect = (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_DATA) ?
> + stripe_sect = (type == R5LOG_PAYLOAD_DATA) ?
> raid5_compute_sector(
> conf, le64_to_cpu(payload->location), 0, &dd,
> NULL)
> @@ -2183,7 +2239,7 @@ r5c_recovery_analyze_meta_block(struct r5l_log *log,
> list_add_tail(&sh->lru, cached_stripe_list);
> }
>
> - if (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_DATA) {
> + if (type == R5LOG_PAYLOAD_DATA) {
> if (!test_bit(STRIPE_R5C_CACHING, &sh->state) &&
> test_bit(R5_Wantwrite, &sh->dev[sh->pd_idx].flags)) {
> r5l_recovery_replay_one_stripe(conf, sh, ctx);
> @@ -2191,7 +2247,7 @@ r5c_recovery_analyze_meta_block(struct r5l_log *log,
> }
> r5l_recovery_load_data(log, sh, ctx, payload,
> log_offset);
> - } else if (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_PARITY)
> + } else if (type == R5LOG_PAYLOAD_PARITY)
> r5l_recovery_load_parity(log, sh, ctx, payload,
> log_offset);
> else
--
Thanks,
Kuai
^ 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