* [PATCH 5/8] raid6: enable lock context analysis
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
The code doesn't have any locking, so this is trivial.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/raid/raid6/Makefile b/lib/raid/raid6/Makefile
index 038d6c74d1ba..5cdb7223de2a 100644
--- a/lib/raid/raid6/Makefile
+++ b/lib/raid/raid6/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
ccflags-y += -I $(src)
ifeq ($(CONFIG_RAID6_PQ_ARCH),y)
--
2.53.0
^ permalink raw reply related
* [PATCH 4/8] xor/kunit: add a benchmark
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
Add a benchmark to test the XOR functions for more representative block
sizes and numbers of disks. Including 64k would be useful here, but
increasing the test buffer size increases the runtime of the functional
kunit test too much unfortunately.
The runtime numbers are reported in GB/s as the numbers of modern
implementations are basically unreadable as MB/s. This means
retro-architectures could report 0, but that is an easy tradeoff.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/Kconfig | 6 ++++
lib/raid/xor/tests/xor_kunit.c | 62 ++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+)
diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig
index 978cd6ba08ac..c4eeb7c716c8 100644
--- a/lib/raid/Kconfig
+++ b/lib/raid/Kconfig
@@ -29,6 +29,12 @@ config XOR_KUNIT_TEST
This is intended to help people writing architecture-specific
optimized versions. If unsure, say N.
+config XOR_BENCHMARK
+ bool "Benchmark for xor_gen"
+ depends on XOR_KUNIT_TEST
+ help
+ Include benchmarks in the KUnit test suite for xor_gen.
+
config RAID6_PQ
tristate
diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
index 659ae3edbc25..648c6da9464c 100644
--- a/lib/raid/xor/tests/xor_kunit.c
+++ b/lib/raid/xor/tests/xor_kunit.c
@@ -125,8 +125,70 @@ static void xor_test(struct kunit *test)
}
}
+static void xor_benchmark(struct kunit *test)
+{
+ static const unsigned int nr_to_test[] = {
+ 4, 5, 6, 7, 8, 10, 12, 15, 16, 32,
+ };
+ static const unsigned int len_to_test[] = {
+ SZ_4K, SZ_16K,
+ };
+ unsigned int i, j, l;
+ u64 t;
+
+ if (!IS_ENABLED(CONFIG_XOR_BENCHMARK))
+ kunit_skip(test, "not enabled");
+
+ /* warm-up */
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ for (l = 0; l < 10; l++) {
+ xor_gen(test_dest, test_buffers, nr_to_test[i],
+ len_to_test[j]);
+ }
+ }
+ }
+
+ /*
+ * Preferably this would be a loop over len_to_test, but the kunit
+ * logging always adds a newline to each logged format string.
+ */
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, " \t%5u bytes\t%5u bytes\n",
+ len_to_test[0], len_to_test[1]);
+
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ unsigned int nr = nr_to_test[i];
+ u64 speed[ARRAY_SIZE(len_to_test)];
+
+ KUNIT_ASSERT_LE(test, nr, XOR_KUNIT_MAX_BUFFERS);
+
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ unsigned int len = len_to_test[j];
+ const unsigned long num_iters = 1000;
+
+ KUNIT_ASSERT_GT(test, len, 0);
+ KUNIT_ASSERT_LE(test, len, XOR_KUNIT_MAX_BYTES);
+
+ preempt_disable();
+ t = ktime_get_ns();
+ for (l = 0; l < num_iters; l++)
+ xor_gen(test_dest, test_buffers, nr, len);
+ t = ktime_get_ns() - t;
+ preempt_enable();
+
+ speed[j] = div64_u64((u64)len * num_iters * nr, t);
+ }
+
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, "%3u disks:\t%5llu GB/s\t%5llu GB/s\n",
+ nr, speed[0], speed[1]);
+ }
+}
+
static struct kunit_case xor_test_cases[] = {
KUNIT_CASE(xor_test),
+ KUNIT_CASE(xor_benchmark),
{},
};
--
2.53.0
^ permalink raw reply related
* [PATCH 3/8] xor/kunit: fix a spelling error
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/tests/xor_kunit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
index 0c2a3a420bf9..659ae3edbc25 100644
--- a/lib/raid/xor/tests/xor_kunit.c
+++ b/lib/raid/xor/tests/xor_kunit.c
@@ -85,7 +85,7 @@ static void xor_test(struct kunit *test)
xor_generate_random_data();
/*
- * If we're not using the entire buffer size, inject randomize
+ * If we're not using the entire buffer size, inject randomized
* alignment into the buffer.
*/
max_alignment = XOR_KUNIT_MAX_BYTES - len;
--
2.53.0
^ permalink raw reply related
* [PATCH 2/8] xor: improve the runtime selection benchmark
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
Use plain ktime_get_ns for the timing, use 4 + 1 disks for a realistic
load, and report the throughput on the data disks instead of the that
on the parity disk, which isn't all that useful.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/xor-core.c | 55 +++++++++++++++++++----------------------
1 file changed, 26 insertions(+), 29 deletions(-)
diff --git a/lib/raid/xor/xor-core.c b/lib/raid/xor/xor-core.c
index 50931fbf0324..a890cec21bbc 100644
--- a/lib/raid/xor/xor-core.c
+++ b/lib/raid/xor/xor-core.c
@@ -10,7 +10,6 @@
#include <linux/gfp.h>
#include <linux/slab.h>
#include <linux/raid/xor.h>
-#include <linux/jiffies.h>
#include <linux/preempt.h>
#include <linux/static_call.h>
#include "xor_impl.h"
@@ -73,59 +72,56 @@ void __init xor_force(struct xor_block_template *tmpl)
forced_template = tmpl;
}
-#define BENCH_SIZE 4096
+#define BENCH_SIZE SZ_4K
+#define NR_SRCS 4
#define REPS 800U
-static void __init
-do_xor_speed(struct xor_block_template *tmpl, void *b1, void *b2)
+static void __init do_xor_speed(struct xor_block_template *tmpl, void *dest,
+ void *srcs[NR_SRCS])
{
- int speed;
- unsigned long reps;
- ktime_t min, start, t0;
- void *srcs[1] = { b2 };
+ u64 t;
+ int i;
preempt_disable();
-
- reps = 0;
- t0 = ktime_get();
- /* delay start until time has advanced */
- while ((start = ktime_get()) == t0)
- cpu_relax();
- do {
+ t = ktime_get_ns();
+ for (i = 0; i < REPS; i++) {
mb(); /* prevent loop optimization */
- tmpl->xor_gen(b1, srcs, 1, BENCH_SIZE);
+ tmpl->xor_gen(dest, srcs, NR_SRCS, BENCH_SIZE);
mb();
- } while (reps++ < REPS || (t0 = ktime_get()) == start);
- min = ktime_sub(t0, start);
-
+ }
+ t = ktime_get_ns() - t;
preempt_enable();
- // bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s]
- speed = (1000 * reps * BENCH_SIZE) / (unsigned int)ktime_to_ns(min);
- tmpl->speed = speed;
+ /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
+ tmpl->speed = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
- pr_info(" %-16s: %5d MB/sec\n", tmpl->name, speed);
+ pr_info(" %-16s: %5d MB/sec\n", tmpl->name, tmpl->speed);
}
static int __init calibrate_xor_blocks(void)
{
- void *b1, *b2;
struct xor_block_template *f, *fastest;
+ void *srcs[NR_SRCS];
+ void *buf, *dest;
+ int i;
if (forced_template)
return 0;
- b1 = kmalloc(PAGE_SIZE * 4, GFP_KERNEL);
- if (!b1) {
+ buf = kmalloc(BENCH_SIZE * (NR_SRCS + 1), GFP_KERNEL);
+ if (!buf) {
pr_warn("xor: Yikes! No memory available.\n");
return -ENOMEM;
}
- b2 = b1 + 2*PAGE_SIZE + BENCH_SIZE;
+ get_random_bytes(buf, BENCH_SIZE * (NR_SRCS + 1));
+ dest = buf;
+ for (i = 0; i < NR_SRCS; i++)
+ srcs[i] = buf + (i + 1) * BENCH_SIZE;
pr_info("xor: measuring software checksum speed\n");
fastest = template_list;
for (f = template_list; f; f = f->next) {
- do_xor_speed(f, b1, b2);
+ do_xor_speed(f, dest, srcs);
if (f->speed > fastest->speed)
fastest = f;
}
@@ -133,9 +129,10 @@ static int __init calibrate_xor_blocks(void)
pr_info("xor: using function: %s (%d MB/sec)\n",
fastest->name, fastest->speed);
- kfree(b1);
+ kfree(buf);
return 0;
}
+#undef NR_SRCS
#ifdef CONFIG_XOR_BLOCKS_ARCH
#include "xor_arch.h" /* $SRCARCH/xor_arch.h */
--
2.53.0
^ permalink raw reply related
* [PATCH 1/8] xor: enable lock context analysis
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090740.1433685-1-hch@lst.de>
The code doesn't have any locking, so this is trivial.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/Makefile | 2 ++
lib/raid/xor/tests/Makefile | 2 ++
2 files changed, 4 insertions(+)
diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile
index e8ecec3c09f9..9b0fad459cdb 100644
--- a/lib/raid/xor/Makefile
+++ b/lib/raid/xor/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
ccflags-y += -I $(src)
obj-$(CONFIG_XOR_BLOCKS) += xor.o
diff --git a/lib/raid/xor/tests/Makefile b/lib/raid/xor/tests/Makefile
index 661e8f6ffd1f..1cce833cd7fd 100644
--- a/lib/raid/xor/tests/Makefile
+++ b/lib/raid/xor/tests/Makefile
@@ -1,3 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
+CONTEXT_ANALYSIS := y
+
obj-$(CONFIG_XOR_KUNIT_TEST) += xor_kunit.o
--
2.53.0
^ permalink raw reply related
* misc lib/raid/ improvements
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
Hi all,
this series improves the benchmark-based algorithm selection and
adds kunit benchmarks for both the XOR and raid6 libraries, and tidies
up a few very minor other bits.
diffstat:
lib/raid/Kconfig | 15 ++++++
lib/raid/raid6/Makefile | 2
lib/raid/raid6/algos.c | 112 +++++++++++++++++++++++++---------------------
lib/raid/raid6/tests/raid6_kunit.c | 66 ++++++++++++++++++++++++++-
lib/raid/xor/Makefile | 2
lib/raid/xor/tests/Makefile | 2
lib/raid/xor/tests/xor_kunit.c | 64 +++++++++++++++++++++++++-
lib/raid/xor/xor-core.c | 55 ++++++++++------------
8 files changed, 236 insertions(+), 82 deletions(-)
^ permalink raw reply
* [PATCH 6/8] raid6: defer implementation selection when built-in
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
Don't hold up early boot and defer the selection just like we've been
doing for the RAID5 XOR code since commit 524ccdbdfb52 ("crypto: xor -
defer load time benchmark to a later time").
This will also allow full use of the timing subsystem for benchmarking.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/algos.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
index 6f5c89ab2b17..e7984bde5157 100644
--- a/lib/raid/raid6/algos.c
+++ b/lib/raid/raid6/algos.c
@@ -236,16 +236,6 @@ static int __init raid6_select_algo(void)
int i, cycle;
int error;
- if (!IS_ENABLED(CONFIG_RAID6_PQ_BENCHMARK) || raid6_nr_algos == 1) {
- pr_info("raid6: skipped pq benchmark and selected %s\n",
- raid6_algos[raid6_nr_algos - 1]->name);
- static_call_update(raid6_gen_syndrome_impl,
- raid6_algos[raid6_nr_algos - 1]->gen_syndrome);
- static_call_update(raid6_xor_syndrome_impl,
- raid6_algos[raid6_nr_algos - 1]->xor_syndrome);
- return 0;
- }
-
/* prepare the buffer and fill it circularly with gfmul table */
disk_ptr = kmalloc(PAGE_SIZE * RAID6_TEST_DISKS, GFP_KERNEL);
if (!disk_ptr) {
@@ -329,13 +319,40 @@ static int __init raid6_init(void)
static_call_update(raid6_recov_datap_impl, raid6_recov_algo->datap);
pr_info("raid6: using %s recovery algorithm\n", raid6_recov_algo->name);
+ if (!IS_ENABLED(CONFIG_RAID6_PQ_BENCHMARK) || raid6_nr_algos == 1) {
+ pr_info("raid6: skipped pq benchmark and selected %s\n",
+ raid6_algos[raid6_nr_algos - 1]->name);
+ goto default_algo;
+ }
+
+#ifdef MODULE
return raid6_select_algo();
+#else
+ /*
+ * Pick the last registered implementation as the temporary default until
+ * calibration happens.
+ */
+#endif
+default_algo:
+ static_call_update(raid6_gen_syndrome_impl,
+ raid6_algos[raid6_nr_algos - 1]->gen_syndrome);
+ static_call_update(raid6_xor_syndrome_impl,
+ raid6_algos[raid6_nr_algos - 1]->xor_syndrome);
+ return 0;
}
static void __exit raid6_exit(void)
{
}
+/*
+ * When built-in we must register the default implementation before md
+ * initializes, but we don't want calibration to run that early as that
+ * would delay the boot process.
+ */
+#ifndef MODULE
+device_initcall(raid6_select_algo);
+#endif
subsys_initcall(raid6_init);
module_exit(raid6_exit);
MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] md/raid10: free r10_bio on write NOWAIT exits
From: sashiko-bot @ 2026-07-08 9:06 UTC (permalink / raw)
To: Guangshuo Li; +Cc: linux-raid, yukuai
In-Reply-To: <20260708084701.737763-1-lgs201920130244@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The MD layer and RAID10 driver sleep in multiple paths while handling a `REQ_NOWAIT` bio, violating the non-blocking semantics of the block layer.
- [High] The driver unconditionally uses `GFP_NOIO` for mempool and bio allocations, which can sleep when memory is low, violating `REQ_NOWAIT` non-blocking semantics.
--
--- Patch [1]: [PATCH] md/raid10: free r10_bio on write NOWAIT exits ---
commit 2e30580bb5bb2ae342c9d924bf8c217f5b6c832f
Author: Guangshuo Li <lgs201920130244@gmail.com>
md/raid10: free r10_bio on write NOWAIT exits
This commit fixes a memory leak in raid10_write_request() where the r10_bio
structure was not returned to the mempool on two REQ_NOWAIT early exit
paths. It ensures free_r10bio() is called before returning false for these
NOWAIT cases.
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0a3cfdd3f5df8..d480344c5b8f3 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1364,6 +1364,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
> DEFINE_WAIT(w);
> /* Bail out if REQ_NOWAIT is set for the bio */
> if (bio->bi_opf & REQ_NOWAIT) {
> + free_r10bio(r10_bio);
> bio_wouldblock_error(bio);
> return false;
> }
[ ... ]
> @@ -1397,6 +1398,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
> md_wakeup_thread(mddev->thread);
> if (bio->bi_opf & REQ_NOWAIT) {
> allow_barrier(conf);
> + free_r10bio(r10_bio);
> bio_wouldblock_error(bio);
> return false;
> }
[Severity: High]
This isn't a bug introduced by this patch, but are there other blocking
operations in the NOWAIT path that could still violate the non-blocking
semantics?
If a REQ_NOWAIT bio does not hit these early exits, it proceeds down to
wait_blocked_dev() later in raid10_write_request(), which does not check
for REQ_NOWAIT:
raid10_find_phys(conf, r10_bio);
wait_blocked_dev(mddev, r10_bio);
If a member device is blocked, wait_blocked_dev() drops the barrier and
sleeps in md_wait_for_blocked_rdev() (which has a 5-second timeout) and
then sleeps again in wait_barrier(conf, false).
Additionally, if the bio crosses a chunk boundary, it reacquires the
barrier without checking for REQ_NOWAIT:
if (r10_bio->sectors < bio_sectors(bio)) {
allow_barrier(conf);
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
wait_barrier(conf, false);
Could these unconditional sleeps cause the thread submitting the async I/O
to stall, potentially exhausting io_uring worker threads?
[Severity: High]
This is also a pre-existing issue, but does the driver unconditionally use
GFP_NOIO for allocations when handling a REQ_NOWAIT bio?
Before raid10_write_request() is even called, __make_request() allocates
the primary r10_bio using GFP_NOIO:
r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
Later, raid10_write_one_disk() clones the bio unconditionally using
GFP_NOIO:
mbio = bio_alloc_clone(rdev->bdev, bio, GFP_NOIO, &mddev->bio_set);
If the mempool is depleted and memory pressure requires direct reclaim,
won't these GFP_NOIO allocations sleep and stall the IO submission thread?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708084701.737763-1-lgs201920130244@gmail.com?part=1
^ permalink raw reply
* [PATCH 5/8] raid6: enable lock context analysis
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
The code doesn't have any locking, so this is trivial.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/raid/raid6/Makefile b/lib/raid/raid6/Makefile
index 038d6c74d1ba..5cdb7223de2a 100644
--- a/lib/raid/raid6/Makefile
+++ b/lib/raid/raid6/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
ccflags-y += -I $(src)
ifeq ($(CONFIG_RAID6_PQ_ARCH),y)
--
2.53.0
^ permalink raw reply related
* [PATCH 4/8] xor/kunit: add a benchmark
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
Add a benchmark to test the XOR functions for more representative block
sizes and numbers of disks. Including 64k would be useful here, but
increasing the test buffer size increases the runtime of the functional
kunit test too much unfortunately.
The runtime numbers are reported in GB/s as the numbers of modern
implementations are basically unreadable as MB/s. This means
retro-architectures could report 0, but that is an easy tradeoff.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/Kconfig | 6 ++++
lib/raid/xor/tests/xor_kunit.c | 62 ++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+)
diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig
index 978cd6ba08ac..c4eeb7c716c8 100644
--- a/lib/raid/Kconfig
+++ b/lib/raid/Kconfig
@@ -29,6 +29,12 @@ config XOR_KUNIT_TEST
This is intended to help people writing architecture-specific
optimized versions. If unsure, say N.
+config XOR_BENCHMARK
+ bool "Benchmark for xor_gen"
+ depends on XOR_KUNIT_TEST
+ help
+ Include benchmarks in the KUnit test suite for xor_gen.
+
config RAID6_PQ
tristate
diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
index 659ae3edbc25..648c6da9464c 100644
--- a/lib/raid/xor/tests/xor_kunit.c
+++ b/lib/raid/xor/tests/xor_kunit.c
@@ -125,8 +125,70 @@ static void xor_test(struct kunit *test)
}
}
+static void xor_benchmark(struct kunit *test)
+{
+ static const unsigned int nr_to_test[] = {
+ 4, 5, 6, 7, 8, 10, 12, 15, 16, 32,
+ };
+ static const unsigned int len_to_test[] = {
+ SZ_4K, SZ_16K,
+ };
+ unsigned int i, j, l;
+ u64 t;
+
+ if (!IS_ENABLED(CONFIG_XOR_BENCHMARK))
+ kunit_skip(test, "not enabled");
+
+ /* warm-up */
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ for (l = 0; l < 10; l++) {
+ xor_gen(test_dest, test_buffers, nr_to_test[i],
+ len_to_test[j]);
+ }
+ }
+ }
+
+ /*
+ * Preferably this would be a loop over len_to_test, but the kunit
+ * logging always adds a newline to each logged format string.
+ */
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, " \t%5u bytes\t%5u bytes\n",
+ len_to_test[0], len_to_test[1]);
+
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ unsigned int nr = nr_to_test[i];
+ u64 speed[ARRAY_SIZE(len_to_test)];
+
+ KUNIT_ASSERT_LE(test, nr, XOR_KUNIT_MAX_BUFFERS);
+
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ unsigned int len = len_to_test[j];
+ const unsigned long num_iters = 1000;
+
+ KUNIT_ASSERT_GT(test, len, 0);
+ KUNIT_ASSERT_LE(test, len, XOR_KUNIT_MAX_BYTES);
+
+ preempt_disable();
+ t = ktime_get_ns();
+ for (l = 0; l < num_iters; l++)
+ xor_gen(test_dest, test_buffers, nr, len);
+ t = ktime_get_ns() - t;
+ preempt_enable();
+
+ speed[j] = div64_u64((u64)len * num_iters * nr, t);
+ }
+
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, "%3u disks:\t%5llu GB/s\t%5llu GB/s\n",
+ nr, speed[0], speed[1]);
+ }
+}
+
static struct kunit_case xor_test_cases[] = {
KUNIT_CASE(xor_test),
+ KUNIT_CASE(xor_benchmark),
{},
};
--
2.53.0
^ permalink raw reply related
* [PATCH 3/8] xor/kunit: fix a spelling error
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/tests/xor_kunit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
index 0c2a3a420bf9..659ae3edbc25 100644
--- a/lib/raid/xor/tests/xor_kunit.c
+++ b/lib/raid/xor/tests/xor_kunit.c
@@ -85,7 +85,7 @@ static void xor_test(struct kunit *test)
xor_generate_random_data();
/*
- * If we're not using the entire buffer size, inject randomize
+ * If we're not using the entire buffer size, inject randomized
* alignment into the buffer.
*/
max_alignment = XOR_KUNIT_MAX_BYTES - len;
--
2.53.0
^ permalink raw reply related
* [PATCH 2/8] xor: improve the runtime selection benchmark
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
Use plain ktime_get_ns for the timing, use 4 + 1 disks for a realistic
load, and report the throughput on the data disks instead of the that
on the parity disk, which isn't all that useful.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/xor-core.c | 55 +++++++++++++++++++----------------------
1 file changed, 26 insertions(+), 29 deletions(-)
diff --git a/lib/raid/xor/xor-core.c b/lib/raid/xor/xor-core.c
index 50931fbf0324..a890cec21bbc 100644
--- a/lib/raid/xor/xor-core.c
+++ b/lib/raid/xor/xor-core.c
@@ -10,7 +10,6 @@
#include <linux/gfp.h>
#include <linux/slab.h>
#include <linux/raid/xor.h>
-#include <linux/jiffies.h>
#include <linux/preempt.h>
#include <linux/static_call.h>
#include "xor_impl.h"
@@ -73,59 +72,56 @@ void __init xor_force(struct xor_block_template *tmpl)
forced_template = tmpl;
}
-#define BENCH_SIZE 4096
+#define BENCH_SIZE SZ_4K
+#define NR_SRCS 4
#define REPS 800U
-static void __init
-do_xor_speed(struct xor_block_template *tmpl, void *b1, void *b2)
+static void __init do_xor_speed(struct xor_block_template *tmpl, void *dest,
+ void *srcs[NR_SRCS])
{
- int speed;
- unsigned long reps;
- ktime_t min, start, t0;
- void *srcs[1] = { b2 };
+ u64 t;
+ int i;
preempt_disable();
-
- reps = 0;
- t0 = ktime_get();
- /* delay start until time has advanced */
- while ((start = ktime_get()) == t0)
- cpu_relax();
- do {
+ t = ktime_get_ns();
+ for (i = 0; i < REPS; i++) {
mb(); /* prevent loop optimization */
- tmpl->xor_gen(b1, srcs, 1, BENCH_SIZE);
+ tmpl->xor_gen(dest, srcs, NR_SRCS, BENCH_SIZE);
mb();
- } while (reps++ < REPS || (t0 = ktime_get()) == start);
- min = ktime_sub(t0, start);
-
+ }
+ t = ktime_get_ns() - t;
preempt_enable();
- // bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s]
- speed = (1000 * reps * BENCH_SIZE) / (unsigned int)ktime_to_ns(min);
- tmpl->speed = speed;
+ /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
+ tmpl->speed = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
- pr_info(" %-16s: %5d MB/sec\n", tmpl->name, speed);
+ pr_info(" %-16s: %5d MB/sec\n", tmpl->name, tmpl->speed);
}
static int __init calibrate_xor_blocks(void)
{
- void *b1, *b2;
struct xor_block_template *f, *fastest;
+ void *srcs[NR_SRCS];
+ void *buf, *dest;
+ int i;
if (forced_template)
return 0;
- b1 = kmalloc(PAGE_SIZE * 4, GFP_KERNEL);
- if (!b1) {
+ buf = kmalloc(BENCH_SIZE * (NR_SRCS + 1), GFP_KERNEL);
+ if (!buf) {
pr_warn("xor: Yikes! No memory available.\n");
return -ENOMEM;
}
- b2 = b1 + 2*PAGE_SIZE + BENCH_SIZE;
+ get_random_bytes(buf, BENCH_SIZE * (NR_SRCS + 1));
+ dest = buf;
+ for (i = 0; i < NR_SRCS; i++)
+ srcs[i] = buf + (i + 1) * BENCH_SIZE;
pr_info("xor: measuring software checksum speed\n");
fastest = template_list;
for (f = template_list; f; f = f->next) {
- do_xor_speed(f, b1, b2);
+ do_xor_speed(f, dest, srcs);
if (f->speed > fastest->speed)
fastest = f;
}
@@ -133,9 +129,10 @@ static int __init calibrate_xor_blocks(void)
pr_info("xor: using function: %s (%d MB/sec)\n",
fastest->name, fastest->speed);
- kfree(b1);
+ kfree(buf);
return 0;
}
+#undef NR_SRCS
#ifdef CONFIG_XOR_BLOCKS_ARCH
#include "xor_arch.h" /* $SRCARCH/xor_arch.h */
--
2.53.0
^ permalink raw reply related
* [PATCH 1/8] xor: enable lock context analysis
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
The code doesn't have any locking, so this is trivial.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/Makefile | 2 ++
lib/raid/xor/tests/Makefile | 2 ++
2 files changed, 4 insertions(+)
diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile
index e8ecec3c09f9..9b0fad459cdb 100644
--- a/lib/raid/xor/Makefile
+++ b/lib/raid/xor/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
ccflags-y += -I $(src)
obj-$(CONFIG_XOR_BLOCKS) += xor.o
diff --git a/lib/raid/xor/tests/Makefile b/lib/raid/xor/tests/Makefile
index 661e8f6ffd1f..1cce833cd7fd 100644
--- a/lib/raid/xor/tests/Makefile
+++ b/lib/raid/xor/tests/Makefile
@@ -1,3 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
+CONTEXT_ANALYSIS := y
+
obj-$(CONFIG_XOR_KUNIT_TEST) += xor_kunit.o
--
2.53.0
^ permalink raw reply related
* misc lib/raid/ improvements
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
Hi all,
this series improves the benchmark-based algorithm selection and
adds kunit benchmarks for both the XOR and raid6 libraries, and tidies
up a few very minor other bits.
diffstat:
lib/raid/Kconfig | 15 ++++++
lib/raid/raid6/Makefile | 2
lib/raid/raid6/algos.c | 112 +++++++++++++++++++++++++---------------------
lib/raid/raid6/tests/raid6_kunit.c | 66 ++++++++++++++++++++++++++-
lib/raid/xor/Makefile | 2
lib/raid/xor/tests/Makefile | 2
lib/raid/xor/tests/xor_kunit.c | 64 +++++++++++++++++++++++++-
lib/raid/xor/xor-core.c | 55 ++++++++++------------
8 files changed, 236 insertions(+), 82 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: Abd-Alrhman Masalkhi @ 2026-07-08 9:05 UTC (permalink / raw)
To: John Garry, song, yukuai, magiclinan, xiao, axboe, vverma,
martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <fbf1a5d6-d362-4eeb-8434-2f4a114a70bd@oracle.com>
Hi John,
On Tue, Jul 07, 2026 at 17:20 +0100, John Garry wrote:
> On 06/07/2026 12:35, Abd-Alrhman Masalkhi wrote:
>>> well it seems to be that everything in the driver is split over
>>> BARRIER_UNIT_SECTOR_SIZE, so is in effect a chunk size.
>>>
>>> Note that atomic_write_hw_unit_max is going to be small always compared
>>> to BARRIER_UNIT_SECTOR_SIZE.
>>>
>>> However, can you check the blk stacking code to make sure that this does
>>> as you want? As I remember, for stacking we take the atomic write limits
>>> of the first bottom device and then stack the other bottom devices and I
>>> don't think that setting atomic_write_hw_unit_max in this way has an
>>> impact - see blk_stack_atomic_writes_limits()
>>>
>> I checked blk_stack_atomic_writes_limits(), and you are right: setting
>> atomic_write_hw_unit_max here alone does not have the intended effect on
>> the final stacked device.
>>
>> Also, since atomic_write_hw_unit_max is expected to always be smaller
>> than BARRIER_UNIT_SECTOR_SIZE,
>
> Yes, always expected to be much smaller.
>
>> it seems that there is no need to set an
>> additional atomic write limit in raid1, or even to set
>> atomic_write_hw_unit_max at all. Is that what you mean?
>
> Even though we expect it to be much smaller, it's good practice to
> ensure this.
>
> One method to do so - which I already mentioned - was to set the chunk
> size to BARRIER_UNIT_SECTOR_SIZE. You were not keen on that.
>
I see, thanks for the clarification. My concern was mainly conceptual,
and I was also worried that setting the chunk size might affect other
operations, such as discard. Since that is not the case, I'll set the
chunk size to BARRIER_UNIT_SECTOR_SIZE and update it accordingly in v2.
> Another method is to set max hw sectors for the RAID1 device to
> BARRIER_UNIT_SECTOR_SIZE.
>
> Thanks,
> John
>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: Abd-Alrhman Masalkhi @ 2026-07-08 8:59 UTC (permalink / raw)
To: yu kuai, yu kuai, song, magiclinan, xiao
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <f30acc2d-9153-4a9c-b6e5-79083bd9fedb@fygo.io>
Hi Kuai,
On Wed, Jul 08, 2026 at 16:32 +0800, yu kuai wrote:
> Hi,
>
> 在 2026/7/7 18:35, Abd-Alrhman Masalkhi 写道:
>> Hi Kuai,
>>
>> On Tue, Jul 07, 2026 at 09:12 +0800, yu kuai wrote:
>>> Hi,
>>>
>>> 在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道:
>>>>> The problem looks real, however, I think this will cause a change that user will be awared,
>>>>> if there are really spares that can be removed from conf, but array is not suspended here,
>>>>> user will still expect rdev will be removed from conf automatically.
>>>>>
>>>>> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
>>>>> supposed to be true, we can release the lock and retry with suspend = true.
>>>>>
>>>> Yes, I see, and your approach is much better. But what do you think
>>>> about taking the lock first and then checking only once?
>>> I don't get what you mean. If we take the lock and then check that array should
>>> suspend, we still have to release the lock before we suspend the array.
>>>
>> Sorry, I was not clear. I meant, do we need to check twice, once before
>> taking the lock and once after? It seems that the check before taking the
>> lock is redundant. since the result would need to be checked again after
>> taking the lock anyway.
>>
>> Could we drop the check before taking the lock and only check whether
>> suspension is needed once while holding it? If suspension is needed, we
>> would release the lock, suspend the array, and then reacquire the lock.
>
> Thanks for the explanation, I understand now. Howerver, I still prefer to check
> first before holding the lock. Because the checking is much lower overhead than
> acquire reconfig_mutex, and the race window that rdev become spare is small, so
> it's unlikely we'll acquire reconfig_mtuex twice.
>
I see, thanks for the explanation. I'll send a v2 shortly.
>>
>>> --
>>> Thanks,
>>> Kuai
>
> --
> Thanks,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* [PATCH] md/raid10: free r10_bio on write NOWAIT exits
From: Guangshuo Li @ 2026-07-08 8:47 UTC (permalink / raw)
To: Song Liu, Yu Kuai, Li Nan, Xiao Ni, Josh Hunt, linux-raid,
linux-kernel
Cc: Guangshuo Li
The change referenced by the Fixes tag made a NOWAIT early exit in
raid10_write_request() free the r10_bio directly instead of using
raid_end_bio_io(), avoiding an unmatched allow_barrier() while still
returning the r10_bio object to the mempool.
The same function has two other NOWAIT early exits. One is before
regular_request_wait(), where the request has not taken a barrier
reference. The other is in the reshape metadata path, where the barrier
reference is released explicitly with allow_barrier(). Both paths end the
bio and return false, but neither returns the r10_bio object to the
mempool.
Free the r10_bio on both NOWAIT exits. Keep the existing barrier handling
unchanged, since only the reshape path has taken a barrier reference.
Fixes: 7d96f3120a7f ("md/raid10: fix deadlock with check operation and nowait requests")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
drivers/md/raid10.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0a3cfdd3f5df..d480344c5b8f 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1364,6 +1364,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
DEFINE_WAIT(w);
/* Bail out if REQ_NOWAIT is set for the bio */
if (bio->bi_opf & REQ_NOWAIT) {
+ free_r10bio(r10_bio);
bio_wouldblock_error(bio);
return false;
}
@@ -1397,6 +1398,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
md_wakeup_thread(mddev->thread);
if (bio->bi_opf & REQ_NOWAIT) {
allow_barrier(conf);
+ free_r10bio(r10_bio);
bio_wouldblock_error(bio);
return false;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: yu kuai @ 2026-07-08 8:32 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, yu kuai, song, magiclinan, xiao
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <m2mrw36qap.fsf@gmail.com>
Hi,
在 2026/7/7 18:35, Abd-Alrhman Masalkhi 写道:
> Hi Kuai,
>
> On Tue, Jul 07, 2026 at 09:12 +0800, yu kuai wrote:
>> Hi,
>>
>> 在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道:
>>>> The problem looks real, however, I think this will cause a change that user will be awared,
>>>> if there are really spares that can be removed from conf, but array is not suspended here,
>>>> user will still expect rdev will be removed from conf automatically.
>>>>
>>>> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
>>>> supposed to be true, we can release the lock and retry with suspend = true.
>>>>
>>> Yes, I see, and your approach is much better. But what do you think
>>> about taking the lock first and then checking only once?
>> I don't get what you mean. If we take the lock and then check that array should
>> suspend, we still have to release the lock before we suspend the array.
>>
> Sorry, I was not clear. I meant, do we need to check twice, once before
> taking the lock and once after? It seems that the check before taking the
> lock is redundant. since the result would need to be checked again after
> taking the lock anyway.
>
> Could we drop the check before taking the lock and only check whether
> suspension is needed once while holding it? If suspension is needed, we
> would release the lock, suspend the array, and then reacquire the lock.
Thanks for the explanation, I understand now. Howerver, I still prefer to check
first before holding the lock. Because the checking is much lower overhead than
acquire reconfig_mutex, and the race window that rdev become spare is small, so
it's unlikely we'll acquire reconfig_mtuex twice.
>
>> --
>> Thanks,
>> Kuai
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: John Garry @ 2026-07-07 16:20 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, axboe,
vverma, martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <m2pl1073l5.fsf@gmail.com>
On 06/07/2026 12:35, Abd-Alrhman Masalkhi wrote:
>> well it seems to be that everything in the driver is split over
>> BARRIER_UNIT_SECTOR_SIZE, so is in effect a chunk size.
>>
>> Note that atomic_write_hw_unit_max is going to be small always compared
>> to BARRIER_UNIT_SECTOR_SIZE.
>>
>> However, can you check the blk stacking code to make sure that this does
>> as you want? As I remember, for stacking we take the atomic write limits
>> of the first bottom device and then stack the other bottom devices and I
>> don't think that setting atomic_write_hw_unit_max in this way has an
>> impact - see blk_stack_atomic_writes_limits()
>>
> I checked blk_stack_atomic_writes_limits(), and you are right: setting
> atomic_write_hw_unit_max here alone does not have the intended effect on
> the final stacked device.
>
> Also, since atomic_write_hw_unit_max is expected to always be smaller
> than BARRIER_UNIT_SECTOR_SIZE,
Yes, always expected to be much smaller.
> it seems that there is no need to set an
> additional atomic write limit in raid1, or even to set
> atomic_write_hw_unit_max at all. Is that what you mean?
Even though we expect it to be much smaller, it's good practice to
ensure this.
One method to do so - which I already mentioned - was to set the chunk
size to BARRIER_UNIT_SECTOR_SIZE. You were not keen on that.
Another method is to set max hw sectors for the RAID1 device to
BARRIER_UNIT_SECTOR_SIZE.
Thanks,
John
^ permalink raw reply
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: Abd-Alrhman Masalkhi @ 2026-07-07 10:35 UTC (permalink / raw)
To: yu kuai, yu kuai, song, magiclinan, xiao
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <4f0b2516-65bd-41fa-abc3-3cd76951ec46@fygo.io>
Hi Kuai,
On Tue, Jul 07, 2026 at 09:12 +0800, yu kuai wrote:
> Hi,
>
> 在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道:
>>> The problem looks real, however, I think this will cause a change that user will be awared,
>>> if there are really spares that can be removed from conf, but array is not suspended here,
>>> user will still expect rdev will be removed from conf automatically.
>>>
>>> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
>>> supposed to be true, we can release the lock and retry with suspend = true.
>>>
>> Yes, I see, and your approach is much better. But what do you think
>> about taking the lock first and then checking only once?
>
> I don't get what you mean. If we take the lock and then check that array should
> suspend, we still have to release the lock before we suspend the array.
>
Sorry, I was not clear. I meant, do we need to check twice, once before
taking the lock and once after? It seems that the check before taking the
lock is redundant. since the result would need to be checked again after
taking the lock anyway.
Could we drop the check before taking the lock and only check whether
suspension is needed once while holding it? If suspension is needed, we
would release the lock, suspend the array, and then reacquire the lock.
> --
> Thanks,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: yu kuai @ 2026-07-07 1:12 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, yu kuai, song, magiclinan, xiao
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <m2se5x6wff.fsf@gmail.com>
Hi,
在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道:
>> The problem looks real, however, I think this will cause a change that user will be awared,
>> if there are really spares that can be removed from conf, but array is not suspended here,
>> user will still expect rdev will be removed from conf automatically.
>>
>> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
>> supposed to be true, we can release the lock and retry with suspend = true.
>>
> Yes, I see, and your approach is much better. But what do you think
> about taking the lock first and then checking only once?
I don't get what you mean. If we take the lock and then check that array should
suspend, we still have to release the lock before we suspend the array.
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: Abd-Alrhman Masalkhi @ 2026-07-06 11:35 UTC (permalink / raw)
To: John Garry, song, yukuai, magiclinan, xiao, axboe, vverma,
martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <856abedc-d2f3-4248-9583-4d7e1a11abc1@oracle.com>
On Fri, Jul 03, 2026 at 09:28 +0100, John Garry wrote:
> On 30/06/2026 09:39, Abd-Alrhman Masalkhi wrote:
>>
>> Hi John,
>>
>> On Mon, Jun 29, 2026 at 15:48 +0100, John Garry wrote:
>>> On 28/06/2026 15:24, Abd-Alrhman Masalkhi wrote:
>>>> Atomic writes in RAID1 must fit within a single barrier unit. Advertise
>>>> this restriction through the queue limits by setting
>>>> atomic_write_hw_unit_max to BARRIER_UNIT_SECTOR_SIZE so that bios which
>>>> would cross a barrier-unit boundary are rejected by the block layer
>>>> before reaching MD.
>>>>
>>>> A bio that passes block-layer validation may still become unserviceable
>>>> within RAID1 due to bad blocks or write-behind constraints. In the former
>>>> case, complete the bio with EIO. In the latter case, disable
>>>> write-behind rather than failing the bio with EIO.
>>>>
>>>> Fixes: f2a38abf5f1c ("md/raid1: Atomic write support")
>>>> Fixes: a4c55c902670 ("md/raid1: simplify raid1_write_request() error handling")
>>>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>>>> ---
>>>> Changes in v2:
>>>> - Drop the early atomic write split check from raid1_write_request().
>>>> - Advertise the atomic write size limit via queue limits.
>>>> - Disable write-behind instead of failing atomic writes when the
>>>> BIO_MAX_VECS limit is encountered.
>>>> - Link to v1: https://urldefense.com/v3/__https://lore.kernel.org/linux-raid/20260623072456.333437-3-abd.masalkhi@gmail.com/__;!!ACWV5N9M2RV99hQ!LbMSGSClRi0PNBqQti5ZNWGDVjDd34-7saYEAwNyBNjpNTjEA7veqM5RHG8KB1QiscarW4UaIefjm19ywSImtIgh$
>>>> ---
>>>> drivers/md/raid1.c | 36 +++++++++++++++++++-----------------
>>>> 1 file changed, 19 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>>>> index afe2ca96ad8c..f322048ab3c2 100644
>>>> --- a/drivers/md/raid1.c
>>>> +++ b/drivers/md/raid1.c
>>>> @@ -1522,6 +1522,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>>>> int first_clone;
>>>> bool write_behind = false;
>>>> bool nowait = bio->bi_opf & REQ_NOWAIT;
>>>> + bool atomic = bio->bi_opf & REQ_ATOMIC;
>>>> bool is_discard = op_is_discard(bio->bi_opf);
>>>> sector_t sector = bio->bi_iter.bi_sector;
>>>>
>>>> @@ -1603,20 +1604,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>>>> }
>>>> if (is_bad) {
>>>> int good_sectors;
>>>> -
>>>> - /*
>>>> - * We cannot atomically write this, so just
>>>> - * error in that case. It could be possible to
>>>> - * atomically write other mirrors, but the
>>>> - * complexity of supporting that is not worth
>>>> - * the benefit.
>>>> - */
>>>> - if (bio->bi_opf & REQ_ATOMIC) {
>>>> - bio->bi_status = BLK_STS_NOTSUPP;
>>>> - bio_endio(bio);
>>>> - goto err_dec_pending;
>>>> - }
>>>> -
>>>> good_sectors = first_bad - sector;
>>>> if (good_sectors < max_sectors)
>>>> max_sectors = good_sectors;
>>>> @@ -1633,10 +1620,24 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>>>> * at a time and thus needs a new bio that can fit the whole payload
>>>> * this bio in page sized chunks.
>>>> */
>>>> - if (write_behind && mddev->bitmap)
>>>> - max_sectors = min_t(int, max_sectors,
>>>> - BIO_MAX_VECS * (PAGE_SIZE >> 9));
>>>> + if (write_behind && mddev->bitmap) {
>>>> + if (atomic && max_sectors > BIO_MAX_VECS * (PAGE_SIZE >> 9))
>>>
>>> where does BIO_MAX_VECS * (PAGE_SIZE >> 9) even come from?
>>>
>>
>> BIO_MAX_VECS * (PAGE_SIZE >> 9) defines the maximum size supported by
>> write-behind. The write-behind copy (alloc_behind_master_bio) uses a
>> single bio, which can hold at most BIO_MAX_VECS pages, making this the
>> largest payload it can carry. With a 4 KiB PAGE_SIZE, that corresponds
>> to 256 pages, or 1 MiB (2048 sectors).
>> This patch changes the behavior for atomic writes that exceed this
>> limit. Instead of failing the write with -EIO when the number of sectors
>> must be reduced, it disables write-behind and proceeds with the atomic
>> write.
>>
>>>> + /*
>>>> + * Atomic writes cannot be split, so disable
>>>> + * write-behind.
>>>> + */
>>>> + write_behind = false;
>
> It's a bit poor to have write_behind initially = false, then allow it to
> be set = true, and then later be set = false. Can this be improved?
>
Yes, I agree that setting write_behind to true and then resetting it to
false later is not ideal. I will rework this.
>>>> + else
>>>> + max_sectors = min_t(int, max_sectors,
>>>> + BIO_MAX_VECS * (PAGE_SIZE >> 9));
>>>> + }
>>>> +
>>>> if (max_sectors < bio_sectors(bio)) {
>>>> + if (atomic) {
>>>> + bio_io_error(bio);
>>>> + goto err_dec_pending;
>>>> + }
>>>> +
>>>> bio = bio_submit_split_bioset(bio, max_sectors,
>>>> &conf->bio_split);
>>>> if (!bio)
>>>> @@ -3229,6 +3230,7 @@ static int raid1_set_limits(struct mddev *mddev)
>>>> lim.max_write_zeroes_sectors = 0;
>>>> lim.max_hw_wzeroes_unmap_sectors = 0;
>>>> lim.logical_block_size = mddev->logical_block_size;
>>>> + lim.atomic_write_hw_unit_max = BARRIER_UNIT_SECTOR_SIZE;
>>>
>>> This BARRIER_UNIT_SECTOR_SIZE is a bit like chunk sectors, no? I am just
>>> wondering if we just should set it to chunk sectors =
>>> BARRIER_UNIT_SECTOR_SIZE
>>>
>>> I assume that it affects more than Reads and writes, e.g. discard also.
>>>
>>
>> BARRIER_UNIT_SECTOR_SIZE is the resync barrier-bucket, not the layout
>> chunk size. Unless I'm missing something, using
>> atomic_write_hw_unit_max seems more appropriate than using the chunk
>> size. That way, the limit only applies to atomic writes instead of
>> affecting other operations such.
>
> well it seems to be that everything in the driver is split over
> BARRIER_UNIT_SECTOR_SIZE, so is in effect a chunk size.
>
> Note that atomic_write_hw_unit_max is going to be small always compared
> to BARRIER_UNIT_SECTOR_SIZE.
>
> However, can you check the blk stacking code to make sure that this does
> as you want? As I remember, for stacking we take the atomic write limits
> of the first bottom device and then stack the other bottom devices and I
> don't think that setting atomic_write_hw_unit_max in this way has an
> impact - see blk_stack_atomic_writes_limits()
>
I checked blk_stack_atomic_writes_limits(), and you are right: setting
atomic_write_hw_unit_max here alone does not have the intended effect on
the final stacked device.
Also, since atomic_write_hw_unit_max is expected to always be smaller
than BARRIER_UNIT_SECTOR_SIZE, it seems that there is no need to set an
additional atomic write limit in raid1, or even to set
atomic_write_hw_unit_max at all. Is that what you mean?
>>
>>>> lim.features |= BLK_FEAT_ATOMIC_WRITES;
>>>> lim.features |= BLK_FEAT_PCI_P2PDMA;
>>>> err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
>>>
>>
>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH 0/8] md/raid5: scalability and rebuild-path improvements
From: yu kuai @ 2026-07-06 2:56 UTC (permalink / raw)
To: Hiroshi Nishida, Song Liu
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, yukuai
In-Reply-To: <20260624155452.211646-1-nishidafmly@gmail.com>
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
^ permalink raw reply
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: Abd-Alrhman Masalkhi @ 2026-07-05 19:58 UTC (permalink / raw)
To: yu kuai, song, magiclinan, xiao, yukuai
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <0a7dc7e2-30af-44de-8c29-b6e27a62fa55@fygo.io>
Hi Kuai,
On Mon, Jul 06, 2026 at 00:16 +0800, yu kuai wrote:
> Hi,
>
> 在 2026/6/30 15:56, Abd-Alrhman Masalkhi 写道:
>> remove_spares() and remove_and_add_spares() modify the array's rdev
>> configuration. These operations are only safe after the array has been
>> suspended.
>>
>> Today, md_start_sync() can call md_choose_sync_action() even when the
>> array has not been suspended. As a result, md_choose_sync_action() may
>> remove or replace rdevs while normal I/O is still accessing them.
>>
>> The race can occur as follows:
>>
>> raid10d Worker Normal IO
>> ____________ _______________________ ______________________
>>
>> raid10_write_request()
>> wait_blocked_dev()
>> set Blocked
>> set Faulty
>> Skip Faulty rdev
>> rrdev->nr_pending++
>> .repl_bio = bio
>> removeable_rdev = false .
>> array not suspended .
>> lock mddev goto err_handle
>> lock mddev (wait)
>> .
>> update sb .
>> clear Blocked .
>> .
>> unlock mddev .
>> lock mddev (acquires)
>> remove_spares()
>> removeable_rdev = true
>>
>> raid10_remove_disk()
>> rdev = replacement
>> replacement = NULL
>> rdev_dec_pending(NULL)
>> unlock mddev (NULL)->nr_pending--
>>
>> In this case, rdev_dec_pending() is called with a NULL pointer,
>> resulting in a NULL pointer dereference when attempting to decrement
>> nr_pending.
>>
>> Fix this by ensuring that remove_spares() and remove_and_add_spares()
>> are only called after the array has been suspended, preventing
>> concurrent rdev configuration changes while normal I/O is in progress.
>>
>> Fixes: bc08041b32ab ("md: suspend array in md_start_sync() if array need reconfiguration")
>> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
>> Closes: https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3
>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>> ---
>> drivers/md/md.c | 18 +++++++++++++-----
>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 66a41d482e59..c85ebb59535b 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -10116,7 +10116,8 @@ static int remove_and_add_spares(struct mddev *mddev,
>> return spares;
>> }
>>
>> -static bool md_choose_sync_action(struct mddev *mddev, int *spares)
>> +static bool md_choose_sync_action(struct mddev *mddev, int *spares,
>> + bool array_suspended)
>> {
>> /* Check if reshape is in progress first. */
>> if (mddev->reshape_position != MaxSector) {
>> @@ -10132,7 +10133,9 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
>>
>> /* Check if resync is in progress. */
>> if (mddev->resync_offset < MaxSector) {
>> - remove_spares(mddev, NULL);
>> + if (array_suspended)
>> + remove_spares(mddev, NULL);
>
> The problem looks real, however, I think this will cause a change that user will be awared,
> if there are really spares that can be removed from conf, but array is not suspended here,
> user will still expect rdev will be removed from conf automatically.
>
> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
> supposed to be true, we can release the lock and retry with suspend = true.
>
Yes, I see, and your approach is much better. But what do you think
about taking the lock first and then checking only once?
>> +
>> set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
>> clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
>> clear_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery);
>> @@ -10144,7 +10147,11 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
>> * also removed and re-added, to allow the personality to fail the
>> * re-add.
>> */
>> - *spares = remove_and_add_spares(mddev, NULL);
>> + if (array_suspended)
>> + *spares = remove_and_add_spares(mddev, NULL);
>> + else
>> + *spares = 0;
>> +
>> if (*spares || test_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery)) {
>> clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
>> clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
>> @@ -10189,11 +10196,12 @@ static void md_start_sync(struct work_struct *ws)
>> * As we only add devices that are already in-sync, we can
>> * activate the spares immediately.
>> */
>> - remove_and_add_spares(mddev, NULL);
>> + if (suspend)
>> + remove_and_add_spares(mddev, NULL);
>> goto not_running;
>> }
>>
>> - if (!md_choose_sync_action(mddev, &spares))
>> + if (!md_choose_sync_action(mddev, &spares, suspend))
>> goto not_running;
>>
>> if (!mddev->pers->sync_request)
>
> --
> Thanks,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: yu kuai @ 2026-07-05 16:16 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, magiclinan, xiao, yukuai
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <20260630075640.1081634-1-abd.masalkhi@gmail.com>
Hi,
在 2026/6/30 15:56, Abd-Alrhman Masalkhi 写道:
> remove_spares() and remove_and_add_spares() modify the array's rdev
> configuration. These operations are only safe after the array has been
> suspended.
>
> Today, md_start_sync() can call md_choose_sync_action() even when the
> array has not been suspended. As a result, md_choose_sync_action() may
> remove or replace rdevs while normal I/O is still accessing them.
>
> The race can occur as follows:
>
> raid10d Worker Normal IO
> ____________ _______________________ ______________________
>
> raid10_write_request()
> wait_blocked_dev()
> set Blocked
> set Faulty
> Skip Faulty rdev
> rrdev->nr_pending++
> .repl_bio = bio
> removeable_rdev = false .
> array not suspended .
> lock mddev goto err_handle
> lock mddev (wait)
> .
> update sb .
> clear Blocked .
> .
> unlock mddev .
> lock mddev (acquires)
> remove_spares()
> removeable_rdev = true
>
> raid10_remove_disk()
> rdev = replacement
> replacement = NULL
> rdev_dec_pending(NULL)
> unlock mddev (NULL)->nr_pending--
>
> In this case, rdev_dec_pending() is called with a NULL pointer,
> resulting in a NULL pointer dereference when attempting to decrement
> nr_pending.
>
> Fix this by ensuring that remove_spares() and remove_and_add_spares()
> are only called after the array has been suspended, preventing
> concurrent rdev configuration changes while normal I/O is in progress.
>
> Fixes: bc08041b32ab ("md: suspend array in md_start_sync() if array need reconfiguration")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> drivers/md/md.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 66a41d482e59..c85ebb59535b 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -10116,7 +10116,8 @@ static int remove_and_add_spares(struct mddev *mddev,
> return spares;
> }
>
> -static bool md_choose_sync_action(struct mddev *mddev, int *spares)
> +static bool md_choose_sync_action(struct mddev *mddev, int *spares,
> + bool array_suspended)
> {
> /* Check if reshape is in progress first. */
> if (mddev->reshape_position != MaxSector) {
> @@ -10132,7 +10133,9 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
>
> /* Check if resync is in progress. */
> if (mddev->resync_offset < MaxSector) {
> - remove_spares(mddev, NULL);
> + if (array_suspended)
> + remove_spares(mddev, NULL);
The problem looks real, however, I think this will cause a change that user will be awared,
if there are really spares that can be removed from conf, but array is not suspended here,
user will still expect rdev will be removed from conf automatically.
In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
supposed to be true, we can release the lock and retry with suspend = true.
> +
> set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
> clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
> clear_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery);
> @@ -10144,7 +10147,11 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
> * also removed and re-added, to allow the personality to fail the
> * re-add.
> */
> - *spares = remove_and_add_spares(mddev, NULL);
> + if (array_suspended)
> + *spares = remove_and_add_spares(mddev, NULL);
> + else
> + *spares = 0;
> +
> if (*spares || test_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery)) {
> clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
> clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
> @@ -10189,11 +10196,12 @@ static void md_start_sync(struct work_struct *ws)
> * As we only add devices that are already in-sync, we can
> * activate the spares immediately.
> */
> - remove_and_add_spares(mddev, NULL);
> + if (suspend)
> + remove_and_add_spares(mddev, NULL);
> goto not_running;
> }
>
> - if (!md_choose_sync_action(mddev, &spares))
> + if (!md_choose_sync_action(mddev, &spares, suspend))
> goto not_running;
>
> if (!mddev->pers->sync_request)
--
Thanks,
Kuai
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox