* [PATCH 1/8] xor: enable lock context analysis
2026-07-08 9:07 misc lib/raid/ improvements Christoph Hellwig
@ 2026-07-08 9:07 ` Christoph Hellwig
2026-07-08 9:07 ` [PATCH 2/8] xor: improve the runtime selection benchmark Christoph Hellwig
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
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 [flat|nested] 15+ messages in thread* [PATCH 2/8] xor: improve the runtime selection benchmark
2026-07-08 9:07 misc lib/raid/ improvements Christoph Hellwig
2026-07-08 9:07 ` [PATCH 1/8] xor: enable lock context analysis Christoph Hellwig
@ 2026-07-08 9:07 ` Christoph Hellwig
2026-07-08 9:18 ` sashiko-bot
2026-07-08 9:07 ` [PATCH 3/8] xor/kunit: fix a spelling error Christoph Hellwig
` (5 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
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 [flat|nested] 15+ messages in thread* Re: [PATCH 2/8] xor: improve the runtime selection benchmark
2026-07-08 9:07 ` [PATCH 2/8] xor: improve the runtime selection benchmark Christoph Hellwig
@ 2026-07-08 9:18 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-07-08 9:18 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A division by zero kernel panic will occur during boot on systems using a coarse clocksource when calculating the benchmark speed.
--
--- Patch [2]: [PATCH 2/8] xor: improve the runtime selection benchmark ---
commit 69c8bf90e6d6fa234ebb61b5b5b7c4dfd2d8e9c1
Author: Christoph Hellwig <hch@lst.de>
xor: improve the runtime selection benchmark
This commit updates the XOR benchmark to use ktime_get_ns() for timing,
increases the workload to 4 source disks, and calculates throughput based
on the data disks.
> diff --git a/lib/raid/xor/xor-core.c b/lib/raid/xor/xor-core.c
> --- a/lib/raid/xor/xor-core.c
> +++ b/lib/raid/xor/xor-core.c
[ ... ]
> @@ -73,59 +72,56 @@ void __init xor_force(struct xor_block_template *tmpl)
[ ... ]
> -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])
> {
[ ... ]
> + 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);
[Severity: High]
Could this cause a division by zero panic during boot on systems with a
coarse clocksource?
If the 800 loop iterations execute very quickly (within the same clock tick)
on a platform without high-resolution timers, or during early boot phases
where jiffies are used as the clocksource, the elapsed time t will evaluate
to 0.
The previous code avoided this by looping until the clock advanced before
and after the benchmark:
while ((start = ktime_get()) == t0)
Since t is now used directly as the divisor in div64_u64(), a 0 value will
trigger an unhandled divide-by-zero hardware exception.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708090740.1433685-1-hch@lst.de?part=2
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/8] xor/kunit: fix a spelling error
2026-07-08 9:07 misc lib/raid/ improvements Christoph Hellwig
2026-07-08 9:07 ` [PATCH 1/8] xor: enable lock context analysis Christoph Hellwig
2026-07-08 9:07 ` [PATCH 2/8] xor: improve the runtime selection benchmark Christoph Hellwig
@ 2026-07-08 9:07 ` Christoph Hellwig
2026-07-08 9:07 ` [PATCH 4/8] xor/kunit: add a benchmark Christoph Hellwig
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
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 [flat|nested] 15+ messages in thread* [PATCH 4/8] xor/kunit: add a benchmark
2026-07-08 9:07 misc lib/raid/ improvements Christoph Hellwig
` (2 preceding siblings ...)
2026-07-08 9:07 ` [PATCH 3/8] xor/kunit: fix a spelling error Christoph Hellwig
@ 2026-07-08 9:07 ` Christoph Hellwig
2026-07-08 9:22 ` sashiko-bot
2026-07-08 9:07 ` [PATCH 5/8] raid6: enable lock context analysis Christoph Hellwig
` (3 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
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 [flat|nested] 15+ messages in thread* Re: [PATCH 4/8] xor/kunit: add a benchmark
2026-07-08 9:07 ` [PATCH 4/8] xor/kunit: add a benchmark Christoph Hellwig
@ 2026-07-08 9:22 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-07-08 9:22 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Potential divide-by-zero kernel panic in `xor_benchmark()` when calculating the benchmark speed if the execution time is 0.
- [Medium] The benchmark holds `preempt_disable()` for the entire duration of 1000 XOR iterations, risking soft lockups on slower hardware or emulators.
--
--- Patch [4]: [PATCH 4/8] xor/kunit: add a benchmark ---
commit 4f82691a9522c9ab25bdc94cf0c1ad74dc8c3b7e
Author: Christoph Hellwig <hch@lst.de>
xor/kunit: add a benchmark
This patch adds a benchmark to test the XOR functions for more
representative block sizes and numbers of disks. It measures the execution
time of 1000 iterations of xor_gen() for varying buffer counts and lengths,
reporting the throughput in GB/s.
> diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
> index 659ae3edbc25b..648c6da9464c4 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)
> +{
[ ... ]
> + 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();
[Severity: Medium]
Does this workload risk triggering the kernel's soft lockup detector?
The test executes a hardcoded 1000 iterations of xor_gen() on up to 32 buffers
of 16KB within a single preempt_disable() block. This amounts to processing
over 500MB of data atomically.
In slower environments like QEMU TCG without KVM, this large computational
workload could take tens of seconds, which might exceed the typical 20-second
soft lockup threshold.
> +
> + speed[j] = div64_u64((u64)len * num_iters * nr, t);
[Severity: High]
Can this result in a divide-by-zero kernel panic?
If the benchmark runs on a system with coarse timekeeping resolution (e.g.,
using the jiffies clocksource), and the CPU completes the 1000 iterations
within a single timer tick, the elapsed time `t` could evaluate to 0.
Since `t` is used directly as the divisor in div64_u64() without being
checked, this might trigger a hardware exception and panic the kernel.
> + }
> +
> + 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]);
> + }
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708090740.1433685-1-hch@lst.de?part=4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/8] raid6: enable lock context analysis
2026-07-08 9:07 misc lib/raid/ improvements Christoph Hellwig
` (3 preceding siblings ...)
2026-07-08 9:07 ` [PATCH 4/8] xor/kunit: add a benchmark Christoph Hellwig
@ 2026-07-08 9:07 ` Christoph Hellwig
2026-07-08 9:07 ` [PATCH 6/8] raid6: defer implementation selection when built-in Christoph Hellwig
` (2 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
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 [flat|nested] 15+ messages in thread* [PATCH 6/8] raid6: defer implementation selection when built-in
2026-07-08 9:07 misc lib/raid/ improvements Christoph Hellwig
` (4 preceding siblings ...)
2026-07-08 9:07 ` [PATCH 5/8] raid6: enable lock context analysis Christoph Hellwig
@ 2026-07-08 9:07 ` Christoph Hellwig
2026-07-08 9:23 ` sashiko-bot
2026-07-08 9:07 ` [PATCH 7/8] raid6: improve the runtime selection benchmark Christoph Hellwig
2026-07-08 9:07 ` [PATCH 8/8] raid6/kunit: add a benchmark Christoph Hellwig
7 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
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 [flat|nested] 15+ messages in thread* Re: [PATCH 6/8] raid6: defer implementation selection when built-in
2026-07-08 9:07 ` [PATCH 6/8] raid6: defer implementation selection when built-in Christoph Hellwig
@ 2026-07-08 9:23 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-07-08 9:23 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The boot-time RAID6 benchmark runs unconditionally when built-in, ignoring the `CONFIG_RAID6_PQ_BENCHMARK` setting and the single-algorithm optimization.
--
--- Patch [6]: [PATCH 6/8] raid6: defer implementation selection when built-in ---
commit 2c9ec4f0b44515f49715b67c2e70f8628c5abd61
Author: Christoph Hellwig <hch@lst.de>
Subject: raid6: defer implementation selection when built-in
This commit defers the RAID6 implementation selection to avoid holding up
early boot. It follows the pattern used in the RAID5 XOR code, allowing
full use of the timing subsystem for benchmarking.
> diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
> index 6f5c89ab2b17b..e7984bde5157f 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);
[ ... ]
> @@ -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
[ ... ]
> +/*
> + * 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
[Severity: Medium]
Will this unconditional device_initcall cause the benchmark to run even when
it is intentionally disabled in the configuration?
While raid6_init() checks CONFIG_RAID6_PQ_BENCHMARK and correctly skips the
early calibration phase, removing that same check from raid6_select_algo()
means this deferred call will unconditionally run the benchmarking loop later
in the boot process. It looks like this bypasses both the config setting and
the single-algorithm optimization entirely.
> subsys_initcall(raid6_init);
> module_exit(raid6_exit);
> MODULE_LICENSE("GPL");
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708090740.1433685-1-hch@lst.de?part=6
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 7/8] raid6: improve the runtime selection benchmark
2026-07-08 9:07 misc lib/raid/ improvements Christoph Hellwig
` (5 preceding siblings ...)
2026-07-08 9:07 ` [PATCH 6/8] raid6: defer implementation selection when built-in Christoph Hellwig
@ 2026-07-08 9:07 ` Christoph Hellwig
2026-07-08 9:20 ` sashiko-bot
2026-07-08 9:07 ` [PATCH 8/8] raid6/kunit: add a benchmark Christoph Hellwig
7 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
Use plain ktime_get_ns for the timing, use 8 + 2 disks for a realistic
load, and report the throughput on the data disks as that is what
storage systems are measured on.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/algos.c | 75 +++++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 41 deletions(-)
diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
index e7984bde5157..c44b986e319e 100644
--- a/lib/raid/raid6/algos.c
+++ b/lib/raid/raid6/algos.c
@@ -152,40 +152,39 @@ void raid6_recov_datap(int disks, size_t bytes, int faila, void **ptrs)
}
EXPORT_SYMBOL_GPL(raid6_recov_datap);
-#define RAID6_TIME_JIFFIES_LG2 4
-#define RAID6_TEST_DISKS 8
+#define BENCH_SIZE SZ_4K
+#define NR_SRCS 8
+#define NR_DISKS (NR_SRCS + 2)
+#define REPS 800U
-static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
- const int disks)
+static int raid6_choose_gen(void *dptrs[NR_DISKS], const int disks)
{
/* work on the second half of the disks */
- int start = (disks >> 1) - 1, stop = disks - 3;
+ int start = (disks / 2) - 1, stop = disks - 3;
const struct raid6_calls *best = NULL;
unsigned long bestgenperf = 0;
unsigned int i;
for (i = 0; i < raid6_nr_algos; i++) {
const struct raid6_calls *algo = raid6_algos[i];
- unsigned long perf = 0, j0, j1;
+ unsigned long perf = 0;
+ u64 t;
+ int i;
preempt_disable();
- j0 = jiffies;
- while ((j1 = jiffies) == j0)
- cpu_relax();
- while (time_before(jiffies,
- j1 + (1<<RAID6_TIME_JIFFIES_LG2))) {
- algo->gen_syndrome(disks, PAGE_SIZE, *dptrs);
- perf++;
- }
+ t = ktime_get_ns();
+ for (i = 0; i < REPS; i++)
+ algo->gen_syndrome(disks, BENCH_SIZE, dptrs);
+ t = ktime_get_ns() - t;
preempt_enable();
+ /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
+ perf = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
if (perf > bestgenperf) {
bestgenperf = perf;
best = algo;
}
- pr_info("raid6: %-8s gen() %5ld MB/s\n", algo->name,
- (perf * HZ * (disks-2)) >>
- (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2));
+ pr_info("raid6: %-8s gen() %5lu MB/s\n", algo->name, perf);
}
if (!best) {
@@ -197,28 +196,22 @@ static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
static_call_update(raid6_xor_syndrome_impl, best->xor_syndrome);
pr_info("raid6: using algorithm %s gen() %ld MB/s\n",
- best->name,
- (bestgenperf * HZ * (disks - 2)) >>
- (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2));
+ best->name, bestgenperf);
if (best->xor_syndrome) {
- unsigned long perf = 0, j0, j1;
+ u64 t;
+ int i;
preempt_disable();
- j0 = jiffies;
- while ((j1 = jiffies) == j0)
- cpu_relax();
- while (time_before(jiffies,
- j1 + (1 << RAID6_TIME_JIFFIES_LG2))) {
- best->xor_syndrome(disks, start, stop,
- PAGE_SIZE, *dptrs);
- perf++;
- }
+ t = ktime_get_ns();
+ for (i = 0; i < REPS; i++)
+ best->xor_syndrome(disks, start, stop, BENCH_SIZE,
+ dptrs);
+ t = ktime_get_ns() - t;
preempt_enable();
- pr_info("raid6: .... xor() %ld MB/s, rmw enabled\n",
- (perf * HZ * (disks - 2)) >>
- (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2 + 1));
+ pr_info("raid6: .... xor() %llu MB/s, rmw enabled\n",
+ div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t));
}
return 0;
@@ -230,14 +223,14 @@ static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
static int __init raid6_select_algo(void)
{
- const int disks = RAID6_TEST_DISKS;
+ const int disks = NR_DISKS;
+ void *dptrs[NR_DISKS];
char *disk_ptr, *p;
- void *dptrs[RAID6_TEST_DISKS];
int i, cycle;
int error;
/* prepare the buffer and fill it circularly with gfmul table */
- disk_ptr = kmalloc(PAGE_SIZE * RAID6_TEST_DISKS, GFP_KERNEL);
+ disk_ptr = kmalloc_array(NR_DISKS, BENCH_SIZE, GFP_KERNEL);
if (!disk_ptr) {
pr_err("raid6: Yikes! No memory available.\n");
return -ENOMEM;
@@ -245,19 +238,19 @@ static int __init raid6_select_algo(void)
p = disk_ptr;
for (i = 0; i < disks; i++)
- dptrs[i] = p + PAGE_SIZE * i;
+ dptrs[i] = p + BENCH_SIZE * i;
- cycle = ((disks - 2) * PAGE_SIZE) / 65536;
+ cycle = ((disks - 2) * BENCH_SIZE) / 65536;
for (i = 0; i < cycle; i++) {
memcpy(p, raid6_gfmul, 65536);
p += 65536;
}
- if ((disks - 2) * PAGE_SIZE % 65536)
- memcpy(p, raid6_gfmul, (disks - 2) * PAGE_SIZE % 65536);
+ if ((disks - 2) * BENCH_SIZE % 65536)
+ memcpy(p, raid6_gfmul, (disks - 2) * BENCH_SIZE % 65536);
/* select raid gen_syndrome function */
- error = raid6_choose_gen(&dptrs, disks);
+ error = raid6_choose_gen(dptrs, disks);
kfree(disk_ptr);
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 7/8] raid6: improve the runtime selection benchmark
2026-07-08 9:07 ` [PATCH 7/8] raid6: improve the runtime selection benchmark Christoph Hellwig
@ 2026-07-08 9:20 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-07-08 9:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Potential division by zero kernel panic when calculating benchmark throughput if the elapsed time `t` evaluates to 0.
- [Medium] The reported throughput for the XOR algorithm is artificially inflated by a factor of 2.
- [Low] The benchmark inner loop variable `int i` redundantly shadows the outer function-scoped variable `unsigned int i`.
--
--- Patch [7]: [PATCH 7/8] raid6: improve the runtime selection benchmark ---
commit ae782577f509b50023ef73d6d96f6fe2f366a500
Author: Christoph Hellwig <hch@lst.de>
raid6: improve the runtime selection benchmark
Use plain ktime_get_ns for the timing, use 8 + 2 disks for a realistic
load, and report the throughput on the data disks as that is what
storage systems are measured on.
> diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
> index e7984bde5157f..c44b986e319e3 100644
> --- a/lib/raid/raid6/algos.c
> +++ b/lib/raid/raid6/algos.c
[ ... ]
> @@ -152,40 +152,39 @@ void raid6_recov_datap(int disks, size_t bytes, int faila, void **ptrs)
> }
> EXPORT_SYMBOL_GPL(raid6_recov_datap);
>
> -#define RAID6_TIME_JIFFIES_LG2 4
> -#define RAID6_TEST_DISKS 8
> +#define BENCH_SIZE SZ_4K
> +#define NR_SRCS 8
> +#define NR_DISKS (NR_SRCS + 2)
> +#define REPS 800U
>
> -static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
> - const int disks)
> +static int raid6_choose_gen(void *dptrs[NR_DISKS], const int disks)
> {
> /* work on the second half of the disks */
> - int start = (disks >> 1) - 1, stop = disks - 3;
> + int start = (disks / 2) - 1, stop = disks - 3;
> const struct raid6_calls *best = NULL;
> unsigned long bestgenperf = 0;
> unsigned int i;
>
> for (i = 0; i < raid6_nr_algos; i++) {
> const struct raid6_calls *algo = raid6_algos[i];
> - unsigned long perf = 0, j0, j1;
> + unsigned long perf = 0;
> + u64 t;
> + int i;
[Severity: Low]
This isn't a bug, but `int i` declared here shadows the function-scoped
`unsigned int i` used for the outer loop.
>
> preempt_disable();
> - j0 = jiffies;
> - while ((j1 = jiffies) == j0)
> - cpu_relax();
> - while (time_before(jiffies,
> - j1 + (1<<RAID6_TIME_JIFFIES_LG2))) {
> - algo->gen_syndrome(disks, PAGE_SIZE, *dptrs);
> - perf++;
> - }
> + t = ktime_get_ns();
> + for (i = 0; i < REPS; i++)
> + algo->gen_syndrome(disks, BENCH_SIZE, dptrs);
> + t = ktime_get_ns() - t;
> preempt_enable();
>
> + /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
> + perf = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
[Severity: High]
Is it possible for `t` to be 0 here, leading to a division by zero panic?
If the system uses a coarse clock source (like jiffies during early boot before
high-resolution timers are initialized) and the benchmark iterations complete
within the same timer tick, `t` might evaluate to 0.
[ ... ]
> @@ -197,28 +196,22 @@ static int raid6_choose_gen(void *(*const dptrs)[RAID6_TEST_DISKS],
> static_call_update(raid6_xor_syndrome_impl, best->xor_syndrome);
>
> pr_info("raid6: using algorithm %s gen() %ld MB/s\n",
> - best->name,
> - (bestgenperf * HZ * (disks - 2)) >>
> - (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2));
> + best->name, bestgenperf);
>
> if (best->xor_syndrome) {
> - unsigned long perf = 0, j0, j1;
> + u64 t;
> + int i;
[Severity: Low]
This isn't a bug, but `int i` declared here also shadows the function-scoped
`unsigned int i`.
>
> preempt_disable();
> - j0 = jiffies;
> - while ((j1 = jiffies) == j0)
> - cpu_relax();
> - while (time_before(jiffies,
> - j1 + (1 << RAID6_TIME_JIFFIES_LG2))) {
> - best->xor_syndrome(disks, start, stop,
> - PAGE_SIZE, *dptrs);
> - perf++;
> - }
> + t = ktime_get_ns();
> + for (i = 0; i < REPS; i++)
> + best->xor_syndrome(disks, start, stop, BENCH_SIZE,
> + dptrs);
> + t = ktime_get_ns() - t;
> preempt_enable();
>
> - pr_info("raid6: .... xor() %ld MB/s, rmw enabled\n",
> - (perf * HZ * (disks - 2)) >>
> - (20 - PAGE_SHIFT + RAID6_TIME_JIFFIES_LG2 + 1));
> + pr_info("raid6: .... xor() %llu MB/s, rmw enabled\n",
> + div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t));
[Severity: Medium]
Does this calculation artificially inflate the reported throughput for the XOR
algorithm by a factor of 2?
The `xor_syndrome` benchmark operates on a subset of disks from `start` to
`stop` (which is `(disks / 2) - 1` through `disks - 3`). For the default
`NR_DISKS` (10), this processes 4 data disks. However, the throughput
calculation multiplies by `NR_SRCS` (8) instead of the actual number of disks
processed. The previous implementation correctly adjusted for this using an
extra bit shift (`+ 1`), which was dropped in this patch.
[Severity: High]
Like the calculation above, could `t` be 0 here leading to a division by zero
panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708090740.1433685-1-hch@lst.de?part=7
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 8/8] raid6/kunit: add a benchmark
2026-07-08 9:07 misc lib/raid/ improvements Christoph Hellwig
` (6 preceding siblings ...)
2026-07-08 9:07 ` [PATCH 7/8] raid6: improve the runtime selection benchmark Christoph Hellwig
@ 2026-07-08 9:07 ` Christoph Hellwig
2026-07-08 9:20 ` sashiko-bot
7 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2026-07-08 9:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
Add a benchmark to test the XOR functions for more representative block
sizes and numbers of disks. This splits the maximum alloc size used for
the benchmark from the max size for the kunit test because recovery
is currently limited to a single page at a time. Hopefully this will be
fixed soon.
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 | 9 ++++
lib/raid/raid6/tests/raid6_kunit.c | 66 +++++++++++++++++++++++++++++-
2 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig
index c4eeb7c716c8..c52d5c205294 100644
--- a/lib/raid/Kconfig
+++ b/lib/raid/Kconfig
@@ -60,6 +60,15 @@ config RAID6_PQ_KUNIT_TEST
This is intended to help people writing architecture-specific
optimized versions. If unsure, say N.
+config RAID6_PQ_KUNIT_BENCHMARK
+ bool "Benchmark for RAID6 PQ"
+ depends on RAID6_PQ_KUNIT_TEST
+ help
+ Include benchmarks in the KUnit test suite for raid P/Q generation.
+
+config RAID6_PQ
+ tristate
+
config RAID6_PQ_BENCHMARK
bool "Automatically choose fastest RAID6 PQ functions"
depends on RAID6_PQ
diff --git a/lib/raid/raid6/tests/raid6_kunit.c b/lib/raid/raid6/tests/raid6_kunit.c
index 9f3e671a1224..f927e3456d17 100644
--- a/lib/raid/raid6/tests/raid6_kunit.c
+++ b/lib/raid/raid6/tests/raid6_kunit.c
@@ -18,6 +18,7 @@ MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
#define RAID6_KUNIT_MAX_BUFFERS 64 /* Including P and Q */
#define RAID6_KUNIT_MAX_FAILURES 2
#define RAID6_KUNIT_MAX_BYTES PAGE_SIZE
+#define RAID6_KUNIT_ALLOC_BYTES SZ_16K
static struct rnd_state rng;
static void *test_buffers[RAID6_KUNIT_MAX_BUFFERS];
@@ -229,6 +230,68 @@ static void raid6_test(struct kunit *test)
raid6_test_one(test);
}
+static void raid6_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_RAID6_PQ_KUNIT_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++) {
+ raid6_gen_syndrome(nr_to_test[i],
+ len_to_test[j], test_buffers);
+ }
+ }
+ }
+
+ /*
+ * 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, RAID6_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, RAID6_KUNIT_ALLOC_BYTES);
+
+ preempt_disable();
+ t = ktime_get_ns();
+ for (l = 0; l < num_iters; l++)
+ raid6_gen_syndrome(nr_to_test[i],
+ len_to_test[j], test_buffers);
+ 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 const void *raid6_gen_params(struct kunit *test, const void *prev,
char *desc)
{
@@ -256,6 +319,7 @@ static const void *raid6_gen_params(struct kunit *test, const void *prev,
static struct kunit_case raid6_test_cases[] = {
KUNIT_CASE_PARAM(raid6_test, raid6_gen_params),
+ KUNIT_CASE(raid6_benchmark),
{},
};
@@ -270,7 +334,7 @@ static int raid6_suite_init(struct kunit_suite *suite)
* so that it is immediately followed by a guard page. This allows
* buffer overreads to be detected, even in assembly code.
*/
- test_buflen = round_up(RAID6_KUNIT_MAX_BYTES, PAGE_SIZE);
+ test_buflen = round_up(RAID6_KUNIT_ALLOC_BYTES, PAGE_SIZE);
for (i = 0; i < RAID6_KUNIT_MAX_FAILURES; i++) {
test_recov_buffers[i] = vmalloc(test_buflen);
if (!test_recov_buffers[i])
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 8/8] raid6/kunit: add a benchmark
2026-07-08 9:07 ` [PATCH 8/8] raid6/kunit: add a benchmark Christoph Hellwig
@ 2026-07-08 9:20 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-07-08 9:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Potential division by zero in benchmark speed calculation if elapsed time `t` evaluates to zero.
- [Medium] Unbounded atomic section (preemption disabled) can cause soft lockups or RCU stalls on slower architectures.
- [Low] Duplicate `RAID6_PQ` Kconfig symbol declaration.
- [Low] Inaccurate commit message referencing XOR functions instead of RAID6 PQ functions.
--
--- Patch [8]: [PATCH 8/8] raid6/kunit: add a benchmark ---
commit 63d135afc471564206ce7921d64339a16ff1484f
Author: Christoph Hellwig <hch@lst.de>
raid6/kunit: add a benchmark
This commit adds a benchmark to the RAID6 KUnit test suite to evaluate P/Q
generation speeds across different disk counts and block sizes. It splits
the max alloc size from the kunit max size and reports the performance in
GB/s.
> Add a benchmark to test the XOR functions for more representative block
> sizes and numbers of disks. This splits the maximum alloc size used for
[Severity: Low]
This isn't a bug, but the commit message states it adds a benchmark for the
"XOR functions", while the patch actually adds a benchmark for the RAID6 P/Q
generation algorithm.
[ ... ]
> diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig
> index c4eeb7c716c89..c52d5c2052946 100644
> --- a/lib/raid/Kconfig
> +++ b/lib/raid/Kconfig
> @@ -60,6 +60,15 @@ config RAID6_PQ_KUNIT_TEST
> This is intended to help people writing architecture-specific
> optimized versions. If unsure, say N.
>
> +config RAID6_PQ_KUNIT_BENCHMARK
> + bool "Benchmark for RAID6 PQ"
> + depends on RAID6_PQ_KUNIT_TEST
> + help
> + Include benchmarks in the KUnit test suite for raid P/Q generation.
> +
> +config RAID6_PQ
> + tristate
> +
[Severity: Low]
Is this duplicate declaration of config RAID6_PQ intentional? It appears to
be defined earlier in this same file.
> config RAID6_PQ_BENCHMARK
> bool "Automatically choose fastest RAID6 PQ functions"
> depends on RAID6_PQ
> diff --git a/lib/raid/raid6/tests/raid6_kunit.c b/lib/raid/raid6/tests/raid6_kunit.c
> index 9f3e671a12241..f927e3456d175 100644
> --- a/lib/raid/raid6/tests/raid6_kunit.c
> +++ b/lib/raid/raid6/tests/raid6_kunit.c
[ ... ]
> @@ -229,6 +230,68 @@ static void raid6_test(struct kunit *test)
> raid6_test_one(test);
> }
>
> +static void raid6_benchmark(struct kunit *test)
> +{
[ ... ]
> + 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, RAID6_KUNIT_ALLOC_BYTES);
> +
> + preempt_disable();
> + t = ktime_get_ns();
> + for (l = 0; l < num_iters; l++)
> + raid6_gen_syndrome(nr_to_test[i],
> + len_to_test[j], test_buffers);
> + t = ktime_get_ns() - t;
> + preempt_enable();
[Severity: Medium]
Could this unbounded atomic section cause soft lockups or RCU stalls on
slower architectures or emulators?
The loop executes 1,000 iterations of raid6_gen_syndrome() with preemption
disabled. On slow hardware or CI emulators, processing this much data might
take multiple seconds and prevent the scheduler from running.
> +
> + speed[j] = div64_u64((u64)len * num_iters * nr, t);
[Severity: Medium]
Is there a risk of a division by zero panic here?
If the KUnit test runs on a system with a coarse-resolution clock (such as
with CONFIG_HIGH_RES_TIMERS disabled) and a sufficiently fast CPU, the
1,000 iterations might complete in less time than a single clock tick.
In this scenario, ktime_get_ns() could return the same value before and
after the loop, causing the elapsed time t to evaluate to exactly 0 before
being passed as the divisor.
> + }
> +
> + 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]);
> + }
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708090740.1433685-1-hch@lst.de?part=8
^ permalink raw reply [flat|nested] 15+ messages in thread