[PATCH 7/8] raid6: improve the runtime selection benchmark

[PATCH 7/8] raid6: improve the runtime selection benchmark

[Christoph Hellwig]

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

Re: [PATCH 7/8] raid6: improve the runtime selection benchmark

[sashiko-bot]

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

misc lib/raid/ improvements v2

[Christoph Hellwig]

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.

Note that Sashiko still complains about long preempt_disable regions.
While that is right in general, it is very much intentional for these
kunit boot / module load time benchmarks.

Changes since v1:
 - fix the deferred implementation backing
 - remove pointless variable shadowing
 - fix up a commit message
 - don't add a duplicate RAID6_PQ symbol
 - fix the calculated RAID6 "XOR" bandwidth
 - floor all timing delta to 1 to avoid entire hypothetical
   divide by zero errors

diffstat:
 Kconfig                   |   12 +++++
 raid6/Makefile            |    2 
 raid6/algos.c             |  102 +++++++++++++++++++++++++---------------------
 raid6/tests/raid6_kunit.c |   66 +++++++++++++++++++++++++++++
 xor/Makefile              |    2 
 xor/tests/Makefile        |    2 
 xor/tests/xor_kunit.c     |   64 ++++++++++++++++++++++++++++
 xor/xor-core.c            |   55 +++++++++++-------------
 8 files changed, 228 insertions(+), 77 deletions(-)

[PATCH 1/8] xor: enable lock context analysis

[Christoph Hellwig]

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

[PATCH 2/8] xor: improve the runtime selection benchmark

[Christoph Hellwig]

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..f9d61b096b11 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 = max(ktime_get_ns() - t, 1);
 	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

[PATCH 3/8] xor/kunit: fix a spelling error

[Christoph Hellwig]

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

[PATCH 4/8] xor/kunit: add a benchmark

[Christoph Hellwig]

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..1fb30b2c7c47 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 = max(ktime_get_ns() - t, 1);
+			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

[PATCH 5/8] raid6: enable lock context analysis

[Christoph Hellwig]

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

[PATCH 6/8] raid6: defer implementation selection when built-in

[Christoph Hellwig]

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 | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
index 6f5c89ab2b17..702e167a543a 100644
--- a/lib/raid/raid6/algos.c
+++ b/lib/raid/raid6/algos.c
@@ -239,10 +239,6 @@ static int __init raid6_select_algo(void)
 	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;
 	}
 
@@ -329,13 +325,34 @@ 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);
 
+	/*
+	 * Pick the last registered implementation as the temporary default until
+	 * calibration happens.
+	 */
+	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);
+
+#ifdef MODULE
 	return raid6_select_algo();
+#else
+	return 0;
+#endif
 }
 
 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

[PATCH 7/8] raid6: improve the runtime selection benchmark

[Christoph Hellwig]

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 | 77 +++++++++++++++++++-----------------------
 1 file changed, 35 insertions(+), 42 deletions(-)

diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
index 702e167a543a..011aa9d0eb3c 100644
--- a/lib/raid/raid6/algos.c
+++ b/lib/raid/raid6/algos.c
@@ -152,40 +152,37 @@ 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;
 	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 = max(ktime_get_ns() - t, 1);
 		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 +194,24 @@ 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;
+		/* work on the second half of the disks */
+		int start = (disks / 2) - 1, stop = disks - 3;
+		u64 t;
 
 		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 = max(ktime_get_ns() - t, 1);
 		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 / 2 * 1000,
+				t));
 	}
 
 	return 0;
@@ -230,9 +223,9 @@ 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;
 
@@ -243,7 +236,7 @@ static int __init raid6_select_algo(void)
 	}
 
 	/* 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;
@@ -251,19 +244,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

[PATCH 8/8] raid6/kunit: add a benchmark

[Christoph Hellwig]

Add a benchmark to test the RAID 6 P/Q 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                   |  6 +++
 lib/raid/raid6/tests/raid6_kunit.c | 66 +++++++++++++++++++++++++++++-
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig
index c4eeb7c716c8..01f007b2522c 100644
--- a/lib/raid/Kconfig
+++ b/lib/raid/Kconfig
@@ -60,6 +60,12 @@ 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_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..7d0a1ec98001 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 = max(ktime_get_ns() - t, 1);
+			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

Re: [PATCH 7/8] raid6: improve the runtime selection benchmark

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The loop variable `i` is shadowed inside `raid6_choose_gen()`.
--

--- Patch [7]: [PATCH 7/8] raid6: improve the runtime selection benchmark ---
commit eff19093e5be2add0169cc2d5063db139e6f9d0c
Author: Christoph Hellwig <hch@lst.de>

raid6: improve the runtime selection benchmark

This commit modifies the RAID6 runtime selection benchmark to use plain
ktime_get_ns for timing. It also updates the benchmark to use 8 + 2 disks
for a more realistic load and reports throughput specifically on the data
disks.

> diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c
> index 702e167a543af..011aa9d0eb3c5 100644
> --- a/lib/raid/raid6/algos.c
> +++ b/lib/raid/raid6/algos.c
[ ... ]
> -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;
>  	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 does this inner declaration of int i shadow the
outer loop variable unsigned int i declared at the beginning of the
function?

This can trigger -Wshadow compiler warnings and might cause confusion when
reading the outer loop over raid6_nr_algos.

>  
>  		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 = max(ktime_get_ns() - t, 1);
>  		preempt_enable();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715144825.95432-1-hch@lst.de?part=7

Re: [PATCH 8/8] raid6/kunit: add a benchmark

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The benchmark loop disables preemption for an extended period, risking soft lockups and RCU stalls on slower architectures or emulators.
--

--- Patch [8]: [PATCH 8/8] raid6/kunit: add a benchmark ---
commit c8db04a9f21fef60a9c9e7ad0c98848e405330de
Author: Christoph Hellwig <hch@lst.de>

raid6/kunit: add a benchmark

This commit adds a benchmark to test the RAID 6 P/Q functions for more
representative block sizes and numbers of disks. The maximum allocation
size is split to allow benchmarking larger sizes, and results are reported
in GB/s.

> diff --git a/lib/raid/raid6/tests/raid6_kunit.c b/lib/raid/raid6/tests/raid6_kunit.c
> index 9f3e671a12241..7d0a1ec980013 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)
[ ... ]
> +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 = max(ktime_get_ns() - t, 1);
> +			preempt_enable();

[Severity: Medium]
Is it safe to disable preemption across this entire 1000-iteration loop?

For the maximum test configuration of 32 disks and 16KB buffers, this loop
processes roughly 512MB of data in a single uninterruptible block. On slower
environments like emulators or User-Mode Linux (which are commonly used for
KUnit tests), could this exceed standard watchdog thresholds and trigger soft
lockups or RCU stalls?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715144825.95432-1-hch@lst.de?part=8

Re: [PATCH 2/8] xor: improve the runtime selection benchmark

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing clock-tick synchronization with a fixed number of loop iterations causes zero-duration time deltas on coarse clocks, leading to artificial throughput inflation, integer overflow, and random algorithm selection.
--

--- Patch [2]: [PATCH 2/8] xor: improve the runtime selection benchmark ---
commit 2205984bf337f5c013e14971e819bb6346e19853
Author: Christoph Hellwig <hch@lst.de>

xor: improve the runtime selection benchmark

This commit modifies the XOR block benchmark to use ktime_get_ns for timing
and switches to 4 + 1 disks for a more realistic load. It also reports the
throughput on the data disks instead of the parity disk.

> diff --git a/lib/raid/xor/xor-core.c b/lib/raid/xor/xor-core.c
> index 50931fbf0324e..f9d61b096b117 100644
> --- a/lib/raid/xor/xor-core.c
> +++ b/lib/raid/xor/xor-core.c

[ ... ]

> -#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 = max(ktime_get_ns() - t, 1);

[Severity: High]
Does removing the clock-tick synchronization create an issue with low
resolution clocks?

If a high-resolution clocksource is not yet available during early boot,
the 800 loop iterations might complete within a single clock tick. If that
happens, the elapsed time t evaluates to 0 and gets clamped to 1.

>  	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]
Can this division silently overflow the 32-bit signed integer tmpl->speed?

If t is clamped to 1 ns, it yields a quotient of approximately 13.1 billion.
This overflows the 32-bit integer and could result in negative or
astronomically high throughput values. 

This might break the calibration logic and cause the kernel to select a
suboptimal XOR algorithm.

>  
> -	pr_info("   %-16s: %5d MB/sec\n", tmpl->name, speed);
> +	pr_info("   %-16s: %5d MB/sec\n", tmpl->name, tmpl->speed);
>  }

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715144825.95432-1-hch@lst.de?part=2

Re: [PATCH 4/8] xor/kunit: add a benchmark

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbounded preemption-disabled region in `xor_benchmark` causes softlockup panics and RCU stall warnings on slower architectures.
--

--- Patch [4]: [PATCH 4/8] xor/kunit: add a benchmark ---
commit f403ad2f80a6eb7a690f9b948152ad30281caf15
Author: Christoph Hellwig <hch@lst.de>
xor/kunit: add a benchmark

This commit adds a benchmark to the XOR KUnit test suite to evaluate
performance across varying block sizes and numbers of disks. The benchmark
reports runtime speeds in GB/s.

> diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
> --- a/lib/raid/xor/tests/xor_kunit.c
> +++ b/lib/raid/xor/tests/xor_kunit.c
[ ... ]
> +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 = max(ktime_get_ns() - t, 1);
> +			preempt_enable();

[Severity: High]
Will this preemption-disabled region cause softlockup panics or RCU stalls
on slower architectures?

In xor_benchmark(), preempt_disable() is called before processing 1,000
iterations of up to 32 blocks of 16KB each. On a slow emulated architecture
without hardware acceleration, this loop will synchronously process over
500 MB of memory.

If the memory bandwidth is low enough, could this tight loop exceed the
default 20-second softlockup and 21-second RCU stall timeouts, resulting
in a kernel panic during CI testing?

Would it be safer to place the preemption-disabled block inside the
iteration loop and accumulate the time, or dynamically scale the number
of iterations based on a time threshold so the scheduler has a chance to run?

> +
> +			speed[j] = div64_u64((u64)len * num_iters * nr, t);
> +		}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715144825.95432-1-hch@lst.de?part=4