[PATCH v3 0/2] lib/prime_numbers: convert self-test to KUnit

[PATCH v3 0/2] lib/prime_numbers: convert self-test to KUnit

[Tamir Duberstein]

This is a clear example of a unit test.

I tested this using:

$ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 math-prime_numbers

On success:
; [08:51:41] ============== math-prime_numbers (1 subtest) ==============
; [08:51:41] [PASSED] prime_numbers_test
; [08:51:41] =============== [PASSED] math-prime_numbers ================
; [08:51:41] ============================================================
; [08:51:41] Testing complete. Ran 1 tests: passed: 1

On failure:
; [08:50:19] ============== math-prime_numbers (1 subtest) ==============
; [08:50:19]     # prime_numbers_test: ASSERTION FAILED at lib/math/tests/prime_numbers_kunit.c:28
; [08:50:19]     Expected slow == fast, but
; [08:50:19]         slow == 0 (0x0)
; [08:50:19]         fast == 1 (0x1)
; [08:50:19] is-prime(2)
; [08:50:19] [FAILED] prime_numbers_test
; [08:50:19] # module: prime_numbers_kunit
; [08:50:19] # math-prime_numbers: primes.{last=61, .sz=64, .primes[]=...x28208a20a08a28ac} = 2-3,5,7,11,13,17,19,23,29,31,37,41,43,47,53,59,61
; [08:50:19] =============== [FAILED] math-prime_numbers ================
; [08:50:19] ============================================================
; [08:50:19] Testing complete. Ran 1 tests: failed: 1

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v3:
- Remove `selftest_max` again; snuck into v2.
- Restore `#include <linux/slab.h>`.
- Sending v3 early because kernel test robot is sad.
- Link to v2: https://lore.kernel.org/r/20250208-prime_numbers-kunit-convert-v2-0-863119447e04@gmail.com

Changes in v2:
- Keep all the details hidden in prime_numbers.c; expose `with_primes`
  for debug logging in the test. (David Gow)
- Link to v1: https://lore.kernel.org/r/20250207-prime_numbers-kunit-convert-v1-0-6067f2b7c713@gmail.com

---
Tamir Duberstein (2):
      lib/math: Hook up tests/Makefile
      lib/prime_numbers: convert self-test to KUnit

 lib/Kconfig.debug                            | 14 +++++
 lib/math/Makefile                            |  4 +-
 lib/math/prime_numbers.c                     | 76 ++++------------------------
 lib/math/prime_numbers_private.h             | 17 +++++++
 lib/math/tests/Makefile                      |  1 +
 lib/math/tests/prime_numbers_kunit.c         | 59 +++++++++++++++++++++
 tools/testing/selftests/lib/config           |  1 -
 tools/testing/selftests/lib/prime_numbers.sh |  4 --
 8 files changed, 102 insertions(+), 74 deletions(-)
---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20250207-prime_numbers-kunit-convert-71c9b3c1d1d4

Best regards,
-- 
Tamir Duberstein <tamird@gmail.com>

[PATCH v3 1/2] lib/math: Hook up tests/Makefile

[Tamir Duberstein]

This Makefile has been dead code since it was added in commit
7fcc9b53216c ("lib/math: Add int_pow test suite"); the tests worked
because of the duplicated rules in the parent directory's Makefile. Wire
up tests/Makefile and remove the duplication.

Fixes: 7fcc9b53216c ("lib/math: Add int_pow test suite")
Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 lib/math/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/math/Makefile b/lib/math/Makefile
index 853f023ae537..d390da9a59fe 100644
--- a/lib/math/Makefile
+++ b/lib/math/Makefile
@@ -5,8 +5,8 @@ obj-$(CONFIG_CORDIC)		+= cordic.o
 obj-$(CONFIG_PRIME_NUMBERS)	+= prime_numbers.o
 obj-$(CONFIG_RATIONAL)		+= rational.o
 
-obj-$(CONFIG_INT_POW_TEST)  += tests/int_pow_kunit.o
 obj-$(CONFIG_TEST_DIV64)	+= test_div64.o
 obj-$(CONFIG_TEST_MULDIV64)	+= test_mul_u64_u64_div_u64.o
 obj-$(CONFIG_RATIONAL_KUNIT_TEST) += rational-test.o
-obj-$(CONFIG_INT_SQRT_KUNIT_TEST) += tests/int_sqrt_kunit.o
\ No newline at end of file
+
+obj-y += tests/

-- 
2.48.1

[PATCH v3 2/2] lib/prime_numbers: convert self-test to KUnit

[Tamir Duberstein]

Extract a private header and convert the prime_numbers self-test to a
KUnit test. I considered parameterizing the test using
`KUNIT_CASE_PARAM` but didn't see how it was possible since the test
logic is entangled with the test parameter generation logic.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 lib/Kconfig.debug                            | 14 +++++
 lib/math/prime_numbers.c                     | 76 ++++------------------------
 lib/math/prime_numbers_private.h             | 17 +++++++
 lib/math/tests/Makefile                      |  1 +
 lib/math/tests/prime_numbers_kunit.c         | 59 +++++++++++++++++++++
 tools/testing/selftests/lib/config           |  1 -
 tools/testing/selftests/lib/prime_numbers.sh |  4 --
 7 files changed, 100 insertions(+), 72 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1af972a92d06..616beaca4a2b 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -3197,6 +3197,20 @@ config INT_SQRT_KUNIT_TEST
 
 	  If unsure, say N
 
+config PRIME_NUMBERS_KUNIT_TEST
+	tristate "Prime number generator test" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	select PRIME_NUMBERS
+	default KUNIT_ALL_TESTS
+	help
+	  This option enables the KUnit test suite for the {is,next}_prime_number
+	  functions.
+
+	  Enabling this option will include tests that compare the prime number
+	  generator functions against a brute force implementation.
+
+	  If unsure, say N
+
 endif # RUNTIME_TESTING_MENU
 
 config ARCH_USE_MEMTEST
diff --git a/lib/math/prime_numbers.c b/lib/math/prime_numbers.c
index 9a17ee9af93a..bb3d66f91364 100644
--- a/lib/math/prime_numbers.c
+++ b/lib/math/prime_numbers.c
@@ -1,16 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0-only
-#define pr_fmt(fmt) "prime numbers: " fmt
 
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/prime_numbers.h>
 #include <linux/slab.h>
 
-struct primes {
-	struct rcu_head rcu;
-	unsigned long last, sz;
-	unsigned long primes[];
-};
+#include "prime_numbers_private.h"
 
 #if BITS_PER_LONG == 64
 static const struct primes small_primes = {
@@ -62,9 +57,13 @@ static const struct primes small_primes = {
 static DEFINE_MUTEX(lock);
 static const struct primes __rcu *primes = RCU_INITIALIZER(&small_primes);
 
-static unsigned long selftest_max;
+#ifdef CONFIG_PRIME_NUMBERS_KUNIT_TEST
+EXPORT_SYMBOL(slow_is_prime_number);
 
-static bool slow_is_prime_number(unsigned long x)
+#else
+static
+#endif
+bool slow_is_prime_number(unsigned long x)
 {
 	unsigned long y = int_sqrt(x);
 
@@ -239,65 +238,11 @@ bool is_prime_number(unsigned long x)
 }
 EXPORT_SYMBOL(is_prime_number);
 
-static void dump_primes(void)
+void with_primes(void *ctx, primes_fn fn)
 {
-	const struct primes *p;
-	char *buf;
-
-	buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-
 	rcu_read_lock();
-	p = rcu_dereference(primes);
-
-	if (buf)
-		bitmap_print_to_pagebuf(true, buf, p->primes, p->sz);
-	pr_info("primes.{last=%lu, .sz=%lu, .primes[]=...x%lx} = %s\n",
-		p->last, p->sz, p->primes[BITS_TO_LONGS(p->sz) - 1], buf);
-
+	fn(ctx, rcu_dereference(primes));
 	rcu_read_unlock();
-
-	kfree(buf);
-}
-
-static int selftest(unsigned long max)
-{
-	unsigned long x, last;
-
-	if (!max)
-		return 0;
-
-	for (last = 0, x = 2; x < max; x++) {
-		bool slow = slow_is_prime_number(x);
-		bool fast = is_prime_number(x);
-
-		if (slow != fast) {
-			pr_err("inconsistent result for is-prime(%lu): slow=%s, fast=%s!\n",
-			       x, slow ? "yes" : "no", fast ? "yes" : "no");
-			goto err;
-		}
-
-		if (!slow)
-			continue;
-
-		if (next_prime_number(last) != x) {
-			pr_err("incorrect result for next-prime(%lu): expected %lu, got %lu\n",
-			       last, x, next_prime_number(last));
-			goto err;
-		}
-		last = x;
-	}
-
-	pr_info("%s(%lu) passed, last prime was %lu\n", __func__, x, last);
-	return 0;
-
-err:
-	dump_primes();
-	return -EINVAL;
-}
-
-static int __init primes_init(void)
-{
-	return selftest(selftest_max);
 }
 
 static void __exit primes_exit(void)
@@ -305,11 +250,8 @@ static void __exit primes_exit(void)
 	free_primes();
 }
 
-module_init(primes_init);
 module_exit(primes_exit);
 
-module_param_named(selftest, selftest_max, ulong, 0400);
-
 MODULE_AUTHOR("Intel Corporation");
 MODULE_DESCRIPTION("Prime number library");
 MODULE_LICENSE("GPL");
diff --git a/lib/math/prime_numbers_private.h b/lib/math/prime_numbers_private.h
new file mode 100644
index 000000000000..9e1d440c5398
--- /dev/null
+++ b/lib/math/prime_numbers_private.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/types.h>
+
+struct primes {
+	struct rcu_head rcu;
+	unsigned long last, sz;
+	unsigned long primes[];
+};
+
+#ifdef CONFIG_PRIME_NUMBERS_KUNIT_TEST
+bool slow_is_prime_number(unsigned long x);
+#endif
+typedef void (*primes_fn)(void *, const struct primes *);
+
+// Calls the callback under RCU lock. The callback must not retain the primes pointer.
+void with_primes(void *ctx, primes_fn fn);
diff --git a/lib/math/tests/Makefile b/lib/math/tests/Makefile
index e1a79f093b2d..da21a592c75a 100644
--- a/lib/math/tests/Makefile
+++ b/lib/math/tests/Makefile
@@ -2,3 +2,4 @@
 
 obj-$(CONFIG_INT_POW_TEST) += int_pow_kunit.o
 obj-$(CONFIG_INT_SQRT_KUNIT_TEST) += int_sqrt_kunit.o
+obj-$(CONFIG_PRIME_NUMBERS_KUNIT_TEST) += prime_numbers_kunit.o
diff --git a/lib/math/tests/prime_numbers_kunit.c b/lib/math/tests/prime_numbers_kunit.c
new file mode 100644
index 000000000000..2f1643208c66
--- /dev/null
+++ b/lib/math/tests/prime_numbers_kunit.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <kunit/test.h>
+#include <linux/module.h>
+#include <linux/prime_numbers.h>
+
+#include "../prime_numbers_private.h"
+
+static void dump_primes(void *ctx, const struct primes *p)
+{
+	static char buf[PAGE_SIZE];
+	struct kunit_suite *suite = ctx;
+
+	bitmap_print_to_pagebuf(true, buf, p->primes, p->sz);
+	kunit_info(suite, "primes.{last=%lu, .sz=%lu, .primes[]=...x%lx} = %s",
+		   p->last, p->sz, p->primes[BITS_TO_LONGS(p->sz) - 1], buf);
+}
+
+static void prime_numbers_test(struct kunit *test)
+{
+	const unsigned long max = 65536;
+	unsigned long x, last, next;
+
+	for (last = 0, x = 2; x < max; x++) {
+		const bool slow = slow_is_prime_number(x);
+		const bool fast = is_prime_number(x);
+
+		KUNIT_ASSERT_EQ_MSG(test, slow, fast, "is-prime(%lu)", x);
+
+		if (!slow)
+			continue;
+
+		next = next_prime_number(last);
+		KUNIT_ASSERT_EQ_MSG(test, next, x, "next-prime(%lu)", last);
+		last = next;
+	}
+}
+
+static void kunit_suite_exit(struct kunit_suite *suite)
+{
+	with_primes(suite, dump_primes);
+}
+
+static struct kunit_case prime_numbers_cases[] = {
+	KUNIT_CASE(prime_numbers_test),
+	{},
+};
+
+static struct kunit_suite prime_numbers_suite = {
+	.name = "math-prime_numbers",
+	.suite_exit = kunit_suite_exit,
+	.test_cases = prime_numbers_cases,
+};
+
+kunit_test_suite(prime_numbers_suite);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_DESCRIPTION("Prime number library");
+MODULE_LICENSE("GPL");
diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config
index dc15aba8d0a3..306a3d4dca98 100644
--- a/tools/testing/selftests/lib/config
+++ b/tools/testing/selftests/lib/config
@@ -1,5 +1,4 @@
 CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
-CONFIG_PRIME_NUMBERS=m
 CONFIG_TEST_BITOPS=m
diff --git a/tools/testing/selftests/lib/prime_numbers.sh b/tools/testing/selftests/lib/prime_numbers.sh
deleted file mode 100755
index 370b79a9cb2e..000000000000
--- a/tools/testing/selftests/lib/prime_numbers.sh
+++ /dev/null
@@ -1,4 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-# Checks fast/slow prime_number generation for inconsistencies
-$(dirname $0)/../kselftest/module.sh "prime numbers" prime_numbers selftest=65536

-- 
2.48.1

Re: [PATCH v3 0/2] lib/prime_numbers: convert self-test to KUnit

[Andrew Morton]

On Sat, 08 Feb 2025 17:52:28 -0500 Tamir Duberstein <tamird@gmail.com> wrote:

> This is a clear example of a unit test.
> 
> I tested this using:
> 
> $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 math-prime_numbers
> 
> On success:
> ; [08:51:41] ============== math-prime_numbers (1 subtest) ==============
> ; [08:51:41] [PASSED] prime_numbers_test
> ; [08:51:41] =============== [PASSED] math-prime_numbers ================
> ; [08:51:41] ============================================================
> ; [08:51:41] Testing complete. Ran 1 tests: passed: 1
> 
> On failure:
> ; [08:50:19] ============== math-prime_numbers (1 subtest) ==============
> ; [08:50:19]     # prime_numbers_test: ASSERTION FAILED at lib/math/tests/prime_numbers_kunit.c:28
> ; [08:50:19]     Expected slow == fast, but
> ; [08:50:19]         slow == 0 (0x0)
> ; [08:50:19]         fast == 1 (0x1)
> ; [08:50:19] is-prime(2)
> ; [08:50:19] [FAILED] prime_numbers_test
> ; [08:50:19] # module: prime_numbers_kunit
> ; [08:50:19] # math-prime_numbers: primes.{last=61, .sz=64, .primes[]=...x28208a20a08a28ac} = 2-3,5,7,11,13,17,19,23,29,31,37,41,43,47,53,59,61
> ; [08:50:19] =============== [FAILED] math-prime_numbers ================
> ; [08:50:19] ============================================================
> ; [08:50:19] Testing complete. Ran 1 tests: failed: 1
> 

I resolved this against "lib/math: add Kunit test suite for gcd()"
(https://lkml.kernel.org/r/20250203075400.3431330-1-eleanor15x@gmail.com)
in the obvious fashion then added this fixup:

From: Andrew Morton <akpm@linux-foundation.org>
Subject: lib-math-hook-up-tests-makefile-fix
Date: Sat Feb  8 03:33:59 PM PST 2025

don't link gcd_kunit.o twice

Cc: David Gow <davidgow@google.com>
Cc: Tamir Duberstein <tamird@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 lib/math/Makefile |    1 -
 1 file changed, 1 deletion(-)

--- a/lib/math/Makefile~lib-math-hook-up-tests-makefile-fix
+++ a/lib/math/Makefile
@@ -8,5 +8,4 @@ obj-$(CONFIG_RATIONAL)		+= rational.o
 obj-$(CONFIG_TEST_DIV64)	+= test_div64.o
 obj-$(CONFIG_TEST_MULDIV64)	+= test_mul_u64_u64_div_u64.o
 obj-$(CONFIG_RATIONAL_KUNIT_TEST) += rational-test.o
-obj-$(CONFIG_GCD_KUNIT_TEST) += tests/gcd_kunit.o
 obj-y += tests/
_

and the x86_64 allmodconfig build failed thusly:

lib/math/tests/prime_numbers_kunit.c: In function 'prime_numbers_test':
lib/math/tests/prime_numbers_kunit.c:25:35: error: implicit declaration of function 'slow_is_prime_number'; did you mean 'is_prime_number'? [-Werror=implicit-function-declaration]
   25 |                 const bool slow = slow_is_prime_number(x);
      |                                   ^~~~~~~~~~~~~~~~~~~~
      |                                   is_prime_number


So, please redo against mm.git's mm-everything branch or its
mm-nonmm-unstable branch and retest carefully.

Re: [PATCH v3 0/2] lib/prime_numbers: convert self-test to KUnit

[Tamir Duberstein]

On Sat, Feb 8, 2025 at 7:25 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Sat, 08 Feb 2025 17:52:28 -0500 Tamir Duberstein <tamird@gmail.com> wrote:
>
> > This is a clear example of a unit test.
> >
> > I tested this using:
> >
> > $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 math-prime_numbers
> >
> > On success:
> > ; [08:51:41] ============== math-prime_numbers (1 subtest) ==============
> > ; [08:51:41] [PASSED] prime_numbers_test
> > ; [08:51:41] =============== [PASSED] math-prime_numbers ================
> > ; [08:51:41] ============================================================
> > ; [08:51:41] Testing complete. Ran 1 tests: passed: 1
> >
> > On failure:
> > ; [08:50:19] ============== math-prime_numbers (1 subtest) ==============
> > ; [08:50:19]     # prime_numbers_test: ASSERTION FAILED at lib/math/tests/prime_numbers_kunit.c:28
> > ; [08:50:19]     Expected slow == fast, but
> > ; [08:50:19]         slow == 0 (0x0)
> > ; [08:50:19]         fast == 1 (0x1)
> > ; [08:50:19] is-prime(2)
> > ; [08:50:19] [FAILED] prime_numbers_test
> > ; [08:50:19] # module: prime_numbers_kunit
> > ; [08:50:19] # math-prime_numbers: primes.{last=61, .sz=64, .primes[]=...x28208a20a08a28ac} = 2-3,5,7,11,13,17,19,23,29,31,37,41,43,47,53,59,61
> > ; [08:50:19] =============== [FAILED] math-prime_numbers ================
> > ; [08:50:19] ============================================================
> > ; [08:50:19] Testing complete. Ran 1 tests: failed: 1
> >
>
> I resolved this against "lib/math: add Kunit test suite for gcd()"
> (https://lkml.kernel.org/r/20250203075400.3431330-1-eleanor15x@gmail.com)
> in the obvious fashion then added this fixup:
>
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: lib-math-hook-up-tests-makefile-fix
> Date: Sat Feb  8 03:33:59 PM PST 2025
>
> don't link gcd_kunit.o twice
>
> Cc: David Gow <davidgow@google.com>
> Cc: Tamir Duberstein <tamird@gmail.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
>  lib/math/Makefile |    1 -
>  1 file changed, 1 deletion(-)
>
> --- a/lib/math/Makefile~lib-math-hook-up-tests-makefile-fix
> +++ a/lib/math/Makefile
> @@ -8,5 +8,4 @@ obj-$(CONFIG_RATIONAL)          += rational.o
>  obj-$(CONFIG_TEST_DIV64)       += test_div64.o
>  obj-$(CONFIG_TEST_MULDIV64)    += test_mul_u64_u64_div_u64.o
>  obj-$(CONFIG_RATIONAL_KUNIT_TEST) += rational-test.o
> -obj-$(CONFIG_GCD_KUNIT_TEST) += tests/gcd_kunit.o
>  obj-y += tests/
> _
>
> and the x86_64 allmodconfig build failed thusly:
>
> lib/math/tests/prime_numbers_kunit.c: In function 'prime_numbers_test':
> lib/math/tests/prime_numbers_kunit.c:25:35: error: implicit declaration of function 'slow_is_prime_number'; did you mean 'is_prime_number'? [-Werror=implicit-function-declaration]
>    25 |                 const bool slow = slow_is_prime_number(x);
>       |                                   ^~~~~~~~~~~~~~~~~~~~
>       |                                   is_prime_number
>
>
> So, please redo against mm.git's mm-everything branch or its
> mm-nonmm-unstable branch and retest carefully.

Thanks for the clear instructions. I was able to reproduce. v4 is on its way.