* [PATCH 4/6] md/raid5: make the stripe batch size a module parameter
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132346.7295-1-nishidafmly@gmail.com>
handle_active_stripes() dequeues up to MAX_STRIPE_BATCH (8) stripes from a
worker group under a single device_lock acquisition, and
raid5_wakeup_stripe_thread() divides group->stripes_cnt by the same value
to decide how many additional workers to wake. The two uses must move
together: the dequeue cap bounds how many stripes one worker drains, while
the spawn divisor sizes the worker fan-out to match.
Make the batch size a per-array value chosen by a new max_stripe_batch
module parameter (1-32, default 8, so behaviour is unchanged out of the
box). Both the dequeue cap and the spawn divisor read the single
conf->max_stripe_batch, so they cannot drift apart. The on-stack batch[]
array in handle_active_stripes() is sized to the STRIPE_BATCH_MAX (32)
upper bound; only conf->max_stripe_batch entries are ever used.
The value is resolved when an array is created. On busy multi-threaded
arrays a larger batch amortises the device_lock over more stripes at some
latency cost; unlike the hash-lock count and the cache-size limit there is
no general hardware signal for the best batch size, so the default is left
at the historical 8 and the value is simply exposed for tuning.
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 15 ++++++++++++---
drivers/md/raid5.h | 12 +++++++++++-
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index e41d3fc92dd0..5f0825c5effe 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -78,6 +78,11 @@ module_param(stripe_cache_size_max, uint, 0644);
MODULE_PARM_DESC(stripe_cache_size_max,
"Maximum the per-array stripe_cache_size may be raised to. 0 (the default) derives the limit from system memory (never below the historical 32768), so large-memory hosts can grow the stripe cache without a recompile while small ones are not offered a limit above what RAM can back. A non-zero value sets a fixed limit");
+static unsigned int max_stripe_batch = STRIPE_BATCH_DEFAULT;
+module_param(max_stripe_batch, uint, 0644);
+MODULE_PARM_DESC(max_stripe_batch,
+ "Number of stripes a worker thread handles per device_lock acquisition, 1-32 (default 8). Larger values amortise the lock over more stripes on busy multi-threaded arrays at some latency cost. Read when an array is created");
+
static bool devices_handle_discard_safely = false;
module_param(devices_handle_discard_safely, bool, 0644);
MODULE_PARM_DESC(devices_handle_discard_safely,
@@ -221,7 +226,7 @@ static void raid5_wakeup_stripe_thread(struct stripe_head *sh)
/* at least one worker should run to avoid race */
queue_work_on(sh->cpu, raid5_wq, &group->workers[0].work);
- thread_cnt = group->stripes_cnt / MAX_STRIPE_BATCH - 1;
+ thread_cnt = group->stripes_cnt / conf->max_stripe_batch - 1;
/* wakeup more workers */
for (i = 1; i < conf->worker_cnt_per_group && thread_cnt > 0; i++) {
if (group->workers[i].working == false) {
@@ -6734,11 +6739,11 @@ static int handle_active_stripes(struct r5conf *conf, int group,
struct list_head *temp_inactive_list)
__must_hold(&conf->device_lock)
{
- struct stripe_head *batch[MAX_STRIPE_BATCH], *sh;
+ struct stripe_head *batch[STRIPE_BATCH_MAX], *sh;
int i, batch_size = 0, hash;
bool release_inactive = false;
- while (batch_size < MAX_STRIPE_BATCH &&
+ while (batch_size < conf->max_stripe_batch &&
(sh = __get_priority_stripe(conf, group)) != NULL)
batch[batch_size++] = sh;
@@ -7640,6 +7645,10 @@ static struct r5conf *setup_conf(struct mddev *mddev)
!conf->temp_inactive_list)
goto abort;
+ /* Resolve the stripe batch size (see STRIPE_BATCH_* in raid5.h). */
+ conf->max_stripe_batch = clamp_t(int, max_stripe_batch,
+ 1, STRIPE_BATCH_MAX);
+
#if PAGE_SIZE != DEFAULT_STRIPE_SIZE
conf->stripe_size = DEFAULT_STRIPE_SIZE;
conf->stripe_shift = ilog2(DEFAULT_STRIPE_SIZE) - 9;
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 10c45fa22955..69a41c1310af 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -490,7 +490,16 @@ struct disk_info {
#define BYPASS_THRESHOLD 1
#define NR_HASH (PAGE_SIZE / sizeof(struct hlist_head))
#define HASH_MASK (NR_HASH - 1)
-#define MAX_STRIPE_BATCH 8
+/*
+ * Stripe batch size: how many stripes a worker dequeues from its group per
+ * device_lock acquisition in handle_active_stripes(); the same value divides
+ * group->stripes_cnt in raid5_wakeup_stripe_thread() to choose how many extra
+ * workers to spawn, so the two stay coupled. Selected per array by the
+ * max_stripe_batch module parameter: STRIPE_BATCH_DEFAULT is the historical
+ * value and STRIPE_BATCH_MAX bounds the on-stack batch[] array.
+ */
+#define STRIPE_BATCH_DEFAULT 8
+#define STRIPE_BATCH_MAX 32
/*
* The stripe cache hash is striped across a power-of-two number of spinlocks,
@@ -690,6 +699,7 @@ struct r5conf {
struct r5worker_group *worker_groups;
int group_cnt;
int worker_cnt_per_group;
+ int max_stripe_batch; /* stripes/dequeue, 1..STRIPE_BATCH_MAX */
struct r5l_log *log;
void *log_private;
--
2.43.0
^ permalink raw reply related
* [PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132346.7295-1-nishidafmly@gmail.com>
raid5_set_cache_size() caps how far the per-array stripe_cache_size sysfs
knob may be raised at a fixed 32768 stripes. The stripe cache costs
roughly max_nr_stripes * (sizeof(stripe_head) + pool_size * (sizeof(bio) +
PAGE_SIZE)), so 32768 stripes is about 1.5GB on a 12-disk array. That
fixed limit is wrong at both ends: a host with hundreds of GB of RAM
backing a wide array cannot grow the cache past ~1.5GB even though it has
the memory to spare, while on a small box 32768 stripes can already exceed
total RAM.
Derive the limit from memory instead. When the new stripe_cache_size_max
module parameter is 0 (the default), the ceiling is the stripe count that
fits in at most 1/8 of RAM for this array's width, but never less than the
historical 32768 -- so the limit only ever grows relative to today and
small systems are unchanged. A non-zero stripe_cache_size_max pins a fixed
ceiling for administrators who want one.
The default stripe count (256), and thus the default memory footprint, is
unchanged; this only changes how high an administrator may raise
stripe_cache_size.
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 37 ++++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 7f72981121fd..e41d3fc92dd0 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -44,6 +44,7 @@
#include <linux/seq_file.h>
#include <linux/cpu.h>
#include <linux/slab.h>
+#include <linux/mm.h>
#include <linux/ratelimit.h>
#include <linux/nodemask.h>
@@ -72,6 +73,11 @@ module_param(nr_stripe_hash_locks, uint, 0644);
MODULE_PARM_DESC(nr_stripe_hash_locks,
"Number of spinlocks the stripe cache hash is striped across, rounded up to a power of two and capped at 32. 0 (the default) auto-sizes it from the online CPU count (never below 8); a non-zero value overrides that. Larger values reduce lock contention on many-core systems at a small per-array memory cost. Read when an array is created");
+static unsigned int stripe_cache_size_max;
+module_param(stripe_cache_size_max, uint, 0644);
+MODULE_PARM_DESC(stripe_cache_size_max,
+ "Maximum the per-array stripe_cache_size may be raised to. 0 (the default) derives the limit from system memory (never below the historical 32768), so large-memory hosts can grow the stripe cache without a recompile while small ones are not offered a limit above what RAM can back. A non-zero value sets a fixed limit");
+
static bool devices_handle_discard_safely = false;
module_param(devices_handle_discard_safely, bool, 0644);
MODULE_PARM_DESC(devices_handle_discard_safely,
@@ -6922,13 +6928,42 @@ raid5_show_stripe_cache_size(struct mddev *mddev, char *page)
return ret;
}
+/*
+ * Upper bound that the per-array stripe_cache_size may be raised to. The
+ * stripe cache costs roughly max_nr_stripes * (sizeof(stripe_head) +
+ * pool_size * (sizeof(bio) + PAGE_SIZE)). The limit was historically a fixed
+ * 32768 stripes, which both under-serves large-memory hosts backing wide
+ * arrays and, on a small box, still permits a cache larger than RAM. Derive
+ * it from memory instead -- at most 1/8 of RAM -- but never below the
+ * historical 32768, so the limit only ever grows relative to today. A
+ * non-zero stripe_cache_size_max module parameter overrides the heuristic.
+ */
+#define RAID5_CACHE_SIZE_FLOOR 32768
+#define RAID5_CACHE_SIZE_RAM_SHIFT 3 /* cap the cache at 1/8 of RAM */
+
+static unsigned long raid5_max_cache_size(struct r5conf *conf)
+{
+ unsigned long per_stripe, limit;
+
+ if (stripe_cache_size_max) {
+ limit = stripe_cache_size_max;
+ } else {
+ per_stripe = sizeof(struct stripe_head) +
+ conf->pool_size * (sizeof(struct bio) + PAGE_SIZE);
+ limit = ((totalram_pages() << PAGE_SHIFT) >>
+ RAID5_CACHE_SIZE_RAM_SHIFT) / per_stripe;
+ limit = max_t(unsigned long, limit, RAID5_CACHE_SIZE_FLOOR);
+ }
+ return min_t(unsigned long, limit, INT_MAX);
+}
+
int
raid5_set_cache_size(struct mddev *mddev, int size)
{
int result = 0;
struct r5conf *conf = mddev->private;
- if (size <= 16 || size > 32768)
+ if (size <= 16 || size > raid5_max_cache_size(conf))
return -EINVAL;
WRITE_ONCE(conf->min_nr_stripes, size);
--
2.43.0
^ permalink raw reply related
* [PATCH 5/6] md/raid5: scale the default stripe cache size with system memory
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132346.7295-1-nishidafmly@gmail.com>
setup_conf() starts every array with min_nr_stripes = NR_STRIPES (256),
about 12MB of stripe cache on a 12-disk array. That fixed default was
chosen for small systems and is never revisited: a server with hundreds
of GB of RAM backing a wide array still creates its arrays with the same
256 stripes, and only benefits from more after an administrator writes
stripe_cache_size by hand.
Auto-size the initial count from memory when the new
stripe_cache_size_default module parameter is 0 (the default): keep the
historical NR_STRIPES up to RAID5_CACHE_DEFAULT_BASE_GB (8GB) of RAM, then
grow the count using about 1/512 of the RAM above that baseline, capped at
RAID5_CACHE_DEFAULT_MAX (4096). A system with 8GB or less is therefore
unchanged -- same count, same footprint -- while larger ones scale up
smoothly (no jump at the baseline) to a bounded maximum. A non-zero
stripe_cache_size_default sets a fixed initial size for administrators who
want one; existing arrays are unaffected, and a reshape still forces at
least its window's worth of stripes.
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 44 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 5f0825c5effe..8e1c2eba4241 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -83,6 +83,11 @@ module_param(max_stripe_batch, uint, 0644);
MODULE_PARM_DESC(max_stripe_batch,
"Number of stripes a worker thread handles per device_lock acquisition, 1-32 (default 8). Larger values amortise the lock over more stripes on busy multi-threaded arrays at some latency cost. Read when an array is created");
+static unsigned int stripe_cache_size_default;
+module_param(stripe_cache_size_default, uint, 0644);
+MODULE_PARM_DESC(stripe_cache_size_default,
+ "Initial stripe_cache_size for newly created arrays. 0 (the default) auto-sizes it from system memory: the historical 256 on small hosts, scaling up with RAM to a capped maximum on larger ones. A non-zero value sets a fixed initial size. Existing arrays are unaffected");
+
static bool devices_handle_discard_safely = false;
module_param(devices_handle_discard_safely, bool, 0644);
MODULE_PARM_DESC(devices_handle_discard_safely,
@@ -7568,6 +7573,17 @@ static unsigned long raid5_cache_count(struct shrinker *shrink,
return max_stripes - min_stripes;
}
+/*
+ * Auto-sizing of the initial stripe cache (stripe_cache_size_default == 0):
+ * stay at the historical NR_STRIPES up to RAID5_CACHE_DEFAULT_BASE_GB of RAM,
+ * then grow the count using about 1/512 of the RAM above that base, capped at
+ * RAID5_CACHE_DEFAULT_MAX. So a system at or below the base is unchanged and
+ * keeps the historical footprint, while larger ones scale.
+ */
+#define RAID5_CACHE_DEFAULT_BASE_GB 8 /* unchanged at or below this much RAM */
+#define RAID5_CACHE_DEFAULT_RAM_SHIFT 9 /* above it: ~1/512 of the extra RAM */
+#define RAID5_CACHE_DEFAULT_MAX 4096
+
static struct r5conf *setup_conf(struct mddev *mddev)
{
struct r5conf *conf;
@@ -7801,15 +7817,37 @@ static struct r5conf *setup_conf(struct mddev *mddev)
conf->prev_algo = conf->algorithm;
}
- conf->min_nr_stripes = NR_STRIPES;
+ /*
+ * Choose the initial stripe cache size. stripe_cache_size_default
+ * selects it: 0 (the default) auto-sizes from memory -- the historical
+ * NR_STRIPES up to RAID5_CACHE_DEFAULT_BASE_GB of RAM, then scaling up
+ * to at most RAID5_CACHE_DEFAULT_MAX -- and a non-zero value sets it
+ * directly. A reshape still forces at least enough stripes for its
+ * window, below.
+ */
+ if (stripe_cache_size_default) {
+ conf->min_nr_stripes = clamp_t(unsigned long,
+ stripe_cache_size_default, 16, INT_MAX);
+ } else {
+ unsigned long per_stripe = sizeof(struct stripe_head) +
+ max_disks * (sizeof(struct bio) + PAGE_SIZE);
+ unsigned long ram = totalram_pages() << PAGE_SHIFT;
+ unsigned long base = (unsigned long)RAID5_CACHE_DEFAULT_BASE_GB << 30;
+ unsigned long extra = ram > base ?
+ ((ram - base) >> RAID5_CACHE_DEFAULT_RAM_SHIFT) / per_stripe : 0;
+
+ conf->min_nr_stripes = clamp_t(unsigned long, NR_STRIPES + extra,
+ NR_STRIPES, RAID5_CACHE_DEFAULT_MAX);
+ }
if (mddev->reshape_position != MaxSector) {
int stripes = max_t(int,
((mddev->chunk_sectors << 9) / RAID5_STRIPE_SIZE(conf)) * 4,
((mddev->new_chunk_sectors << 9) / RAID5_STRIPE_SIZE(conf)) * 4);
- conf->min_nr_stripes = max(NR_STRIPES, stripes);
- if (conf->min_nr_stripes != NR_STRIPES)
+ if (stripes > conf->min_nr_stripes) {
+ conf->min_nr_stripes = stripes;
pr_info("md/raid:%s: force stripe size %d for reshape\n",
mdname(mddev), conf->min_nr_stripes);
+ }
}
memory = conf->min_nr_stripes * (sizeof(struct stripe_head) +
max_disks * ((sizeof(struct bio) + PAGE_SIZE))) / 1024;
--
2.43.0
^ permalink raw reply related
* [PATCH 6/6] md/raid5: derive the default group_thread_cnt from the hardware
From: Hiroshi Nishida @ 2026-07-10 13:23 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Hiroshi Nishida
In-Reply-To: <20260710132346.7295-1-nishidafmly@gmail.com>
setup_conf() starts every array with worker_cnt_per_group = 0, i.e. a
single raid5d thread and no worker groups. On a many-core host backing a
fast, wide array that single thread is the bottleneck; the multi-threading
that group_thread_cnt enables has to be turned on by hand, per array,
after every assembly.
Pick a starting value from the CPU count instead. When the new
group_thread_cnt_default module parameter is left at -1 (the default),
raid5_default_group_thread_cnt() uses half the online CPU count spread
across the NUMA nodes, capped at 256:
gtc = num_online_cpus() / (2 * nr_nodes)
The count is per NUMA node, matching alloc_thread_groups(), so dividing by
the node count keeps the total near half the CPUs regardless of socket
count. It is only a ceiling: raid5_wakeup_stripe_thread() wakes workers
in proportion to the queued stripe count, so a lightly loaded array uses
far fewer than the maximum. A lone worker is not worth its overhead over
raid5d, so a result of 1 collapses back to 0, which keeps boxes with two
or fewer CPUs single-threaded.
Measured on a 16-disk raid6 array of NVMe SSDs (a 32-vCPU host: 16 cores,
two NUMA nodes; steady state, interleaved runs): the derived count of 8
raises throughput 2.1-3.2x over the single-threaded default -- 4K random
write from ~39k to ~100k IOPS (2.6x), mixed database/OLTP/high-concurrency
2.1-2.5x, partial stripe write 3.2x. There is no regression on smaller
machines: a 4-CPU box gets 2 workers and is faster or equal on every
workload, and a box with two or fewer CPUs gets 0 and is byte-for-byte
unchanged.
group_thread_cnt_default overrides the heuristic (0 forces the historical
single-threaded behaviour; a positive value pins a count, capped at 256),
and the existing per-array group_thread_cnt sysfs attribute continues to
override it at runtime and still accepts up to its own 8192 limit.
Signed-off-by: Hiroshi Nishida <nishidafmly@gmail.com>
---
drivers/md/raid5.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 8e1c2eba4241..6e91eb0ad575 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -88,6 +88,11 @@ module_param(stripe_cache_size_default, uint, 0644);
MODULE_PARM_DESC(stripe_cache_size_default,
"Initial stripe_cache_size for newly created arrays. 0 (the default) auto-sizes it from system memory: the historical 256 on small hosts, scaling up with RAM to a capped maximum on larger ones. A non-zero value sets a fixed initial size. Existing arrays are unaffected");
+static int group_thread_cnt_default = -1;
+module_param(group_thread_cnt_default, int, 0644);
+MODULE_PARM_DESC(group_thread_cnt_default,
+ "Initial group_thread_cnt (raid5 worker threads per NUMA node) for newly created arrays. A negative value (the default, -1) auto-sizes it from the CPU count; 0 forces the single-threaded raid5d; a positive value sets a fixed count (capped at 256). The per-array group_thread_cnt sysfs attribute overrides this and allows larger values");
+
static bool devices_handle_discard_safely = false;
module_param(devices_handle_discard_safely, bool, 0644);
MODULE_PARM_DESC(devices_handle_discard_safely,
@@ -7584,6 +7589,31 @@ static unsigned long raid5_cache_count(struct shrinker *shrink,
#define RAID5_CACHE_DEFAULT_RAM_SHIFT 9 /* above it: ~1/512 of the extra RAM */
#define RAID5_CACHE_DEFAULT_MAX 4096
+/*
+ * Default group_thread_cnt (worker_cnt_per_group) for a new array when the
+ * group_thread_cnt_default module parameter is left at -1. The historical
+ * default is 0 -- a single raid5d thread -- which cannot keep a fast, wide
+ * array busy on a many-core host. Derive a starting point from the CPU
+ * count: half the online CPUs divided across the NUMA nodes (this is a
+ * per-node count -- see alloc_thread_groups() -- so the total lands near
+ * half the online CPUs regardless of socket count), capped at
+ * RAID5_AUTO_GROUP_THREAD_MAX. This is only a ceiling:
+ * raid5_wakeup_stripe_thread() wakes workers in proportion to the queued
+ * stripe count, so a lightly loaded array uses far fewer. A lone worker is
+ * not worth its overhead, so 1 collapses back to 0. The group_thread_cnt
+ * sysfs attribute overrides this per array.
+ */
+#define RAID5_AUTO_GROUP_THREAD_MAX 256
+
+static int raid5_default_group_thread_cnt(void)
+{
+ unsigned int gtc = num_online_cpus() / (2 * num_possible_nodes());
+
+ if (gtc > RAID5_AUTO_GROUP_THREAD_MAX)
+ gtc = RAID5_AUTO_GROUP_THREAD_MAX;
+ return gtc == 1 ? 0 : gtc;
+}
+
static struct r5conf *setup_conf(struct mddev *mddev)
{
struct r5conf *conf;
@@ -7594,6 +7624,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
int i;
int group_cnt;
struct r5worker_group *new_group;
+ int def_threads;
int ret = -ENOMEM;
if (mddev->new_level != 5
@@ -7677,10 +7708,18 @@ static struct r5conf *setup_conf(struct mddev *mddev)
goto abort;
for (i = 0; i < PENDING_IO_MAX; i++)
list_add(&conf->pending_data[i].sibling, &conf->free_list);
- /* Don't enable multi-threading by default*/
- if (!alloc_thread_groups(conf, 0, &group_cnt, &new_group)) {
+ /*
+ * Multi-threading defaults to a hardware-derived worker count (see
+ * raid5_default_group_thread_cnt()); group_thread_cnt_default overrides
+ * the choice, and the group_thread_cnt sysfs attribute overrides it per
+ * array.
+ */
+ def_threads = group_thread_cnt_default < 0 ?
+ raid5_default_group_thread_cnt() :
+ min(group_thread_cnt_default, RAID5_AUTO_GROUP_THREAD_MAX);
+ if (!alloc_thread_groups(conf, def_threads, &group_cnt, &new_group)) {
conf->group_cnt = group_cnt;
- conf->worker_cnt_per_group = 0;
+ conf->worker_cnt_per_group = def_threads;
conf->worker_groups = new_group;
} else
goto abort;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory
From: sashiko-bot @ 2026-07-10 13:34 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132346.7295-4-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Integer overflow in `raid5_max_cache_size` on 32-bit platforms with 4GB+ RAM.
--
--- Patch [3]: [PATCH 3/6] md/raid5: scale the stripe_cache_size limit with system memory ---
commit 9086a4a6d9e318381885104d84e3f0ec9bbeec13
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: scale the stripe_cache_size limit with system memory
This commit derives the upper bound for the per-array stripe_cache_size from
system memory rather than a fixed limit. It scales the maximum cache size to
1/8 of RAM, allowing large-memory hosts to grow the cache while preventing
small systems from exceeding total RAM.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 7f72981121fd5..e41d3fc92dd05 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -6922,13 +6928,42 @@ raid5_show_stripe_cache_size(struct mddev *mddev, char *page)
> return ret;
> }
>
> +/*
> + * Upper bound that the per-array stripe_cache_size may be raised to. The
> + * stripe cache costs roughly max_nr_stripes * (sizeof(stripe_head) +
> + * pool_size * (sizeof(bio) + PAGE_SIZE)). The limit was historically a fixed
> + * 32768 stripes, which both under-serves large-memory hosts backing wide
> + * arrays and, on a small box, still permits a cache larger than RAM. Derive
> + * it from memory instead -- at most 1/8 of RAM -- but never below the
> + * historical 32768, so the limit only ever grows relative to today. A
> + * non-zero stripe_cache_size_max module parameter overrides the heuristic.
> + */
> +#define RAID5_CACHE_SIZE_FLOOR 32768
> +#define RAID5_CACHE_SIZE_RAM_SHIFT 3 /* cap the cache at 1/8 of RAM */
> +
> +static unsigned long raid5_max_cache_size(struct r5conf *conf)
> +{
> + unsigned long per_stripe, limit;
> +
> + if (stripe_cache_size_max) {
> + limit = stripe_cache_size_max;
> + } else {
> + per_stripe = sizeof(struct stripe_head) +
> + conf->pool_size * (sizeof(struct bio) + PAGE_SIZE);
> + limit = ((totalram_pages() << PAGE_SHIFT) >>
> + RAID5_CACHE_SIZE_RAM_SHIFT) / per_stripe;
[Severity: Medium]
Will this calculation overflow on 32-bit platforms with 4GB or more of RAM?
Since totalram_pages() returns an unsigned long, on a 32-bit system with high
memory (e.g., using PAE), totalram_pages() can be 1,048,576 or more.
Left-shifting this by PAGE_SHIFT (12) will overflow the 32-bit integer,
yielding a severely truncated value or 0.
This seems like it would cause the dynamic limit to inappropriately clamp to
the RAID5_CACHE_SIZE_FLOOR on such systems, breaking the intended memory-based
scaling.
> + limit = max_t(unsigned long, limit, RAID5_CACHE_SIZE_FLOOR);
> + }
> + return min_t(unsigned long, limit, INT_MAX);
> +}
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH 2/2] md/raid5: reserve stripe cache for user I/O during rebuild
From: sashiko-bot @ 2026-07-10 13:34 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: yukuai, linux-raid
In-Reply-To: <20260710132339.7284-3-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added stripe cache reservation logic is completely ineffective because it fails to throttle the calling loop in md_do_sync().
--
--- Patch [2]: [PATCH 2/2] md/raid5: reserve stripe cache for user I/O during rebuild ---
commit fb8a80692f7a386d69a8bee1d9b364db1d33380f
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: reserve stripe cache for user I/O during rebuild
This commit attempts to prevent burst-starvation of user I/O during a RAID5
rebuild by reserving a portion of the stripe cache. It adds checks in the
read-ahead loop of raid5_sync_request() to yield when there is competing
user I/O.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 574880e4f23fe..6575bdb84bb23 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -6656,6 +6656,27 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
> submitted < RAID5_SYNC_WINDOW && win_sector < max_sector &&
> win_sector < mddev->resync_max;
> submitted++, win_sector += RAID5_STRIPE_SECTORS(conf)) {
> + /*
> + * Yield to user I/O: stop the read-ahead if anyone is waiting
> + * for a stripe. The check is intentionally racy -- a waiter
> + * appearing just after is serviced by the next sync_request
> + * call, so no barrier is needed.
> + */
> + if (waitqueue_active(&conf->wait_for_stripe))
> + break;
> + /*
> + * Reserve cache for user I/O only when it is actually competing.
> + * preread_active_stripes counts stripes queued for write I/O
> + * (including the read phase of RMW); sync stripes never set
> + * STRIPE_PREREAD_ACTIVE, so during a pure rebuild it stays zero
> + * and the window fills freely. Competing user reads do not bump
> + * the counter but are caught by the waitqueue_active() check
> + * above.
> + */
> + if (atomic_read(&conf->preread_active_stripes) > 0 &&
> + atomic_read(&conf->active_stripes) >=
> + conf->max_nr_stripes / RAID5_SYNC_HWMARK)
> + break;
[Severity: High]
Does this actually throttle the rebuild as intended?
When we break out of this loop here, raid5_sync_request() returns the number
of sectors processed so far back to md_do_sync(). Since md_do_sync()
continuously loops calling the sync_request handler until the rate-limit window
is met, it will immediately call raid5_sync_request() again.
When raid5_sync_request() is re-entered, the very first stripe is allocated
unconditionally without checking the high-water mark:
drivers/md/raid5.c:raid5_sync_request() {
...
sh = raid5_get_active_stripe(conf, NULL, sector_nr, R5_GAS_NOBLOCK);
...
}
Will the sync thread just rapidly consume stripes one-by-one, completely
bypassing the 50% cache reservation and filling the cache 100% anyway?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132339.7284-1-nishidafmly@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 1/2] md/raid5: submit a window of stripes during resync/recovery
From: sashiko-bot @ 2026-07-10 13:36 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132339.7284-2-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The batched sync loop can greedily drain the stripe cache without yielding, causing severe latency spikes and potentially starving user I/O.
--
--- Patch [1]: [PATCH 1/2] md/raid5: submit a window of stripes during resync/recovery ---
commit 2b3cdb45d82bdea6112cba5dad14b59ec3acb23a
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: submit a window of stripes during resync/recovery
This patch updates the RAID5 sync path to dispatch a batch of up to 32
stripes per call instead of just one. It throttles only on the first stripe,
allowing subsequent stripes to be submitted without blocking to improve
resync performance.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 0c5c9fb0606ee..574880e4f23fe 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -6640,13 +6632,42 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
[ ... ]
> + /* Submit remaining stripes in the window non-blocking. Stop early
> + * if the stripe cache is full: the disk queue is already saturated.
> + * Bound by resync_max so a user- or cluster-imposed sync ceiling is
> + * not overshot.
> + */
> + win_sector = sector_nr + RAID5_STRIPE_SECTORS(conf);
> + for (submitted = 1;
> + submitted < RAID5_SYNC_WINDOW && win_sector < max_sector &&
> + win_sector < mddev->resync_max;
> + submitted++, win_sector += RAID5_STRIPE_SECTORS(conf)) {
> + sh = raid5_get_active_stripe(conf, NULL, win_sector,
> + R5_GAS_NOBLOCK);
> + if (!sh)
> + break;
[Severity: High]
Can this batched sync loop greedily drain the stripe cache without yielding to
user I/O?
During array resync or recovery, this loop acquires up to 32 free stripes
sequentially. Could this starve user I/O threads that are blocked on the
conf->wait_for_stripe waitqueue, since it doesn't check if user I/O is waiting
(e.g., via waitqueue_active(&conf->wait_for_stripe)) before getting the next
stripe?
> + md_bitmap_start_sync(mddev, win_sector, &sync_blocks,
> + still_degraded);
> + set_bit(STRIPE_SYNC_REQUESTED, &sh->state);
> + set_bit(STRIPE_HANDLE, &sh->state);
> + raid5_release_stripe(sh);
> + }
> +
> + return submitted * RAID5_STRIPE_SECTORS(conf);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132339.7284-1-nishidafmly@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 4/6] md/raid5: make the stripe batch size a module parameter
From: sashiko-bot @ 2026-07-10 13:36 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: yukuai, linux-raid
In-Reply-To: <20260710132346.7295-5-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Large unsigned values of `max_stripe_batch` are incorrectly clamped to 1 instead of 32.
--
--- Patch [4]: [PATCH 4/6] md/raid5: make the stripe batch size a module parameter ---
commit 5fef4dd203fcdfd7a223e9904fbea20f876f265b
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: make the stripe batch size a module parameter
This commit makes the stripe batch size a configurable module parameter to
allow tuning for busy multi-threaded arrays. It synchronizes the dequeue
cap and the spawn divisor by tying both to a single per-array variable.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -7640,6 +7645,10 @@ static struct r5conf *setup_conf(struct mddev *mddev)
> !conf->temp_inactive_list)
> goto abort;
>
> + /* Resolve the stripe batch size (see STRIPE_BATCH_* in raid5.h). */
> + conf->max_stripe_batch = clamp_t(int, max_stripe_batch,
> + 1, STRIPE_BATCH_MAX);
[Severity: Low]
Does this code incorrectly clamp large unsigned values to 1 instead of 32?
Since max_stripe_batch is defined as an unsigned int, if an administrator
provides a value greater than INT_MAX, the clamp_t(int, ...) cast will treat
it as a negative number.
This will cause it to be clamped to the minimum bound of 1, rather than the
intended maximum bound of STRIPE_BATCH_MAX.
> +
> #if PAGE_SIZE != DEFAULT_STRIPE_SIZE
> conf->stripe_size = DEFAULT_STRIPE_SIZE;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH 5/6] md/raid5: scale the default stripe cache size with system memory
From: sashiko-bot @ 2026-07-10 13:37 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132346.7295-6-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] 32-bit arithmetic overflow in `setup_conf` causes incorrect auto-sizing of the RAID5 stripe cache on 32-bit systems.
- [Medium] TOCTOU race condition when reading the module parameter `stripe_cache_size_default` allows bypassing intended bounds.
- [Medium] The commit message incorrectly claims that 'existing arrays are unaffected' by the new auto-sizing default.
--
--- Patch [5]: [PATCH 5/6] md/raid5: scale the default stripe cache size with system memory ---
commit 879380beda117fd029123c90fae16a214ae91020
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: scale the default stripe cache size with system memory
setup_conf() starts every array with min_nr_stripes = NR_STRIPES (256),
about 12MB of stripe cache on a 12-disk array. That fixed default was
chosen for small systems and is never revisited. Auto-size the initial
count from memory when the new stripe_cache_size_default module parameter
is 0.
> Auto-size the initial count from memory when the new
> stripe_cache_size_default module parameter is 0 (the default): keep the
> historical NR_STRIPES up to RAID5_CACHE_DEFAULT_BASE_GB (8GB) of RAM, then
> grow the count using about 1/512 of the RAM above that baseline, capped at
> RAID5_CACHE_DEFAULT_MAX (4096). A system with 8GB or less is therefore
> unchanged -- same count, same footprint -- while larger ones scale up
> smoothly (no jump at the baseline) to a bounded maximum. A non-zero
> stripe_cache_size_default sets a fixed initial size for administrators who
> want one; existing arrays are unaffected, and a reshape still forces at
> least its window's worth of stripes.
[Severity: Medium]
Is the claim that existing arrays are unaffected accurate?
Because setup_conf() is invoked during array assembly and the
stripe_cache_size is not persisted in the superblock, it appears an
existing array will boot with the dynamically scaled cache size instead
of the historical 256 stripes.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 5f0825c5effec..8e1c2eba42417 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -7801,15 +7817,37 @@ static struct r5conf *setup_conf(struct mddev *mddev)
[ ... ]
> + if (stripe_cache_size_default) {
> + conf->min_nr_stripes = clamp_t(unsigned long,
> + stripe_cache_size_default, 16, INT_MAX);
[Severity: Medium]
Could this read of stripe_cache_size_default race with a concurrent sysfs
modification?
If stripe_cache_size_default is changed to 0 exactly between the if
condition and the clamp_t evaluation, conf->min_nr_stripes would be clamped
to 16 rather than the historical 256.
Would it be safer to read this module parameter once into a local variable?
> + } else {
> + unsigned long per_stripe = sizeof(struct stripe_head) +
> + max_disks * (sizeof(struct bio) + PAGE_SIZE);
> + unsigned long ram = totalram_pages() << PAGE_SHIFT;
> + unsigned long base = (unsigned long)RAID5_CACHE_DEFAULT_BASE_GB << 30;
[Severity: High]
Do these calculations overflow on 32-bit systems?
Since unsigned long is 32 bits on those platforms, calculating base as
8 << 30 will wrap to 0.
Similarly, calculating ram by shifting totalram_pages() by PAGE_SHIFT will
wrap for systems with 4GB or more of memory, potentially breaking the
scaling logic for higher memory systems.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=5
^ permalink raw reply
* Re: [PATCH 1/6] md/raid5: size the worker group array by nr_node_ids
From: sashiko-bot @ 2026-07-10 13:39 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: yukuai, linux-raid
In-Reply-To: <20260710132346.7295-2-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Holding reconfig_mutex while waiting for raid5_wq workers to finish can cause an ABBA deadlock if workers wait for MD_SB_CHANGE_PENDING to be cleared.
--
--- Patch [1]: [PATCH 1/6] md/raid5: size the worker group array by nr_node_ids ---
commit 4f7875645a221b114b23eee045960c40b057b078
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: size the worker group array by nr_node_ids
This patch fixes an out-of-bounds array access when the NUMA node map is
sparse by sizing the worker_groups array using nr_node_ids rather than
num_possible_nodes().
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 0c5c9fb0606ee..d8807114a6932 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7322,7 +7322,13 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt, int *group_cnt,
> *worker_groups = NULL;
> return 0;
> }
> - *group_cnt = num_possible_nodes();
> + /*
> + * worker_groups is indexed by cpu_to_group() == cpu_to_node(), a node
> + * id, so it must have room for the largest possible id. Size it by
> + * nr_node_ids (one past that id), not num_possible_nodes(), which is
> + * only the node count and is smaller on a sparse node map.
> + */
> + *group_cnt = nr_node_ids;
[Severity: High]
This is a pre-existing issue, but while reviewing this allocation change,
I noticed a potential deadlock risk in the caller,
raid5_store_group_thread_cnt(), when configuring these worker groups.
When writing to the group_thread_cnt sysfs attribute,
raid5_store_group_thread_cnt() acquires reconfig_mutex via
mddev_suspend_and_lock() and then blocks waiting for workers in
raid5_wq to finish:
drivers/md/raid5.c:raid5_store_group_thread_cnt() {
err = mddev_suspend_and_lock(mddev);
...
raid5_quiesce(mddev, true);
if (new != conf->worker_cnt_per_group) {
old_groups = conf->worker_groups;
if (old_groups)
flush_workqueue(raid5_wq);
err = alloc_thread_groups(conf, new, &group_cnt, &new_groups);
}
However, the raid5 workers can block waiting for the md_thread to clear
MD_SB_CHANGE_PENDING:
drivers/md/raid5.c:raid5_do_work() {
...
wait_event_lock_irq(mddev->sb_wait,
!test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags),
conf->device_lock);
}
The md_thread requires the reconfig_mutex to clear this flag:
drivers/md/md.c:md_check_recovery() {
...
if (mddev_trylock(mddev)) {
...
if (mddev->sb_flags)
md_update_sb(mddev, 0);
}
Does this sequence create a deadlock where the sysfs thread holds
reconfig_mutex and waits for raid5_wq workers, while the workers wait for
the md_thread to clear MD_SB_CHANGE_PENDING, but the md_thread fails to
acquire reconfig_mutex via mddev_trylock()?
> size = sizeof(struct r5worker) * cnt;
> workers = kcalloc(size, *group_cnt, GFP_NOIO);
> *worker_groups = kzalloc_objs(struct r5worker_group, *group_cnt,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 2/2] md: widen badblock sectors param from int to sector_t
From: sashiko-bot @ 2026-07-10 13:40 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132329.7273-3-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Arithmetic underflow in `is_badblock()` when a physical bad block overlaps `data_offset`.
- [High] Discarded macro results in `block/badblocks.c` make block alignment a no-op.
--
--- Patch [2]: [PATCH 2/2] md: widen badblock sectors param from int to sector_t ---
commit 19521e4f1c32c01e7a6122a185436691618d94c1
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md: widen badblock sectors param from int to sector_t
This commit changes the sectors parameter in md wrappers (rdev_set_badblocks,
rdev_clear_badblocks, and rdev_has_badblock) from int to sector_t. This
matches the core API and is_badblock() to prevent a truncation point.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c81..61f40fa41e781 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> @@ -10553,7 +10553,7 @@ EXPORT_SYMBOL(md_finish_reshape);
> /* Bad block management */
>
> /* Returns true on success, false on failure */
> -bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
> +bool rdev_set_badblocks(struct md_rdev *rdev, sector_t s, sector_t sectors,
> int is_new)
> {
[Severity: High]
This isn't a bug introduced by this patch, but since this code interacts
with the core badblocks API, I noticed a pre-existing bug in how block
alignment is handled in block/badblocks.c.
In _badblocks_set(), the code attempts to align sectors:
block/badblocks.c:_badblocks_set() {
...
rounddown(s, 1 << bb->shift);
roundup(next, 1 << bb->shift);
sectors = next - s;
...
}
Similar logic exists in _badblocks_clear() and badblocks_check().
Because rounddown() and roundup() are macros that evaluate to the rounded
value but do not modify their arguments in-place, aren't the alignment
results being ignored here, leaving s and next unaligned?
If bb->shift > 0, wouldn't this break bad block alignment, fragment the
bad block table, and risk premature table exhaustion?
[ ... ]
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index b9ad26844799d..95835a3286aac 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
[ ... ]
> @@ -311,7 +311,7 @@ static inline int is_badblock(struct md_rdev *rdev, sector_t s, sector_t sectors
> }
>
> static inline int rdev_has_badblock(struct md_rdev *rdev, sector_t s,
> - int sectors)
> + sector_t sectors)
> {
> sector_t first_bad;
[Severity: High]
This is a pre-existing issue, but while reviewing the badblock sector types,
I noticed a logic error in is_badblock() located just above this function.
drivers/md/md.h:is_badblock() {
...
int rv = badblocks_check(&rdev->badblocks, rdev->data_offset + s,
sectors, first_bad, bad_sectors);
if (rv)
*first_bad -= rdev->data_offset;
...
}
If userspace tools (e.g., mdadm via sysfs bb_store) write a physical bad
block that starts before rdev->data_offset but extends into the array's data
area, badblocks_check() returns a *first_bad physical sector less than
data_offset.
Could the unconditional subtraction (*first_bad -= rdev->data_offset)
underflow here?
Would this create a massive false first_bad sector value, causing callers
like raid1 to mistakenly issue I/O to a known bad sector?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132329.7273-1-nishidafmly@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 2/6] md/raid5: size stripe-cache hash locks from the CPU count
From: sashiko-bot @ 2026-07-10 13:41 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132346.7295-3-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Time-of-check to time-of-use (TOCTOU) race condition in `nr_stripe_hash_locks` module parameter evaluation can lead to undefined behavior and a potential kernel panic during array creation.
Pre-existing issues:
- [High] The worker threads (`r5worker->work`) are not explicitly canceled or flushed during array teardown, which can lead to a Use-After-Free.
--
--- Patch [2]: [PATCH 2/6] md/raid5: size stripe-cache hash locks from the CPU count ---
commit bae2376a7fe52c6b3de7d64514b554d3101957fe
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: size stripe-cache hash locks from the CPU count
This commit modifies the stripe cache hash to use a per-array dynamic count
of spinlocks instead of a fixed 8, automatically sizing it based on the
online CPU count to reduce lock contention on many-core systems.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index d8807114a6932..7f72981121fd5 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -7448,6 +7453,9 @@ static void free_conf(struct r5conf *conf)
> kfree(conf->disks);
> bioset_exit(&conf->bio_split);
> kfree(conf->stripe_hashtbl);
> + kfree(conf->hash_locks);
> + kfree(conf->inactive_list);
> + kfree(conf->temp_inactive_list);
> kfree(conf->pending_data);
[Severity: High]
This is a pre-existing issue, but looking at the teardown sequence, could
the worker threads access freed memory here?
During array teardown, md_stop() eventually causes a wait for
active_stripes to reach 0. In a worker thread executing raid5_do_work(),
active_stripes drops to 0 when handle_active_stripes() finishes releasing
the final stripes.
At this point, raid5_quiesce() wakes up and teardown proceeds to call
free_thread_groups() which frees the worker structures without calling
flush_workqueue() or cancel_work_sync().
The still-executing worker thread could then drop its lock and continue
to execute flush_deferred_bios() and r5l_flush_stripe_to_raid() using the
freed structures.
Should the worker threads be explicitly canceled or flushed before the
structures are freed?
[ ... ]
> @@ -7565,6 +7573,38 @@ static struct r5conf *setup_conf(struct mddev *mddev)
> if (conf == NULL)
> goto abort;
>
> + /*
> + * Size the stripe cache hash-lock striping per array. The
> + * nr_stripe_hash_locks module parameter selects the count: 0 (the
> + * default) auto-sizes it from the online CPU count so busier machines
> + * stripe the locks wider, while a non-zero value overrides that. The
> + * result is rounded up to a power of two (the hash uses it as a
> + * bitmask) and capped at NR_STRIPE_HASH_LOCKS_MAX; the auto path also
> + * keeps at least the historical NR_STRIPE_HASH_LOCKS_DEFAULT so small
> + * systems are unchanged. Allocating per array means an untuned system
> + * uses no more memory than before.
> + */
> + if (nr_stripe_hash_locks == 0)
> + conf->nr_hash_locks = clamp_t(unsigned int,
> + roundup_pow_of_two(num_online_cpus()),
> + NR_STRIPE_HASH_LOCKS_DEFAULT,
> + NR_STRIPE_HASH_LOCKS_MAX);
> + else
> + conf->nr_hash_locks = roundup_pow_of_two(
> + min_t(unsigned int, nr_stripe_hash_locks,
> + NR_STRIPE_HASH_LOCKS_MAX));
[Severity: High]
Could this be vulnerable to a time-of-check to time-of-use race if the
sysfs parameter is modified concurrently?
If the module parameter is changed from a non-zero value to 0 right after
the check:
Thread 1 in setup_conf():
if (nr_stripe_hash_locks == 0)
// check fails, proceeds to else block
Thread 2 writing to sysfs:
nr_stripe_hash_locks = 0;
Thread 1 inside the else block:
conf->nr_hash_locks = roundup_pow_of_two(min_t(..., 0, ...));
In this scenario, roundup_pow_of_two() would evaluate to 0. When kcalloc()
is subsequently called, it would return the zero size pointer placeholder
value (16).
The safety check testing for a null pointer would fail to catch this since
16 is non-null, and the kernel could panic when spin_lock_init()
dereferences it.
Would it be safer to read the parameter into a local variable first?
unsigned int locks = READ_ONCE(nr_stripe_hash_locks);
if (locks == 0)
conf->nr_hash_locks = clamp_t(...);
else
conf->nr_hash_locks = roundup_pow_of_two(...);
> + conf->hash_locks_mask = conf->nr_hash_locks - 1;
> + conf->hash_locks = kcalloc(conf->nr_hash_locks,
> + sizeof(*conf->hash_locks), GFP_KERNEL);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 6/6] md/raid5: derive the default group_thread_cnt from the hardware
From: sashiko-bot @ 2026-07-10 13:42 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: yukuai, linux-raid
In-Reply-To: <20260710132346.7295-7-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] Double-read of the sysfs-modifiable `group_thread_cnt_default` module parameter can cause an array initialization failure due to a large negative worker count.
- [Medium] Massive overallocation of worker threads and worker groups on systems with sparse NUMA node maps.
Pre-existing issues:
- [High] A failure to register the CPU hotplug instance in `raid5_alloc_percpu()` causes a NULL pointer dereference (Oops) during error cleanup.
--
--- Patch [6]: [PATCH 6/6] md/raid5: derive the default group_thread_cnt from the hardware ---
commit 9c6dbf46ca41e580ada80950d4adaba43e7b0100
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md/raid5: derive the default group_thread_cnt from the hardware
This commit updates the RAID5 subsystem to automatically configure the number of
worker threads per NUMA node based on the hardware CPU count, rather than
defaulting to a single raid5d thread. It calculates a default value bounded by
a cap, and allows overriding this via a new module parameter or the existing
sysfs attribute.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 8e1c2eba42417..6e91eb0ad5759 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -7584,6 +7589,31 @@ static unsigned long raid5_cache_count(struct shrinker *shrink,
> #define RAID5_CACHE_DEFAULT_RAM_SHIFT 9 /* above it: ~1/512 of the extra RAM */
> #define RAID5_CACHE_DEFAULT_MAX 4096
>
> +/*
> + * Default group_thread_cnt (worker_cnt_per_group) for a new array when the
> + * group_thread_cnt_default module parameter is left at -1. The historical
> + * default is 0 -- a single raid5d thread -- which cannot keep a fast, wide
> + * array busy on a many-core host. Derive a starting point from the CPU
> + * count: half the online CPUs divided across the NUMA nodes (this is a
> + * per-node count -- see alloc_thread_groups() -- so the total lands near
> + * half the online CPUs regardless of socket count), capped at
> + * RAID5_AUTO_GROUP_THREAD_MAX. This is only a ceiling:
> + * raid5_wakeup_stripe_thread() wakes workers in proportion to the queued
> + * stripe count, so a lightly loaded array uses far fewer. A lone worker is
> + * not worth its overhead, so 1 collapses back to 0. The group_thread_cnt
> + * sysfs attribute overrides this per array.
> + */
> +#define RAID5_AUTO_GROUP_THREAD_MAX 256
> +
> +static int raid5_default_group_thread_cnt(void)
> +{
> + unsigned int gtc = num_online_cpus() / (2 * num_possible_nodes());
[Severity: Medium]
Could this lead to massive overallocation of worker threads on systems with
sparse NUMA node maps?
Here in raid5_default_group_thread_cnt() in drivers/md/raid5.c, gtc is
calculated using num_possible_nodes(), which returns the actual count of
possible nodes. However, alloc_thread_groups() allocates threads based on
nr_node_ids:
drivers/md/raid5.c:alloc_thread_groups() {
...
*group_cnt = nr_node_ids;
size = sizeof(struct r5worker) * cnt;
workers = kcalloc(size, *group_cnt, GFP_NOIO);
...
}
If a system has sparse NUMA node IDs (e.g., nodes 0 and 64, making
num_possible_nodes() == 2 but nr_node_ids == 65), would multiplying the
large gtc value by the highest node ID cause hundreds of unused struct
r5worker and struct r5worker_group instances to be allocated?
> +
> + if (gtc > RAID5_AUTO_GROUP_THREAD_MAX)
> + gtc = RAID5_AUTO_GROUP_THREAD_MAX;
> + return gtc == 1 ? 0 : gtc;
> +}
> +
> static struct r5conf *setup_conf(struct mddev *mddev)
> {
> struct r5conf *conf;
[ ... ]
> @@ -7677,10 +7708,18 @@ static struct r5conf *setup_conf(struct mddev *mddev)
> goto abort;
> for (i = 0; i < PENDING_IO_MAX; i++)
> list_add(&conf->pending_data[i].sibling, &conf->free_list);
> - /* Don't enable multi-threading by default*/
> - if (!alloc_thread_groups(conf, 0, &group_cnt, &new_group)) {
> + /*
> + * Multi-threading defaults to a hardware-derived worker count (see
> + * raid5_default_group_thread_cnt()); group_thread_cnt_default overrides
> + * the choice, and the group_thread_cnt sysfs attribute overrides it per
> + * array.
> + */
> + def_threads = group_thread_cnt_default < 0 ?
> + raid5_default_group_thread_cnt() :
> + min(group_thread_cnt_default, RAID5_AUTO_GROUP_THREAD_MAX);
[Severity: Medium]
Is it possible for a concurrent sysfs write to cause an array initialization
failure due to a large negative worker count?
The module parameter group_thread_cnt_default is evaluated twice without
READ_ONCE() (once in the ternary condition, and once in the min() macro).
Could an administrator writing to the sysfs parameter concurrently with array
assembly cause the ternary condition to read a positive value, and the min()
macro to read a negative value, resulting in def_threads becoming -1?
> + if (!alloc_thread_groups(conf, def_threads, &group_cnt, &new_group)) {
> conf->group_cnt = group_cnt;
> - conf->worker_cnt_per_group = 0;
> + conf->worker_cnt_per_group = def_threads;
> conf->worker_groups = new_group;
> } else
> goto abort;
[Severity: High]
This isn't a bug introduced by this patch, but does the cleanup path for
setup failures lead to a NULL pointer dereference?
If cpuhp_state_add_instance() in raid5_alloc_percpu() previously returned
an error (like -ENOMEM) and failed to register the hotplug node, the error
bubbles up and causes a jump to the abort label.
In the cleanup path, raid5_free_percpu() unconditionally calls
cpuhp_state_remove_instance():
drivers/md/raid5.c:raid5_free_percpu() {
if (!conf->percpu)
return;
cpuhp_state_remove_instance(CPUHP_MD_RAID5_PREPARE, &conf->node);
...
}
Because conf->node is completely zero-initialized and was never successfully
hashed, its pprev pointer is NULL. Won't the subsequent hlist_del() in the
teardown path dereference this NULL pointer and crash the kernel? Should we
check hlist_unhashed(&conf->node) before removing the instance?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132346.7295-1-nishidafmly@gmail.com?part=6
^ permalink raw reply
* Re: [PATCH 0/8] md/raid5: scalability and rebuild-path improvements
From: Hiroshi Nishida @ 2026-07-10 13:45 UTC (permalink / raw)
To: yukuai; +Cc: Song Liu, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <CAE1ksKTLcamf2jX6s6vaV5keXwS1pmJa_cV7nPZ84CNUSQRZxw@mail.gmail.com>
The cover quotes the 2.1-3.2x headline; here is the full per-workload data
behind it, in case it helps review.
The same raid456.ko is used in both arms -- only the module parameters
differ at array-create -- so this isolates the *defaults*, not code changes.
Real NVMe SSDs, not a ramdisk.
Setup: GCP n2-standard-32 -- 32 vCPUs = 16 physical cores (a vCPU is one
hyperthread), 2 NUMA nodes, 125 GB RAM, 16x 375 GB local NVMe. Kernel: full
build of the 6-patch series (base 55b77337), non-debug. raid6, 64k chunk,
bitmap=none, --assume-clean. Workloads: a 5-test fio suite (direct=1,
libaio, 30 s each): 4K random write (RMW), DB-mixed 75/25 8K,
high-concurrency 70/30 4K, OLTP 70/30 16K, partial-stripe 8K.
AUTO = all defaults left auto (the hardware-derived values); STOCK = the
historical fixed values (hash 8 / ceiling 32768 / batch 8 / cache 256 /
group_thread_cnt 0). On this host AUTO resolves to group_thread_cnt=8,
stripe_cache_size~3886, 32 hash locks.
1) Wide 16-disk raid6 (14+2), steady state, interleaved N=5 (STOCK and AUTO
alternate each round so SSD drift cancels; cv 1-8%):
Workload STOCK IOPS AUTO IOPS AUTO/STOCK
4K random write 39,147 100,069 2.56x
DB mixed 75/25 8K 88,040 208,284 2.37x
High-conc 70/30 4K 122,765 257,952 2.10x
OLTP 70/30 16K 39,597 100,033 2.53x
Partial-stripe 8K 20,491 64,511 3.15x
2) Low-end 4-CPU / 8 GB, 6-disk raid6 (4+2), N=3 -- no-regression control.
4 online CPUs + 8 GB, single node (num_online_cpus()=4 -> gtc=2); the
cache stays 256 and the hash count stays 8, so AUTO differs from STOCK
only in group_thread_cnt (2 vs 0):
Workload STOCK IOPS AUTO IOPS AUTO/STOCK
4K random write 52,017 81,132 1.56x
DB mixed 125,043 126,188 1.01x
High-conc 180,570 217,074 1.20x
OLTP 54,308 55,840 1.03x
Partial-stripe 27,060 42,751 1.58x
No regression on any workload; modest write gains even here. A
slower-disk NAS is more device-bound (the gains flatten) but still shows
no regression, and a box with <=2 online CPUs derives gtc=0 =
byte-for-byte stock.
The gain is essentially patch 6 (worker groups on by default); patches 2-5
(cache and lock sizing) are throughput-neutral on real SSD and earn their
place as configurability plus sane defaults, not a speed claim. Patch 1 is
the NUMA-sizing prerequisite and does not affect throughput.
Thanks,
2026年7月10日(金) 6:09 Hiroshi Nishida <nishidafmly@gmail.com>:
>
> Hi Yu Kuai,
>
> Thanks again for the review, and in particular for:
>
> > I can accept make those values configurable, but not direct
> > modifications.
>
> I've reworked the tunables along exactly those lines and will send them as
> a separate, self-contained series, "md/raid5: size stripe-cache and worker
> tuning from the hardware".
>
> Rather than raising any fixed constant, each value now derives a default
> from the hardware and stays overridable:
>
> - the stripe-cache hash lock count (was a fixed 8) is sized from the CPU
> count, clamped to 8..32;
> - the stripe_cache_size ceiling (was a fixed 32768) scales with memory,
> but never drops below 32768;
> - the initial stripe_cache_size (was a fixed 256) scales gently with
> memory, 256..4096;
> - the default group_thread_cnt (was 0, i.e. single-threaded) is derived
> from the CPU count (half the online CPUs per NUMA node, capped);
> - the stripe batch size (was a fixed 8) is exposed as a parameter.
>
> Ahead of those, the series opens with a one-line prerequisite fix:
> alloc_thread_groups() sizes the per-node worker_groups[] array by
> num_possible_nodes() but indexes it by cpu_to_node(), so a sparse NUMA node
> map can index off the end -- reachable once the worker-group default (the
> last patch) is on. It is Fixes:-tagged and can be taken on its own.
>
> The key point for the consumer-NAS concern you raised: each default only
> rises on hardware that can back it -- the lock and worker counts scale with
> the core count, the cache sizes with RAM -- so a genuinely small system (a
> few cores and a few GB) keeps today's values and footprint. And each one is
> overridable via a module parameter (group_thread_cnt also via its existing
> sysfs attribute), including all the way back to today's behaviour. So nothing is
> imposed; the default simply tracks the machine instead of a constant, and
> a wide array on a large host no longer needs a recompile or manual
> per-array tuning to use the memory and cores it has.
>
> And on performance -- on real NVMe this time, not a ramdisk: on a 16-disk
> raid6 array (a 32-vCPU / 16-core host, steady state, interleaved runs) the
> hardware-derived defaults run 2.1-3.2x the stock defaults (4K random write
> ~39k -> ~100k IOPS), essentially all of it from patch 6 turning on the
> worker groups. Patches 2-5 (the cache and lock sizing) are
> throughput-neutral on real SSD, as you'd expect -- they earn their place as
> configurability and sane defaults, not a speed claim. And there is no
> regression at the small end: a 4-CPU box derives 2 workers and is never
> slower on any workload, and a box with two or fewer CPUs derives 0 and is
> byte-for-byte unchanged.
>
> It has been through KASAN + lockdep + DEBUG_LIST on RAID5
> (create/rebuild/scrub, plus the bitmap add/remove that drives the
> lock-all-hash-locks quiesce path), at both the derived defaults and pinned
> values including nr_stripe_hash_locks=32.
>
> The two unrelated parts of the original series -- the type-widening /
> correctness fixes and the resync/recovery dispatch changes -- I'll send as
> their own small series, as discussed.
>
> Thanks,
>
> 2026年7月5日(日) 19:56 yu kuai <yukuai@fygo.io>:
> >
> > Hi,
> >
> > 在 2026/6/24 23:54, Hiroshi Nishida 写道:
> > > This series collects small, individually low-risk md/raid5 changes for
> > > large, many-core, many-disk arrays. Their common theme is reducing
> > > per-stripe and stripe-cache contention, so the benefit appears mainly
> > > when the raid5 stripe-handling worker threads are in use
> > > (group_thread_cnt > 0); at the default group_thread_cnt = 0 (a single
> > > handling thread) the series is essentially neutral.
> > >
> > > - patches 1-3 remove signed arithmetic from a hot-path divisor, lift an
> > > arbitrary stripe-cache size cap, and widen a badblock length argument
> > > that currently truncates large ranges;
> > > - patch 4 raises NR_STRIPE_HASH_LOCKS (8 -> 32) to spread stripe-hash
> > > contention on high core-count systems;
> > > - patches 5 and 8 reduce per-stripe overhead in the resync/recovery
> > > path and bound the share of the stripe cache a rebuild may hold while
> > > user I/O is competing;
> > > - patch 6 allocates each worker group's array on its own NUMA node;
> > > - patch 7 raises MAX_STRIPE_BATCH (8 -> 32).
> > >
> > > Measured effect, treatment vs baseline, % change in mean IOPS (N=3),
> > > swept over group_thread_cnt (RAID6 4+2, 22-core host, ramdisk members):
> >
> > Testing with ramdisk does serve as a useful reference, but it does not reflect
> > real world usage.
> >
> > >
> > > workload gtc=0 gtc=2 gtc=4 gtc=8
> > > random 4K write (RMW) +4.2% +8.1% +17.4% +6.5%
> > > DB mixed 75/25 8K +0.4% +4.2% +10.3% +4.7%
> > > high-concurrency 70/30 4K +3.9% +1.2% +10.0% +0.2%
> > > OLTP 70/30 16K -0.3% +4.7% +10.1% +9.3%
> > > partial-stripe write 8K +1.1% +4.8% +11.2% +14.2%
> >
> > With a quick review I saw many static configurations is changed, I agree
> > these changes can improve arrays with ssd/nvme and a system with large
> > memory available. However, we already tested with hdd and about 8G memory
> > available, these changes will not improve performance at all, with the
> > extra memory overhead.
> >
> > I can accept make those values configurable, but not direct modifications.
> > As validation is required for numerous scenarios. Memory resources are precious
> > especially for most consumer NAS devices.
> >
> > >
> > > At the default single handling thread (group_thread_cnt = 0) the series is
> > > neutral (no regression). As worker threads are added the gain grows,
> > > peaking broadly around group_thread_cnt = 4 at roughly +10-17% across the
> > > whole mix; at gtc = 8 the write-heavy workloads keep gaining while the
> > > read-heavy high-concurrency case has saturated. (Per-run cv was <1%
> > > except the random-write test, ~5-9%, from a cold first run.)
> > >
> > > These numbers are on a ramdisk, which removes device latency and so
> > > overstates the CPU-side contention effect relative to a real device;
> > > they show the direction and the group_thread_cnt dependence, not an
> > > absolute speedup. The stripe-hash/batch patches (4, 7) and the cache cap
> > > (2) drive this; patch 6 only matters on multi-socket systems (not
> > > exercised above) and patches 5/8 act on the resync/recovery path rather
> > > than this steady-state workload.
> > >
> > > Reproduction (stock mdadm + fio):
> > > mdadm --create /dev/md0 --level=6 --raid-devices=6 --chunk=512 \
> > > --assume-clean <6 members>
> > > echo 16384 > /sys/block/md0/md/stripe_cache_size
> > > echo N > /sys/block/md0/md/group_thread_cnt # N = 0,2,4,8
> > > fio --filename=/dev/md0 --direct=1 --ioengine=libaio --group_reporting \
> > > --time_based --runtime=15 --name=w <per-workload opts>:
> > > random write : --rw=randwrite --bs=4k --numjobs=4 --iodepth=32
> > > DB mixed : --rw=randrw --rwmixread=75 --bs=8k --numjobs=8 --iodepth=16
> > > high-concur. : --rw=randrw --rwmixread=70 --bs=4k --numjobs=16 --iodepth=8
> > > OLTP : --rw=randrw --rwmixread=70 --bs=16k --numjobs=6 --iodepth=16
> > > partial-stripe : --rw=randwrite --bs=8k --numjobs=4 --iodepth=32
> > >
> > > Each patch stands on its own; I am happy to drop or defer any that is not
> > > justified on its own merit.
> > >
> > > Functional testing on RAID5 and RAID6: create, fail a member, rebuild
> > > onto a spare / re-add, full data read-back verified, and scrub
> > > ("check") reporting mismatch_cnt == 0. The series was also exercised
> > > with KASAN and lockdep enabled -- including heavy group_thread_cnt
> > > churn on a multi-node setup to stress the per-NUMA-node worker
> > > allocation and the raid5_quiesce hash-lock-all path -- with no reports.
> > >
> > > Hiroshi Nishida (8):
> > > md: change chunk_sectors and stripe cache counts to unsigned int
> > > md/raid5: raise stripe cache limit from 32768 to 262144
> > > md: widen badblock sectors param from int to sector_t
> > > md/raid5: raise NR_STRIPE_HASH_LOCKS from 8 to 32
> > > md/raid5: submit a window of stripes during resync/recovery
> > > md/raid5: allocate worker groups per NUMA node
> > > md/raid5: raise MAX_STRIPE_BATCH from 8 to 32
> > > md/raid5: reserve stripe cache for user I/O during rebuild
> > >
> > > drivers/md/md.c | 4 +-
> > > drivers/md/md.h | 10 ++--
> > > drivers/md/raid5.c | 129 ++++++++++++++++++++++++++++++++-------------
> > > drivers/md/raid5.h | 33 ++++++++----
> > > 4 files changed, 121 insertions(+), 55 deletions(-)
> > >
> > > base-commit: 55b77337bdd088c77461588e5ec094421b89911b
> > >
> > --
> > Thanks,
> > Kuai
>
>
>
> --
> Hiroshi Nishida
> nishidafmly@gmail.com
--
Hiroshi Nishida
nishidafmly@gmail.com
^ permalink raw reply
* Re: [PATCH 1/2] md: change chunk_sectors and stripe cache counts to unsigned int
From: sashiko-bot @ 2026-07-10 15:44 UTC (permalink / raw)
To: Hiroshi Nishida; +Cc: linux-raid, yukuai
In-Reply-To: <20260710132329.7273-2-nishidafmly@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] Lockless read race on total_cached evaluates to a negative value, triggering unsigned promotion and false cache pressure signals.
Pre-existing issues:
- [Critical] The calculation of total sectors per chunk stripe (chunk size multiplied by data disks) is performed using 32-bit arithmetic, which can overflow. This leads to a divide-by-zero kernel panic or incorrect boundary math if a maliciously large chunk size is provided.
--
--- Patch [1]: [PATCH 1/2] md: change chunk_sectors and stripe cache counts to unsigned int ---
commit 6fa5b599e890f5ca04407ab1005af1cc096d709e
Author: Hiroshi Nishida <nishidafmly@gmail.com>
md: change chunk_sectors and stripe cache counts to unsigned int
This commit changes various chunk and stripe counts from signed to
unsigned int in the md/raid5 subsystem to prevent negative values.
It allows the compiler to optimize division and modulo operations by
power-of-two chunk sizes to right shifts in the hot I/O path.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 0c5c9fb0606ee..28828e083c2b3 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -7929,7 +7929,7 @@ static int raid5_run(struct mddev *mddev)
> sector_t here_new, here_old;
> int old_disks;
> int max_degraded = (mddev->level == 6 ? 2 : 1);
> - int chunk_sectors;
> + unsigned int chunk_sectors;
> int new_data_disks;
[Severity: Critical]
This isn't a bug introduced by this patch, but changing the types to
unsigned int fails to resolve an underlying 32-bit wrapping regression.
Does this code overflow when calculating total sectors per chunk stripe?
In raid5_run(), if a maliciously crafted RAID superblock sets chunksize
to 0x08000000 (64 GiB) and raid_disks to 33, the multiplication
(chunk_sectors * new_data_disks) evaluates to 0x08000000 * 32.
This wraps to 0 in 32-bit arithmetic, which is then passed to sector_div():
if (sector_div(here_new, chunk_sectors * new_data_disks)) {
This triggers a divide-by-zero panic. A similar issue occurs in
make_discard_request() where DIV_ROUND_UP_SECTOR_T() is called with a
wrapped stripe_sectors value.
> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
> index cb5feae04db27..5cd9d0f36b6e0 100644
> --- a/drivers/md/raid5.h
> +++ b/drivers/md/raid5.h
[ ... ]
> @@ -572,12 +572,12 @@ struct r5conf {
> /* only protect corresponding hash list and inactive_list */
> spinlock_t hash_locks[NR_STRIPE_HASH_LOCKS];
> struct mddev *mddev;
> - int chunk_sectors;
> + unsigned int chunk_sectors;
> int level, algorithm, rmw_level;
> int max_degraded;
> int raid_disks;
> - int max_nr_stripes;
> - int min_nr_stripes;
> + unsigned int max_nr_stripes;
> + unsigned int min_nr_stripes;
[Severity: Medium]
Does changing min_nr_stripes to unsigned int introduce a spurious cache
flush regression due to type promotion?
In drivers/md/raid5-cache.c:r5c_do_reclaim(), total_cached can evaluate
to -1 if r5c_finish_stripe_write_out() concurrently decrements
r5c_flushing_partial_stripes and r5c_cached_partial_stripes during a
lockless read.
Since min_nr_stripes is now unsigned, the -1 is promoted to UINT_MAX in
this condition:
if (total_cached > conf->min_nr_stripes * 3 / 4 ||
This makes the cache pressure check spuriously evaluate to true, leading
to unnecessary CPU wakeups and a performance regression during writeback
workloads.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710132329.7273-1-nishidafmly@gmail.com?part=1
^ permalink raw reply
* Re: LBS setup
From: Piergiorgio Sartor @ 2026-07-10 16:20 UTC (permalink / raw)
To: Piergiorgio Sartor; +Cc: linux-raid
In-Reply-To: <ahLSj1Ua5WJ0GTgY@lazy.lzy>
Hi again,
not sure, but has anybody comments
on the topic?
Thanks,
bye,
pg
On Sun, May 24, 2026 at 12:27:27PM +0200, Piergiorgio Sartor wrote:
> Hi all,
>
> I've some questions about some recent
> report from kernel.
> I assume from 7.x.y, from what I understood.
> Not sure about the <= 6.18 reported.
>
> In the boot log there is:
>
> md0: echo current LBS to md/logical_block_size to prevent data loss issues from LBS changes.
> Note: After setting, array will not be assembled in old kernels (<= 6.18)
>
> I found the docs stating:
>
> logical_block_size
> ... Write the value before starting array. ...
>
> Now, should we really copy the value from:
>
> /sys/class/block/md0/queue/logical_block_size
>
> to:
>
> /sys/class/block/md0/md/logical_block_size
>
> ???
>
> Note, the first in my system is 512, the
> second is 0.
>
> Second, the docs says "before starting array".
> In case this is due (copy the value), how to do
> for a system array?
> I mean, the array where the whole OS lies.
>
> If needed, is there any way to do this automatically?
> Using mdadm or something?
>
> Thanks,
>
> bye,
>
> --
>
> piergiorgio
>
--
piergiorgio
^ permalink raw reply
* Re: LBS setup
From: Piergiorgio Sartor @ 2026-07-10 17:54 UTC (permalink / raw)
To: Wol; +Cc: Piergiorgio Sartor, linux-raid
In-Reply-To: <811c24a5-0c72-4b83-af19-609e42e12613@youngman.org.uk>
Hi Wol
thanks for the answer.
My question is about "what to do?".
Not much what happened.
The log says we should copy some size
from somewhere to somewhere else.
The docs says this should be done
with array at rest.
Is there any documentation about this?
Should we just ignore the log?
What should be done, if any?
The log:
md1: echo current LBS to md/logical_block_size to prevent data loss issues from LBS changes.
Note: After setting, array will not be assembled in old kernels (<= 6.18)
The docs:
logical_block_size
Configure the array’s logical block size in bytes.
This attribute is only supported for 1.x meta.
*Write the value before starting array.* (emphasis mine)
The final array LBS uses the maximum between this
configuration and LBS of all combined devices.
Note that LBS cannot exceed PAGE_SIZE before RAID
supports folio.
WARNING: Arrays created on new kernel cannot be
assembled at old kernel due to padding check,
Set module parameter ‘check_new_feature’ to
false to bypass, but data loss may occur.
The kernel is pretty new, actually 7.1.3,
from Fedora.
Old kernels is for me no problem, it would
be a problem losing the array.
There is no procedure explained, documentation
is, for me, quite dry.
It would be good to have some comments from
whom wrote it (and the code as well).
Thanks,
bye,
pg
On Fri, Jul 10, 2026 at 06:10:47PM +0100, Wol wrote:
> On 10/07/2026 17:20, Piergiorgio Sartor wrote:
> > Hi again,
> >
> > not sure, but has anybody comments
> > on the topic?
>
> I don't understand what you're asking, but I do think I remember the events
> you're talking about.
>
> Basically, some change accidentally slipped into the wild with a kernel,
> probably a 6.x, and people started creating kernels with the new kernel.
>
> Then, as people started upgrading kernels, they suddenly realised that this
> change affected the raid format, such that new kernels couldn't read old
> arrays, and vice versa. More importantly, and more dangerously, it led to
> arrays being trashed!
>
> What to do? They couldn't pull old kernels obviously, there were too many
> arrays out there. They couldn't pull new kernels, there were already some
> arrays out there.
>
> So they rushed out a fix, such that if a new kernel created an array, it
> flagged it as being a new array. If you booted a new kernel with an array
> that didn't have the flag IT WOULDN'T LOAD THE ARRAY. And they trusted to
> luck that people wouldn't downgrade an array and try to load a new array on
> an old kernel.
>
> So this now meant that all of a sudden new kernels wouldn't boot on old
> systems, until the user explicitly set the flag on the array to say "this is
> an old-style array". If they happened to be upgrading from one of the few
> early new-style kernels, they could set the flag to "this is a new-style
> array".
>
> Point is, this code was meant to make it very hard to upgrade your kernel
> without being prompted what sort of array you had, so that the kernel devs
> could try to ensure you didn't lose your array.
>
> A pretty awful way of doing it, but nobody could think of anything better.
>
> Hopefully that explains what's going on there, and you'll be able to work
> out your own answer from this information.
>
> Cheers,
> Wol
--
piergiorgio
^ permalink raw reply
* Re: LBS setup
From: Wol @ 2026-07-10 17:10 UTC (permalink / raw)
To: Piergiorgio Sartor; +Cc: linux-raid
In-Reply-To: <alEb50Nmqt8IW2Nu@lazy.lzy>
On 10/07/2026 17:20, Piergiorgio Sartor wrote:
> Hi again,
>
> not sure, but has anybody comments
> on the topic?
I don't understand what you're asking, but I do think I remember the
events you're talking about.
Basically, some change accidentally slipped into the wild with a kernel,
probably a 6.x, and people started creating kernels with the new kernel.
Then, as people started upgrading kernels, they suddenly realised that
this change affected the raid format, such that new kernels couldn't
read old arrays, and vice versa. More importantly, and more dangerously,
it led to arrays being trashed!
What to do? They couldn't pull old kernels obviously, there were too
many arrays out there. They couldn't pull new kernels, there were
already some arrays out there.
So they rushed out a fix, such that if a new kernel created an array, it
flagged it as being a new array. If you booted a new kernel with an
array that didn't have the flag IT WOULDN'T LOAD THE ARRAY. And they
trusted to luck that people wouldn't downgrade an array and try to load
a new array on an old kernel.
So this now meant that all of a sudden new kernels wouldn't boot on old
systems, until the user explicitly set the flag on the array to say
"this is an old-style array". If they happened to be upgrading from one
of the few early new-style kernels, they could set the flag to "this is
a new-style array".
Point is, this code was meant to make it very hard to upgrade your
kernel without being prompted what sort of array you had, so that the
kernel devs could try to ensure you didn't lose your array.
A pretty awful way of doing it, but nobody could think of anything better.
Hopefully that explains what's going on there, and you'll be able to
work out your own answer from this information.
Cheers,
Wol
^ permalink raw reply
* Re: LBS setup
From: Wol @ 2026-07-10 20:43 UTC (permalink / raw)
To: Piergiorgio Sartor; +Cc: linux-raid
In-Reply-To: <alExwDILVwd3nVvK@lazy.lzy>
On 10/07/2026 18:54, Piergiorgio Sartor wrote:
> Hi Wol
>
> thanks for the answer.
>
> My question is about "what to do?".
> Not much what happened.
>
> The log says we should copy some size
> from somewhere to somewhere else.
> The docs says this should be done
> with array at rest.
>
> Is there any documentation about this?
> Should we just ignore the log?
> What should be done, if any?
>
> The log:
>
> md1: echo current LBS to md/logical_block_size to prevent data loss issues from LBS changes.
> Note: After setting, array will not be assembled in old kernels (<= 6.18)
Ah. Personally I wouldn't do anything, then. Are you using any systems
with kernel 6.18 or earlier?
Are you likely to use any kernels 6.18 or earlier in the future?
If the answer to both questions is no, then you're not going to have any
trouble ...
Cheers,
Wol
^ permalink raw reply
* [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
From: Chen Cheng @ 2026-07-11 2:51 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
sb_flags is checked without a lock in md, raid1, raid5, and raid10.
KCSAN reports these reads as data races.
The write side uses atomic bit ops.
The read side still has plain loads in a few places.
Use READ_ONCE() for the lockless reads of sb_flags.
KCSAN reports #1:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_write_start
write (marked) to 0xffff8e39f897f030 of 8 bytes by task 248146 on cpu 8:
md_write_start+0x5dd/0x910
raid10_make_request+0x9b/0x1080
md_handle_request+0x4a2/0xa40
[........]
read to 0xffff8e39f897f030 of 8 bytes by task 250445 on cpu 11:
md_check_recovery+0x574/0x900
raid10d+0xb7/0x2950
[........]
KCSAN reports #2:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_write_start
write (marked) to 0xffff8e39e953f030 of 8 bytes by task 540091 on cpu 11:
md_write_start+0x5dd/0x910
raid1_make_request+0x141/0x1990
[........]
read to 0xffff8e39e953f030 of 8 bytes by task 580822 on cpu 0:
md_check_recovery+0x574/0x900
raid1d+0xcc/0x3840
[........]
value changed: 0x0000000000000002 -> 0x0000000000000006
KCSAN reports #3:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_do_sync.cold
write (marked) to 0xffff8e39e9404030 of 8 bytes by task 492473 on cpu 6:
md_do_sync.cold+0x3f6/0x1686
[........]
read to 0xffff8e39e9404030 of 8 bytes by task 492402 on cpu 3:
md_check_recovery+0x16d/0x900
raid1d+0xcc/0x3840
[........]
value changed: 0x0000000000000000 -> 0x0000000000000002
KCSAN reports #4:
======================================
BUG: KCSAN: data-race in md_do_sync.cold / raid5d
write (marked) to 0xffff8e39c35cb030 of 8 bytes by task 192196 on cpu 10:
md_do_sync.cold+0x3f6/0x1686
md_thread+0x15a/0x2d0
[........]
read to 0xffff8e39c35cb030 of 8 bytes by task 190759 on cpu 5:
raid5d+0x7f9/0xba0
md_thread+0x15a/0x2d0
[........]
value changed: 0x0000000000000000 -> 0x0000000000000002
Fixes: 2953079c692d ("md: separate flags for superblock changes")
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
v1 -> v2
- pass checkpatch.pl check.
---
drivers/md/md.c | 8 ++++----
drivers/md/raid1.c | 2 +-
drivers/md/raid10.c | 4 ++--
drivers/md/raid5.c | 7 ++++---
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 096bb64e87bd..e72bf2010287 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6830,11 +6830,11 @@ int md_run(struct mddev *mddev)
* via sysfs - until a lack of spares is confirmed.
*/
set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
- if (mddev->sb_flags)
+ if (READ_ONCE(mddev->sb_flags))
md_update_sb(mddev, 0);
if (IS_ENABLED(CONFIG_MD_BITMAP) && !mddev->bitmap_info.file &&
!mddev->bitmap_info.offset)
md_bitmap_set_none(mddev);
@@ -7024,11 +7024,11 @@ static void __md_stop_writes(struct mddev *mddev)
mddev->bitmap_ops->flush(mddev);
}
if (md_is_rdwr(mddev) &&
((!mddev->in_sync && !mddev_is_clustered(mddev)) ||
- mddev->sb_flags)) {
+ READ_ONCE(mddev->sb_flags))) {
/* mark array as shutdown cleanly */
if (!mddev_is_clustered(mddev))
mddev->in_sync = 1;
md_update_sb(mddev, 1);
}
@@ -10294,11 +10294,11 @@ static bool md_should_do_recovery(struct mddev *mddev)
/*
* MD_SB_CHANGE_PENDING indicates that the array is switching from clean to
* active, and no action is needed for now.
* All other MD_SB_* flags require to update the superblock.
*/
- if (mddev->sb_flags & ~ (1<<MD_SB_CHANGE_PENDING))
+ if (READ_ONCE(mddev->sb_flags) & ~(1<<MD_SB_CHANGE_PENDING))
return true;
/*
* If the array is not using external metadata and there has been no data
* written for some time, then the array's status needs to be set to
@@ -10423,11 +10423,11 @@ void md_check_recovery(struct mddev *mddev)
spin_lock(&mddev->lock);
set_in_sync(mddev);
spin_unlock(&mddev->lock);
}
- if (mddev->sb_flags)
+ if (READ_ONCE(mddev->sb_flags))
md_update_sb(mddev, 0);
/*
* Never start a new sync thread if MD_RECOVERY_RUNNING is
* still set.
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 29b58583e381..bd6808656edb 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2738,11 +2738,11 @@ static void raid1d(struct md_thread *thread)
handle_read_error(conf, r1_bio);
else
WARN_ON_ONCE(1);
cond_resched();
- if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
+ if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
md_check_recovery(mddev);
}
blk_finish_plug(&plug);
}
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index adaf9e432e25..3ffa5a19964d 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3030,11 +3030,11 @@ static void raid10d(struct md_thread *thread)
handle_read_error(mddev, r10_bio);
else
WARN_ON_ONCE(1);
cond_resched();
- if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
+ if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
md_check_recovery(mddev);
}
blk_finish_plug(&plug);
}
@@ -4698,11 +4698,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
else
mddev->curr_resync_completed = conf->reshape_progress;
conf->reshape_checkpoint = jiffies;
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
- wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
+ wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
test_bit(MD_RECOVERY_INTR, &mddev->recovery));
if (test_bit(MD_RECOVERY_INTR, &mddev->recovery)) {
allow_barrier(conf);
return sectors_done;
}
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ded6a69f7795..cb58b4353995 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -1219,11 +1219,11 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
break;
if (bad < 0) {
set_bit(BlockedBadBlocks, &rdev->flags);
if (!conf->mddev->external &&
- conf->mddev->sb_flags) {
+ READ_ONCE(conf->mddev->sb_flags)) {
/* It is very unlikely, but we might
* still need to write out the
* bad block log - better give it
* a chance*/
md_check_recovery(conf->mddev);
@@ -6469,11 +6469,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
rdev->recovery_offset = sector_nr;
conf->reshape_checkpoint = jiffies;
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
- wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
+ wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
test_bit(MD_RECOVERY_INTR, &mddev->recovery));
if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
return 0;
spin_lock_irq(&conf->device_lock);
conf->reshape_safe = mddev->reshape_position;
@@ -6913,11 +6913,12 @@ static void raid5d(struct md_thread *thread)
conf->temp_inactive_list);
if (!batch_size && !released)
break;
handled += batch_size;
- if (mddev->sb_flags & ~(1 << MD_SB_CHANGE_PENDING)) {
+ if (READ_ONCE(mddev->sb_flags) &
+ ~(1 << MD_SB_CHANGE_PENDING)) {
spin_unlock_irq(&conf->device_lock);
md_check_recovery(mddev);
spin_lock_irq(&conf->device_lock);
}
}
--
2.54.0
^ permalink raw reply related
* Re: [RFC PATCH] md,dm: caller-owned memalloc_noio token in mddev_suspend()/resume()
From: Chen Cheng @ 2026-07-11 3:53 UTC (permalink / raw)
To: yukuai, linux-raid, dm-devel; +Cc: linux-kernel
In-Reply-To: <da94b991-fe62-4a8b-b448-c4ee226b6b4f@fygo.io>
在 2026/7/5 21:10, yu kuai 写道:
> Hi,
>
> 在 2026/6/24 18:13, Chen Cheng 写道:
>> From: Chen Cheng <chencheng@fnnas.com>
>>
>> Save token as mddev-scoped in mddev->noio_flags cause PF_MEMALLOC_NOIO
>> leak into task A, while task B restores a token that it never saved.
>>
>> scenario:
>>
>> task A mddev task B
>> ====== ======= ============
>> write suspend_lo
>> mddev_suspend()
>> suspended == 0
>> drain active_io
>> suspended = 1
>> A: noio_A = memalloc_noio_save()
>> A returns with PF_MEMALLOC_NOIO set
>>
>> write suspend_hi
>> mddev_suspend()
>> suspended == 1
>> suspended = 2
>> B returns
>> (no save)
>>
>> mddev_resume()
>> suspended = 1
>> not last resume
>> A returns
>> A still has PF_MEMALLOC_NOIO <-- leaked
>>
>> mddev_resume()
>> suspended = 0
>> memalloc_noio_restore(noio_A)
>> (restores A's token in B)
>>
>> Fixed by:
>> - return each caller's noio_flags from mddev_suspend()
>> - pass that token back into mddev_resume()
>> - update the suspend-and-lock helpers to carry the token
>> - store the token in struct raid_set for dm-raid paths where suspend
>> and resume are paired across callbacks
>>
>> Validation:
>> repeatedly updates the array's suspend_lo and suspend_hi sysfs from many
>> concurrent userspace workers. That makes multiple tasks to call
>> mddev_suspend()/mddev_resume() concurrently.
>>
>> Each worker:
>> - reads its initial /proc/self/stat flags and verifies that PF_MEMALLOC_NOIO is not already
>> set
>> - writes 0 to either suspend_lo or suspend_hi
>> - immediately reads its own task flags again
>> - reports success if flags & PF_MEMALLOC_NOIO is true after the write returns
>>
>> Link: https://github.com/chencheng-fnnas/reproducer/blob/main/repro-md-noio-token-leak.sh
>>
>> Fixes: 78f57ef9d50a ("md: use memalloc scope APIs in mddev_suspend()/mddev_resume()")
>>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> drivers/md/dm-raid.c | 7 ++--
>> drivers/md/md-autodetect.c | 5 ++-
>> drivers/md/md-bitmap.c | 12 +++---
>> drivers/md/md.c | 85 ++++++++++++++++++++++----------------
>> drivers/md/md.h | 23 ++++++-----
>> drivers/md/raid5-cache.c | 11 +++--
>> drivers/md/raid5.c | 25 ++++++-----
>> 7 files changed, 97 insertions(+), 71 deletions(-)
>>
>> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
>> index 8f5a5e1342a9..d89207e3722a 100644
>> --- a/drivers/md/dm-raid.c
>> +++ b/drivers/md/dm-raid.c
>> @@ -239,10 +239,11 @@ struct raid_set {
>> int raid_disks;
>> int delta_disks;
>> int data_offset;
>> int raid10_copies;
>> int requested_bitmap_chunk_sectors;
>> + unsigned int suspend_noio_flags;
>>
>> struct mddev md;
>> struct raid_type *raid_type;
>>
>> sector_t array_sectors;
>> @@ -3251,11 +3252,11 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>> /* Start raid set read-only and assumed clean to change in raid_resume() */
>> rs->md.ro = MD_RDONLY;
>> rs->md.in_sync = 1;
>>
>> /* Has to be held on running the array */
>> - mddev_suspend_and_lock_nointr(&rs->md);
>> + mddev_suspend_and_lock_nointr(&rs->md, &rs->suspend_noio_flags);
>>
>> /* Keep array frozen until resume. */
>> md_frozen_sync_thread(&rs->md);
>>
>> r = md_run(&rs->md);
>> @@ -3863,11 +3864,11 @@ static void raid_postsuspend(struct dm_target *ti)
>> /*
>> * sync_thread must be stopped during suspend, and writes have
>> * to be stopped before suspending to avoid deadlocks.
>> */
>> md_stop_writes(&rs->md);
>> - mddev_suspend(&rs->md, false);
>> + mddev_suspend(&rs->md, false, &rs->suspend_noio_flags);
>> rs->md.ro = MD_RDONLY;
>> }
>> clear_bit(MD_DM_SUSPENDING, &mddev->flags);
>>
>> }
>> @@ -4141,11 +4142,11 @@ static void raid_resume(struct dm_target *ti)
>> lockdep_is_held(&mddev->reconfig_mutex)));
>> clear_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
>> mddev->ro = MD_RDWR;
>> mddev->in_sync = 0;
>> md_unfrozen_sync_thread(mddev);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, rs->suspend_noio_flags);
>> }
>> }
>
> For mdraid, changes looks fine. However, for dm-raid, for example:
>
> dmsetup suspend ...
>
> In this case, array will be suspended while task returned to user. Take a look at
> the commit to introduce memalloc_noio_save(), it's supposed to be called for mdraid
> arrays to allocate memory while array is suspended, I don't see why dm-raid need it.
> So I'll suggest just bypass the memalloc_noio_save() for dm-raid where mddev_suspend()
> is not paired with mddev_resume() in the same task context.
>
Hi Kuai.
I check the commit you mentioned., and I truelly misused
memalloc_noio_save() in dmraid side, really thanks for point-out.
thanks,
cheng.
>>
>> static struct target_type raid_target = {
>> .name = "raid",
>> diff --git a/drivers/md/md-autodetect.c b/drivers/md/md-autodetect.c
>> index 4b80165afd23..58e062cd0580 100644
>> --- a/drivers/md/md-autodetect.c
>> +++ b/drivers/md/md-autodetect.c
>> @@ -126,10 +126,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
>> dev_t devices[MD_SB_DISKS + 1], mdev;
>> struct mdu_array_info_s ainfo = { };
>> struct mddev *mddev;
>> int err = 0, i;
>> char name[16];
>> + unsigned int noio_flags;
>>
>> if (args->partitioned) {
>> mdev = MKDEV(mdp_major, args->minor << MdpMinorShift);
>> sprintf(name, "md_d%d", args->minor);
>> } else {
>> @@ -173,11 +174,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
>> if (IS_ERR(mddev)) {
>> pr_err("md: md_alloc failed - cannot start array %s\n", name);
>> return;
>> }
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err) {
>> pr_err("md: failed to lock array %s\n", name);
>> goto out_mddev_put;
>> }
>>
>> @@ -219,11 +220,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
>> if (!err)
>> err = do_md_run(mddev);
>> if (err)
>> pr_warn("md: starting %s failed\n", name);
>> out_unlock:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> out_mddev_put:
>> mddev_put(mddev);
>> }
>>
>> static int __init raid_setup(char *str)
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index 028b9ca8ce52..74b7f569a3f4 100644
>> --- a/drivers/md/md-bitmap.c
>> +++ b/drivers/md/md-bitmap.c
>> @@ -2620,13 +2620,14 @@ location_show(struct mddev *mddev, char *page)
>> }
>>
>> static ssize_t
>> location_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> + unsigned int noio_flags;
>> int rv;
>>
>> - rv = mddev_suspend_and_lock(mddev);
>> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (rv)
>> return rv;
>>
>> if (mddev->pers) {
>> if (mddev->recovery || mddev->sync_thread) {
>> @@ -2711,11 +2712,11 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
>> set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
>> md_wakeup_thread(mddev->thread);
>> }
>> rv = 0;
>> out:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> if (rv)
>> return rv;
>> return len;
>>
>> merge_err:
>> @@ -2831,17 +2832,18 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> unsigned long backlog;
>> unsigned long old_mwb = mddev->bitmap_info.max_write_behind;
>> struct md_rdev *rdev;
>> bool has_write_mostly = false;
>> + unsigned int noio_flags;
>> int rv = kstrtoul(buf, 10, &backlog);
>> if (rv)
>> return rv;
>> if (backlog > COUNTER_MAX)
>> return -EINVAL;
>>
>> - rv = mddev_suspend_and_lock(mddev);
>> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (rv)
>> return rv;
>>
>> /*
>> * Without write mostly device, it doesn't make sense to set
>> @@ -2854,11 +2856,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
>> }
>> }
>> if (!has_write_mostly) {
>> pr_warn_ratelimited("%s: can't set backlog, no write mostly device available\n",
>> mdname(mddev));
>> - mddev_unlock(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return -EINVAL;
>> }
>>
>> mddev->bitmap_info.max_write_behind = backlog;
>> if (!backlog && mddev->serial_info_pool) {
>> @@ -2871,11 +2873,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
>> mddev_create_serial_pool(mddev, rdev);
>> }
>> if (old_mwb != backlog)
>> bitmap_update_sb(mddev->bitmap);
>>
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return len;
>> }
>>
>> static struct md_sysfs_entry bitmap_backlog =
>> __ATTR(backlog, S_IRUGO|S_IWUSR, backlog_show, backlog_store);
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 1377c407614c..86d938dee50a 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -459,11 +459,12 @@ static void md_submit_bio(struct bio *bio)
>>
>> /*
>> * Make sure no new requests are submitted to the device, and any requests that
>> * have been submitted are completely handled.
>> */
>> -int mddev_suspend(struct mddev *mddev, bool interruptible)
>> +int mddev_suspend(struct mddev *mddev, bool interruptible,
>> + unsigned int *noio_flags)
>> {
>> int err = 0;
>>
>> /*
>> * hold reconfig_mutex to wait for normal io will deadlock, because
>> @@ -478,10 +479,11 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
>> mutex_lock(&mddev->suspend_mutex);
>> if (err)
>> return err;
>>
>> if (mddev->suspended) {
>> + *noio_flags = memalloc_noio_save();
>> WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
>> mutex_unlock(&mddev->suspend_mutex);
>> return 0;
>> }
>>
>> @@ -515,31 +517,30 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
>> * prevent deadlock.
>> */
>> WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
>>
>> /* restrict memory reclaim I/O during raid array is suspend */
>> - mddev->noio_flag = memalloc_noio_save();
>> + *noio_flags = memalloc_noio_save();
>>
>> mutex_unlock(&mddev->suspend_mutex);
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(mddev_suspend);
>>
>> -static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
>> +static void __mddev_resume(struct mddev *mddev, bool recovery_needed,
>> + unsigned int noio_flags)
>> {
>> lockdep_assert_not_held(&mddev->reconfig_mutex);
>>
>> mutex_lock(&mddev->suspend_mutex);
>> + memalloc_noio_restore(noio_flags);
>> WRITE_ONCE(mddev->suspended, mddev->suspended - 1);
>> if (mddev->suspended) {
>> mutex_unlock(&mddev->suspend_mutex);
>> return;
>> }
>>
>> - /* entred the memalloc scope from mddev_suspend() */
>> - memalloc_noio_restore(mddev->noio_flag);
>> -
>> percpu_ref_resurrect(&mddev->active_io);
>> wake_up(&mddev->sb_wait);
>>
>> if (recovery_needed)
>> set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
>> @@ -547,13 +548,13 @@ static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
>> md_wakeup_thread(mddev->sync_thread); /* possibly kick off a reshape */
>>
>> mutex_unlock(&mddev->suspend_mutex);
>> }
>>
>> -void mddev_resume(struct mddev *mddev)
>> +void mddev_resume(struct mddev *mddev, unsigned int noio_flags)
>> {
>> - return __mddev_resume(mddev, true);
>> + return __mddev_resume(mddev, true, noio_flags);
>> }
>> EXPORT_SYMBOL_GPL(mddev_resume);
>>
>> /* sync bdev before setting device to readonly or stopping raid*/
>> static int mddev_set_closing_and_sync_blockdev(struct mddev *mddev, int opener_num)
>> @@ -3737,10 +3738,11 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
>> {
>> struct rdev_sysfs_entry *entry = container_of(attr, struct rdev_sysfs_entry, attr);
>> struct md_rdev *rdev = container_of(kobj, struct md_rdev, kobj);
>> struct kernfs_node *kn = NULL;
>> bool suspend = false;
>> + unsigned int noio_flags = 0;
>> ssize_t rv;
>> struct mddev *mddev = READ_ONCE(rdev->mddev);
>>
>> if (!entry->store)
>> return -EIO;
>> @@ -3756,17 +3758,17 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
>> cmd_match(page, "writemostly") ||
>> cmd_match(page, "-writemostly"))
>> suspend = true;
>> }
>>
>> - rv = suspend ? mddev_suspend_and_lock(mddev) : mddev_lock(mddev);
>> + rv = suspend ? mddev_suspend_and_lock(mddev, &noio_flags) : mddev_lock(mddev);
>> if (!rv) {
>> if (rdev->mddev == NULL)
>> rv = -ENODEV;
>> else
>> rv = entry->store(rdev, page, length);
>> - suspend ? mddev_unlock_and_resume(mddev) : mddev_unlock(mddev);
>> + suspend ? mddev_unlock_and_resume(mddev, noio_flags) : mddev_unlock(mddev);
>> }
>>
>> if (kn)
>> sysfs_unbreak_active_protection(kn);
>>
>> @@ -4049,15 +4051,16 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
>> size_t slen = len;
>> struct md_personality *pers, *oldpers;
>> long level;
>> void *priv, *oldpriv;
>> struct md_rdev *rdev;
>> + unsigned int noio_flags;
>>
>> if (slen == 0 || slen >= sizeof(clevel))
>> return -EINVAL;
>>
>> - rv = mddev_suspend_and_lock(mddev);
>> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (rv)
>> return rv;
>>
>> if (mddev->pers == NULL) {
>> memcpy(mddev->clevel, buf, slen);
>> @@ -4231,11 +4234,11 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
>> md_update_sb(mddev, 1);
>> sysfs_notify_dirent_safe(mddev->sysfs_level);
>> md_new_event();
>> rv = len;
>> out_unlock:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return rv;
>> }
>>
>> static struct md_sysfs_entry md_level =
>> __ATTR(level, S_IRUGO|S_IWUSR, level_show, level_store);
>> @@ -4410,17 +4413,18 @@ static int update_raid_disks(struct mddev *mddev, int raid_disks);
>>
>> static ssize_t
>> raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> unsigned int n;
>> + unsigned int noio_flags;
>> int err;
>>
>> err = kstrtouint(buf, 10, &n);
>> if (err < 0)
>> return err;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> if (mddev->pers) {
>> if (n != mddev->raid_disks)
>> err = update_raid_disks(mddev, n);
>> @@ -4442,11 +4446,11 @@ raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
>> mddev->raid_disks = n;
>> mddev->reshape_backwards = (mddev->delta_disks < 0);
>> } else
>> mddev->raid_disks = n;
>> out_unlock:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return err ? err : len;
>> }
>> static struct md_sysfs_entry md_raid_disks =
>> __ATTR(raid_disks, S_IRUGO|S_IWUSR, raid_disks_show, raid_disks_store);
>>
>> @@ -4822,10 +4826,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
>> char *e;
>> int major = simple_strtoul(buf, &e, 10);
>> int minor;
>> dev_t dev;
>> struct md_rdev *rdev;
>> + unsigned int noio_flags;
>> int err;
>>
>> if (!*buf || *e != ':' || !e[1] || e[1] == '\n')
>> return -EINVAL;
>> minor = simple_strtoul(e+1, &e, 10);
>> @@ -4834,11 +4839,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
>> dev = MKDEV(major, minor);
>> if (major != MAJOR(dev) ||
>> minor != MINOR(dev))
>> return -EOVERFLOW;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> if (mddev->persistent) {
>> rdev = md_import_device(dev, mddev->major_version,
>> mddev->minor_version);
>> @@ -4855,18 +4860,18 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
>> rdev = md_import_device(dev, -2, -1);
>> else
>> rdev = md_import_device(dev, -1, -1);
>>
>> if (IS_ERR(rdev)) {
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return PTR_ERR(rdev);
>> }
>> err = bind_rdev_to_array(rdev, mddev);
>> out:
>> if (err)
>> export_rdev(rdev);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> if (!err)
>> md_new_event();
>> return err ? err : len;
>> }
>>
>> @@ -5257,28 +5262,29 @@ static int mddev_start_reshape(struct mddev *mddev)
>> static ssize_t
>> action_store(struct mddev *mddev, const char *page, size_t len)
>> {
>> int ret;
>> enum sync_action action;
>> + unsigned int noio_flags = 0;
>>
>> if (!mddev->pers || !mddev->pers->sync_request)
>> return -EINVAL;
>>
>> action = md_sync_action_by_name(page);
>> retry:
>> if (work_busy(&mddev->sync_work))
>> flush_work(&mddev->sync_work);
>>
>> ret = (action == ACTION_RESHAPE) ?
>> - mddev_suspend_and_lock(mddev) :
>> + mddev_suspend_and_lock(mddev, &noio_flags) :
>> mddev_lock(mddev);
>> if (ret)
>> return ret;
>>
>> if (work_busy(&mddev->sync_work)) {
>> if (action == ACTION_RESHAPE)
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> else
>> mddev_unlock(mddev);
>> goto retry;
>> }
>>
>> @@ -5349,11 +5355,11 @@ action_store(struct mddev *mddev, const char *page, size_t len)
>> sysfs_notify_dirent_safe(mddev->sysfs_action);
>> ret = len;
>>
>> out:
>> if (action == ACTION_RESHAPE)
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> else
>> mddev_unlock(mddev);
>> return ret;
>> }
>>
>> @@ -5640,24 +5646,25 @@ suspend_lo_show(struct mddev *mddev, char *page)
>>
>> static ssize_t
>> suspend_lo_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> unsigned long long new;
>> + unsigned int noio_flags;
>> int err;
>>
>> err = kstrtoull(buf, 10, &new);
>> if (err < 0)
>> return err;
>> if (new != (sector_t)new)
>> return -EINVAL;
>>
>> - err = mddev_suspend(mddev, true);
>> + err = mddev_suspend(mddev, true, &noio_flags);
>> if (err)
>> return err;
>>
>> WRITE_ONCE(mddev->suspend_lo, new);
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, noio_flags);
>>
>> return len;
>> }
>> static struct md_sysfs_entry md_suspend_lo =
>> __ATTR(suspend_lo, S_IRUGO|S_IWUSR, suspend_lo_show, suspend_lo_store);
>> @@ -5671,24 +5678,25 @@ suspend_hi_show(struct mddev *mddev, char *page)
>>
>> static ssize_t
>> suspend_hi_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> unsigned long long new;
>> + unsigned int noio_flags;
>> int err;
>>
>> err = kstrtoull(buf, 10, &new);
>> if (err < 0)
>> return err;
>> if (new != (sector_t)new)
>> return -EINVAL;
>>
>> - err = mddev_suspend(mddev, true);
>> + err = mddev_suspend(mddev, true, &noio_flags);
>> if (err)
>> return err;
>>
>> WRITE_ONCE(mddev->suspend_hi, new);
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, noio_flags);
>>
>> return len;
>> }
>> static struct md_sysfs_entry md_suspend_hi =
>> __ATTR(suspend_hi, S_IRUGO|S_IWUSR, suspend_hi_show, suspend_hi_store);
>> @@ -5928,19 +5936,20 @@ static ssize_t serialize_policy_show(struct mddev *mddev, char *page)
>> static ssize_t
>> serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
>> {
>> int err;
>> bool value;
>> + unsigned int noio_flags;
>>
>> err = kstrtobool(buf, &value);
>> if (err)
>> return err;
>>
>> if (value == test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
>> return len;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> if (mddev->pers == NULL || (mddev->pers->head.id != ID_RAID1)) {
>> pr_err("md: serialize_policy is only effective for raid1\n");
>> err = -EINVAL;
>> @@ -5953,11 +5962,11 @@ serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
>> } else {
>> mddev_destroy_serial_pool(mddev, NULL);
>> clear_bit(MD_SERIALIZE_POLICY, &mddev->flags);
>> }
>> unlock:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return err ?: len;
>> }
>>
>> static struct md_sysfs_entry md_serialize_policy =
>> __ATTR(serialize_policy, S_IRUGO | S_IWUSR, serialize_policy_show,
>> @@ -6263,21 +6272,22 @@ EXPORT_SYMBOL_GPL(mddev_stack_new_rdev);
>>
>> /* update the optimal I/O size after a reshape */
>> void mddev_update_io_opt(struct mddev *mddev, unsigned int nr_stripes)
>> {
>> struct queue_limits lim;
>> + unsigned int noio_flags;
>>
>> if (mddev_is_dm(mddev))
>> return;
>>
>> /* don't bother updating io_opt if we can't suspend the array */
>> - if (mddev_suspend(mddev, false) < 0)
>> + if (mddev_suspend(mddev, false, &noio_flags) < 0)
>> return;
>> lim = queue_limits_start_update(mddev->gendisk->queue);
>> lim.io_opt = lim.io_min * nr_stripes;
>> queue_limits_commit_update(mddev->gendisk->queue, &lim);
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, noio_flags);
>> }
>> EXPORT_SYMBOL_GPL(mddev_update_io_opt);
>>
>> static void mddev_delayed_delete(struct work_struct *ws)
>> {
>> @@ -7255,10 +7265,11 @@ static void autorun_array(struct mddev *mddev)
>> */
>> static void autorun_devices(int part)
>> {
>> struct md_rdev *rdev0, *rdev, *tmp;
>> struct mddev *mddev;
>> + unsigned int noio_flags;
>>
>> pr_info("md: autorun ...\n");
>> while (!list_empty(&pending_raid_disks)) {
>> int unit;
>> dev_t dev;
>> @@ -7295,27 +7306,27 @@ static void autorun_devices(int part)
>>
>> mddev = md_alloc(dev, NULL);
>> if (IS_ERR(mddev))
>> break;
>>
>> - if (mddev_suspend_and_lock(mddev))
>> + if (mddev_suspend_and_lock(mddev, &noio_flags))
>> pr_warn("md: %s locked, cannot run\n", mdname(mddev));
>> else if (mddev->raid_disks || mddev->major_version
>> || !list_empty(&mddev->disks)) {
>> pr_warn("md: %s already running, cannot run %pg\n",
>> mdname(mddev), rdev0->bdev);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> } else {
>> pr_debug("md: created %s\n", mdname(mddev));
>> mddev->persistent = 1;
>> rdev_for_each_list(rdev, tmp, &candidates) {
>> list_del_init(&rdev->same_set);
>> if (bind_rdev_to_array(rdev, mddev))
>> export_rdev(rdev);
>> }
>> autorun_array(mddev);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> }
>> /* on success, candidates will be empty, on error
>> * it won't...
>> */
>> rdev_for_each_list(rdev, tmp, &candidates) {
>> @@ -8329,10 +8340,11 @@ static int __md_set_array_info(struct mddev *mddev, void __user *argp)
>>
>> static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
>> unsigned int cmd, unsigned long arg)
>> {
>> int err = 0;
>> + unsigned int noio_flags = 0;
>> void __user *argp = (void __user *)arg;
>> struct mddev *mddev = NULL;
>>
>> err = md_ioctl_valid(cmd);
>> if (err)
>> @@ -8380,11 +8392,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
>> }
>>
>> if (!md_is_rdwr(mddev))
>> flush_work(&mddev->sync_work);
>>
>> - err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev) :
>> + err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev, &noio_flags) :
>> mddev_lock(mddev);
>> if (err) {
>> pr_debug("md: ioctl lock interrupted, reason %d, cmd %d\n",
>> err, cmd);
>> goto out;
>> @@ -8511,11 +8523,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
>> unlock:
>> if (mddev->hold_active == UNTIL_IOCTL &&
>> err != -EINVAL)
>> mddev->hold_active = 0;
>>
>> - md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev) :
>> + md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev, noio_flags) :
>> mddev_unlock(mddev);
>>
>> out:
>> if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
>> clear_bit(MD_CLOSING, &mddev->flags);
>> @@ -10180,20 +10192,21 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
>> static void md_start_sync(struct work_struct *ws)
>> {
>> struct mddev *mddev = container_of(ws, struct mddev, sync_work);
>> int spares = 0;
>> bool suspend = false;
>> + unsigned int noio_flags = 0;
>> char *name;
>>
>> /*
>> * If reshape is still in progress, spares won't be added or removed
>> * from conf until reshape is done.
>> */
>> if (mddev->reshape_position == MaxSector &&
>> md_spares_need_change(mddev)) {
>> suspend = true;
>> - mddev_suspend(mddev, false);
>> + mddev_suspend(mddev, false, &noio_flags);
>> }
>>
>> mddev_lock_nointr(mddev);
>> if (!md_is_rdwr(mddev)) {
>> /*
>> @@ -10237,11 +10250,11 @@ static void md_start_sync(struct work_struct *ws)
>> * not set it again. Otherwise, we may cause issue like this one:
>> * https://bugzilla.kernel.org/show_bug.cgi?id=218200
>> * Therefore, use __mddev_resume(mddev, false).
>> */
>> if (suspend)
>> - __mddev_resume(mddev, false);
>> + __mddev_resume(mddev, false, noio_flags);
>> md_wakeup_thread(mddev->sync_thread);
>> sysfs_notify_dirent_safe(mddev->sysfs_action);
>> md_new_event();
>> return;
>>
>> @@ -10257,11 +10270,11 @@ static void md_start_sync(struct work_struct *ws)
>> * not set it again. Otherwise, we may cause issue like this one:
>> * https://bugzilla.kernel.org/show_bug.cgi?id=218200
>> * Therefore, use __mddev_resume(mddev, false).
>> */
>> if (suspend)
>> - __mddev_resume(mddev, false);
>> + __mddev_resume(mddev, false, noio_flags);
>>
>> wake_up(&resync_wait);
>> if (test_and_clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery) &&
>> mddev->sysfs_action)
>> sysfs_notify_dirent_safe(mddev->sysfs_action);
>> diff --git a/drivers/md/md.h b/drivers/md/md.h
>> index d8daf0f75cbb..3337cd21eb30 100644
>> --- a/drivers/md/md.h
>> +++ b/drivers/md/md.h
>> @@ -619,11 +619,10 @@ struct mddev {
>> mempool_t *serial_info_pool;
>> void (*sync_super)(struct mddev *mddev, struct md_rdev *rdev);
>> struct md_cluster_info *cluster_info;
>> struct md_cluster_operations *cluster_ops;
>> unsigned int good_device_nr; /* good device num within cluster raid */
>> - unsigned int noio_flag; /* for memalloc scope API */
>>
>> /*
>> * Temporarily store rdev that will be finally removed when
>> * reconfig_mutex is unlocked, protected by reconfig_mutex.
>> */
>> @@ -953,12 +952,13 @@ extern void md_stop(struct mddev *mddev);
>> extern void md_stop_writes(struct mddev *mddev);
>> extern int md_rdev_init(struct md_rdev *rdev);
>> extern void md_rdev_clear(struct md_rdev *rdev);
>>
>> extern bool md_handle_request(struct mddev *mddev, struct bio *bio);
>> -extern int mddev_suspend(struct mddev *mddev, bool interruptible);
>> -extern void mddev_resume(struct mddev *mddev);
>> +extern int mddev_suspend(struct mddev *mddev, bool interruptible,
>> + unsigned int *noio_flags);
>> +extern void mddev_resume(struct mddev *mddev, unsigned int noio_flags);
>> extern void md_idle_sync_thread(struct mddev *mddev);
>> extern void md_frozen_sync_thread(struct mddev *mddev);
>> extern void md_unfrozen_sync_thread(struct mddev *mddev);
>>
>> extern void md_update_sb(struct mddev *mddev, int force);
>> @@ -999,35 +999,38 @@ static inline void mddev_check_write_zeroes(struct mddev *mddev, struct bio *bio
>> if (bio_op(bio) == REQ_OP_WRITE_ZEROES &&
>> !bio->bi_bdev->bd_disk->queue->limits.max_write_zeroes_sectors)
>> mddev->gendisk->queue->limits.max_write_zeroes_sectors = 0;
>> }
>>
>> -static inline int mddev_suspend_and_lock(struct mddev *mddev)
>> +static inline int mddev_suspend_and_lock(struct mddev *mddev,
>> + unsigned int *noio_flags)
>> {
>> int ret;
>>
>> - ret = mddev_suspend(mddev, true);
>> + ret = mddev_suspend(mddev, true, noio_flags);
>> if (ret)
>> return ret;
>>
>> ret = mddev_lock(mddev);
>> if (ret)
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, *noio_flags);
>>
>> return ret;
>> }
>>
>> -static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev)
>> +static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev,
>> + unsigned int *noio_flags)
>> {
>> - mddev_suspend(mddev, false);
>> + mddev_suspend(mddev, false, noio_flags);
>> mddev_lock_nointr(mddev);
>> }
>>
>> -static inline void mddev_unlock_and_resume(struct mddev *mddev)
>> +static inline void mddev_unlock_and_resume(struct mddev *mddev,
>> + unsigned int noio_flags)
>> {
>> mddev_unlock(mddev);
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, noio_flags);
>> }
>>
>> struct mdu_array_info_s;
>> struct mdu_disk_info_s;
>>
>> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
>> index 7b7546bfa21f..6f8e3a624456 100644
>> --- a/drivers/md/raid5-cache.c
>> +++ b/drivers/md/raid5-cache.c
>> @@ -693,13 +693,15 @@ static void r5c_disable_writeback_async(struct work_struct *work)
>> !READ_ONCE(conf->log) ||
>> !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
>>
>> log = READ_ONCE(conf->log);
>> if (log) {
>> - mddev_suspend(mddev, false);
>> + unsigned int noio_flags;
>> +
>> + mddev_suspend(mddev, false, &noio_flags);
>> log->r5c_journal_mode = R5C_JOURNAL_MODE_WRITE_THROUGH;
>> - mddev_resume(mddev);
>> + mddev_resume(mddev, noio_flags);
>> }
>> }
>>
>> static void r5l_submit_current_io(struct r5l_log *log)
>> {
>> @@ -2603,10 +2605,11 @@ EXPORT_SYMBOL(r5c_journal_mode_set);
>> static ssize_t r5c_journal_mode_store(struct mddev *mddev,
>> const char *page, size_t length)
>> {
>> int mode = ARRAY_SIZE(r5c_journal_mode_str);
>> size_t len = length;
>> + unsigned int noio_flags;
>> int ret;
>>
>> if (len < 2)
>> return -EINVAL;
>>
>> @@ -2615,15 +2618,15 @@ static ssize_t r5c_journal_mode_store(struct mddev *mddev,
>>
>> while (mode--)
>> if (strlen(r5c_journal_mode_str[mode]) == len &&
>> !strncmp(page, r5c_journal_mode_str[mode], len))
>> break;
>> - ret = mddev_suspend_and_lock(mddev);
>> + ret = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (ret)
>> return ret;
>> ret = r5c_journal_mode_set(mddev, mode);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return ret ?: length;
>> }
>>
>> struct md_sysfs_entry
>> r5c_journal_mode = __ATTR(journal_mode, 0644,
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index 65ae7d8930fc..6062c4b62cc8 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -6992,11 +6992,11 @@ raid5_show_stripe_size(struct mddev *mddev, char *page)
>> #if PAGE_SIZE != DEFAULT_STRIPE_SIZE
>> static ssize_t
>> raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
>> {
>> struct r5conf *conf;
>> - unsigned long new;
>> + unsigned long new, noio_flags;
>> int err;
>> int size;
>>
>> if (len >= PAGE_SIZE)
>> return -EINVAL;
>> @@ -7011,11 +7011,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
>> if (new % DEFAULT_STRIPE_SIZE != 0 ||
>> new > PAGE_SIZE || new == 0 ||
>> new != roundup_pow_of_two(new))
>> return -EINVAL;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>>
>> conf = mddev->private;
>> if (!conf) {
>> @@ -7049,11 +7049,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
>> err = -ENOMEM;
>> }
>> mutex_unlock(&conf->cache_size_mutex);
>>
>> out_unlock:
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return err ?: len;
>> }
>>
>> static struct md_sysfs_entry
>> raid5_stripe_size = __ATTR(stripe_size, 0644,
>> @@ -7127,19 +7127,20 @@ raid5_show_skip_copy(struct mddev *mddev, char *page)
>> static ssize_t
>> raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
>> {
>> struct r5conf *conf;
>> unsigned long new;
>> + unsigned int noio_flags;
>> int err;
>>
>> if (len >= PAGE_SIZE)
>> return -EINVAL;
>> if (kstrtoul(page, 10, &new))
>> return -EINVAL;
>> new = !!new;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> conf = mddev->private;
>> if (!conf)
>> err = -ENODEV;
>> @@ -7152,11 +7153,11 @@ raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
>> lim.features |= BLK_FEAT_STABLE_WRITES;
>> else
>> lim.features &= ~BLK_FEAT_STABLE_WRITES;
>> err = queue_limits_commit_update(q, &lim);
>> }
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return err ?: len;
>> }
>>
>> static struct md_sysfs_entry
>> raid5_skip_copy = __ATTR(skip_copy, S_IRUGO | S_IWUSR,
>> @@ -7195,10 +7196,11 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt,
>> static ssize_t
>> raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
>> {
>> struct r5conf *conf;
>> unsigned int new;
>> + unsigned int noio_flags;
>> int err;
>> struct r5worker_group *new_groups, *old_groups;
>> int group_cnt;
>>
>> if (len >= PAGE_SIZE)
>> @@ -7207,16 +7209,16 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
>> return -EINVAL;
>> /* 8192 should be big enough */
>> if (new > 8192)
>> return -EINVAL;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> conf = mddev->private;
>> if (!conf) {
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return -ENODEV;
>> }
>> raid5_quiesce(mddev, true);
>>
>> if (new != conf->worker_cnt_per_group) {
>> @@ -7237,11 +7239,11 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
>> kfree(old_groups);
>> }
>> }
>>
>> raid5_quiesce(mddev, false);
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>>
>> return err ?: len;
>> }
>>
>> static struct md_sysfs_entry
>> @@ -8940,18 +8942,19 @@ static void *raid6_takeover(struct mddev *mddev)
>> }
>>
>> static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
>> {
>> struct r5conf *conf;
>> + unsigned int noio_flags;
>> int err;
>>
>> - err = mddev_suspend_and_lock(mddev);
>> + err = mddev_suspend_and_lock(mddev, &noio_flags);
>> if (err)
>> return err;
>> conf = mddev->private;
>> if (!conf) {
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>> return -ENODEV;
>> }
>>
>> if (strncmp(buf, "ppl", 3) == 0) {
>> /* ppl only works with RAID 5 */
>> @@ -8990,11 +8993,11 @@ static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
>> }
>>
>> if (!err)
>> md_update_sb(mddev, 1);
>>
>> - mddev_unlock_and_resume(mddev);
>> + mddev_unlock_and_resume(mddev, noio_flags);
>>
>> return err;
>> }
>>
>> static int raid5_start(struct mddev *mddev)
>
^ permalink raw reply
* [PATCH v1] md,dm: caller-owned memalloc_noio token in mddev_suspend()/resume()
From: Chen Cheng @ 2026-07-11 6:48 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
Save token as mddev-scoped in mddev->noio_flags cause PF_MEMALLOC_NOIO
leak into task A, while task B restores a token that it never saved.
scenario:
task A mddev task B
====== ======= ============
write suspend_lo
mddev_suspend()
suspended == 0
drain active_io
suspended = 1
A: noio_A = memalloc_noio_save()
A returns with PF_MEMALLOC_NOIO set
write suspend_hi
mddev_suspend()
suspended == 1
suspended = 2
B returns
(no save)
mddev_resume()
suspended = 1
not last resume
A returns
A still has PF_MEMALLOC_NOIO <-- leaked
mddev_resume()
suspended = 0
memalloc_noio_restore(noio_A)
(restores A's token in B)
Fixed by:
for md-raid side:
- return each caller's noio_flags from mddev_suspend()
- pass that token back into mddev_resume()
- update the suspend-and-lock helpers to carry the token
for dm-raid side:
- just bypass because: dm-raid call suspend() or resume() will return to
userland, so is not paired in the same task context.
Validation:
repeatedly updates the array's suspend_lo and suspend_hi sysfs from many
concurrent userspace workers. That makes multiple tasks to call
mddev_suspend()/mddev_resume() concurrently.
Each worker:
- reads its initial /proc/self/stat flags and verifies that PF_MEMALLOC_NOIO is not already
set
- writes 0 to either suspend_lo or suspend_hi
- immediately reads its own task flags again
- reports success if flags & PF_MEMALLOC_NOIO is true after the write returns
Link: https://github.com/chencheng-fnnas/reproducer/blob/main/repro-md-noio-token-leak.sh
Fixes: 78f57ef9d50a ("md: use memalloc scope APIs in mddev_suspend()/mddev_resume()")
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
rfc -> v1:
- dm-raid side just bypass because suspend and resume not in the
same task context..
---
drivers/md/dm-raid.c | 6 +--
drivers/md/md-autodetect.c | 5 ++-
drivers/md/md-bitmap.c | 12 ++---
drivers/md/md.c | 90 +++++++++++++++++++++++---------------
drivers/md/md.h | 23 +++++-----
drivers/md/raid5-cache.c | 11 +++--
drivers/md/raid5.c | 24 +++++-----
7 files changed, 101 insertions(+), 70 deletions(-)
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 8f5a5e1342a9..ce0f562c5da3 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -3251,11 +3251,11 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
/* Start raid set read-only and assumed clean to change in raid_resume() */
rs->md.ro = MD_RDONLY;
rs->md.in_sync = 1;
/* Has to be held on running the array */
- mddev_suspend_and_lock_nointr(&rs->md);
+ mddev_suspend_and_lock_nointr(&rs->md, NULL);
/* Keep array frozen until resume. */
md_frozen_sync_thread(&rs->md);
r = md_run(&rs->md);
@@ -3863,11 +3863,11 @@ static void raid_postsuspend(struct dm_target *ti)
/*
* sync_thread must be stopped during suspend, and writes have
* to be stopped before suspending to avoid deadlocks.
*/
md_stop_writes(&rs->md);
- mddev_suspend(&rs->md, false);
+ mddev_suspend(&rs->md, false, NULL);
rs->md.ro = MD_RDONLY;
}
clear_bit(MD_DM_SUSPENDING, &mddev->flags);
}
@@ -4141,11 +4141,11 @@ static void raid_resume(struct dm_target *ti)
lockdep_is_held(&mddev->reconfig_mutex)));
clear_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
mddev->ro = MD_RDWR;
mddev->in_sync = 0;
md_unfrozen_sync_thread(mddev);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, 0);
}
}
static struct target_type raid_target = {
.name = "raid",
diff --git a/drivers/md/md-autodetect.c b/drivers/md/md-autodetect.c
index 4b80165afd23..58e062cd0580 100644
--- a/drivers/md/md-autodetect.c
+++ b/drivers/md/md-autodetect.c
@@ -126,10 +126,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
dev_t devices[MD_SB_DISKS + 1], mdev;
struct mdu_array_info_s ainfo = { };
struct mddev *mddev;
int err = 0, i;
char name[16];
+ unsigned int noio_flags;
if (args->partitioned) {
mdev = MKDEV(mdp_major, args->minor << MdpMinorShift);
sprintf(name, "md_d%d", args->minor);
} else {
@@ -173,11 +174,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
if (IS_ERR(mddev)) {
pr_err("md: md_alloc failed - cannot start array %s\n", name);
return;
}
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err) {
pr_err("md: failed to lock array %s\n", name);
goto out_mddev_put;
}
@@ -219,11 +220,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
if (!err)
err = do_md_run(mddev);
if (err)
pr_warn("md: starting %s failed\n", name);
out_unlock:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
out_mddev_put:
mddev_put(mddev);
}
static int __init raid_setup(char *str)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 028b9ca8ce52..74b7f569a3f4 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2620,13 +2620,14 @@ location_show(struct mddev *mddev, char *page)
}
static ssize_t
location_store(struct mddev *mddev, const char *buf, size_t len)
{
+ unsigned int noio_flags;
int rv;
- rv = mddev_suspend_and_lock(mddev);
+ rv = mddev_suspend_and_lock(mddev, &noio_flags);
if (rv)
return rv;
if (mddev->pers) {
if (mddev->recovery || mddev->sync_thread) {
@@ -2711,11 +2712,11 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
}
rv = 0;
out:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
if (rv)
return rv;
return len;
merge_err:
@@ -2831,17 +2832,18 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
{
unsigned long backlog;
unsigned long old_mwb = mddev->bitmap_info.max_write_behind;
struct md_rdev *rdev;
bool has_write_mostly = false;
+ unsigned int noio_flags;
int rv = kstrtoul(buf, 10, &backlog);
if (rv)
return rv;
if (backlog > COUNTER_MAX)
return -EINVAL;
- rv = mddev_suspend_and_lock(mddev);
+ rv = mddev_suspend_and_lock(mddev, &noio_flags);
if (rv)
return rv;
/*
* Without write mostly device, it doesn't make sense to set
@@ -2854,11 +2856,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
}
}
if (!has_write_mostly) {
pr_warn_ratelimited("%s: can't set backlog, no write mostly device available\n",
mdname(mddev));
- mddev_unlock(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return -EINVAL;
}
mddev->bitmap_info.max_write_behind = backlog;
if (!backlog && mddev->serial_info_pool) {
@@ -2871,11 +2873,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
mddev_create_serial_pool(mddev, rdev);
}
if (old_mwb != backlog)
bitmap_update_sb(mddev->bitmap);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return len;
}
static struct md_sysfs_entry bitmap_backlog =
__ATTR(backlog, S_IRUGO|S_IWUSR, backlog_show, backlog_store);
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 1377c407614c..04e10769d5c6 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -459,11 +459,12 @@ static void md_submit_bio(struct bio *bio)
/*
* Make sure no new requests are submitted to the device, and any requests that
* have been submitted are completely handled.
*/
-int mddev_suspend(struct mddev *mddev, bool interruptible)
+int mddev_suspend(struct mddev *mddev, bool interruptible,
+ unsigned int *noio_flags)
{
int err = 0;
/*
* hold reconfig_mutex to wait for normal io will deadlock, because
@@ -477,11 +478,16 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
else
mutex_lock(&mddev->suspend_mutex);
if (err)
return err;
+ if (noio_flags)
+ *noio_flags = 0;
+
if (mddev->suspended) {
+ if (noio_flags && !mddev_is_dm(mddev))
+ *noio_flags = memalloc_noio_save();
WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
mutex_unlock(&mddev->suspend_mutex);
return 0;
}
@@ -515,31 +521,31 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
* prevent deadlock.
*/
WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
/* restrict memory reclaim I/O during raid array is suspend */
- mddev->noio_flag = memalloc_noio_save();
+ if (noio_flags && !mddev_is_dm(mddev))
+ *noio_flags = memalloc_noio_save();
mutex_unlock(&mddev->suspend_mutex);
return 0;
}
EXPORT_SYMBOL_GPL(mddev_suspend);
-static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
+static void __mddev_resume(struct mddev *mddev, bool recovery_needed,
+ unsigned int noio_flags)
{
lockdep_assert_not_held(&mddev->reconfig_mutex);
mutex_lock(&mddev->suspend_mutex);
+ memalloc_noio_restore(noio_flags);
WRITE_ONCE(mddev->suspended, mddev->suspended - 1);
if (mddev->suspended) {
mutex_unlock(&mddev->suspend_mutex);
return;
}
- /* entred the memalloc scope from mddev_suspend() */
- memalloc_noio_restore(mddev->noio_flag);
-
percpu_ref_resurrect(&mddev->active_io);
wake_up(&mddev->sb_wait);
if (recovery_needed)
set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
@@ -547,13 +553,13 @@ static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
md_wakeup_thread(mddev->sync_thread); /* possibly kick off a reshape */
mutex_unlock(&mddev->suspend_mutex);
}
-void mddev_resume(struct mddev *mddev)
+void mddev_resume(struct mddev *mddev, unsigned int noio_flags)
{
- return __mddev_resume(mddev, true);
+ return __mddev_resume(mddev, true, noio_flags);
}
EXPORT_SYMBOL_GPL(mddev_resume);
/* sync bdev before setting device to readonly or stopping raid*/
static int mddev_set_closing_and_sync_blockdev(struct mddev *mddev, int opener_num)
@@ -3737,10 +3743,11 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
{
struct rdev_sysfs_entry *entry = container_of(attr, struct rdev_sysfs_entry, attr);
struct md_rdev *rdev = container_of(kobj, struct md_rdev, kobj);
struct kernfs_node *kn = NULL;
bool suspend = false;
+ unsigned int noio_flags = 0;
ssize_t rv;
struct mddev *mddev = READ_ONCE(rdev->mddev);
if (!entry->store)
return -EIO;
@@ -3756,17 +3763,17 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
cmd_match(page, "writemostly") ||
cmd_match(page, "-writemostly"))
suspend = true;
}
- rv = suspend ? mddev_suspend_and_lock(mddev) : mddev_lock(mddev);
+ rv = suspend ? mddev_suspend_and_lock(mddev, &noio_flags) : mddev_lock(mddev);
if (!rv) {
if (rdev->mddev == NULL)
rv = -ENODEV;
else
rv = entry->store(rdev, page, length);
- suspend ? mddev_unlock_and_resume(mddev) : mddev_unlock(mddev);
+ suspend ? mddev_unlock_and_resume(mddev, noio_flags) : mddev_unlock(mddev);
}
if (kn)
sysfs_unbreak_active_protection(kn);
@@ -4049,15 +4056,16 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
size_t slen = len;
struct md_personality *pers, *oldpers;
long level;
void *priv, *oldpriv;
struct md_rdev *rdev;
+ unsigned int noio_flags;
if (slen == 0 || slen >= sizeof(clevel))
return -EINVAL;
- rv = mddev_suspend_and_lock(mddev);
+ rv = mddev_suspend_and_lock(mddev, &noio_flags);
if (rv)
return rv;
if (mddev->pers == NULL) {
memcpy(mddev->clevel, buf, slen);
@@ -4231,11 +4239,11 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
md_update_sb(mddev, 1);
sysfs_notify_dirent_safe(mddev->sysfs_level);
md_new_event();
rv = len;
out_unlock:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return rv;
}
static struct md_sysfs_entry md_level =
__ATTR(level, S_IRUGO|S_IWUSR, level_show, level_store);
@@ -4410,17 +4418,18 @@ static int update_raid_disks(struct mddev *mddev, int raid_disks);
static ssize_t
raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
{
unsigned int n;
+ unsigned int noio_flags;
int err;
err = kstrtouint(buf, 10, &n);
if (err < 0)
return err;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
if (mddev->pers) {
if (n != mddev->raid_disks)
err = update_raid_disks(mddev, n);
@@ -4442,11 +4451,11 @@ raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
mddev->raid_disks = n;
mddev->reshape_backwards = (mddev->delta_disks < 0);
} else
mddev->raid_disks = n;
out_unlock:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err ? err : len;
}
static struct md_sysfs_entry md_raid_disks =
__ATTR(raid_disks, S_IRUGO|S_IWUSR, raid_disks_show, raid_disks_store);
@@ -4822,10 +4831,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
char *e;
int major = simple_strtoul(buf, &e, 10);
int minor;
dev_t dev;
struct md_rdev *rdev;
+ unsigned int noio_flags;
int err;
if (!*buf || *e != ':' || !e[1] || e[1] == '\n')
return -EINVAL;
minor = simple_strtoul(e+1, &e, 10);
@@ -4834,11 +4844,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
dev = MKDEV(major, minor);
if (major != MAJOR(dev) ||
minor != MINOR(dev))
return -EOVERFLOW;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
if (mddev->persistent) {
rdev = md_import_device(dev, mddev->major_version,
mddev->minor_version);
@@ -4855,18 +4865,18 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
rdev = md_import_device(dev, -2, -1);
else
rdev = md_import_device(dev, -1, -1);
if (IS_ERR(rdev)) {
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return PTR_ERR(rdev);
}
err = bind_rdev_to_array(rdev, mddev);
out:
if (err)
export_rdev(rdev);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
if (!err)
md_new_event();
return err ? err : len;
}
@@ -5257,28 +5267,29 @@ static int mddev_start_reshape(struct mddev *mddev)
static ssize_t
action_store(struct mddev *mddev, const char *page, size_t len)
{
int ret;
enum sync_action action;
+ unsigned int noio_flags = 0;
if (!mddev->pers || !mddev->pers->sync_request)
return -EINVAL;
action = md_sync_action_by_name(page);
retry:
if (work_busy(&mddev->sync_work))
flush_work(&mddev->sync_work);
ret = (action == ACTION_RESHAPE) ?
- mddev_suspend_and_lock(mddev) :
+ mddev_suspend_and_lock(mddev, &noio_flags) :
mddev_lock(mddev);
if (ret)
return ret;
if (work_busy(&mddev->sync_work)) {
if (action == ACTION_RESHAPE)
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
else
mddev_unlock(mddev);
goto retry;
}
@@ -5349,11 +5360,11 @@ action_store(struct mddev *mddev, const char *page, size_t len)
sysfs_notify_dirent_safe(mddev->sysfs_action);
ret = len;
out:
if (action == ACTION_RESHAPE)
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
else
mddev_unlock(mddev);
return ret;
}
@@ -5640,24 +5651,25 @@ suspend_lo_show(struct mddev *mddev, char *page)
static ssize_t
suspend_lo_store(struct mddev *mddev, const char *buf, size_t len)
{
unsigned long long new;
+ unsigned int noio_flags;
int err;
err = kstrtoull(buf, 10, &new);
if (err < 0)
return err;
if (new != (sector_t)new)
return -EINVAL;
- err = mddev_suspend(mddev, true);
+ err = mddev_suspend(mddev, true, &noio_flags);
if (err)
return err;
WRITE_ONCE(mddev->suspend_lo, new);
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags);
return len;
}
static struct md_sysfs_entry md_suspend_lo =
__ATTR(suspend_lo, S_IRUGO|S_IWUSR, suspend_lo_show, suspend_lo_store);
@@ -5671,24 +5683,25 @@ suspend_hi_show(struct mddev *mddev, char *page)
static ssize_t
suspend_hi_store(struct mddev *mddev, const char *buf, size_t len)
{
unsigned long long new;
+ unsigned int noio_flags;
int err;
err = kstrtoull(buf, 10, &new);
if (err < 0)
return err;
if (new != (sector_t)new)
return -EINVAL;
- err = mddev_suspend(mddev, true);
+ err = mddev_suspend(mddev, true, &noio_flags);
if (err)
return err;
WRITE_ONCE(mddev->suspend_hi, new);
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags);
return len;
}
static struct md_sysfs_entry md_suspend_hi =
__ATTR(suspend_hi, S_IRUGO|S_IWUSR, suspend_hi_show, suspend_hi_store);
@@ -5928,19 +5941,20 @@ static ssize_t serialize_policy_show(struct mddev *mddev, char *page)
static ssize_t
serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
{
int err;
bool value;
+ unsigned int noio_flags;
err = kstrtobool(buf, &value);
if (err)
return err;
if (value == test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
return len;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
if (mddev->pers == NULL || (mddev->pers->head.id != ID_RAID1)) {
pr_err("md: serialize_policy is only effective for raid1\n");
err = -EINVAL;
@@ -5953,11 +5967,11 @@ serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
} else {
mddev_destroy_serial_pool(mddev, NULL);
clear_bit(MD_SERIALIZE_POLICY, &mddev->flags);
}
unlock:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err ?: len;
}
static struct md_sysfs_entry md_serialize_policy =
__ATTR(serialize_policy, S_IRUGO | S_IWUSR, serialize_policy_show,
@@ -6263,21 +6277,22 @@ EXPORT_SYMBOL_GPL(mddev_stack_new_rdev);
/* update the optimal I/O size after a reshape */
void mddev_update_io_opt(struct mddev *mddev, unsigned int nr_stripes)
{
struct queue_limits lim;
+ unsigned int noio_flags;
if (mddev_is_dm(mddev))
return;
/* don't bother updating io_opt if we can't suspend the array */
- if (mddev_suspend(mddev, false) < 0)
+ if (mddev_suspend(mddev, false, &noio_flags) < 0)
return;
lim = queue_limits_start_update(mddev->gendisk->queue);
lim.io_opt = lim.io_min * nr_stripes;
queue_limits_commit_update(mddev->gendisk->queue, &lim);
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags);
}
EXPORT_SYMBOL_GPL(mddev_update_io_opt);
static void mddev_delayed_delete(struct work_struct *ws)
{
@@ -7255,10 +7270,11 @@ static void autorun_array(struct mddev *mddev)
*/
static void autorun_devices(int part)
{
struct md_rdev *rdev0, *rdev, *tmp;
struct mddev *mddev;
+ unsigned int noio_flags;
pr_info("md: autorun ...\n");
while (!list_empty(&pending_raid_disks)) {
int unit;
dev_t dev;
@@ -7295,27 +7311,27 @@ static void autorun_devices(int part)
mddev = md_alloc(dev, NULL);
if (IS_ERR(mddev))
break;
- if (mddev_suspend_and_lock(mddev))
+ if (mddev_suspend_and_lock(mddev, &noio_flags))
pr_warn("md: %s locked, cannot run\n", mdname(mddev));
else if (mddev->raid_disks || mddev->major_version
|| !list_empty(&mddev->disks)) {
pr_warn("md: %s already running, cannot run %pg\n",
mdname(mddev), rdev0->bdev);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
} else {
pr_debug("md: created %s\n", mdname(mddev));
mddev->persistent = 1;
rdev_for_each_list(rdev, tmp, &candidates) {
list_del_init(&rdev->same_set);
if (bind_rdev_to_array(rdev, mddev))
export_rdev(rdev);
}
autorun_array(mddev);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
}
/* on success, candidates will be empty, on error
* it won't...
*/
rdev_for_each_list(rdev, tmp, &candidates) {
@@ -8329,10 +8345,11 @@ static int __md_set_array_info(struct mddev *mddev, void __user *argp)
static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
unsigned int cmd, unsigned long arg)
{
int err = 0;
+ unsigned int noio_flags = 0;
void __user *argp = (void __user *)arg;
struct mddev *mddev = NULL;
err = md_ioctl_valid(cmd);
if (err)
@@ -8380,11 +8397,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
}
if (!md_is_rdwr(mddev))
flush_work(&mddev->sync_work);
- err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev) :
+ err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev, &noio_flags) :
mddev_lock(mddev);
if (err) {
pr_debug("md: ioctl lock interrupted, reason %d, cmd %d\n",
err, cmd);
goto out;
@@ -8511,11 +8528,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
unlock:
if (mddev->hold_active == UNTIL_IOCTL &&
err != -EINVAL)
mddev->hold_active = 0;
- md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev) :
+ md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev, noio_flags) :
mddev_unlock(mddev);
out:
if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
clear_bit(MD_CLOSING, &mddev->flags);
@@ -10180,20 +10197,21 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
static void md_start_sync(struct work_struct *ws)
{
struct mddev *mddev = container_of(ws, struct mddev, sync_work);
int spares = 0;
bool suspend = false;
+ unsigned int noio_flags = 0;
char *name;
/*
* If reshape is still in progress, spares won't be added or removed
* from conf until reshape is done.
*/
if (mddev->reshape_position == MaxSector &&
md_spares_need_change(mddev)) {
suspend = true;
- mddev_suspend(mddev, false);
+ mddev_suspend(mddev, false, &noio_flags);
}
mddev_lock_nointr(mddev);
if (!md_is_rdwr(mddev)) {
/*
@@ -10237,11 +10255,11 @@ static void md_start_sync(struct work_struct *ws)
* not set it again. Otherwise, we may cause issue like this one:
* https://bugzilla.kernel.org/show_bug.cgi?id=218200
* Therefore, use __mddev_resume(mddev, false).
*/
if (suspend)
- __mddev_resume(mddev, false);
+ __mddev_resume(mddev, false, noio_flags);
md_wakeup_thread(mddev->sync_thread);
sysfs_notify_dirent_safe(mddev->sysfs_action);
md_new_event();
return;
@@ -10257,11 +10275,11 @@ static void md_start_sync(struct work_struct *ws)
* not set it again. Otherwise, we may cause issue like this one:
* https://bugzilla.kernel.org/show_bug.cgi?id=218200
* Therefore, use __mddev_resume(mddev, false).
*/
if (suspend)
- __mddev_resume(mddev, false);
+ __mddev_resume(mddev, false, noio_flags);
wake_up(&resync_wait);
if (test_and_clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery) &&
mddev->sysfs_action)
sysfs_notify_dirent_safe(mddev->sysfs_action);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index d8daf0f75cbb..7d92deea34f2 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -619,11 +619,10 @@ struct mddev {
mempool_t *serial_info_pool;
void (*sync_super)(struct mddev *mddev, struct md_rdev *rdev);
struct md_cluster_info *cluster_info;
struct md_cluster_operations *cluster_ops;
unsigned int good_device_nr; /* good device num within cluster raid */
- unsigned int noio_flag; /* for memalloc scope API */
/*
* Temporarily store rdev that will be finally removed when
* reconfig_mutex is unlocked, protected by reconfig_mutex.
*/
@@ -953,12 +952,13 @@ extern void md_stop(struct mddev *mddev);
extern void md_stop_writes(struct mddev *mddev);
extern int md_rdev_init(struct md_rdev *rdev);
extern void md_rdev_clear(struct md_rdev *rdev);
extern bool md_handle_request(struct mddev *mddev, struct bio *bio);
-extern int mddev_suspend(struct mddev *mddev, bool interruptible);
-extern void mddev_resume(struct mddev *mddev);
+extern int mddev_suspend(struct mddev *mddev, bool interruptible,
+ unsigned int *noio_flags);
+extern void mddev_resume(struct mddev *mddev, unsigned int noio_flags);
extern void md_idle_sync_thread(struct mddev *mddev);
extern void md_frozen_sync_thread(struct mddev *mddev);
extern void md_unfrozen_sync_thread(struct mddev *mddev);
extern void md_update_sb(struct mddev *mddev, int force);
@@ -999,35 +999,38 @@ static inline void mddev_check_write_zeroes(struct mddev *mddev, struct bio *bio
if (bio_op(bio) == REQ_OP_WRITE_ZEROES &&
!bio->bi_bdev->bd_disk->queue->limits.max_write_zeroes_sectors)
mddev->gendisk->queue->limits.max_write_zeroes_sectors = 0;
}
-static inline int mddev_suspend_and_lock(struct mddev *mddev)
+static inline int mddev_suspend_and_lock(struct mddev *mddev,
+ unsigned int *noio_flags)
{
int ret;
- ret = mddev_suspend(mddev, true);
+ ret = mddev_suspend(mddev, true, noio_flags);
if (ret)
return ret;
ret = mddev_lock(mddev);
if (ret)
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags ? *noio_flags : 0);
return ret;
}
-static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev)
+static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev,
+ unsigned int *noio_flags)
{
- mddev_suspend(mddev, false);
+ mddev_suspend(mddev, false, noio_flags);
mddev_lock_nointr(mddev);
}
-static inline void mddev_unlock_and_resume(struct mddev *mddev)
+static inline void mddev_unlock_and_resume(struct mddev *mddev,
+ unsigned int noio_flags)
{
mddev_unlock(mddev);
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags);
}
struct mdu_array_info_s;
struct mdu_disk_info_s;
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 7b7546bfa21f..6f8e3a624456 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -693,13 +693,15 @@ static void r5c_disable_writeback_async(struct work_struct *work)
!READ_ONCE(conf->log) ||
!test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
log = READ_ONCE(conf->log);
if (log) {
- mddev_suspend(mddev, false);
+ unsigned int noio_flags;
+
+ mddev_suspend(mddev, false, &noio_flags);
log->r5c_journal_mode = R5C_JOURNAL_MODE_WRITE_THROUGH;
- mddev_resume(mddev);
+ mddev_resume(mddev, noio_flags);
}
}
static void r5l_submit_current_io(struct r5l_log *log)
{
@@ -2603,10 +2605,11 @@ EXPORT_SYMBOL(r5c_journal_mode_set);
static ssize_t r5c_journal_mode_store(struct mddev *mddev,
const char *page, size_t length)
{
int mode = ARRAY_SIZE(r5c_journal_mode_str);
size_t len = length;
+ unsigned int noio_flags;
int ret;
if (len < 2)
return -EINVAL;
@@ -2615,15 +2618,15 @@ static ssize_t r5c_journal_mode_store(struct mddev *mddev,
while (mode--)
if (strlen(r5c_journal_mode_str[mode]) == len &&
!strncmp(page, r5c_journal_mode_str[mode], len))
break;
- ret = mddev_suspend_and_lock(mddev);
+ ret = mddev_suspend_and_lock(mddev, &noio_flags);
if (ret)
return ret;
ret = r5c_journal_mode_set(mddev, mode);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return ret ?: length;
}
struct md_sysfs_entry
r5c_journal_mode = __ATTR(journal_mode, 0644,
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 65ae7d8930fc..482872ae7198 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6993,10 +6993,11 @@ raid5_show_stripe_size(struct mddev *mddev, char *page)
static ssize_t
raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
{
struct r5conf *conf;
unsigned long new;
+ unsigned int noio_flags;
int err;
int size;
if (len >= PAGE_SIZE)
return -EINVAL;
@@ -7011,11 +7012,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
if (new % DEFAULT_STRIPE_SIZE != 0 ||
new > PAGE_SIZE || new == 0 ||
new != roundup_pow_of_two(new))
return -EINVAL;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
conf = mddev->private;
if (!conf) {
@@ -7049,11 +7050,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
err = -ENOMEM;
}
mutex_unlock(&conf->cache_size_mutex);
out_unlock:
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err ?: len;
}
static struct md_sysfs_entry
raid5_stripe_size = __ATTR(stripe_size, 0644,
@@ -7127,19 +7128,20 @@ raid5_show_skip_copy(struct mddev *mddev, char *page)
static ssize_t
raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
{
struct r5conf *conf;
unsigned long new;
+ unsigned int noio_flags;
int err;
if (len >= PAGE_SIZE)
return -EINVAL;
if (kstrtoul(page, 10, &new))
return -EINVAL;
new = !!new;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
conf = mddev->private;
if (!conf)
err = -ENODEV;
@@ -7152,11 +7154,11 @@ raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
lim.features |= BLK_FEAT_STABLE_WRITES;
else
lim.features &= ~BLK_FEAT_STABLE_WRITES;
err = queue_limits_commit_update(q, &lim);
}
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err ?: len;
}
static struct md_sysfs_entry
raid5_skip_copy = __ATTR(skip_copy, S_IRUGO | S_IWUSR,
@@ -7195,10 +7197,11 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt,
static ssize_t
raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
{
struct r5conf *conf;
unsigned int new;
+ unsigned int noio_flags;
int err;
struct r5worker_group *new_groups, *old_groups;
int group_cnt;
if (len >= PAGE_SIZE)
@@ -7207,16 +7210,16 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
return -EINVAL;
/* 8192 should be big enough */
if (new > 8192)
return -EINVAL;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
conf = mddev->private;
if (!conf) {
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return -ENODEV;
}
raid5_quiesce(mddev, true);
if (new != conf->worker_cnt_per_group) {
@@ -7237,11 +7240,11 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
kfree(old_groups);
}
}
raid5_quiesce(mddev, false);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err ?: len;
}
static struct md_sysfs_entry
@@ -8940,18 +8943,19 @@ static void *raid6_takeover(struct mddev *mddev)
}
static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
{
struct r5conf *conf;
+ unsigned int noio_flags;
int err;
- err = mddev_suspend_and_lock(mddev);
+ err = mddev_suspend_and_lock(mddev, &noio_flags);
if (err)
return err;
conf = mddev->private;
if (!conf) {
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return -ENODEV;
}
if (strncmp(buf, "ppl", 3) == 0) {
/* ppl only works with RAID 5 */
@@ -8990,11 +8994,11 @@ static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
}
if (!err)
md_update_sb(mddev, 1);
- mddev_unlock_and_resume(mddev);
+ mddev_unlock_and_resume(mddev, noio_flags);
return err;
}
static int raid5_start(struct mddev *mddev)
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v6 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio()
From: Chen Cheng @ 2026-07-11 8:38 UTC (permalink / raw)
To: yukuai, linux-raid; +Cc: linux-kernel
In-Reply-To: <fb8b048b-3912-44e8-9f56-a25989cd80ee@fygo.io>
在 2026/7/5 15:48, yu kuai 写道:
> Hi,
>
> 在 2026/6/23 20:38, Chen Cheng 写道:
>> From: Chen Cheng <chencheng@fnnas.com>
>>
>> origin flow:
>>
>> bio_endio(master_bio); /* may drop active_io to zero */
>> allow_barrier(conf);
>> free_r10bio(r10_bio); /* reads conf->geo, returns to pool */
>>
>> one scenario is:
>>
>> CPU A (softirq, raid_end_bio_io) CPU B (action_store) --> reshape
>> ================================ ===============================
>> bio_endio(master_bio)
>> md_end_clone_io
>> percpu_ref_put -> 0
>> wait_event wakeup, and,
>> mddev_suspend return
>> raid10_start_reshape:
>> setup_geo(&conf->geo, new)
>> ...
>> mempool_destroy(old_pool)
>> conf->r10bio_pool = new_pool
>> allow_barrier(conf)
>> free_r10bio(r10_bio)
>> put_all_bios:
>> for (i=0; i<conf->geo.raid_disks; i++)
>> ==> old obj, new geo, OOB
>> mempool_free(r10_bio, conf->r10bio_pool)
>> ==> old-geometry obj freed into new pool
>>
>> so .. fix by reorder the flow:
>>
>> free_r10bio(r10_bio)
>> allow_barrier(conf)
>> bio_endio(master_io)
>>
>> raid_end_discard_bio() is exactly the same.
>>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> drivers/md/raid10.c | 17 ++++++++++-------
>> 1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index d740744a9746..e44a9b6e95c7 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -330,24 +330,25 @@ static void reschedule_retry(struct r10bio *r10_bio)
>> */
>> static void raid_end_bio_io(struct r10bio *r10_bio)
>> {
>> struct bio *bio = r10_bio->master_bio;
>> struct r10conf *conf = r10_bio->mddev->private;
>> + unsigned long state = r10_bio->state;
>>
>> - if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
>> - if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
>> - bio->bi_status = BLK_STS_IOERR;
>> - bio_endio(bio);
>> - }
>> + free_r10bio(r10_bio);
>>
>> /*
>> * Wake up any possible resync thread that waits for the device
>> * to go idle.
>> */
>> allow_barrier(conf);
>>
>> - free_r10bio(r10_bio);
>> + if (!test_and_set_bit(R10BIO_Returned, &state)) {
>> + if (!test_bit(R10BIO_Uptodate, &state))
>> + bio->bi_status = BLK_STS_IOERR;
>> + bio_endio(bio);
>> + }
>
> Why is this moved to the end? I feel it's not necessary.
>
yes, point is let endio after free r10bio.
free_r10bio(r10_bio) -> allow_barrier(conf) -> bio_endio(master_io) or
free_r10bio(r10_bio) -> bio_endio(master_io) -> allow_barrier(conf)
both order is correct to fix the bug.
>> }
>>
>> /*
>> * Update disk head position estimator based on IRQ completion info.
>> */
>> @@ -1580,13 +1581,15 @@ static void raid_end_discard_bio(struct r10bio *r10bio)
>> if (!test_bit(R10BIO_Discard, &r10bio->state)) {
>> first_r10bio = (struct r10bio *)r10bio->master_bio;
>> free_r10bio(r10bio);
>> r10bio = first_r10bio;
>> } else {
>> + struct bio *master_bio = r10bio->master_bio;
>> +
>> md_write_end(r10bio->mddev);
>> - bio_endio(r10bio->master_bio);
>> free_r10bio(r10bio);
>> + bio_endio(master_bio);
>> break;
>> }
>> }
>> }
>>
>
^ permalink raw reply
* [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape
From: Chen Cheng @ 2026-07-11 10:03 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
Hi,
This series fixes slab out-of-bounds accesses in raid10 when reshape changes
the number of raid disks while regular I/O is still reusing r10bio objects
allocated under the previous geometry.
The bug is reproducible with a simple 4-disk to 5-disk reshape under write
load, for example:
mdadm -C /dev/md777 -l10 -n4 /dev/sda /dev/sdb /dev/sdc /dev/sdd
mkfs.ext4 /dev/md777
mount /dev/md777 /mnt/test
fsstress -d /mnt/test -n 24000 -p 8 -l 24 &
mdadm /dev/md777 --add /dev/sde
mdadm --grow /dev/md777 --raid-devices=5 \
--backup-file=/tmp/md-reshape-backup
KASAN report:
BUG: KASAN: slab-out-of-bounds in free_r10bio+0x1c4/0x260 [raid10]
Read of size 8 at addr ffff00008c2dfac8 by task ksoftirqd/0/15
free_r10bio
raid_end_bio_io
one_write_done
raid10_end_write_request
This series addresses the problem in three steps:
1. ensure the sync_action=reshape caller suspends and locks before start_reshape
2. resize r10bio_pool when reshape grows raid_disks
3. reorder the r10bio free flow before bio_endio in the regular and discard
completion paths
Changes in v7:
- patch #1, refactor out suspend from retry block
- patch #3, raid_end_bio_io() refactor to:
free(r10_bio) -> free(master_bio) -> allow_barrier
Changes in v6:
- suspend the array in action_store() after flush_work()
- free r10bio before ending the discard master bio
Changes in v5 (suggested by Yu Kuai):
- simplify patch 2
- switch patch 3 from bounding reused r10bio devs[] walks by used_nr_devs
to reordering the free/endio flow
Changes in v4:
- make the sync_action=reshape path invoke mddev_suspend_and_lock() before
calling start_reshape()
- leave the md-cluster and dm-raid paths unchanged; they still reach
start_reshape() with the mddev locked but without suspend
Changes in v3:
- replace freeze_array()/unfreeze_array() in raid10_start_reshape() with
mddev_suspend_and_lock_nointr()/mddev_unlock_and_resume(); freeze_array()
can return while retry-list items still hold pool objects, while
mddev_suspend() provides the correct upper-layer quiesce interface
Changes in v2:
- add this cover letter
- convert r10bio_pool to a fixed-size kmalloc mempool
- rebuild r10bio_pool inside the freeze window before switching live reshape
geometry
- switch raid10_quiesce() to freeze_array()/unfreeze_array()
Testing:
- reproduced the original KASAN slab-out-of-bounds on 4-disk -> 5-disk
raid10 reshape with fsstress
- verified that this series fixes that reproducer
- exercised the 5-disk -> 4-disk reshape direction as well
Thanks,
Chen Cheng
Chen Cheng (3):
md: suspend array when sync_action=reshape
md/raid10: resize r10bio_pool for reshape
md/raid10: free r10bio before ending master_bio in raid_end_bio_io()
and raid_end_discard_bio()
drivers/md/md.c | 17 +++++++++----
drivers/md/raid10.c | 61 ++++++++++++++++++++++++++++++++-------------
drivers/md/raid10.h | 2 +-
3 files changed, 56 insertions(+), 24 deletions(-)
--
2.54.0
^ permalink raw reply
* [PATCH v7 1/3] md: suspend array when sync_action=reshape
From: Chen Cheng @ 2026-07-11 10:03 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
In-Reply-To: <20260711100352.425177-1-chencheng@fnnas.com>
From: Chen Cheng <chencheng@fnnas.com>
raid10 needs to resize/swap r10bio_pool when reshape changes
raid_disks, and, don't let new requests keep allocating r10bio
objects from the old pool while that transition is in progress.
suspend and lock array before mddev_start_reshape(), and resume
it on exit.
Other sync_action ops are unchanged.
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/md.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 096bb64e87bd..8ee2dcc116eb 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5261,25 +5261,32 @@ action_store(struct mddev *mddev, const char *page, size_t len)
enum sync_action action;
if (!mddev->pers || !mddev->pers->sync_request)
return -EINVAL;
+ action = md_sync_action_by_name(page);
+ if (action == ACTION_RESHAPE) {
+ ret = mddev_suspend(mddev, true);
+ if (ret)
+ return ret;
+ }
retry:
if (work_busy(&mddev->sync_work))
flush_work(&mddev->sync_work);
ret = mddev_lock(mddev);
- if (ret)
+ if (ret) {
+ if (action == ACTION_RESHAPE)
+ mddev_resume(mddev);
return ret;
+ }
if (work_busy(&mddev->sync_work)) {
mddev_unlock(mddev);
goto retry;
}
- action = md_sync_action_by_name(page);
-
/* TODO: mdadm rely on "idle" to start sync_thread. */
if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) {
switch (action) {
case ACTION_FROZEN:
md_frozen_sync_thread(mddev);
@@ -5345,10 +5352,12 @@ action_store(struct mddev *mddev, const char *page, size_t len)
sysfs_notify_dirent_safe(mddev->sysfs_action);
ret = len;
out:
mddev_unlock(mddev);
+ if (action == ACTION_RESHAPE)
+ mddev_resume(mddev);
return ret;
}
static struct md_sysfs_entry md_scan_mode =
__ATTR_PREALLOC(sync_action, S_IRUGO|S_IWUSR, action_show, action_store);
--
2.54.0
^ permalink raw reply related
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