* [PATCH 0/3] bitmap: convert self-test to KUnit
@ 2025-02-07 20:14 Tamir Duberstein
2025-02-07 20:14 ` [PATCH 1/3] bitmap: remove _check_eq_u32_array Tamir Duberstein
` (4 more replies)
0 siblings, 5 replies; 27+ messages in thread
From: Tamir Duberstein @ 2025-02-07 20:14 UTC (permalink / raw)
To: David Gow, John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Yury Norov, Rasmus Villemoes,
Shuah Khan, Kees Cook
Cc: Muhammad Usama Anjum, linux-kernel, linux-m68k, linuxppc-dev,
linux-kselftest, Tamir Duberstein
This is one of just 3 remaining "Test Module" kselftests (the others
being printf and scanf), the rest having been converted to KUnit.
I tested this using:
$ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 bitmap.
I've already sent out a conversion series for each of printf[0] and scanf[1].
There was a previous attempt[2] to do this in July 2024. Please bear
with me as I try to understand and address the objections from that
time. I've spoken with Muhammad Usama Anjum, the author of that series,
and received their approval to "take over" this work. Here we go...
On 7/26/24 11:45 PM, John Hubbard wrote:
>
> This changes the situation from "works for Linus' tab completion
> case", to "causes a tab completion problem"! :)
>
> I think a tests/ subdir is how we eventually decided to do this [1],
> right?
>
> So:
>
> lib/tests/bitmap_kunit.c
>
> [1] https://lore.kernel.org/20240724201354.make.730-kees@kernel.org
This is true and unfortunate, but not trivial to fix because new
kallsyms tests were placed in lib/tests in commit 84b4a51fce4c
("selftests: add new kallsyms selftests") *after* the KUnit filename
best practices were adopted.
I propose that the KUnit maintainers blaze this trail using
`string_kunit.c` which currently still lives in lib/ despite the KUnit
docs giving it as an example at lib/tests/.
On 7/27/24 12:24 AM, Shuah Khan wrote:
>
> This change will take away the ability to run bitmap tests during
> boot on a non-kunit kernel.
>
> Nack on this change. I wan to see all tests that are being removed
> from lib because they have been converted - also it doesn't make
> sense to convert some tests like this one that add the ability test
> during boot.
This point was also discussed in another thread[3] in which:
On 7/27/24 12:35 AM, Shuah Khan wrote:
>
> Please make sure you aren't taking away the ability to run these tests during
> boot.
>
> It doesn't make sense to convert every single test especially when it
> is intended to be run during boot without dependencies - not as a kunit test
> but a regression test during boot.
>
> bitmap is one example - pay attention to the config help test - bitmap
> one clearly states it runs regression testing during boot. Any test that
> says that isn't a candidate for conversion.
>
> I am going to nack any such conversions.
The crux of the argument seems to be that the config help text is taken
to describe the author's intent with the fragment "at boot". I think
this may be a case of confirmation bias: I see at least the following
KUnit tests with "at boot" in their help text:
- CPUMASK_KUNIT_TEST
- BITFIELD_KUNIT
- CHECKSUM_KUNIT
- UTIL_MACROS_KUNIT
It seems to me that the inference being made is that any test that runs
"at boot" is intended to be run by both developers and users, but I find
no evidence that bitmap in particular would ever provide additional
value when run by users.
There's further discussion about KUnit not being "ideal for cases where
people would want to check a subsystem on a running kernel", but I find
no evidence that bitmap in particular is actually testing the running
kernel; it is a unit test of the bitmap functions, which is also stated
in the config help text.
David Gow made many of the same points in his final reply[4], which was
never replied to.
Link: https://lore.kernel.org/all/20250207-printf-kunit-convert-v2-0-057b23860823@gmail.com/T/#u [0]
Link: https://lore.kernel.org/all/20250207-scanf-kunit-convert-v4-0-a23e2afaede8@gmail.com/T/#u [1]
Link: https://lore.kernel.org/all/20240726110658.2281070-1-usama.anjum@collabora.com/T/#u [2]
Link: https://lore.kernel.org/all/327831fb-47ab-4555-8f0b-19a8dbcaacd7@collabora.com/T/#u [3]
Link: https://lore.kernel.org/all/CABVgOSmMoPD3JfzVd4VTkzGL2fZCo8LfwzaVSzeFimPrhgLa5w@mail.gmail.com/ [4]
Thanks for your attention.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Tamir Duberstein (3):
bitmap: remove _check_eq_u32_array
bitmap: convert self-test to KUnit
bitmap: break kunit into test cases
MAINTAINERS | 2 +-
arch/m68k/configs/amiga_defconfig | 1 -
arch/m68k/configs/apollo_defconfig | 1 -
arch/m68k/configs/atari_defconfig | 1 -
arch/m68k/configs/bvme6000_defconfig | 1 -
arch/m68k/configs/hp300_defconfig | 1 -
arch/m68k/configs/mac_defconfig | 1 -
arch/m68k/configs/multi_defconfig | 1 -
arch/m68k/configs/mvme147_defconfig | 1 -
arch/m68k/configs/mvme16x_defconfig | 1 -
arch/m68k/configs/q40_defconfig | 1 -
arch/m68k/configs/sun3_defconfig | 1 -
arch/m68k/configs/sun3x_defconfig | 1 -
arch/powerpc/configs/ppc64_defconfig | 1 -
lib/Kconfig.debug | 24 +-
lib/Makefile | 2 +-
lib/{test_bitmap.c => bitmap_kunit.c} | 454 +++++++++++++---------------------
tools/testing/selftests/lib/bitmap.sh | 3 -
tools/testing/selftests/lib/config | 1 -
19 files changed, 195 insertions(+), 304 deletions(-)
---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20250207-bitmap-kunit-convert-92d3147b2eee
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/3] bitmap: remove _check_eq_u32_array
2025-02-07 20:14 [PATCH 0/3] bitmap: convert self-test to KUnit Tamir Duberstein
@ 2025-02-07 20:14 ` Tamir Duberstein
2025-02-08 9:07 ` David Gow
` (2 more replies)
2025-02-07 20:14 ` [PATCH 2/3] bitmap: convert self-test to KUnit Tamir Duberstein
` (3 subsequent siblings)
4 siblings, 3 replies; 27+ messages in thread
From: Tamir Duberstein @ 2025-02-07 20:14 UTC (permalink / raw)
To: David Gow, John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Yury Norov, Rasmus Villemoes,
Shuah Khan, Kees Cook
Cc: Muhammad Usama Anjum, linux-kernel, linux-m68k, linuxppc-dev,
linux-kselftest, Tamir Duberstein
This has been unused since commit 3aa56885e516 ("bitmap: replace
bitmap_{from,to}_u32array") in 2018. Remove it to avoid the need to port
it to KUnit in this series.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
lib/test_bitmap.c | 28 ----------------------------
1 file changed, 28 deletions(-)
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 65a75d58ed9e..c83829ef557f 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -100,34 +100,6 @@ __check_eq_pbl(const char *srcfile, unsigned int line,
return true;
}
-static bool __init
-__check_eq_u32_array(const char *srcfile, unsigned int line,
- const u32 *exp_arr, unsigned int exp_len,
- const u32 *arr, unsigned int len) __used;
-static bool __init
-__check_eq_u32_array(const char *srcfile, unsigned int line,
- const u32 *exp_arr, unsigned int exp_len,
- const u32 *arr, unsigned int len)
-{
- if (exp_len != len) {
- pr_warn("[%s:%u] array length differ: expected %u, got %u\n",
- srcfile, line,
- exp_len, len);
- return false;
- }
-
- if (memcmp(exp_arr, arr, len*sizeof(*arr))) {
- pr_warn("[%s:%u] array contents differ\n", srcfile, line);
- print_hex_dump(KERN_WARNING, " exp: ", DUMP_PREFIX_OFFSET,
- 32, 4, exp_arr, exp_len*sizeof(*exp_arr), false);
- print_hex_dump(KERN_WARNING, " got: ", DUMP_PREFIX_OFFSET,
- 32, 4, arr, len*sizeof(*arr), false);
- return false;
- }
-
- return true;
-}
-
static bool __init __check_eq_clump8(const char *srcfile, unsigned int line,
const unsigned int offset,
const unsigned int size,
--
2.48.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/3] bitmap: convert self-test to KUnit
2025-02-07 20:14 [PATCH 0/3] bitmap: convert self-test to KUnit Tamir Duberstein
2025-02-07 20:14 ` [PATCH 1/3] bitmap: remove _check_eq_u32_array Tamir Duberstein
@ 2025-02-07 20:14 ` Tamir Duberstein
2025-02-08 9:07 ` David Gow
` (2 more replies)
2025-02-07 20:14 ` [PATCH 3/3] bitmap: break kunit into test cases Tamir Duberstein
` (2 subsequent siblings)
4 siblings, 3 replies; 27+ messages in thread
From: Tamir Duberstein @ 2025-02-07 20:14 UTC (permalink / raw)
To: David Gow, John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Yury Norov, Rasmus Villemoes,
Shuah Khan, Kees Cook
Cc: Muhammad Usama Anjum, linux-kernel, linux-m68k, linuxppc-dev,
linux-kselftest, Tamir Duberstein
Convert the bitmap() self-test to a KUnit test.
In the interest of keeping the patch reasonably-sized this doesn't
refactor the tests into proper parameterized tests - it's all one big
test case.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
MAINTAINERS | 2 +-
arch/m68k/configs/amiga_defconfig | 1 -
arch/m68k/configs/apollo_defconfig | 1 -
arch/m68k/configs/atari_defconfig | 1 -
arch/m68k/configs/bvme6000_defconfig | 1 -
arch/m68k/configs/hp300_defconfig | 1 -
arch/m68k/configs/mac_defconfig | 1 -
arch/m68k/configs/multi_defconfig | 1 -
arch/m68k/configs/mvme147_defconfig | 1 -
arch/m68k/configs/mvme16x_defconfig | 1 -
arch/m68k/configs/q40_defconfig | 1 -
arch/m68k/configs/sun3_defconfig | 1 -
arch/m68k/configs/sun3x_defconfig | 1 -
arch/powerpc/configs/ppc64_defconfig | 1 -
lib/Kconfig.debug | 24 ++-
lib/Makefile | 2 +-
lib/{test_bitmap.c => bitmap_kunit.c} | 322 +++++++++++++---------------------
tools/testing/selftests/lib/bitmap.sh | 3 -
tools/testing/selftests/lib/config | 1 -
19 files changed, 145 insertions(+), 222 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 896a307fa065..9824d4053748 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4016,11 +4016,11 @@ F: include/linux/nodemask_types.h
F: include/vdso/bits.h
F: lib/bitmap-str.c
F: lib/bitmap.c
+F: lib/bitmap_kunit.c
F: lib/cpumask.c
F: lib/cpumask_kunit.c
F: lib/find_bit.c
F: lib/find_bit_benchmark.c
-F: lib/test_bitmap.c
F: tools/include/linux/bitfield.h
F: tools/include/linux/bitmap.h
F: tools/include/linux/bits.h
diff --git a/arch/m68k/configs/amiga_defconfig b/arch/m68k/configs/amiga_defconfig
index dbf2ea561c85..3c9d7b58cb8a 100644
--- a/arch/m68k/configs/amiga_defconfig
+++ b/arch/m68k/configs/amiga_defconfig
@@ -624,7 +624,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/m68k/configs/apollo_defconfig b/arch/m68k/configs/apollo_defconfig
index b0fd199cc0a4..b94c87a7cbdb 100644
--- a/arch/m68k/configs/apollo_defconfig
+++ b/arch/m68k/configs/apollo_defconfig
@@ -581,7 +581,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/m68k/configs/atari_defconfig b/arch/m68k/configs/atari_defconfig
index bb5b2d3b6c10..823ba96c4486 100644
--- a/arch/m68k/configs/atari_defconfig
+++ b/arch/m68k/configs/atari_defconfig
@@ -601,7 +601,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/m68k/configs/bvme6000_defconfig b/arch/m68k/configs/bvme6000_defconfig
index 8315a13bab73..0fa985129200 100644
--- a/arch/m68k/configs/bvme6000_defconfig
+++ b/arch/m68k/configs/bvme6000_defconfig
@@ -573,7 +573,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/m68k/configs/hp300_defconfig b/arch/m68k/configs/hp300_defconfig
index 350370657e5f..75ed6eb547c6 100644
--- a/arch/m68k/configs/hp300_defconfig
+++ b/arch/m68k/configs/hp300_defconfig
@@ -583,7 +583,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/m68k/configs/mac_defconfig b/arch/m68k/configs/mac_defconfig
index f942b4755702..149c398f80a8 100644
--- a/arch/m68k/configs/mac_defconfig
+++ b/arch/m68k/configs/mac_defconfig
@@ -600,7 +600,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/m68k/configs/multi_defconfig b/arch/m68k/configs/multi_defconfig
index b1eaad02efab..34e6eaa47a18 100644
--- a/arch/m68k/configs/multi_defconfig
+++ b/arch/m68k/configs/multi_defconfig
@@ -687,7 +687,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/m68k/configs/mvme147_defconfig b/arch/m68k/configs/mvme147_defconfig
index 6309a4442bb3..9e9f8883b38d 100644
--- a/arch/m68k/configs/mvme147_defconfig
+++ b/arch/m68k/configs/mvme147_defconfig
@@ -573,7 +573,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/m68k/configs/mvme16x_defconfig b/arch/m68k/configs/mvme16x_defconfig
index 3feb0731f814..5ed08e78c4fd 100644
--- a/arch/m68k/configs/mvme16x_defconfig
+++ b/arch/m68k/configs/mvme16x_defconfig
@@ -574,7 +574,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/m68k/configs/q40_defconfig b/arch/m68k/configs/q40_defconfig
index ea04b1b0da7d..4e0eea94a6a0 100644
--- a/arch/m68k/configs/q40_defconfig
+++ b/arch/m68k/configs/q40_defconfig
@@ -590,7 +590,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/m68k/configs/sun3_defconfig b/arch/m68k/configs/sun3_defconfig
index f52d9af92153..690e2156b5f7 100644
--- a/arch/m68k/configs/sun3_defconfig
+++ b/arch/m68k/configs/sun3_defconfig
@@ -570,7 +570,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/m68k/configs/sun3x_defconfig b/arch/m68k/configs/sun3x_defconfig
index f348447824da..71486fd428da 100644
--- a/arch/m68k/configs/sun3x_defconfig
+++ b/arch/m68k/configs/sun3x_defconfig
@@ -571,7 +571,6 @@ CONFIG_TEST_HEXDUMP=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index 465eb96c755e..cfd235d90c95 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -446,7 +446,6 @@ CONFIG_TEST_STRING_HELPERS=m
CONFIG_TEST_KSTRTOX=m
CONFIG_TEST_PRINTF=m
CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
CONFIG_TEST_UUID=m
CONFIG_TEST_XARRAY=m
CONFIG_TEST_MAPLE_TREE=m
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1af972a92d06..fd3dcb0677b5 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2427,6 +2427,23 @@ config ASYNC_RAID6_TEST
config TEST_HEXDUMP
tristate "Test functions located in the hexdump module at runtime"
+config BITMAP_KUNIT_TEST
+ tristate "KUnit test bitmap_*() family of functions at runtime" if !KUNIT_ALL_TESTS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ Enable this option to test the bitmap functions at boot.
+
+ KUnit tests run during boot and output the results to the debug log
+ in TAP format (http://testanything.org/). Only useful for kernel devs
+ running the KUnit test harness, and not intended for inclusion into a
+ production build.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
config STRING_KUNIT_TEST
tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS
depends on KUNIT
@@ -2446,13 +2463,6 @@ config TEST_PRINTF
config TEST_SCANF
tristate "Test scanf() family of functions at runtime"
-config TEST_BITMAP
- tristate "Test bitmap_*() family of functions at runtime"
- help
- Enable this option to test the bitmap functions at boot.
-
- If unsure, say N.
-
config TEST_UUID
tristate "Test functions located in the uuid module at runtime"
diff --git a/lib/Makefile b/lib/Makefile
index d5cfc7afbbb8..d735e1b70606 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -87,7 +87,7 @@ obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
obj-$(CONFIG_TEST_PRINTF) += test_printf.o
obj-$(CONFIG_TEST_SCANF) += test_scanf.o
-obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
+obj-$(CONFIG_BITMAP_KUNIT_TEST) += bitmap_kunit.o
ifeq ($(CONFIG_CC_IS_CLANG)$(CONFIG_KASAN),yy)
# FIXME: Clang breaks test_bitmap_const_eval when KASAN and GCOV are enabled
GCOV_PROFILE_test_bitmap.o := n
diff --git a/lib/test_bitmap.c b/lib/bitmap_kunit.c
similarity index 83%
rename from lib/test_bitmap.c
rename to lib/bitmap_kunit.c
index c83829ef557f..0605228288d6 100644
--- a/lib/test_bitmap.c
+++ b/lib/bitmap_kunit.c
@@ -3,10 +3,8 @@
* Test cases for bitmap API.
*/
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
+#include <kunit/test.h>
#include <linux/bitmap.h>
-#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/printk.h>
@@ -14,16 +12,17 @@
#include <linux/string.h>
#include <linux/uaccess.h>
-#include "../tools/testing/selftests/kselftest_module.h"
-
#define EXP1_IN_BITS (sizeof(exp1) * 8)
-KSTM_MODULE_GLOBALS();
+static char pbl_buffer[PAGE_SIZE];
+static char print_buf[PAGE_SIZE * 2];
+
+static struct kunit *kunittest;
-static char pbl_buffer[PAGE_SIZE] __initdata;
-static char print_buf[PAGE_SIZE * 2] __initdata;
+#define tc_err(fmt, ...) \
+ KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
-static const unsigned long exp1[] __initconst = {
+static const unsigned long exp1[] = {
BITMAP_FROM_U64(1),
BITMAP_FROM_U64(2),
BITMAP_FROM_U64(0x0000ffff),
@@ -41,130 +40,63 @@ static const unsigned long exp1[] __initconst = {
BITMAP_FROM_U64(0x80000000),
};
-static const unsigned long exp2[] __initconst = {
+static const unsigned long exp2[] = {
BITMAP_FROM_U64(0x3333333311111111ULL),
BITMAP_FROM_U64(0xffffffff77777777ULL),
};
/* Fibonacci sequence */
-static const unsigned long exp2_to_exp3_mask[] __initconst = {
+static const unsigned long exp2_to_exp3_mask[] = {
BITMAP_FROM_U64(0x008000020020212eULL),
};
/* exp3_0_1 = (exp2[0] & ~exp2_to_exp3_mask) | (exp2[1] & exp2_to_exp3_mask) */
-static const unsigned long exp3_0_1[] __initconst = {
+static const unsigned long exp3_0_1[] = {
BITMAP_FROM_U64(0x33b3333311313137ULL),
};
/* exp3_1_0 = (exp2[1] & ~exp2_to_exp3_mask) | (exp2[0] & exp2_to_exp3_mask) */
-static const unsigned long exp3_1_0[] __initconst = {
+static const unsigned long exp3_1_0[] = {
BITMAP_FROM_U64(0xff7fffff77575751ULL),
};
-static bool __init
-__check_eq_ulong(const char *srcfile, unsigned int line,
- const unsigned long exp_ulong, unsigned long x)
-{
- if (exp_ulong != x) {
- pr_err("[%s:%u] expected %lu, got %lu\n",
- srcfile, line, exp_ulong, x);
- return false;
- }
- return true;
-}
-
-static bool __init
-__check_eq_bitmap(const char *srcfile, unsigned int line,
- const unsigned long *exp_bmap, const unsigned long *bmap,
- unsigned int nbits)
-{
- if (!bitmap_equal(exp_bmap, bmap, nbits)) {
- pr_warn("[%s:%u] bitmaps contents differ: expected \"%*pbl\", got \"%*pbl\"\n",
- srcfile, line,
- nbits, exp_bmap, nbits, bmap);
- return false;
- }
- return true;
-}
-
-static bool __init
-__check_eq_pbl(const char *srcfile, unsigned int line,
- const char *expected_pbl,
- const unsigned long *bitmap, unsigned int nbits)
-{
- snprintf(pbl_buffer, sizeof(pbl_buffer), "%*pbl", nbits, bitmap);
- if (strcmp(expected_pbl, pbl_buffer)) {
- pr_warn("[%s:%u] expected \"%s\", got \"%s\"\n",
- srcfile, line,
- expected_pbl, pbl_buffer);
- return false;
- }
- return true;
-}
-
-static bool __init __check_eq_clump8(const char *srcfile, unsigned int line,
- const unsigned int offset,
- const unsigned int size,
- const unsigned char *const clump_exp,
- const unsigned long *const clump)
-{
- unsigned long exp;
-
- if (offset >= size) {
- pr_warn("[%s:%u] bit offset for clump out-of-bounds: expected less than %u, got %u\n",
- srcfile, line, size, offset);
- return false;
- }
-
- exp = clump_exp[offset / 8];
- if (!exp) {
- pr_warn("[%s:%u] bit offset for zero clump: expected nonzero clump, got bit offset %u with clump value 0",
- srcfile, line, offset);
- return false;
- }
-
- if (*clump != exp) {
- pr_warn("[%s:%u] expected clump value of 0x%lX, got clump value of 0x%lX",
- srcfile, line, exp, *clump);
- return false;
- }
-
- return true;
-}
-
-static bool __init
-__check_eq_str(const char *srcfile, unsigned int line,
- const char *exp_str, const char *str,
- unsigned int len)
-{
- bool eq;
-
- eq = strncmp(exp_str, str, len) == 0;
- if (!eq)
- pr_err("[%s:%u] expected %s, got %s\n", srcfile, line, exp_str, str);
-
- return eq;
-}
-
-#define __expect_eq(suffix, ...) \
- ({ \
- int result = 0; \
- total_tests++; \
- if (!__check_eq_ ## suffix(__FILE__, __LINE__, \
- ##__VA_ARGS__)) { \
- failed_tests++; \
- result = 1; \
+#define expect_eq_ulong(exp_ulong, x) KUNIT_EXPECT_EQ(kunittest, exp_ulong, x)
+
+#define expect_eq_bitmap(exp_bmap, bmap, nbits) \
+ KUNIT_EXPECT_TRUE_MSG(kunittest, bitmap_equal(exp_bmap, bmap, nbits), \
+ "bitmaps contents differ: expected \"%*pbl\", got \"%*pbl\"", \
+ nbits, exp_bmap, nbits, bmap)
+
+#define expect_eq_pbl(expected_pbl, bitmap, nbits) do { \
+ { \
+ snprintf(pbl_buffer, sizeof(pbl_buffer), "%*pbl", nbits, bitmap); \
+ KUNIT_EXPECT_STREQ(kunittest, expected_pbl, pbl_buffer); \
+ } \
+ } while (0)
+
+#define expect_eq_clump8(offset, size, clump_exp, clump) do { \
+ { \
+ unsigned long exp; \
+ \
+ KUNIT_EXPECT_LT_MSG(kunittest, offset, size, \
+ "bit offset for clump out-of-bounds"); \
+ \
+ exp = clump_exp[offset / 8]; \
+ KUNIT_EXPECT_NE_MSG(kunittest, exp, 0, \
+ "bit offset %u for zero clump", offset); \
+ \
+ KUNIT_EXPECT_EQ(kunittest, *clump, exp); \
+ } \
+ } while (0)
+
+#define expect_eq_str(exp_str, str, len) \
+ { \
+ if (strncmp(exp_str, str, len) != 0) { \
+ tc_err("expected %s, got %s", exp_str, str); \
} \
- result; \
- })
+ }
-#define expect_eq_ulong(...) __expect_eq(ulong, ##__VA_ARGS__)
#define expect_eq_uint(x, y) expect_eq_ulong((unsigned int)(x), (unsigned int)(y))
-#define expect_eq_bitmap(...) __expect_eq(bitmap, ##__VA_ARGS__)
-#define expect_eq_pbl(...) __expect_eq(pbl, ##__VA_ARGS__)
-#define expect_eq_u32_array(...) __expect_eq(u32_array, ##__VA_ARGS__)
-#define expect_eq_clump8(...) __expect_eq(clump8, ##__VA_ARGS__)
-#define expect_eq_str(...) __expect_eq(str, ##__VA_ARGS__)
-static void __init test_zero_clear(void)
+static void test_zero_clear(void)
{
DECLARE_BITMAP(bmap, 1024);
@@ -193,7 +125,7 @@ static void __init test_zero_clear(void)
expect_eq_pbl("", bmap, 1024);
}
-static void __init test_find_nth_bit(void)
+static void test_find_nth_bit(void)
{
unsigned long b, bit, cnt = 0;
DECLARE_BITMAP(bmap, 64 * 3);
@@ -234,7 +166,7 @@ static void __init test_find_nth_bit(void)
}
}
-static void __init test_fill_set(void)
+static void test_fill_set(void)
{
DECLARE_BITMAP(bmap, 1024);
@@ -263,7 +195,7 @@ static void __init test_fill_set(void)
expect_eq_pbl("0-1023", bmap, 1024);
}
-static void __init test_copy(void)
+static void test_copy(void)
{
DECLARE_BITMAP(bmap1, 1024);
DECLARE_BITMAP(bmap2, 1024);
@@ -302,7 +234,7 @@ static void __init test_copy(void)
expect_eq_pbl("0-108,128-1023", bmap2, 1024);
}
-static void __init test_bitmap_region(void)
+static void test_bitmap_region(void)
{
int pos, order;
@@ -327,7 +259,7 @@ static void __init test_bitmap_region(void)
#define EXP2_IN_BITS (sizeof(exp2) * 8)
-static void __init test_replace(void)
+static void test_replace(void)
{
unsigned int nbits = 64;
unsigned int nlongs = DIV_ROUND_UP(nbits, BITS_PER_LONG);
@@ -352,23 +284,23 @@ static void __init test_replace(void)
expect_eq_bitmap(bmap, exp3_1_0, nbits);
}
-static const unsigned long sg_mask[] __initconst = {
+static const unsigned long sg_mask[] = {
BITMAP_FROM_U64(0x000000000000035aULL),
};
-static const unsigned long sg_src[] __initconst = {
+static const unsigned long sg_src[] = {
BITMAP_FROM_U64(0x0000000000000667ULL),
};
-static const unsigned long sg_gather_exp[] __initconst = {
+static const unsigned long sg_gather_exp[] = {
BITMAP_FROM_U64(0x0000000000000029ULL),
};
-static const unsigned long sg_scatter_exp[] __initconst = {
+static const unsigned long sg_scatter_exp[] = {
BITMAP_FROM_U64(0x000000000000021aULL),
};
-static void __init test_bitmap_sg(void)
+static void test_bitmap_sg(void)
{
unsigned int nbits = 64;
DECLARE_BITMAP(bmap_gather, 100);
@@ -404,7 +336,7 @@ struct test_bitmap_parselist{
const int flags;
};
-static const struct test_bitmap_parselist parselist_tests[] __initconst = {
+static const struct test_bitmap_parselist parselist_tests[] = {
#define step (sizeof(u64) / sizeof(unsigned long))
{0, "0", &exp1[0], 8, 0},
@@ -489,7 +421,7 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
};
-static void __init test_bitmap_parselist(void)
+static void test_bitmap_parselist(void)
{
int i;
int err;
@@ -504,30 +436,28 @@ static void __init test_bitmap_parselist(void)
time = ktime_get() - time;
if (err != ptest.errno) {
- pr_err("parselist: %d: input is %s, errno is %d, expected %d\n",
+ tc_err("parselist: %d: input is %s, errno is %d, expected %d",
i, ptest.in, err, ptest.errno);
- failed_tests++;
continue;
}
if (!err && ptest.expected
&& !__bitmap_equal(bmap, ptest.expected, ptest.nbits)) {
- pr_err("parselist: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
+ tc_err("parselist: %d: input is %s, result is 0x%lx, expected 0x%lx",
i, ptest.in, bmap[0],
*ptest.expected);
- failed_tests++;
continue;
}
if (ptest.flags & PARSE_TIME)
- pr_info("parselist: %d: input is '%s' OK, Time: %llu\n",
+ kunit_info(kunittest, "parselist: %d: input is '%s' OK, Time: %llu",
i, ptest.in, time);
#undef ptest
}
}
-static void __init test_bitmap_printlist(void)
+static void test_bitmap_printlist(void)
{
unsigned long *bmap = kmalloc(PAGE_SIZE, GFP_KERNEL);
char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
@@ -548,37 +478,35 @@ static void __init test_bitmap_printlist(void)
time = ktime_get() - time;
if (ret != slen + 1) {
- pr_err("bitmap_print_to_pagebuf: result is %d, expected %d\n", ret, slen);
- failed_tests++;
+ tc_err("bitmap_print_to_pagebuf: result is %d, expected %d", ret, slen);
goto out;
}
if (strncmp(buf, expected, slen)) {
- pr_err("bitmap_print_to_pagebuf: result is %s, expected %s\n", buf, expected);
- failed_tests++;
+ tc_err("bitmap_print_to_pagebuf: result is %s, expected %s", buf, expected);
goto out;
}
- pr_info("bitmap_print_to_pagebuf: input is '%s', Time: %llu\n", buf, time);
+ kunit_info(kunittest, "bitmap_print_to_pagebuf: input is '%s', Time: %llu", buf, time);
out:
kfree(buf);
kfree(bmap);
}
-static const unsigned long parse_test[] __initconst = {
+static const unsigned long parse_test[] = {
BITMAP_FROM_U64(0),
BITMAP_FROM_U64(1),
BITMAP_FROM_U64(0xdeadbeef),
BITMAP_FROM_U64(0x100000000ULL),
};
-static const unsigned long parse_test2[] __initconst = {
+static const unsigned long parse_test2[] = {
BITMAP_FROM_U64(0x100000000ULL), BITMAP_FROM_U64(0xdeadbeef),
BITMAP_FROM_U64(0x100000000ULL), BITMAP_FROM_U64(0xbaadf00ddeadbeef),
BITMAP_FROM_U64(0x100000000ULL), BITMAP_FROM_U64(0x0badf00ddeadbeef),
};
-static const struct test_bitmap_parselist parse_tests[] __initconst = {
+static const struct test_bitmap_parselist parse_tests[] = {
{0, "", &parse_test[0 * step], 32, 0},
{0, " ", &parse_test[0 * step], 32, 0},
{0, "0", &parse_test[0 * step], 32, 0},
@@ -605,7 +533,7 @@ static const struct test_bitmap_parselist parse_tests[] __initconst = {
#undef step
};
-static void __init test_bitmap_parse(void)
+static void test_bitmap_parse(void)
{
int i;
int err;
@@ -621,28 +549,26 @@ static void __init test_bitmap_parse(void)
time = ktime_get() - time;
if (err != test.errno) {
- pr_err("parse: %d: input is %s, errno is %d, expected %d\n",
+ tc_err("parse: %d: input is %s, errno is %d, expected %d",
i, test.in, err, test.errno);
- failed_tests++;
continue;
}
if (!err && test.expected
&& !__bitmap_equal(bmap, test.expected, test.nbits)) {
- pr_err("parse: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
+ tc_err("parse: %d: input is %s, result is 0x%lx, expected 0x%lx",
i, test.in, bmap[0],
*test.expected);
- failed_tests++;
continue;
}
if (test.flags & PARSE_TIME)
- pr_info("parse: %d: input is '%s' OK, Time: %llu\n",
+ kunit_info(kunittest, "parse: %d: input is '%s' OK, Time: %llu",
i, test.in, time);
}
}
-static void __init test_bitmap_arr32(void)
+static void test_bitmap_arr32(void)
{
unsigned int nbits, next_bit;
u32 arr[EXP1_IN_BITS / 32];
@@ -658,10 +584,8 @@ static void __init test_bitmap_arr32(void)
next_bit = find_next_bit(bmap2,
round_up(nbits, BITS_PER_LONG), nbits);
if (next_bit < round_up(nbits, BITS_PER_LONG)) {
- pr_err("bitmap_copy_arr32(nbits == %d:"
- " tail is not safely cleared: %d\n",
+ tc_err("bitmap_copy_arr32(nbits == %d: tail is not safely cleared: %d",
nbits, next_bit);
- failed_tests++;
}
if (nbits < EXP1_IN_BITS - 32)
@@ -670,7 +594,7 @@ static void __init test_bitmap_arr32(void)
}
}
-static void __init test_bitmap_arr64(void)
+static void test_bitmap_arr64(void)
{
unsigned int nbits, next_bit;
u64 arr[EXP1_IN_BITS / 64];
@@ -686,17 +610,15 @@ static void __init test_bitmap_arr64(void)
next_bit = find_next_bit(bmap2, round_up(nbits, BITS_PER_LONG), nbits);
if (next_bit < round_up(nbits, BITS_PER_LONG)) {
- pr_err("bitmap_copy_arr64(nbits == %d:"
- " tail is not safely cleared: %d\n", nbits, next_bit);
- failed_tests++;
+ tc_err("bitmap_copy_arr64(nbits == %d: tail is not safely cleared: %d",
+ nbits, next_bit);
}
if ((nbits % 64) &&
(arr[(nbits - 1) / 64] & ~GENMASK_ULL((nbits - 1) % 64, 0))) {
- pr_err("bitmap_to_arr64(nbits == %d): tail is not safely cleared: 0x%016llx (must be 0x%016llx)\n",
+ tc_err("bitmap_to_arr64(nbits == %d): tail is not safely cleared: 0x%016llx (must be 0x%016llx)",
nbits, arr[(nbits - 1) / 64],
GENMASK_ULL((nbits - 1) % 64, 0));
- failed_tests++;
}
if (nbits < EXP1_IN_BITS - 64)
@@ -704,7 +626,7 @@ static void __init test_bitmap_arr64(void)
}
}
-static void noinline __init test_mem_optimisations(void)
+static noinline void test_mem_optimisations(void)
{
DECLARE_BITMAP(bmap1, 1024);
DECLARE_BITMAP(bmap2, 1024);
@@ -718,30 +640,25 @@ static void noinline __init test_mem_optimisations(void)
bitmap_set(bmap1, start, nbits);
__bitmap_set(bmap2, start, nbits);
if (!bitmap_equal(bmap1, bmap2, 1024)) {
- printk("set not equal %d %d\n", start, nbits);
- failed_tests++;
+ tc_err("set not equal %d %d", start, nbits);
}
if (!__bitmap_equal(bmap1, bmap2, 1024)) {
- printk("set not __equal %d %d\n", start, nbits);
- failed_tests++;
+ tc_err("set not __equal %d %d", start, nbits);
}
bitmap_clear(bmap1, start, nbits);
__bitmap_clear(bmap2, start, nbits);
if (!bitmap_equal(bmap1, bmap2, 1024)) {
- printk("clear not equal %d %d\n", start, nbits);
- failed_tests++;
+ tc_err("clear not equal %d %d", start, nbits);
}
if (!__bitmap_equal(bmap1, bmap2, 1024)) {
- printk("clear not __equal %d %d\n", start,
- nbits);
- failed_tests++;
+ tc_err("clear not __equal %d %d", start, nbits);
}
}
}
}
-static const unsigned char clump_exp[] __initconst = {
+static const unsigned char clump_exp[] = {
0x01, /* 1 bit set */
0x02, /* non-edge 1 bit set */
0x00, /* zero bits set */
@@ -752,7 +669,7 @@ static const unsigned char clump_exp[] __initconst = {
0x05, /* non-adjacent 2 bits set */
};
-static void __init test_for_each_set_clump8(void)
+static void test_for_each_set_clump8(void)
{
#define CLUMP_EXP_NUMBITS 64
DECLARE_BITMAP(bits, CLUMP_EXP_NUMBITS);
@@ -774,7 +691,7 @@ static void __init test_for_each_set_clump8(void)
expect_eq_clump8(start, CLUMP_EXP_NUMBITS, clump_exp, &clump);
}
-static void __init test_for_each_set_bit_wrap(void)
+static void test_for_each_set_bit_wrap(void)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -799,7 +716,7 @@ static void __init test_for_each_set_bit_wrap(void)
}
}
-static void __init test_for_each_set_bit(void)
+static void test_for_each_set_bit(void)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -821,7 +738,7 @@ static void __init test_for_each_set_bit(void)
expect_eq_bitmap(orig, copy, 500);
}
-static void __init test_for_each_set_bit_from(void)
+static void test_for_each_set_bit_from(void)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -851,7 +768,7 @@ static void __init test_for_each_set_bit_from(void)
}
}
-static void __init test_for_each_clear_bit(void)
+static void test_for_each_clear_bit(void)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -873,7 +790,7 @@ static void __init test_for_each_clear_bit(void)
expect_eq_bitmap(orig, copy, 500);
}
-static void __init test_for_each_clear_bit_from(void)
+static void test_for_each_clear_bit_from(void)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -903,7 +820,7 @@ static void __init test_for_each_clear_bit_from(void)
}
}
-static void __init test_for_each_set_bitrange(void)
+static void test_for_each_set_bitrange(void)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -925,7 +842,7 @@ static void __init test_for_each_set_bitrange(void)
expect_eq_bitmap(orig, copy, 500);
}
-static void __init test_for_each_clear_bitrange(void)
+static void test_for_each_clear_bitrange(void)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -947,7 +864,7 @@ static void __init test_for_each_clear_bitrange(void)
expect_eq_bitmap(orig, copy, 500);
}
-static void __init test_for_each_set_bitrange_from(void)
+static void test_for_each_set_bitrange_from(void)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -977,7 +894,7 @@ static void __init test_for_each_set_bitrange_from(void)
}
}
-static void __init test_for_each_clear_bitrange_from(void)
+static void test_for_each_clear_bitrange_from(void)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -1048,7 +965,7 @@ static struct test_bitmap_cut test_cut[] = {
},
};
-static void __init test_bitmap_cut(void)
+static void test_bitmap_cut(void)
{
unsigned long b[5], *in = &b[1], *out = &b[0]; /* Partial overlap */
int i;
@@ -1071,14 +988,14 @@ struct test_bitmap_print {
const char *list;
};
-static const unsigned long small_bitmap[] __initconst = {
+static const unsigned long small_bitmap[] = {
BITMAP_FROM_U64(0x3333333311111111ULL),
};
-static const char small_mask[] __initconst = "33333333,11111111\n";
-static const char small_list[] __initconst = "0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61\n";
+static const char small_mask[] = "33333333,11111111\n";
+static const char small_list[] = "0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61\n";
-static const unsigned long large_bitmap[] __initconst = {
+static const unsigned long large_bitmap[] = {
BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
@@ -1101,7 +1018,7 @@ static const unsigned long large_bitmap[] __initconst = {
BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
};
-static const char large_mask[] __initconst = "33333333,11111111,33333333,11111111,"
+static const char large_mask[] = "33333333,11111111,33333333,11111111,"
"33333333,11111111,33333333,11111111,"
"33333333,11111111,33333333,11111111,"
"33333333,11111111,33333333,11111111,"
@@ -1122,7 +1039,7 @@ static const char large_mask[] __initconst = "33333333,11111111,33333333,1111111
"33333333,11111111,33333333,11111111,"
"33333333,11111111,33333333,11111111\n";
-static const char large_list[] __initconst = /* more than 4KB */
+static const char large_list[] = /* more than 4KB */
"0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61,64,68,72,76,80,84,88,92,96-97,100-101,104-1"
"05,108-109,112-113,116-117,120-121,124-125,128,132,136,140,144,148,152,156,160-161,164-165,168-169,172-173,176-1"
"77,180-181,184-185,188-189,192,196,200,204,208,212,216,220,224-225,228-229,232-233,236-237,240-241,244-245,248-2"
@@ -1164,12 +1081,12 @@ static const char large_list[] __initconst = /* more than 4KB */
"2489,2492-2493,2496,2500,2504,2508,2512,2516,2520,2524,2528-2529,2532-2533,2536-2537,2540-2541,2544-2545,2548-25"
"49,2552-2553,2556-2557\n";
-static const struct test_bitmap_print test_print[] __initconst = {
+static const struct test_bitmap_print test_print[] = {
{ small_bitmap, sizeof(small_bitmap) * BITS_PER_BYTE, small_mask, small_list },
{ large_bitmap, sizeof(large_bitmap) * BITS_PER_BYTE, large_mask, large_list },
};
-static void __init test_bitmap_print_buf(void)
+static void test_bitmap_print_buf(void)
{
int i;
@@ -1201,7 +1118,7 @@ static void __init test_bitmap_print_buf(void)
* FIXME: Clang breaks compile-time evaluations when KASAN and GCOV are enabled.
* To workaround it, GCOV is force-disabled in Makefile for this configuration.
*/
-static void __init test_bitmap_const_eval(void)
+static void test_bitmap_const_eval(void)
{
DECLARE_BITMAP(bitmap, BITS_PER_LONG);
unsigned long initvar = BIT(2);
@@ -1269,7 +1186,7 @@ static void __init test_bitmap_const_eval(void)
/*
* Helper function to test bitmap_write() overwriting the chosen byte pattern.
*/
-static void __init test_bitmap_write_helper(const char *pattern)
+static void test_bitmap_write_helper(const char *pattern)
{
DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
DECLARE_BITMAP(exp_bitmap, TEST_BIT_LEN);
@@ -1323,7 +1240,7 @@ static void __init test_bitmap_write_helper(const char *pattern)
}
}
-static void __init test_bitmap_read_write(void)
+static void test_bitmap_read_write(void)
{
unsigned char *pattern[3] = {"", "all:1/2", "all"};
DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
@@ -1372,7 +1289,7 @@ static void __init test_bitmap_read_write(void)
test_bitmap_write_helper(pattern[pi]);
}
-static void __init test_bitmap_read_perf(void)
+static void test_bitmap_read_perf(void)
{
DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
unsigned int cnt, nbits, i;
@@ -1395,10 +1312,10 @@ static void __init test_bitmap_read_perf(void)
}
}
time = ktime_get() - time;
- pr_info("Time spent in %s:\t%llu\n", __func__, time);
+ kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
}
-static void __init test_bitmap_write_perf(void)
+static void test_bitmap_write_perf(void)
{
DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
unsigned int cnt, nbits, i;
@@ -1417,13 +1334,15 @@ static void __init test_bitmap_write_perf(void)
}
}
time = ktime_get() - time;
- pr_info("Time spent in %s:\t%llu\n", __func__, time);
+ kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
}
#undef TEST_BIT_LEN
-static void __init selftest(void)
+static void bitmap_test(struct kunit *test)
{
+ kunittest = test;
+
test_zero_clear();
test_fill_set();
test_copy();
@@ -1456,7 +1375,18 @@ static void __init selftest(void)
test_for_each_set_bit_wrap();
}
-KSTM_MODULE_LOADERS(test_bitmap);
+static struct kunit_case bitmap_test_cases[] = {
+ KUNIT_CASE(bitmap_test),
+ {}
+};
+
+static struct kunit_suite bitmap_test_suite = {
+ .name = "bitmap",
+ .test_cases = bitmap_test_cases,
+};
+
+kunit_test_suite(bitmap_test_suite);
+
MODULE_AUTHOR("david decotigny <david.decotigny@googlers.com>");
MODULE_DESCRIPTION("Test cases for bitmap API");
MODULE_LICENSE("GPL");
diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh
deleted file mode 100755
index 00a416fbc0ef..000000000000
--- a/tools/testing/selftests/lib/bitmap.sh
+++ /dev/null
@@ -1,3 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-$(dirname $0)/../kselftest/module.sh "bitmap" test_bitmap
diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config
index dc15aba8d0a3..8817520d1f42 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
--
2.48.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/3] bitmap: break kunit into test cases
2025-02-07 20:14 [PATCH 0/3] bitmap: convert self-test to KUnit Tamir Duberstein
2025-02-07 20:14 ` [PATCH 1/3] bitmap: remove _check_eq_u32_array Tamir Duberstein
2025-02-07 20:14 ` [PATCH 2/3] bitmap: convert self-test to KUnit Tamir Duberstein
@ 2025-02-07 20:14 ` Tamir Duberstein
2025-02-08 9:07 ` David Gow
2025-02-10 5:23 ` Muhammad Usama Anjum
2025-02-08 9:07 ` [PATCH 0/3] bitmap: convert self-test to KUnit David Gow
2025-02-08 17:53 ` Yury Norov
4 siblings, 2 replies; 27+ messages in thread
From: Tamir Duberstein @ 2025-02-07 20:14 UTC (permalink / raw)
To: David Gow, John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Yury Norov, Rasmus Villemoes,
Shuah Khan, Kees Cook
Cc: Muhammad Usama Anjum, linux-kernel, linux-m68k, linuxppc-dev,
linux-kselftest, Tamir Duberstein
Move some tests into `bitmap_test_cases` and parameterize
`test_bitmap_print_buf`. This gives us nicer output in the event of a
failure.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
lib/bitmap_kunit.c | 182 ++++++++++++++++++++++++++---------------------------
1 file changed, 89 insertions(+), 93 deletions(-)
diff --git a/lib/bitmap_kunit.c b/lib/bitmap_kunit.c
index 0605228288d6..f7b90f6d5f49 100644
--- a/lib/bitmap_kunit.c
+++ b/lib/bitmap_kunit.c
@@ -17,8 +17,6 @@
static char pbl_buffer[PAGE_SIZE];
static char print_buf[PAGE_SIZE * 2];
-static struct kunit *kunittest;
-
#define tc_err(fmt, ...) \
KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
@@ -96,7 +94,7 @@ static const unsigned long exp3_1_0[] = {
#define expect_eq_uint(x, y) expect_eq_ulong((unsigned int)(x), (unsigned int)(y))
-static void test_zero_clear(void)
+static void test_zero_clear(struct kunit *kunittest)
{
DECLARE_BITMAP(bmap, 1024);
@@ -125,7 +123,7 @@ static void test_zero_clear(void)
expect_eq_pbl("", bmap, 1024);
}
-static void test_find_nth_bit(void)
+static void test_find_nth_bit(struct kunit *kunittest)
{
unsigned long b, bit, cnt = 0;
DECLARE_BITMAP(bmap, 64 * 3);
@@ -166,7 +164,7 @@ static void test_find_nth_bit(void)
}
}
-static void test_fill_set(void)
+static void test_fill_set(struct kunit *kunittest)
{
DECLARE_BITMAP(bmap, 1024);
@@ -195,7 +193,7 @@ static void test_fill_set(void)
expect_eq_pbl("0-1023", bmap, 1024);
}
-static void test_copy(void)
+static void test_copy(struct kunit *kunittest)
{
DECLARE_BITMAP(bmap1, 1024);
DECLARE_BITMAP(bmap2, 1024);
@@ -234,7 +232,7 @@ static void test_copy(void)
expect_eq_pbl("0-108,128-1023", bmap2, 1024);
}
-static void test_bitmap_region(void)
+static void test_bitmap_region(struct kunit *kunittest)
{
int pos, order;
@@ -259,7 +257,7 @@ static void test_bitmap_region(void)
#define EXP2_IN_BITS (sizeof(exp2) * 8)
-static void test_replace(void)
+static void test_replace(struct kunit *kunittest)
{
unsigned int nbits = 64;
unsigned int nlongs = DIV_ROUND_UP(nbits, BITS_PER_LONG);
@@ -300,7 +298,7 @@ static const unsigned long sg_scatter_exp[] = {
BITMAP_FROM_U64(0x000000000000021aULL),
};
-static void test_bitmap_sg(void)
+static void test_bitmap_sg(struct kunit *kunittest)
{
unsigned int nbits = 64;
DECLARE_BITMAP(bmap_gather, 100);
@@ -421,7 +419,7 @@ static const struct test_bitmap_parselist parselist_tests[] = {
};
-static void test_bitmap_parselist(void)
+static void test_bitmap_parselist(struct kunit *kunittest)
{
int i;
int err;
@@ -457,7 +455,7 @@ static void test_bitmap_parselist(void)
}
}
-static void test_bitmap_printlist(void)
+static void test_bitmap_printlist(struct kunit *kunittest)
{
unsigned long *bmap = kmalloc(PAGE_SIZE, GFP_KERNEL);
char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
@@ -533,7 +531,7 @@ static const struct test_bitmap_parselist parse_tests[] = {
#undef step
};
-static void test_bitmap_parse(void)
+static void test_bitmap_parse(struct kunit *kunittest)
{
int i;
int err;
@@ -568,7 +566,7 @@ static void test_bitmap_parse(void)
}
}
-static void test_bitmap_arr32(void)
+static void test_bitmap_arr32(struct kunit *kunittest)
{
unsigned int nbits, next_bit;
u32 arr[EXP1_IN_BITS / 32];
@@ -594,7 +592,7 @@ static void test_bitmap_arr32(void)
}
}
-static void test_bitmap_arr64(void)
+static void test_bitmap_arr64(struct kunit *kunittest)
{
unsigned int nbits, next_bit;
u64 arr[EXP1_IN_BITS / 64];
@@ -626,7 +624,7 @@ static void test_bitmap_arr64(void)
}
}
-static noinline void test_mem_optimisations(void)
+static noinline void test_mem_optimisations(struct kunit *kunittest)
{
DECLARE_BITMAP(bmap1, 1024);
DECLARE_BITMAP(bmap2, 1024);
@@ -669,7 +667,7 @@ static const unsigned char clump_exp[] = {
0x05, /* non-adjacent 2 bits set */
};
-static void test_for_each_set_clump8(void)
+static void test_for_each_set_clump8(struct kunit *kunittest)
{
#define CLUMP_EXP_NUMBITS 64
DECLARE_BITMAP(bits, CLUMP_EXP_NUMBITS);
@@ -691,7 +689,7 @@ static void test_for_each_set_clump8(void)
expect_eq_clump8(start, CLUMP_EXP_NUMBITS, clump_exp, &clump);
}
-static void test_for_each_set_bit_wrap(void)
+static void test_for_each_set_bit_wrap(struct kunit *kunittest)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -716,7 +714,7 @@ static void test_for_each_set_bit_wrap(void)
}
}
-static void test_for_each_set_bit(void)
+static void test_for_each_set_bit(struct kunit *kunittest)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -738,7 +736,7 @@ static void test_for_each_set_bit(void)
expect_eq_bitmap(orig, copy, 500);
}
-static void test_for_each_set_bit_from(void)
+static void test_for_each_set_bit_from(struct kunit *kunittest)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -768,7 +766,7 @@ static void test_for_each_set_bit_from(void)
}
}
-static void test_for_each_clear_bit(void)
+static void test_for_each_clear_bit(struct kunit *kunittest)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -790,7 +788,7 @@ static void test_for_each_clear_bit(void)
expect_eq_bitmap(orig, copy, 500);
}
-static void test_for_each_clear_bit_from(void)
+static void test_for_each_clear_bit_from(struct kunit *kunittest)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -820,7 +818,7 @@ static void test_for_each_clear_bit_from(void)
}
}
-static void test_for_each_set_bitrange(void)
+static void test_for_each_set_bitrange(struct kunit *kunittest)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -842,7 +840,7 @@ static void test_for_each_set_bitrange(void)
expect_eq_bitmap(orig, copy, 500);
}
-static void test_for_each_clear_bitrange(void)
+static void test_for_each_clear_bitrange(struct kunit *kunittest)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -864,7 +862,7 @@ static void test_for_each_clear_bitrange(void)
expect_eq_bitmap(orig, copy, 500);
}
-static void test_for_each_set_bitrange_from(void)
+static void test_for_each_set_bitrange_from(struct kunit *kunittest)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -894,7 +892,7 @@ static void test_for_each_set_bitrange_from(void)
}
}
-static void test_for_each_clear_bitrange_from(void)
+static void test_for_each_clear_bitrange_from(struct kunit *kunittest)
{
DECLARE_BITMAP(orig, 500);
DECLARE_BITMAP(copy, 500);
@@ -965,7 +963,7 @@ static struct test_bitmap_cut test_cut[] = {
},
};
-static void test_bitmap_cut(void)
+static void test_bitmap_cut(struct kunit *kunittest)
{
unsigned long b[5], *in = &b[1], *out = &b[0]; /* Partial overlap */
int i;
@@ -986,8 +984,14 @@ struct test_bitmap_print {
unsigned long nbits;
const char *mask;
const char *list;
+ const char *name;
};
+static void param_to_desc(const struct test_bitmap_print *param, char *desc)
+{
+ strscpy(desc, param->name, KUNIT_PARAM_DESC_SIZE);
+}
+
static const unsigned long small_bitmap[] = {
BITMAP_FROM_U64(0x3333333311111111ULL),
};
@@ -1082,35 +1086,34 @@ static const char large_list[] = /* more than 4KB */
"49,2552-2553,2556-2557\n";
static const struct test_bitmap_print test_print[] = {
- { small_bitmap, sizeof(small_bitmap) * BITS_PER_BYTE, small_mask, small_list },
- { large_bitmap, sizeof(large_bitmap) * BITS_PER_BYTE, large_mask, large_list },
+ { small_bitmap, sizeof(small_bitmap) * BITS_PER_BYTE, small_mask, small_list, "small" },
+ { large_bitmap, sizeof(large_bitmap) * BITS_PER_BYTE, large_mask, large_list, "large" },
};
-static void test_bitmap_print_buf(void)
+KUNIT_ARRAY_PARAM(test_print, test_print, param_to_desc);
+
+static void test_bitmap_print_buf(struct kunit *kunittest)
{
- int i;
+ int n;
- for (i = 0; i < ARRAY_SIZE(test_print); i++) {
- const struct test_bitmap_print *t = &test_print[i];
- int n;
+ const struct test_bitmap_print *t = kunittest->param_value;
- n = bitmap_print_bitmask_to_buf(print_buf, t->bitmap, t->nbits,
- 0, 2 * PAGE_SIZE);
- expect_eq_uint(strlen(t->mask) + 1, n);
- expect_eq_str(t->mask, print_buf, n);
+ n = bitmap_print_bitmask_to_buf(print_buf, t->bitmap, t->nbits,
+ 0, 2 * PAGE_SIZE);
+ expect_eq_uint(strlen(t->mask) + 1, n);
+ expect_eq_str(t->mask, print_buf, n);
+ n = bitmap_print_list_to_buf(print_buf, t->bitmap, t->nbits,
+ 0, 2 * PAGE_SIZE);
+ expect_eq_uint(strlen(t->list) + 1, n);
+ expect_eq_str(t->list, print_buf, n);
+
+ /* test by non-zero offset */
+ if (strlen(t->list) > PAGE_SIZE) {
n = bitmap_print_list_to_buf(print_buf, t->bitmap, t->nbits,
- 0, 2 * PAGE_SIZE);
- expect_eq_uint(strlen(t->list) + 1, n);
- expect_eq_str(t->list, print_buf, n);
-
- /* test by non-zero offset */
- if (strlen(t->list) > PAGE_SIZE) {
- n = bitmap_print_list_to_buf(print_buf, t->bitmap, t->nbits,
- PAGE_SIZE, PAGE_SIZE);
- expect_eq_uint(strlen(t->list) + 1 - PAGE_SIZE, n);
- expect_eq_str(t->list + PAGE_SIZE, print_buf, n);
- }
+ PAGE_SIZE, PAGE_SIZE);
+ expect_eq_uint(strlen(t->list) + 1 - PAGE_SIZE, n);
+ expect_eq_str(t->list + PAGE_SIZE, print_buf, n);
}
}
@@ -1118,7 +1121,7 @@ static void test_bitmap_print_buf(void)
* FIXME: Clang breaks compile-time evaluations when KASAN and GCOV are enabled.
* To workaround it, GCOV is force-disabled in Makefile for this configuration.
*/
-static void test_bitmap_const_eval(void)
+static void test_bitmap_const_eval(struct kunit *kunittest)
{
DECLARE_BITMAP(bitmap, BITS_PER_LONG);
unsigned long initvar = BIT(2);
@@ -1186,7 +1189,7 @@ static void test_bitmap_const_eval(void)
/*
* Helper function to test bitmap_write() overwriting the chosen byte pattern.
*/
-static void test_bitmap_write_helper(const char *pattern)
+static void test_bitmap_write_helper(struct kunit *kunittest, const char *pattern)
{
DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
DECLARE_BITMAP(exp_bitmap, TEST_BIT_LEN);
@@ -1240,7 +1243,7 @@ static void test_bitmap_write_helper(const char *pattern)
}
}
-static void test_bitmap_read_write(void)
+static void test_bitmap_read_write(struct kunit *kunittest)
{
unsigned char *pattern[3] = {"", "all:1/2", "all"};
DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
@@ -1286,10 +1289,10 @@ static void test_bitmap_read_write(void)
}
for (pi = 0; pi < ARRAY_SIZE(pattern); pi++)
- test_bitmap_write_helper(pattern[pi]);
+ test_bitmap_write_helper(kunittest, pattern[pi]);
}
-static void test_bitmap_read_perf(void)
+static void test_bitmap_read_perf(struct kunit *kunittest)
{
DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
unsigned int cnt, nbits, i;
@@ -1312,10 +1315,10 @@ static void test_bitmap_read_perf(void)
}
}
time = ktime_get() - time;
- kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
+ kunit_info(kunittest, "Time spent:\t%llu\n", time);
}
-static void test_bitmap_write_perf(void)
+static void test_bitmap_write_perf(struct kunit *kunittest)
{
DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
unsigned int cnt, nbits, i;
@@ -1334,49 +1337,42 @@ static void test_bitmap_write_perf(void)
}
}
time = ktime_get() - time;
- kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
+ kunit_info(kunittest, "Time spent:\t%llu\n", time);
}
#undef TEST_BIT_LEN
-static void bitmap_test(struct kunit *test)
-{
- kunittest = test;
-
- test_zero_clear();
- test_fill_set();
- test_copy();
- test_bitmap_region();
- test_replace();
- test_bitmap_sg();
- test_bitmap_arr32();
- test_bitmap_arr64();
- test_bitmap_parse();
- test_bitmap_parselist();
- test_bitmap_printlist();
- test_mem_optimisations();
- test_bitmap_cut();
- test_bitmap_print_buf();
- test_bitmap_const_eval();
- test_bitmap_read_write();
- test_bitmap_read_perf();
- test_bitmap_write_perf();
-
- test_find_nth_bit();
- test_for_each_set_bit();
- test_for_each_set_bit_from();
- test_for_each_clear_bit();
- test_for_each_clear_bit_from();
- test_for_each_set_bitrange();
- test_for_each_clear_bitrange();
- test_for_each_set_bitrange_from();
- test_for_each_clear_bitrange_from();
- test_for_each_set_clump8();
- test_for_each_set_bit_wrap();
-}
-
static struct kunit_case bitmap_test_cases[] = {
- KUNIT_CASE(bitmap_test),
+ KUNIT_CASE(test_zero_clear),
+ KUNIT_CASE(test_fill_set),
+ KUNIT_CASE(test_copy),
+ KUNIT_CASE(test_bitmap_region),
+ KUNIT_CASE(test_replace),
+ KUNIT_CASE(test_bitmap_sg),
+ KUNIT_CASE(test_bitmap_arr32),
+ KUNIT_CASE(test_bitmap_arr64),
+ KUNIT_CASE(test_bitmap_parse),
+ KUNIT_CASE(test_bitmap_parselist),
+ KUNIT_CASE(test_bitmap_printlist),
+ KUNIT_CASE(test_mem_optimisations),
+ KUNIT_CASE(test_bitmap_cut),
+ KUNIT_CASE_PARAM(test_bitmap_print_buf, test_print_gen_params),
+ KUNIT_CASE(test_bitmap_const_eval),
+ KUNIT_CASE(test_bitmap_read_write),
+ KUNIT_CASE(test_bitmap_read_perf),
+ KUNIT_CASE(test_bitmap_write_perf),
+
+ KUNIT_CASE(test_find_nth_bit),
+ KUNIT_CASE(test_for_each_set_bit),
+ KUNIT_CASE(test_for_each_set_bit_from),
+ KUNIT_CASE(test_for_each_clear_bit),
+ KUNIT_CASE(test_for_each_clear_bit_from),
+ KUNIT_CASE(test_for_each_set_bitrange),
+ KUNIT_CASE(test_for_each_clear_bitrange),
+ KUNIT_CASE(test_for_each_set_bitrange_from),
+ KUNIT_CASE(test_for_each_clear_bitrange_from),
+ KUNIT_CASE(test_for_each_set_clump8),
+ KUNIT_CASE(test_for_each_set_bit_wrap),
{}
};
--
2.48.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-07 20:14 [PATCH 0/3] bitmap: convert self-test to KUnit Tamir Duberstein
` (2 preceding siblings ...)
2025-02-07 20:14 ` [PATCH 3/3] bitmap: break kunit into test cases Tamir Duberstein
@ 2025-02-08 9:07 ` David Gow
2025-02-08 17:53 ` Yury Norov
4 siblings, 0 replies; 27+ messages in thread
From: David Gow @ 2025-02-08 9:07 UTC (permalink / raw)
To: Tamir Duberstein
Cc: John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Yury Norov, Rasmus Villemoes,
Shuah Khan, Kees Cook, Muhammad Usama Anjum, linux-kernel,
linux-m68k, linuxppc-dev, linux-kselftest
[-- Attachment #1: Type: text/plain, Size: 7782 bytes --]
On Sat, 8 Feb 2025 at 04:14, Tamir Duberstein <tamird@gmail.com> wrote:
>
> This is one of just 3 remaining "Test Module" kselftests (the others
> being printf and scanf), the rest having been converted to KUnit.
Thanks a lot, Tamir: these are great!
>
> I tested this using:
>
> $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 bitmap.
I have also tested this across several architectures, including arm,
m68k, i386, x86_64, powerpc64, and UML, and it works well here.
For anyone put off by the long command, testing it under UML is
$ ./tools/testing/kunit/kunit.py run bitmap
It should also automatically run on boot for any kernel with it
built-in, and run when the module is loaded if it's enabled as a
module.
>
> I've already sent out a conversion series for each of printf[0] and scanf[1].
>
> There was a previous attempt[2] to do this in July 2024. Please bear
> with me as I try to understand and address the objections from that
> time. I've spoken with Muhammad Usama Anjum, the author of that series,
> and received their approval to "take over" this work. Here we go...
>
> On 7/26/24 11:45 PM, John Hubbard wrote:
> >
> > This changes the situation from "works for Linus' tab completion
> > case", to "causes a tab completion problem"! :)
> >
> > I think a tests/ subdir is how we eventually decided to do this [1],
> > right?
> >
> > So:
> >
> > lib/tests/bitmap_kunit.c
> >
> > [1] https://lore.kernel.org/20240724201354.make.730-kees@kernel.org
>
> This is true and unfortunate, but not trivial to fix because new
> kallsyms tests were placed in lib/tests in commit 84b4a51fce4c
> ("selftests: add new kallsyms selftests") *after* the KUnit filename
> best practices were adopted.
>
> I propose that the KUnit maintainers blaze this trail using
> `string_kunit.c` which currently still lives in lib/ despite the KUnit
> docs giving it as an example at lib/tests/.
>
> On 7/27/24 12:24 AM, Shuah Khan wrote:
> >
> > This change will take away the ability to run bitmap tests during
> > boot on a non-kunit kernel.
> >
> > Nack on this change. I wan to see all tests that are being removed
> > from lib because they have been converted - also it doesn't make
> > sense to convert some tests like this one that add the ability test
> > during boot.
>
> This point was also discussed in another thread[3] in which:
>
> On 7/27/24 12:35 AM, Shuah Khan wrote:
> >
> > Please make sure you aren't taking away the ability to run these tests during
> > boot.
> >
> > It doesn't make sense to convert every single test especially when it
> > is intended to be run during boot without dependencies - not as a kunit test
> > but a regression test during boot.
> >
> > bitmap is one example - pay attention to the config help test - bitmap
> > one clearly states it runs regression testing during boot. Any test that
> > says that isn't a candidate for conversion.
> >
> > I am going to nack any such conversions.
>
> The crux of the argument seems to be that the config help text is taken
> to describe the author's intent with the fragment "at boot". I think
> this may be a case of confirmation bias: I see at least the following
> KUnit tests with "at boot" in their help text:
> - CPUMASK_KUNIT_TEST
> - BITFIELD_KUNIT
> - CHECKSUM_KUNIT
> - UTIL_MACROS_KUNIT
>
> It seems to me that the inference being made is that any test that runs
> "at boot" is intended to be run by both developers and users, but I find
> no evidence that bitmap in particular would ever provide additional
> value when run by users.
I admit to never quite understanding the "at boot" wording as an
objection here, as KUnit tests can run at boot (and do by default),
and are often regression tests. I'd not object if anyone wanted this
stated more clearly in the new config option's help text, though.
The line between 'developers' and 'users' in the kernel world is
necessarily thin, but I equally think users who would want to be able
to run test modules are unlikely to be unable to run KUnit tests if
they so desire. The only difficulty (which I admit could be annoying)
is that it's not possible to run the test against a kernel built with
CONFIG_KUNIT=n. Personally, I have my doubts that anyone is deriving
value from running test_bitmap against a system which was not compiled
for testing -- particularly since it's now quite common for distros to
ship kernels to users with CONFIG_KUNIT=m (IIRC, Red Hat is doing
this, and Android at least were considering it).
> There's further discussion about KUnit not being "ideal for cases where
> people would want to check a subsystem on a running kernel", but I find
> no evidence that bitmap in particular is actually testing the running
> kernel; it is a unit test of the bitmap functions, which is also stated
> in the config help text.
Again, I think the only issue here is the CONFIG_KUNIT=n argument
above. This is a real issue, but I can't imagine a case where someone
has a running system which has broken due to an issue in the bitmap
code which can't easily be reproduced on a fresh kernel with
CONFIG_KUNIT enabled. (Though that could just be a limitation of my
imagination, so if that has happened to someone, I'd love to hear the
story!)
>
> David Gow made many of the same points in his final reply[4], which was
> never replied to.
>
> Link: https://lore.kernel.org/all/20250207-printf-kunit-convert-v2-0-057b23860823@gmail.com/T/#u [0]
> Link: https://lore.kernel.org/all/20250207-scanf-kunit-convert-v4-0-a23e2afaede8@gmail.com/T/#u [1]
> Link: https://lore.kernel.org/all/20240726110658.2281070-1-usama.anjum@collabora.com/T/#u [2]
> Link: https://lore.kernel.org/all/327831fb-47ab-4555-8f0b-19a8dbcaacd7@collabora.com/T/#u [3]
> Link: https://lore.kernel.org/all/CABVgOSmMoPD3JfzVd4VTkzGL2fZCo8LfwzaVSzeFimPrhgLa5w@mail.gmail.com/ [4]
>
> Thanks for your attention.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
My only remaining concerns are that:
- We've not misinterpreted any of the objections to the previous
versions (and ideally that everyone's happy, or at least contentedly
resigned, for this to go through),
- This goes upstream in a way that minimises the conflicts in the
various defconfigs. Given the number of these ports, it'd be great to
either get them to all go through in the same tree or otherwise make
sure the resolutions (while trivial) are as non-annoying as possible.
Cheers,
-- David
> Tamir Duberstein (3):
> bitmap: remove _check_eq_u32_array
> bitmap: convert self-test to KUnit
> bitmap: break kunit into test cases
>
> MAINTAINERS | 2 +-
> arch/m68k/configs/amiga_defconfig | 1 -
> arch/m68k/configs/apollo_defconfig | 1 -
> arch/m68k/configs/atari_defconfig | 1 -
> arch/m68k/configs/bvme6000_defconfig | 1 -
> arch/m68k/configs/hp300_defconfig | 1 -
> arch/m68k/configs/mac_defconfig | 1 -
> arch/m68k/configs/multi_defconfig | 1 -
> arch/m68k/configs/mvme147_defconfig | 1 -
> arch/m68k/configs/mvme16x_defconfig | 1 -
> arch/m68k/configs/q40_defconfig | 1 -
> arch/m68k/configs/sun3_defconfig | 1 -
> arch/m68k/configs/sun3x_defconfig | 1 -
> arch/powerpc/configs/ppc64_defconfig | 1 -
> lib/Kconfig.debug | 24 +-
> lib/Makefile | 2 +-
> lib/{test_bitmap.c => bitmap_kunit.c} | 454 +++++++++++++---------------------
> tools/testing/selftests/lib/bitmap.sh | 3 -
> tools/testing/selftests/lib/config | 1 -
> 19 files changed, 195 insertions(+), 304 deletions(-)
> ---
> base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
> change-id: 20250207-bitmap-kunit-convert-92d3147b2eee
>
> Best regards,
> --
> Tamir Duberstein <tamird@gmail.com>
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] bitmap: remove _check_eq_u32_array
2025-02-07 20:14 ` [PATCH 1/3] bitmap: remove _check_eq_u32_array Tamir Duberstein
@ 2025-02-08 9:07 ` David Gow
2025-02-08 18:00 ` Yury Norov
2025-02-10 5:22 ` Muhammad Usama Anjum
2 siblings, 0 replies; 27+ messages in thread
From: David Gow @ 2025-02-08 9:07 UTC (permalink / raw)
To: Tamir Duberstein
Cc: John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Yury Norov, Rasmus Villemoes,
Shuah Khan, Kees Cook, Muhammad Usama Anjum, linux-kernel,
linux-m68k, linuxppc-dev, linux-kselftest
[-- Attachment #1: Type: text/plain, Size: 2156 bytes --]
On Sat, 8 Feb 2025 at 04:14, Tamir Duberstein <tamird@gmail.com> wrote:
>
> This has been unused since commit 3aa56885e516 ("bitmap: replace
> bitmap_{from,to}_u32array") in 2018. Remove it to avoid the need to port
> it to KUnit in this series.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
Makes sense.
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> lib/test_bitmap.c | 28 ----------------------------
> 1 file changed, 28 deletions(-)
>
> diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
> index 65a75d58ed9e..c83829ef557f 100644
> --- a/lib/test_bitmap.c
> +++ b/lib/test_bitmap.c
> @@ -100,34 +100,6 @@ __check_eq_pbl(const char *srcfile, unsigned int line,
> return true;
> }
>
> -static bool __init
> -__check_eq_u32_array(const char *srcfile, unsigned int line,
> - const u32 *exp_arr, unsigned int exp_len,
> - const u32 *arr, unsigned int len) __used;
> -static bool __init
> -__check_eq_u32_array(const char *srcfile, unsigned int line,
> - const u32 *exp_arr, unsigned int exp_len,
> - const u32 *arr, unsigned int len)
> -{
> - if (exp_len != len) {
> - pr_warn("[%s:%u] array length differ: expected %u, got %u\n",
> - srcfile, line,
> - exp_len, len);
> - return false;
> - }
> -
> - if (memcmp(exp_arr, arr, len*sizeof(*arr))) {
> - pr_warn("[%s:%u] array contents differ\n", srcfile, line);
> - print_hex_dump(KERN_WARNING, " exp: ", DUMP_PREFIX_OFFSET,
> - 32, 4, exp_arr, exp_len*sizeof(*exp_arr), false);
> - print_hex_dump(KERN_WARNING, " got: ", DUMP_PREFIX_OFFSET,
> - 32, 4, arr, len*sizeof(*arr), false);
> - return false;
> - }
> -
> - return true;
> -}
> -
> static bool __init __check_eq_clump8(const char *srcfile, unsigned int line,
> const unsigned int offset,
> const unsigned int size,
>
> --
> 2.48.1
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] bitmap: convert self-test to KUnit
2025-02-07 20:14 ` [PATCH 2/3] bitmap: convert self-test to KUnit Tamir Duberstein
@ 2025-02-08 9:07 ` David Gow
2025-02-10 5:22 ` Muhammad Usama Anjum
2025-02-10 7:42 ` Geert Uytterhoeven
2 siblings, 0 replies; 27+ messages in thread
From: David Gow @ 2025-02-08 9:07 UTC (permalink / raw)
To: Tamir Duberstein
Cc: John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Yury Norov, Rasmus Villemoes,
Shuah Khan, Kees Cook, Muhammad Usama Anjum, linux-kernel,
linux-m68k, linuxppc-dev, linux-kselftest
[-- Attachment #1: Type: text/plain, Size: 41922 bytes --]
On Sat, 8 Feb 2025 at 04:14, Tamir Duberstein <tamird@gmail.com> wrote:
>
> Convert the bitmap() self-test to a KUnit test.
>
> In the interest of keeping the patch reasonably-sized this doesn't
> refactor the tests into proper parameterized tests - it's all one big
> test case.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
Thanks very much for picking this up.
Personally, I'm very much in favour of this, particularly once the
refactor in the next patch lands.
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> MAINTAINERS | 2 +-
> arch/m68k/configs/amiga_defconfig | 1 -
> arch/m68k/configs/apollo_defconfig | 1 -
> arch/m68k/configs/atari_defconfig | 1 -
> arch/m68k/configs/bvme6000_defconfig | 1 -
> arch/m68k/configs/hp300_defconfig | 1 -
> arch/m68k/configs/mac_defconfig | 1 -
> arch/m68k/configs/multi_defconfig | 1 -
> arch/m68k/configs/mvme147_defconfig | 1 -
> arch/m68k/configs/mvme16x_defconfig | 1 -
> arch/m68k/configs/q40_defconfig | 1 -
> arch/m68k/configs/sun3_defconfig | 1 -
> arch/m68k/configs/sun3x_defconfig | 1 -
> arch/powerpc/configs/ppc64_defconfig | 1 -
> lib/Kconfig.debug | 24 ++-
> lib/Makefile | 2 +-
> lib/{test_bitmap.c => bitmap_kunit.c} | 322 +++++++++++++---------------------
> tools/testing/selftests/lib/bitmap.sh | 3 -
> tools/testing/selftests/lib/config | 1 -
> 19 files changed, 145 insertions(+), 222 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 896a307fa065..9824d4053748 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4016,11 +4016,11 @@ F: include/linux/nodemask_types.h
> F: include/vdso/bits.h
> F: lib/bitmap-str.c
> F: lib/bitmap.c
> +F: lib/bitmap_kunit.c
> F: lib/cpumask.c
> F: lib/cpumask_kunit.c
> F: lib/find_bit.c
> F: lib/find_bit_benchmark.c
> -F: lib/test_bitmap.c
> F: tools/include/linux/bitfield.h
> F: tools/include/linux/bitmap.h
> F: tools/include/linux/bits.h
> diff --git a/arch/m68k/configs/amiga_defconfig b/arch/m68k/configs/amiga_defconfig
> index dbf2ea561c85..3c9d7b58cb8a 100644
> --- a/arch/m68k/configs/amiga_defconfig
> +++ b/arch/m68k/configs/amiga_defconfig
> @@ -624,7 +624,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/apollo_defconfig b/arch/m68k/configs/apollo_defconfig
> index b0fd199cc0a4..b94c87a7cbdb 100644
> --- a/arch/m68k/configs/apollo_defconfig
> +++ b/arch/m68k/configs/apollo_defconfig
> @@ -581,7 +581,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/atari_defconfig b/arch/m68k/configs/atari_defconfig
> index bb5b2d3b6c10..823ba96c4486 100644
> --- a/arch/m68k/configs/atari_defconfig
> +++ b/arch/m68k/configs/atari_defconfig
> @@ -601,7 +601,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/bvme6000_defconfig b/arch/m68k/configs/bvme6000_defconfig
> index 8315a13bab73..0fa985129200 100644
> --- a/arch/m68k/configs/bvme6000_defconfig
> +++ b/arch/m68k/configs/bvme6000_defconfig
> @@ -573,7 +573,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/hp300_defconfig b/arch/m68k/configs/hp300_defconfig
> index 350370657e5f..75ed6eb547c6 100644
> --- a/arch/m68k/configs/hp300_defconfig
> +++ b/arch/m68k/configs/hp300_defconfig
> @@ -583,7 +583,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/mac_defconfig b/arch/m68k/configs/mac_defconfig
> index f942b4755702..149c398f80a8 100644
> --- a/arch/m68k/configs/mac_defconfig
> +++ b/arch/m68k/configs/mac_defconfig
> @@ -600,7 +600,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/multi_defconfig b/arch/m68k/configs/multi_defconfig
> index b1eaad02efab..34e6eaa47a18 100644
> --- a/arch/m68k/configs/multi_defconfig
> +++ b/arch/m68k/configs/multi_defconfig
> @@ -687,7 +687,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/mvme147_defconfig b/arch/m68k/configs/mvme147_defconfig
> index 6309a4442bb3..9e9f8883b38d 100644
> --- a/arch/m68k/configs/mvme147_defconfig
> +++ b/arch/m68k/configs/mvme147_defconfig
> @@ -573,7 +573,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/mvme16x_defconfig b/arch/m68k/configs/mvme16x_defconfig
> index 3feb0731f814..5ed08e78c4fd 100644
> --- a/arch/m68k/configs/mvme16x_defconfig
> +++ b/arch/m68k/configs/mvme16x_defconfig
> @@ -574,7 +574,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/q40_defconfig b/arch/m68k/configs/q40_defconfig
> index ea04b1b0da7d..4e0eea94a6a0 100644
> --- a/arch/m68k/configs/q40_defconfig
> +++ b/arch/m68k/configs/q40_defconfig
> @@ -590,7 +590,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/sun3_defconfig b/arch/m68k/configs/sun3_defconfig
> index f52d9af92153..690e2156b5f7 100644
> --- a/arch/m68k/configs/sun3_defconfig
> +++ b/arch/m68k/configs/sun3_defconfig
> @@ -570,7 +570,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/sun3x_defconfig b/arch/m68k/configs/sun3x_defconfig
> index f348447824da..71486fd428da 100644
> --- a/arch/m68k/configs/sun3x_defconfig
> +++ b/arch/m68k/configs/sun3x_defconfig
> @@ -571,7 +571,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
> index 465eb96c755e..cfd235d90c95 100644
> --- a/arch/powerpc/configs/ppc64_defconfig
> +++ b/arch/powerpc/configs/ppc64_defconfig
> @@ -446,7 +446,6 @@ CONFIG_TEST_STRING_HELPERS=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 1af972a92d06..fd3dcb0677b5 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2427,6 +2427,23 @@ config ASYNC_RAID6_TEST
> config TEST_HEXDUMP
> tristate "Test functions located in the hexdump module at runtime"
>
> +config BITMAP_KUNIT_TEST
> + tristate "KUnit test bitmap_*() family of functions at runtime" if !KUNIT_ALL_TESTS
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + Enable this option to test the bitmap functions at boot.
> +
> + KUnit tests run during boot and output the results to the debug log
> + in TAP format (http://testanything.org/). Only useful for kernel devs
> + running the KUnit test harness, and not intended for inclusion into a
> + production build.
> +
> + For more information on KUnit and unit tests in general please refer
> + to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> + If unsure, say N.
> +
> config STRING_KUNIT_TEST
> tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS
> depends on KUNIT
> @@ -2446,13 +2463,6 @@ config TEST_PRINTF
> config TEST_SCANF
> tristate "Test scanf() family of functions at runtime"
>
> -config TEST_BITMAP
> - tristate "Test bitmap_*() family of functions at runtime"
> - help
> - Enable this option to test the bitmap functions at boot.
> -
> - If unsure, say N.
> -
> config TEST_UUID
> tristate "Test functions located in the uuid module at runtime"
>
> diff --git a/lib/Makefile b/lib/Makefile
> index d5cfc7afbbb8..d735e1b70606 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -87,7 +87,7 @@ obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
> obj-$(CONFIG_TEST_PRINTF) += test_printf.o
> obj-$(CONFIG_TEST_SCANF) += test_scanf.o
>
> -obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
> +obj-$(CONFIG_BITMAP_KUNIT_TEST) += bitmap_kunit.o
> ifeq ($(CONFIG_CC_IS_CLANG)$(CONFIG_KASAN),yy)
> # FIXME: Clang breaks test_bitmap_const_eval when KASAN and GCOV are enabled
> GCOV_PROFILE_test_bitmap.o := n
> diff --git a/lib/test_bitmap.c b/lib/bitmap_kunit.c
> similarity index 83%
> rename from lib/test_bitmap.c
> rename to lib/bitmap_kunit.c
> index c83829ef557f..0605228288d6 100644
> --- a/lib/test_bitmap.c
> +++ b/lib/bitmap_kunit.c
> @@ -3,10 +3,8 @@
> * Test cases for bitmap API.
> */
>
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> +#include <kunit/test.h>
> #include <linux/bitmap.h>
> -#include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/printk.h>
> @@ -14,16 +12,17 @@
> #include <linux/string.h>
> #include <linux/uaccess.h>
>
> -#include "../tools/testing/selftests/kselftest_module.h"
> -
> #define EXP1_IN_BITS (sizeof(exp1) * 8)
>
> -KSTM_MODULE_GLOBALS();
> +static char pbl_buffer[PAGE_SIZE];
> +static char print_buf[PAGE_SIZE * 2];
> +
> +static struct kunit *kunittest;
>
> -static char pbl_buffer[PAGE_SIZE] __initdata;
> -static char print_buf[PAGE_SIZE * 2] __initdata;
> +#define tc_err(fmt, ...) \
> + KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
>
> -static const unsigned long exp1[] __initconst = {
> +static const unsigned long exp1[] = {
> BITMAP_FROM_U64(1),
> BITMAP_FROM_U64(2),
> BITMAP_FROM_U64(0x0000ffff),
> @@ -41,130 +40,63 @@ static const unsigned long exp1[] __initconst = {
> BITMAP_FROM_U64(0x80000000),
> };
>
> -static const unsigned long exp2[] __initconst = {
> +static const unsigned long exp2[] = {
> BITMAP_FROM_U64(0x3333333311111111ULL),
> BITMAP_FROM_U64(0xffffffff77777777ULL),
> };
>
> /* Fibonacci sequence */
> -static const unsigned long exp2_to_exp3_mask[] __initconst = {
> +static const unsigned long exp2_to_exp3_mask[] = {
> BITMAP_FROM_U64(0x008000020020212eULL),
> };
> /* exp3_0_1 = (exp2[0] & ~exp2_to_exp3_mask) | (exp2[1] & exp2_to_exp3_mask) */
> -static const unsigned long exp3_0_1[] __initconst = {
> +static const unsigned long exp3_0_1[] = {
> BITMAP_FROM_U64(0x33b3333311313137ULL),
> };
> /* exp3_1_0 = (exp2[1] & ~exp2_to_exp3_mask) | (exp2[0] & exp2_to_exp3_mask) */
> -static const unsigned long exp3_1_0[] __initconst = {
> +static const unsigned long exp3_1_0[] = {
> BITMAP_FROM_U64(0xff7fffff77575751ULL),
> };
>
> -static bool __init
> -__check_eq_ulong(const char *srcfile, unsigned int line,
> - const unsigned long exp_ulong, unsigned long x)
> -{
> - if (exp_ulong != x) {
> - pr_err("[%s:%u] expected %lu, got %lu\n",
> - srcfile, line, exp_ulong, x);
> - return false;
> - }
> - return true;
> -}
> -
> -static bool __init
> -__check_eq_bitmap(const char *srcfile, unsigned int line,
> - const unsigned long *exp_bmap, const unsigned long *bmap,
> - unsigned int nbits)
> -{
> - if (!bitmap_equal(exp_bmap, bmap, nbits)) {
> - pr_warn("[%s:%u] bitmaps contents differ: expected \"%*pbl\", got \"%*pbl\"\n",
> - srcfile, line,
> - nbits, exp_bmap, nbits, bmap);
> - return false;
> - }
> - return true;
> -}
> -
> -static bool __init
> -__check_eq_pbl(const char *srcfile, unsigned int line,
> - const char *expected_pbl,
> - const unsigned long *bitmap, unsigned int nbits)
> -{
> - snprintf(pbl_buffer, sizeof(pbl_buffer), "%*pbl", nbits, bitmap);
> - if (strcmp(expected_pbl, pbl_buffer)) {
> - pr_warn("[%s:%u] expected \"%s\", got \"%s\"\n",
> - srcfile, line,
> - expected_pbl, pbl_buffer);
> - return false;
> - }
> - return true;
> -}
> -
> -static bool __init __check_eq_clump8(const char *srcfile, unsigned int line,
> - const unsigned int offset,
> - const unsigned int size,
> - const unsigned char *const clump_exp,
> - const unsigned long *const clump)
> -{
> - unsigned long exp;
> -
> - if (offset >= size) {
> - pr_warn("[%s:%u] bit offset for clump out-of-bounds: expected less than %u, got %u\n",
> - srcfile, line, size, offset);
> - return false;
> - }
> -
> - exp = clump_exp[offset / 8];
> - if (!exp) {
> - pr_warn("[%s:%u] bit offset for zero clump: expected nonzero clump, got bit offset %u with clump value 0",
> - srcfile, line, offset);
> - return false;
> - }
> -
> - if (*clump != exp) {
> - pr_warn("[%s:%u] expected clump value of 0x%lX, got clump value of 0x%lX",
> - srcfile, line, exp, *clump);
> - return false;
> - }
> -
> - return true;
> -}
> -
> -static bool __init
> -__check_eq_str(const char *srcfile, unsigned int line,
> - const char *exp_str, const char *str,
> - unsigned int len)
> -{
> - bool eq;
> -
> - eq = strncmp(exp_str, str, len) == 0;
> - if (!eq)
> - pr_err("[%s:%u] expected %s, got %s\n", srcfile, line, exp_str, str);
> -
> - return eq;
> -}
> -
> -#define __expect_eq(suffix, ...) \
> - ({ \
> - int result = 0; \
> - total_tests++; \
> - if (!__check_eq_ ## suffix(__FILE__, __LINE__, \
> - ##__VA_ARGS__)) { \
> - failed_tests++; \
> - result = 1; \
> +#define expect_eq_ulong(exp_ulong, x) KUNIT_EXPECT_EQ(kunittest, exp_ulong, x)
> +
> +#define expect_eq_bitmap(exp_bmap, bmap, nbits) \
> + KUNIT_EXPECT_TRUE_MSG(kunittest, bitmap_equal(exp_bmap, bmap, nbits), \
> + "bitmaps contents differ: expected \"%*pbl\", got \"%*pbl\"", \
> + nbits, exp_bmap, nbits, bmap)
> +
> +#define expect_eq_pbl(expected_pbl, bitmap, nbits) do { \
> + { \
> + snprintf(pbl_buffer, sizeof(pbl_buffer), "%*pbl", nbits, bitmap); \
> + KUNIT_EXPECT_STREQ(kunittest, expected_pbl, pbl_buffer); \
> + } \
> + } while (0)
> +
> +#define expect_eq_clump8(offset, size, clump_exp, clump) do { \
> + { \
> + unsigned long exp; \
> + \
> + KUNIT_EXPECT_LT_MSG(kunittest, offset, size, \
> + "bit offset for clump out-of-bounds"); \
> + \
> + exp = clump_exp[offset / 8]; \
> + KUNIT_EXPECT_NE_MSG(kunittest, exp, 0, \
> + "bit offset %u for zero clump", offset); \
> + \
> + KUNIT_EXPECT_EQ(kunittest, *clump, exp); \
> + } \
> + } while (0)
> +
> +#define expect_eq_str(exp_str, str, len) \
> + { \
> + if (strncmp(exp_str, str, len) != 0) { \
> + tc_err("expected %s, got %s", exp_str, str); \
> } \
> - result; \
> - })
> + }
>
> -#define expect_eq_ulong(...) __expect_eq(ulong, ##__VA_ARGS__)
> #define expect_eq_uint(x, y) expect_eq_ulong((unsigned int)(x), (unsigned int)(y))
> -#define expect_eq_bitmap(...) __expect_eq(bitmap, ##__VA_ARGS__)
> -#define expect_eq_pbl(...) __expect_eq(pbl, ##__VA_ARGS__)
> -#define expect_eq_u32_array(...) __expect_eq(u32_array, ##__VA_ARGS__)
> -#define expect_eq_clump8(...) __expect_eq(clump8, ##__VA_ARGS__)
> -#define expect_eq_str(...) __expect_eq(str, ##__VA_ARGS__)
>
> -static void __init test_zero_clear(void)
> +static void test_zero_clear(void)
> {
> DECLARE_BITMAP(bmap, 1024);
>
> @@ -193,7 +125,7 @@ static void __init test_zero_clear(void)
> expect_eq_pbl("", bmap, 1024);
> }
>
> -static void __init test_find_nth_bit(void)
> +static void test_find_nth_bit(void)
> {
> unsigned long b, bit, cnt = 0;
> DECLARE_BITMAP(bmap, 64 * 3);
> @@ -234,7 +166,7 @@ static void __init test_find_nth_bit(void)
> }
> }
>
> -static void __init test_fill_set(void)
> +static void test_fill_set(void)
> {
> DECLARE_BITMAP(bmap, 1024);
>
> @@ -263,7 +195,7 @@ static void __init test_fill_set(void)
> expect_eq_pbl("0-1023", bmap, 1024);
> }
>
> -static void __init test_copy(void)
> +static void test_copy(void)
> {
> DECLARE_BITMAP(bmap1, 1024);
> DECLARE_BITMAP(bmap2, 1024);
> @@ -302,7 +234,7 @@ static void __init test_copy(void)
> expect_eq_pbl("0-108,128-1023", bmap2, 1024);
> }
>
> -static void __init test_bitmap_region(void)
> +static void test_bitmap_region(void)
> {
> int pos, order;
>
> @@ -327,7 +259,7 @@ static void __init test_bitmap_region(void)
>
> #define EXP2_IN_BITS (sizeof(exp2) * 8)
>
> -static void __init test_replace(void)
> +static void test_replace(void)
> {
> unsigned int nbits = 64;
> unsigned int nlongs = DIV_ROUND_UP(nbits, BITS_PER_LONG);
> @@ -352,23 +284,23 @@ static void __init test_replace(void)
> expect_eq_bitmap(bmap, exp3_1_0, nbits);
> }
>
> -static const unsigned long sg_mask[] __initconst = {
> +static const unsigned long sg_mask[] = {
> BITMAP_FROM_U64(0x000000000000035aULL),
> };
>
> -static const unsigned long sg_src[] __initconst = {
> +static const unsigned long sg_src[] = {
> BITMAP_FROM_U64(0x0000000000000667ULL),
> };
>
> -static const unsigned long sg_gather_exp[] __initconst = {
> +static const unsigned long sg_gather_exp[] = {
> BITMAP_FROM_U64(0x0000000000000029ULL),
> };
>
> -static const unsigned long sg_scatter_exp[] __initconst = {
> +static const unsigned long sg_scatter_exp[] = {
> BITMAP_FROM_U64(0x000000000000021aULL),
> };
>
> -static void __init test_bitmap_sg(void)
> +static void test_bitmap_sg(void)
> {
> unsigned int nbits = 64;
> DECLARE_BITMAP(bmap_gather, 100);
> @@ -404,7 +336,7 @@ struct test_bitmap_parselist{
> const int flags;
> };
>
> -static const struct test_bitmap_parselist parselist_tests[] __initconst = {
> +static const struct test_bitmap_parselist parselist_tests[] = {
> #define step (sizeof(u64) / sizeof(unsigned long))
>
> {0, "0", &exp1[0], 8, 0},
> @@ -489,7 +421,7 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
>
> };
>
> -static void __init test_bitmap_parselist(void)
> +static void test_bitmap_parselist(void)
> {
> int i;
> int err;
> @@ -504,30 +436,28 @@ static void __init test_bitmap_parselist(void)
> time = ktime_get() - time;
>
> if (err != ptest.errno) {
> - pr_err("parselist: %d: input is %s, errno is %d, expected %d\n",
> + tc_err("parselist: %d: input is %s, errno is %d, expected %d",
> i, ptest.in, err, ptest.errno);
> - failed_tests++;
> continue;
> }
>
> if (!err && ptest.expected
> && !__bitmap_equal(bmap, ptest.expected, ptest.nbits)) {
> - pr_err("parselist: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
> + tc_err("parselist: %d: input is %s, result is 0x%lx, expected 0x%lx",
> i, ptest.in, bmap[0],
> *ptest.expected);
> - failed_tests++;
> continue;
> }
>
> if (ptest.flags & PARSE_TIME)
> - pr_info("parselist: %d: input is '%s' OK, Time: %llu\n",
> + kunit_info(kunittest, "parselist: %d: input is '%s' OK, Time: %llu",
> i, ptest.in, time);
>
> #undef ptest
> }
> }
>
> -static void __init test_bitmap_printlist(void)
> +static void test_bitmap_printlist(void)
> {
> unsigned long *bmap = kmalloc(PAGE_SIZE, GFP_KERNEL);
> char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> @@ -548,37 +478,35 @@ static void __init test_bitmap_printlist(void)
> time = ktime_get() - time;
>
> if (ret != slen + 1) {
> - pr_err("bitmap_print_to_pagebuf: result is %d, expected %d\n", ret, slen);
> - failed_tests++;
> + tc_err("bitmap_print_to_pagebuf: result is %d, expected %d", ret, slen);
> goto out;
> }
>
> if (strncmp(buf, expected, slen)) {
> - pr_err("bitmap_print_to_pagebuf: result is %s, expected %s\n", buf, expected);
> - failed_tests++;
> + tc_err("bitmap_print_to_pagebuf: result is %s, expected %s", buf, expected);
> goto out;
> }
>
> - pr_info("bitmap_print_to_pagebuf: input is '%s', Time: %llu\n", buf, time);
> + kunit_info(kunittest, "bitmap_print_to_pagebuf: input is '%s', Time: %llu", buf, time);
> out:
> kfree(buf);
> kfree(bmap);
> }
>
> -static const unsigned long parse_test[] __initconst = {
> +static const unsigned long parse_test[] = {
> BITMAP_FROM_U64(0),
> BITMAP_FROM_U64(1),
> BITMAP_FROM_U64(0xdeadbeef),
> BITMAP_FROM_U64(0x100000000ULL),
> };
>
> -static const unsigned long parse_test2[] __initconst = {
> +static const unsigned long parse_test2[] = {
> BITMAP_FROM_U64(0x100000000ULL), BITMAP_FROM_U64(0xdeadbeef),
> BITMAP_FROM_U64(0x100000000ULL), BITMAP_FROM_U64(0xbaadf00ddeadbeef),
> BITMAP_FROM_U64(0x100000000ULL), BITMAP_FROM_U64(0x0badf00ddeadbeef),
> };
>
> -static const struct test_bitmap_parselist parse_tests[] __initconst = {
> +static const struct test_bitmap_parselist parse_tests[] = {
> {0, "", &parse_test[0 * step], 32, 0},
> {0, " ", &parse_test[0 * step], 32, 0},
> {0, "0", &parse_test[0 * step], 32, 0},
> @@ -605,7 +533,7 @@ static const struct test_bitmap_parselist parse_tests[] __initconst = {
> #undef step
> };
>
> -static void __init test_bitmap_parse(void)
> +static void test_bitmap_parse(void)
> {
> int i;
> int err;
> @@ -621,28 +549,26 @@ static void __init test_bitmap_parse(void)
> time = ktime_get() - time;
>
> if (err != test.errno) {
> - pr_err("parse: %d: input is %s, errno is %d, expected %d\n",
> + tc_err("parse: %d: input is %s, errno is %d, expected %d",
> i, test.in, err, test.errno);
> - failed_tests++;
> continue;
> }
>
> if (!err && test.expected
> && !__bitmap_equal(bmap, test.expected, test.nbits)) {
> - pr_err("parse: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
> + tc_err("parse: %d: input is %s, result is 0x%lx, expected 0x%lx",
> i, test.in, bmap[0],
> *test.expected);
> - failed_tests++;
> continue;
> }
>
> if (test.flags & PARSE_TIME)
> - pr_info("parse: %d: input is '%s' OK, Time: %llu\n",
> + kunit_info(kunittest, "parse: %d: input is '%s' OK, Time: %llu",
> i, test.in, time);
> }
> }
>
> -static void __init test_bitmap_arr32(void)
> +static void test_bitmap_arr32(void)
> {
> unsigned int nbits, next_bit;
> u32 arr[EXP1_IN_BITS / 32];
> @@ -658,10 +584,8 @@ static void __init test_bitmap_arr32(void)
> next_bit = find_next_bit(bmap2,
> round_up(nbits, BITS_PER_LONG), nbits);
> if (next_bit < round_up(nbits, BITS_PER_LONG)) {
> - pr_err("bitmap_copy_arr32(nbits == %d:"
> - " tail is not safely cleared: %d\n",
> + tc_err("bitmap_copy_arr32(nbits == %d: tail is not safely cleared: %d",
> nbits, next_bit);
> - failed_tests++;
> }
>
> if (nbits < EXP1_IN_BITS - 32)
> @@ -670,7 +594,7 @@ static void __init test_bitmap_arr32(void)
> }
> }
>
> -static void __init test_bitmap_arr64(void)
> +static void test_bitmap_arr64(void)
> {
> unsigned int nbits, next_bit;
> u64 arr[EXP1_IN_BITS / 64];
> @@ -686,17 +610,15 @@ static void __init test_bitmap_arr64(void)
>
> next_bit = find_next_bit(bmap2, round_up(nbits, BITS_PER_LONG), nbits);
> if (next_bit < round_up(nbits, BITS_PER_LONG)) {
> - pr_err("bitmap_copy_arr64(nbits == %d:"
> - " tail is not safely cleared: %d\n", nbits, next_bit);
> - failed_tests++;
> + tc_err("bitmap_copy_arr64(nbits == %d: tail is not safely cleared: %d",
> + nbits, next_bit);
> }
>
> if ((nbits % 64) &&
> (arr[(nbits - 1) / 64] & ~GENMASK_ULL((nbits - 1) % 64, 0))) {
> - pr_err("bitmap_to_arr64(nbits == %d): tail is not safely cleared: 0x%016llx (must be 0x%016llx)\n",
> + tc_err("bitmap_to_arr64(nbits == %d): tail is not safely cleared: 0x%016llx (must be 0x%016llx)",
> nbits, arr[(nbits - 1) / 64],
> GENMASK_ULL((nbits - 1) % 64, 0));
> - failed_tests++;
> }
>
> if (nbits < EXP1_IN_BITS - 64)
> @@ -704,7 +626,7 @@ static void __init test_bitmap_arr64(void)
> }
> }
>
> -static void noinline __init test_mem_optimisations(void)
> +static noinline void test_mem_optimisations(void)
> {
> DECLARE_BITMAP(bmap1, 1024);
> DECLARE_BITMAP(bmap2, 1024);
> @@ -718,30 +640,25 @@ static void noinline __init test_mem_optimisations(void)
> bitmap_set(bmap1, start, nbits);
> __bitmap_set(bmap2, start, nbits);
> if (!bitmap_equal(bmap1, bmap2, 1024)) {
> - printk("set not equal %d %d\n", start, nbits);
> - failed_tests++;
> + tc_err("set not equal %d %d", start, nbits);
> }
> if (!__bitmap_equal(bmap1, bmap2, 1024)) {
> - printk("set not __equal %d %d\n", start, nbits);
> - failed_tests++;
> + tc_err("set not __equal %d %d", start, nbits);
> }
>
> bitmap_clear(bmap1, start, nbits);
> __bitmap_clear(bmap2, start, nbits);
> if (!bitmap_equal(bmap1, bmap2, 1024)) {
> - printk("clear not equal %d %d\n", start, nbits);
> - failed_tests++;
> + tc_err("clear not equal %d %d", start, nbits);
> }
> if (!__bitmap_equal(bmap1, bmap2, 1024)) {
> - printk("clear not __equal %d %d\n", start,
> - nbits);
> - failed_tests++;
> + tc_err("clear not __equal %d %d", start, nbits);
> }
> }
> }
> }
>
> -static const unsigned char clump_exp[] __initconst = {
> +static const unsigned char clump_exp[] = {
> 0x01, /* 1 bit set */
> 0x02, /* non-edge 1 bit set */
> 0x00, /* zero bits set */
> @@ -752,7 +669,7 @@ static const unsigned char clump_exp[] __initconst = {
> 0x05, /* non-adjacent 2 bits set */
> };
>
> -static void __init test_for_each_set_clump8(void)
> +static void test_for_each_set_clump8(void)
> {
> #define CLUMP_EXP_NUMBITS 64
> DECLARE_BITMAP(bits, CLUMP_EXP_NUMBITS);
> @@ -774,7 +691,7 @@ static void __init test_for_each_set_clump8(void)
> expect_eq_clump8(start, CLUMP_EXP_NUMBITS, clump_exp, &clump);
> }
>
> -static void __init test_for_each_set_bit_wrap(void)
> +static void test_for_each_set_bit_wrap(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -799,7 +716,7 @@ static void __init test_for_each_set_bit_wrap(void)
> }
> }
>
> -static void __init test_for_each_set_bit(void)
> +static void test_for_each_set_bit(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -821,7 +738,7 @@ static void __init test_for_each_set_bit(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void __init test_for_each_set_bit_from(void)
> +static void test_for_each_set_bit_from(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -851,7 +768,7 @@ static void __init test_for_each_set_bit_from(void)
> }
> }
>
> -static void __init test_for_each_clear_bit(void)
> +static void test_for_each_clear_bit(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -873,7 +790,7 @@ static void __init test_for_each_clear_bit(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void __init test_for_each_clear_bit_from(void)
> +static void test_for_each_clear_bit_from(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -903,7 +820,7 @@ static void __init test_for_each_clear_bit_from(void)
> }
> }
>
> -static void __init test_for_each_set_bitrange(void)
> +static void test_for_each_set_bitrange(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -925,7 +842,7 @@ static void __init test_for_each_set_bitrange(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void __init test_for_each_clear_bitrange(void)
> +static void test_for_each_clear_bitrange(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -947,7 +864,7 @@ static void __init test_for_each_clear_bitrange(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void __init test_for_each_set_bitrange_from(void)
> +static void test_for_each_set_bitrange_from(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -977,7 +894,7 @@ static void __init test_for_each_set_bitrange_from(void)
> }
> }
>
> -static void __init test_for_each_clear_bitrange_from(void)
> +static void test_for_each_clear_bitrange_from(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -1048,7 +965,7 @@ static struct test_bitmap_cut test_cut[] = {
> },
> };
>
> -static void __init test_bitmap_cut(void)
> +static void test_bitmap_cut(void)
> {
> unsigned long b[5], *in = &b[1], *out = &b[0]; /* Partial overlap */
> int i;
> @@ -1071,14 +988,14 @@ struct test_bitmap_print {
> const char *list;
> };
>
> -static const unsigned long small_bitmap[] __initconst = {
> +static const unsigned long small_bitmap[] = {
> BITMAP_FROM_U64(0x3333333311111111ULL),
> };
>
> -static const char small_mask[] __initconst = "33333333,11111111\n";
> -static const char small_list[] __initconst = "0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61\n";
> +static const char small_mask[] = "33333333,11111111\n";
> +static const char small_list[] = "0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61\n";
>
> -static const unsigned long large_bitmap[] __initconst = {
> +static const unsigned long large_bitmap[] = {
> BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
> BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
> BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
> @@ -1101,7 +1018,7 @@ static const unsigned long large_bitmap[] __initconst = {
> BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
> };
>
> -static const char large_mask[] __initconst = "33333333,11111111,33333333,11111111,"
> +static const char large_mask[] = "33333333,11111111,33333333,11111111,"
> "33333333,11111111,33333333,11111111,"
> "33333333,11111111,33333333,11111111,"
> "33333333,11111111,33333333,11111111,"
> @@ -1122,7 +1039,7 @@ static const char large_mask[] __initconst = "33333333,11111111,33333333,1111111
> "33333333,11111111,33333333,11111111,"
> "33333333,11111111,33333333,11111111\n";
>
> -static const char large_list[] __initconst = /* more than 4KB */
> +static const char large_list[] = /* more than 4KB */
> "0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61,64,68,72,76,80,84,88,92,96-97,100-101,104-1"
> "05,108-109,112-113,116-117,120-121,124-125,128,132,136,140,144,148,152,156,160-161,164-165,168-169,172-173,176-1"
> "77,180-181,184-185,188-189,192,196,200,204,208,212,216,220,224-225,228-229,232-233,236-237,240-241,244-245,248-2"
> @@ -1164,12 +1081,12 @@ static const char large_list[] __initconst = /* more than 4KB */
> "2489,2492-2493,2496,2500,2504,2508,2512,2516,2520,2524,2528-2529,2532-2533,2536-2537,2540-2541,2544-2545,2548-25"
> "49,2552-2553,2556-2557\n";
>
> -static const struct test_bitmap_print test_print[] __initconst = {
> +static const struct test_bitmap_print test_print[] = {
> { small_bitmap, sizeof(small_bitmap) * BITS_PER_BYTE, small_mask, small_list },
> { large_bitmap, sizeof(large_bitmap) * BITS_PER_BYTE, large_mask, large_list },
> };
>
> -static void __init test_bitmap_print_buf(void)
> +static void test_bitmap_print_buf(void)
> {
> int i;
>
> @@ -1201,7 +1118,7 @@ static void __init test_bitmap_print_buf(void)
> * FIXME: Clang breaks compile-time evaluations when KASAN and GCOV are enabled.
> * To workaround it, GCOV is force-disabled in Makefile for this configuration.
> */
> -static void __init test_bitmap_const_eval(void)
> +static void test_bitmap_const_eval(void)
> {
> DECLARE_BITMAP(bitmap, BITS_PER_LONG);
> unsigned long initvar = BIT(2);
> @@ -1269,7 +1186,7 @@ static void __init test_bitmap_const_eval(void)
> /*
> * Helper function to test bitmap_write() overwriting the chosen byte pattern.
> */
> -static void __init test_bitmap_write_helper(const char *pattern)
> +static void test_bitmap_write_helper(const char *pattern)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> DECLARE_BITMAP(exp_bitmap, TEST_BIT_LEN);
> @@ -1323,7 +1240,7 @@ static void __init test_bitmap_write_helper(const char *pattern)
> }
> }
>
> -static void __init test_bitmap_read_write(void)
> +static void test_bitmap_read_write(void)
> {
> unsigned char *pattern[3] = {"", "all:1/2", "all"};
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> @@ -1372,7 +1289,7 @@ static void __init test_bitmap_read_write(void)
> test_bitmap_write_helper(pattern[pi]);
> }
>
> -static void __init test_bitmap_read_perf(void)
> +static void test_bitmap_read_perf(void)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> unsigned int cnt, nbits, i;
> @@ -1395,10 +1312,10 @@ static void __init test_bitmap_read_perf(void)
> }
> }
> time = ktime_get() - time;
> - pr_info("Time spent in %s:\t%llu\n", __func__, time);
> + kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
> }
>
> -static void __init test_bitmap_write_perf(void)
> +static void test_bitmap_write_perf(void)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> unsigned int cnt, nbits, i;
> @@ -1417,13 +1334,15 @@ static void __init test_bitmap_write_perf(void)
> }
> }
> time = ktime_get() - time;
> - pr_info("Time spent in %s:\t%llu\n", __func__, time);
> + kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
> }
>
> #undef TEST_BIT_LEN
>
> -static void __init selftest(void)
> +static void bitmap_test(struct kunit *test)
> {
> + kunittest = test;
> +
> test_zero_clear();
> test_fill_set();
> test_copy();
> @@ -1456,7 +1375,18 @@ static void __init selftest(void)
> test_for_each_set_bit_wrap();
> }
>
> -KSTM_MODULE_LOADERS(test_bitmap);
> +static struct kunit_case bitmap_test_cases[] = {
> + KUNIT_CASE(bitmap_test),
> + {}
> +};
> +
> +static struct kunit_suite bitmap_test_suite = {
> + .name = "bitmap",
> + .test_cases = bitmap_test_cases,
> +};
> +
> +kunit_test_suite(bitmap_test_suite);
> +
> MODULE_AUTHOR("david decotigny <david.decotigny@googlers.com>");
> MODULE_DESCRIPTION("Test cases for bitmap API");
> MODULE_LICENSE("GPL");
> diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh
> deleted file mode 100755
> index 00a416fbc0ef..000000000000
> --- a/tools/testing/selftests/lib/bitmap.sh
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -$(dirname $0)/../kselftest/module.sh "bitmap" test_bitmap
If getting rid of this is going to seriously inconvenience anyone,
there's no fundamental reason why we can't provide a replacement
wrapper script with the same name, which modprobes the new module. The
output should be largely compatible, and the only real issue is that
CONFIG_KUNIT is required.
> diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config
> index dc15aba8d0a3..8817520d1f42 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
>
> --
> 2.48.1
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] bitmap: break kunit into test cases
2025-02-07 20:14 ` [PATCH 3/3] bitmap: break kunit into test cases Tamir Duberstein
@ 2025-02-08 9:07 ` David Gow
2025-02-08 12:33 ` Tamir Duberstein
2025-02-10 5:23 ` Muhammad Usama Anjum
1 sibling, 1 reply; 27+ messages in thread
From: David Gow @ 2025-02-08 9:07 UTC (permalink / raw)
To: Tamir Duberstein
Cc: John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Yury Norov, Rasmus Villemoes,
Shuah Khan, Kees Cook, Muhammad Usama Anjum, linux-kernel,
linux-m68k, linuxppc-dev, linux-kselftest
[-- Attachment #1: Type: text/plain, Size: 16105 bytes --]
On Sat, 8 Feb 2025 at 04:14, Tamir Duberstein <tamird@gmail.com> wrote:
>
> Move some tests into `bitmap_test_cases` and parameterize
> `test_bitmap_print_buf`. This gives us nicer output in the event of a
> failure.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
I very much like this cleanup: the static global 'kunittest' was annoying me.
I think there's an argument to get rid of tc_err() entirely, and just
use KUNIT_FAIL directly, but I'm happy either way: it is a lot of
churn.
Reviewed-by: David Gow <davidgow@google.com>
Thanks,
-- David
> lib/bitmap_kunit.c | 182 ++++++++++++++++++++++++++---------------------------
> 1 file changed, 89 insertions(+), 93 deletions(-)
>
> diff --git a/lib/bitmap_kunit.c b/lib/bitmap_kunit.c
> index 0605228288d6..f7b90f6d5f49 100644
> --- a/lib/bitmap_kunit.c
> +++ b/lib/bitmap_kunit.c
> @@ -17,8 +17,6 @@
> static char pbl_buffer[PAGE_SIZE];
> static char print_buf[PAGE_SIZE * 2];
>
> -static struct kunit *kunittest;
> -
> #define tc_err(fmt, ...) \
> KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
>
> @@ -96,7 +94,7 @@ static const unsigned long exp3_1_0[] = {
>
> #define expect_eq_uint(x, y) expect_eq_ulong((unsigned int)(x), (unsigned int)(y))
>
> -static void test_zero_clear(void)
> +static void test_zero_clear(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bmap, 1024);
>
> @@ -125,7 +123,7 @@ static void test_zero_clear(void)
> expect_eq_pbl("", bmap, 1024);
> }
>
> -static void test_find_nth_bit(void)
> +static void test_find_nth_bit(struct kunit *kunittest)
> {
> unsigned long b, bit, cnt = 0;
> DECLARE_BITMAP(bmap, 64 * 3);
> @@ -166,7 +164,7 @@ static void test_find_nth_bit(void)
> }
> }
>
> -static void test_fill_set(void)
> +static void test_fill_set(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bmap, 1024);
>
> @@ -195,7 +193,7 @@ static void test_fill_set(void)
> expect_eq_pbl("0-1023", bmap, 1024);
> }
>
> -static void test_copy(void)
> +static void test_copy(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bmap1, 1024);
> DECLARE_BITMAP(bmap2, 1024);
> @@ -234,7 +232,7 @@ static void test_copy(void)
> expect_eq_pbl("0-108,128-1023", bmap2, 1024);
> }
>
> -static void test_bitmap_region(void)
> +static void test_bitmap_region(struct kunit *kunittest)
> {
> int pos, order;
>
> @@ -259,7 +257,7 @@ static void test_bitmap_region(void)
>
> #define EXP2_IN_BITS (sizeof(exp2) * 8)
>
> -static void test_replace(void)
> +static void test_replace(struct kunit *kunittest)
> {
> unsigned int nbits = 64;
> unsigned int nlongs = DIV_ROUND_UP(nbits, BITS_PER_LONG);
> @@ -300,7 +298,7 @@ static const unsigned long sg_scatter_exp[] = {
> BITMAP_FROM_U64(0x000000000000021aULL),
> };
>
> -static void test_bitmap_sg(void)
> +static void test_bitmap_sg(struct kunit *kunittest)
> {
> unsigned int nbits = 64;
> DECLARE_BITMAP(bmap_gather, 100);
> @@ -421,7 +419,7 @@ static const struct test_bitmap_parselist parselist_tests[] = {
>
> };
>
> -static void test_bitmap_parselist(void)
> +static void test_bitmap_parselist(struct kunit *kunittest)
> {
> int i;
> int err;
> @@ -457,7 +455,7 @@ static void test_bitmap_parselist(void)
> }
> }
>
> -static void test_bitmap_printlist(void)
> +static void test_bitmap_printlist(struct kunit *kunittest)
> {
> unsigned long *bmap = kmalloc(PAGE_SIZE, GFP_KERNEL);
> char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> @@ -533,7 +531,7 @@ static const struct test_bitmap_parselist parse_tests[] = {
> #undef step
> };
>
> -static void test_bitmap_parse(void)
> +static void test_bitmap_parse(struct kunit *kunittest)
> {
> int i;
> int err;
> @@ -568,7 +566,7 @@ static void test_bitmap_parse(void)
> }
> }
>
> -static void test_bitmap_arr32(void)
> +static void test_bitmap_arr32(struct kunit *kunittest)
> {
> unsigned int nbits, next_bit;
> u32 arr[EXP1_IN_BITS / 32];
> @@ -594,7 +592,7 @@ static void test_bitmap_arr32(void)
> }
> }
>
> -static void test_bitmap_arr64(void)
> +static void test_bitmap_arr64(struct kunit *kunittest)
> {
> unsigned int nbits, next_bit;
> u64 arr[EXP1_IN_BITS / 64];
> @@ -626,7 +624,7 @@ static void test_bitmap_arr64(void)
> }
> }
>
> -static noinline void test_mem_optimisations(void)
> +static noinline void test_mem_optimisations(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bmap1, 1024);
> DECLARE_BITMAP(bmap2, 1024);
> @@ -669,7 +667,7 @@ static const unsigned char clump_exp[] = {
> 0x05, /* non-adjacent 2 bits set */
> };
>
> -static void test_for_each_set_clump8(void)
> +static void test_for_each_set_clump8(struct kunit *kunittest)
> {
> #define CLUMP_EXP_NUMBITS 64
> DECLARE_BITMAP(bits, CLUMP_EXP_NUMBITS);
> @@ -691,7 +689,7 @@ static void test_for_each_set_clump8(void)
> expect_eq_clump8(start, CLUMP_EXP_NUMBITS, clump_exp, &clump);
> }
>
> -static void test_for_each_set_bit_wrap(void)
> +static void test_for_each_set_bit_wrap(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -716,7 +714,7 @@ static void test_for_each_set_bit_wrap(void)
> }
> }
>
> -static void test_for_each_set_bit(void)
> +static void test_for_each_set_bit(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -738,7 +736,7 @@ static void test_for_each_set_bit(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void test_for_each_set_bit_from(void)
> +static void test_for_each_set_bit_from(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -768,7 +766,7 @@ static void test_for_each_set_bit_from(void)
> }
> }
>
> -static void test_for_each_clear_bit(void)
> +static void test_for_each_clear_bit(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -790,7 +788,7 @@ static void test_for_each_clear_bit(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void test_for_each_clear_bit_from(void)
> +static void test_for_each_clear_bit_from(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -820,7 +818,7 @@ static void test_for_each_clear_bit_from(void)
> }
> }
>
> -static void test_for_each_set_bitrange(void)
> +static void test_for_each_set_bitrange(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -842,7 +840,7 @@ static void test_for_each_set_bitrange(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void test_for_each_clear_bitrange(void)
> +static void test_for_each_clear_bitrange(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -864,7 +862,7 @@ static void test_for_each_clear_bitrange(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void test_for_each_set_bitrange_from(void)
> +static void test_for_each_set_bitrange_from(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -894,7 +892,7 @@ static void test_for_each_set_bitrange_from(void)
> }
> }
>
> -static void test_for_each_clear_bitrange_from(void)
> +static void test_for_each_clear_bitrange_from(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -965,7 +963,7 @@ static struct test_bitmap_cut test_cut[] = {
> },
> };
>
> -static void test_bitmap_cut(void)
> +static void test_bitmap_cut(struct kunit *kunittest)
> {
> unsigned long b[5], *in = &b[1], *out = &b[0]; /* Partial overlap */
> int i;
> @@ -986,8 +984,14 @@ struct test_bitmap_print {
> unsigned long nbits;
> const char *mask;
> const char *list;
> + const char *name;
> };
>
> +static void param_to_desc(const struct test_bitmap_print *param, char *desc)
> +{
> + strscpy(desc, param->name, KUNIT_PARAM_DESC_SIZE);
> +}
> +
> static const unsigned long small_bitmap[] = {
> BITMAP_FROM_U64(0x3333333311111111ULL),
> };
> @@ -1082,35 +1086,34 @@ static const char large_list[] = /* more than 4KB */
> "49,2552-2553,2556-2557\n";
>
> static const struct test_bitmap_print test_print[] = {
> - { small_bitmap, sizeof(small_bitmap) * BITS_PER_BYTE, small_mask, small_list },
> - { large_bitmap, sizeof(large_bitmap) * BITS_PER_BYTE, large_mask, large_list },
> + { small_bitmap, sizeof(small_bitmap) * BITS_PER_BYTE, small_mask, small_list, "small" },
> + { large_bitmap, sizeof(large_bitmap) * BITS_PER_BYTE, large_mask, large_list, "large" },
> };
>
> -static void test_bitmap_print_buf(void)
> +KUNIT_ARRAY_PARAM(test_print, test_print, param_to_desc);
> +
> +static void test_bitmap_print_buf(struct kunit *kunittest)
> {
> - int i;
> + int n;
>
> - for (i = 0; i < ARRAY_SIZE(test_print); i++) {
> - const struct test_bitmap_print *t = &test_print[i];
> - int n;
> + const struct test_bitmap_print *t = kunittest->param_value;
>
> - n = bitmap_print_bitmask_to_buf(print_buf, t->bitmap, t->nbits,
> - 0, 2 * PAGE_SIZE);
> - expect_eq_uint(strlen(t->mask) + 1, n);
> - expect_eq_str(t->mask, print_buf, n);
> + n = bitmap_print_bitmask_to_buf(print_buf, t->bitmap, t->nbits,
> + 0, 2 * PAGE_SIZE);
> + expect_eq_uint(strlen(t->mask) + 1, n);
> + expect_eq_str(t->mask, print_buf, n);
>
> + n = bitmap_print_list_to_buf(print_buf, t->bitmap, t->nbits,
> + 0, 2 * PAGE_SIZE);
> + expect_eq_uint(strlen(t->list) + 1, n);
> + expect_eq_str(t->list, print_buf, n);
> +
> + /* test by non-zero offset */
> + if (strlen(t->list) > PAGE_SIZE) {
> n = bitmap_print_list_to_buf(print_buf, t->bitmap, t->nbits,
> - 0, 2 * PAGE_SIZE);
> - expect_eq_uint(strlen(t->list) + 1, n);
> - expect_eq_str(t->list, print_buf, n);
> -
> - /* test by non-zero offset */
> - if (strlen(t->list) > PAGE_SIZE) {
> - n = bitmap_print_list_to_buf(print_buf, t->bitmap, t->nbits,
> - PAGE_SIZE, PAGE_SIZE);
> - expect_eq_uint(strlen(t->list) + 1 - PAGE_SIZE, n);
> - expect_eq_str(t->list + PAGE_SIZE, print_buf, n);
> - }
> + PAGE_SIZE, PAGE_SIZE);
> + expect_eq_uint(strlen(t->list) + 1 - PAGE_SIZE, n);
> + expect_eq_str(t->list + PAGE_SIZE, print_buf, n);
> }
> }
>
> @@ -1118,7 +1121,7 @@ static void test_bitmap_print_buf(void)
> * FIXME: Clang breaks compile-time evaluations when KASAN and GCOV are enabled.
> * To workaround it, GCOV is force-disabled in Makefile for this configuration.
> */
> -static void test_bitmap_const_eval(void)
> +static void test_bitmap_const_eval(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bitmap, BITS_PER_LONG);
> unsigned long initvar = BIT(2);
> @@ -1186,7 +1189,7 @@ static void test_bitmap_const_eval(void)
> /*
> * Helper function to test bitmap_write() overwriting the chosen byte pattern.
> */
> -static void test_bitmap_write_helper(const char *pattern)
> +static void test_bitmap_write_helper(struct kunit *kunittest, const char *pattern)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> DECLARE_BITMAP(exp_bitmap, TEST_BIT_LEN);
> @@ -1240,7 +1243,7 @@ static void test_bitmap_write_helper(const char *pattern)
> }
> }
>
> -static void test_bitmap_read_write(void)
> +static void test_bitmap_read_write(struct kunit *kunittest)
> {
> unsigned char *pattern[3] = {"", "all:1/2", "all"};
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> @@ -1286,10 +1289,10 @@ static void test_bitmap_read_write(void)
> }
>
> for (pi = 0; pi < ARRAY_SIZE(pattern); pi++)
> - test_bitmap_write_helper(pattern[pi]);
> + test_bitmap_write_helper(kunittest, pattern[pi]);
> }
>
> -static void test_bitmap_read_perf(void)
> +static void test_bitmap_read_perf(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> unsigned int cnt, nbits, i;
> @@ -1312,10 +1315,10 @@ static void test_bitmap_read_perf(void)
> }
> }
> time = ktime_get() - time;
> - kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
> + kunit_info(kunittest, "Time spent:\t%llu\n", time);
> }
>
> -static void test_bitmap_write_perf(void)
> +static void test_bitmap_write_perf(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> unsigned int cnt, nbits, i;
> @@ -1334,49 +1337,42 @@ static void test_bitmap_write_perf(void)
> }
> }
> time = ktime_get() - time;
> - kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
> + kunit_info(kunittest, "Time spent:\t%llu\n", time);
> }
>
> #undef TEST_BIT_LEN
>
> -static void bitmap_test(struct kunit *test)
> -{
> - kunittest = test;
> -
> - test_zero_clear();
> - test_fill_set();
> - test_copy();
> - test_bitmap_region();
> - test_replace();
> - test_bitmap_sg();
> - test_bitmap_arr32();
> - test_bitmap_arr64();
> - test_bitmap_parse();
> - test_bitmap_parselist();
> - test_bitmap_printlist();
> - test_mem_optimisations();
> - test_bitmap_cut();
> - test_bitmap_print_buf();
> - test_bitmap_const_eval();
> - test_bitmap_read_write();
> - test_bitmap_read_perf();
> - test_bitmap_write_perf();
> -
> - test_find_nth_bit();
> - test_for_each_set_bit();
> - test_for_each_set_bit_from();
> - test_for_each_clear_bit();
> - test_for_each_clear_bit_from();
> - test_for_each_set_bitrange();
> - test_for_each_clear_bitrange();
> - test_for_each_set_bitrange_from();
> - test_for_each_clear_bitrange_from();
> - test_for_each_set_clump8();
> - test_for_each_set_bit_wrap();
> -}
> -
> static struct kunit_case bitmap_test_cases[] = {
> - KUNIT_CASE(bitmap_test),
> + KUNIT_CASE(test_zero_clear),
> + KUNIT_CASE(test_fill_set),
> + KUNIT_CASE(test_copy),
> + KUNIT_CASE(test_bitmap_region),
> + KUNIT_CASE(test_replace),
> + KUNIT_CASE(test_bitmap_sg),
> + KUNIT_CASE(test_bitmap_arr32),
> + KUNIT_CASE(test_bitmap_arr64),
> + KUNIT_CASE(test_bitmap_parse),
> + KUNIT_CASE(test_bitmap_parselist),
> + KUNIT_CASE(test_bitmap_printlist),
> + KUNIT_CASE(test_mem_optimisations),
> + KUNIT_CASE(test_bitmap_cut),
> + KUNIT_CASE_PARAM(test_bitmap_print_buf, test_print_gen_params),
> + KUNIT_CASE(test_bitmap_const_eval),
> + KUNIT_CASE(test_bitmap_read_write),
> + KUNIT_CASE(test_bitmap_read_perf),
> + KUNIT_CASE(test_bitmap_write_perf),
> +
> + KUNIT_CASE(test_find_nth_bit),
> + KUNIT_CASE(test_for_each_set_bit),
> + KUNIT_CASE(test_for_each_set_bit_from),
> + KUNIT_CASE(test_for_each_clear_bit),
> + KUNIT_CASE(test_for_each_clear_bit_from),
> + KUNIT_CASE(test_for_each_set_bitrange),
> + KUNIT_CASE(test_for_each_clear_bitrange),
> + KUNIT_CASE(test_for_each_set_bitrange_from),
> + KUNIT_CASE(test_for_each_clear_bitrange_from),
> + KUNIT_CASE(test_for_each_set_clump8),
> + KUNIT_CASE(test_for_each_set_bit_wrap),
> {}
> };
>
>
> --
> 2.48.1
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] bitmap: break kunit into test cases
2025-02-08 9:07 ` David Gow
@ 2025-02-08 12:33 ` Tamir Duberstein
0 siblings, 0 replies; 27+ messages in thread
From: Tamir Duberstein @ 2025-02-08 12:33 UTC (permalink / raw)
To: David Gow
Cc: John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Yury Norov, Rasmus Villemoes,
Shuah Khan, Kees Cook, Muhammad Usama Anjum, linux-kernel,
linux-m68k, linuxppc-dev, linux-kselftest
On Sat, Feb 8, 2025 at 4:07 AM David Gow <davidgow@google.com> wrote:
>
> On Sat, 8 Feb 2025 at 04:14, Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > Move some tests into `bitmap_test_cases` and parameterize
> > `test_bitmap_print_buf`. This gives us nicer output in the event of a
> > failure.
> >
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
>
> I very much like this cleanup: the static global 'kunittest' was annoying me.
>
> I think there's an argument to get rid of tc_err() entirely, and just
> use KUNIT_FAIL directly, but I'm happy either way: it is a lot of
> churn.
I'm happy to do that. Pulling on that thread ended up producing a nice
cleanup. There's no way to add a patch to the series without sending
it all again, is there?
> Reviewed-by: David Gow <davidgow@google.com>
Thanks for the review!
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-07 20:14 [PATCH 0/3] bitmap: convert self-test to KUnit Tamir Duberstein
` (3 preceding siblings ...)
2025-02-08 9:07 ` [PATCH 0/3] bitmap: convert self-test to KUnit David Gow
@ 2025-02-08 17:53 ` Yury Norov
2025-02-08 18:42 ` Tamir Duberstein
2025-02-10 7:54 ` Geert Uytterhoeven
4 siblings, 2 replies; 27+ messages in thread
From: Yury Norov @ 2025-02-08 17:53 UTC (permalink / raw)
To: Tamir Duberstein
Cc: David Gow, John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rasmus Villemoes, Shuah Khan,
Kees Cook, Muhammad Usama Anjum, linux-kernel, linux-m68k,
linuxppc-dev, linux-kselftest
On Fri, Feb 07, 2025 at 03:14:01PM -0500, Tamir Duberstein wrote:
> This is one of just 3 remaining "Test Module" kselftests (the others
> being printf and scanf), the rest having been converted to KUnit.
>
> I tested this using:
>
> $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 bitmap.
>
> I've already sent out a conversion series for each of printf[0] and scanf[1].
>
> There was a previous attempt[2] to do this in July 2024. Please bear
> with me as I try to understand and address the objections from that
> time. I've spoken with Muhammad Usama Anjum, the author of that series,
> and received their approval to "take over" this work. Here we go...
Take over means that you'd at least add the Co-developed-by tag.
>
> On 7/26/24 11:45 PM, John Hubbard wrote:
> >
> > This changes the situation from "works for Linus' tab completion
> > case", to "causes a tab completion problem"! :)
> >
> > I think a tests/ subdir is how we eventually decided to do this [1],
> > right?
> >
> > So:
> >
> > lib/tests/bitmap_kunit.c
> >
> > [1] https://lore.kernel.org/20240724201354.make.730-kees@kernel.org
>
> This is true and unfortunate, but not trivial to fix because new
> kallsyms tests were placed in lib/tests in commit 84b4a51fce4c
> ("selftests: add new kallsyms selftests") *after* the KUnit filename
> best practices were adopted.
>
> I propose that the KUnit maintainers blaze this trail using
> `string_kunit.c` which currently still lives in lib/ despite the KUnit
> docs giving it as an example at lib/tests/.
>
> On 7/27/24 12:24 AM, Shuah Khan wrote:
> >
> > This change will take away the ability to run bitmap tests during
> > boot on a non-kunit kernel.
> >
> > Nack on this change. I wan to see all tests that are being removed
> > from lib because they have been converted - also it doesn't make
> > sense to convert some tests like this one that add the ability test
> > during boot.
>
> This point was also discussed in another thread[3] in which:
>
> On 7/27/24 12:35 AM, Shuah Khan wrote:
> >
> > Please make sure you aren't taking away the ability to run these tests during
> > boot.
> >
> > It doesn't make sense to convert every single test especially when it
> > is intended to be run during boot without dependencies - not as a kunit test
> > but a regression test during boot.
> >
> > bitmap is one example - pay attention to the config help test - bitmap
> > one clearly states it runs regression testing during boot. Any test that
> > says that isn't a candidate for conversion.
> >
> > I am going to nack any such conversions.
>
> The crux of the argument seems to be that the config help text is taken
> to describe the author's intent with the fragment "at boot". I think
KUNIT is disabled in defconfig, at least on x86_64. It is also disabled
on my Ubuntu 24.04 machine. If I take your patches, I'll be unable to
boot-test bitmaps. Even worse, I'll be unable to build the standalone
test from sources as a module and load it later.
Or I misunderstand it, and there's a way to build some particular KUNIT
test without enabling KUNIT in config and/or re-compiling the whole kernel?
Please teach me, if so
Unless you give me a way to build and run the test in true
production environment, I'm not going with KUNITs. Sorry.
> this may be a case of confirmation bias: I see at least the following
> KUnit tests with "at boot" in their help text:
> - CPUMASK_KUNIT_TEST
This one doesn't count because the test was not converted, it's
originally written as a KUNIT test. I am happy when people bring new
tests in the most comfortable way for them, and I don't want to push
them to use this framework or another. So I didn't object, and I'm
thankful for this contribution to Sander.
> - BITFIELD_KUNIT
Same here. Plus, it was written long before I took over bitfields.
> - CHECKSUM_KUNIT
> - UTIL_MACROS_KUNIT
> It seems to me that the inference being made is that any test that runs
> "at boot" is intended to be run by both developers and users, but I find
> no evidence that bitmap in particular would ever provide additional
> value when run by users.
This is my evidence: sometimes people report performance or whatever
issues on their systems, suspecting bitmaps guilty. I ask them to run
the bitmap or find_bit test to narrow the problem. Sometimes I need to
test a hardware I have no access to, and I have to (kindly!) ask people
to build a small test and run it. I don't want to ask them to rebuild
the whole kernel, or even to build something else.
https://lore.kernel.org/all/YuWk3titnOiQACzC@yury-laptop/
> There's further discussion about KUnit not being "ideal for cases where
> people would want to check a subsystem on a running kernel", but I find
> no evidence that bitmap in particular is actually testing the running
> kernel; it is a unit test of the bitmap functions, which is also stated
> in the config help text.
>
> David Gow made many of the same points in his final reply[4], which was
> never replied to.
Nice summary for the discussion. Unfortunately you missed my concerns.
Which are:
Pros:
- Now we switch to KUNITs because KUNITs are so good
Cons:
- Wipes git history;
- Bloats the test's source code;
- Adds dependencies;
- Doesn't run on most popular distros and defconfig;
So, no.
Thanks,
Yury
> Link: https://lore.kernel.org/all/20250207-printf-kunit-convert-v2-0-057b23860823@gmail.com/T/#u [0]
> Link: https://lore.kernel.org/all/20250207-scanf-kunit-convert-v4-0-a23e2afaede8@gmail.com/T/#u [1]
> Link: https://lore.kernel.org/all/20240726110658.2281070-1-usama.anjum@collabora.com/T/#u [2]
> Link: https://lore.kernel.org/all/327831fb-47ab-4555-8f0b-19a8dbcaacd7@collabora.com/T/#u [3]
> Link: https://lore.kernel.org/all/CABVgOSmMoPD3JfzVd4VTkzGL2fZCo8LfwzaVSzeFimPrhgLa5w@mail.gmail.com/ [4]
>
> Thanks for your attention.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> Tamir Duberstein (3):
> bitmap: remove _check_eq_u32_array
> bitmap: convert self-test to KUnit
> bitmap: break kunit into test cases
>
> MAINTAINERS | 2 +-
> arch/m68k/configs/amiga_defconfig | 1 -
> arch/m68k/configs/apollo_defconfig | 1 -
> arch/m68k/configs/atari_defconfig | 1 -
> arch/m68k/configs/bvme6000_defconfig | 1 -
> arch/m68k/configs/hp300_defconfig | 1 -
> arch/m68k/configs/mac_defconfig | 1 -
> arch/m68k/configs/multi_defconfig | 1 -
> arch/m68k/configs/mvme147_defconfig | 1 -
> arch/m68k/configs/mvme16x_defconfig | 1 -
> arch/m68k/configs/q40_defconfig | 1 -
> arch/m68k/configs/sun3_defconfig | 1 -
> arch/m68k/configs/sun3x_defconfig | 1 -
> arch/powerpc/configs/ppc64_defconfig | 1 -
> lib/Kconfig.debug | 24 +-
> lib/Makefile | 2 +-
> lib/{test_bitmap.c => bitmap_kunit.c} | 454 +++++++++++++---------------------
> tools/testing/selftests/lib/bitmap.sh | 3 -
> tools/testing/selftests/lib/config | 1 -
> 19 files changed, 195 insertions(+), 304 deletions(-)
> ---
> base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
> change-id: 20250207-bitmap-kunit-convert-92d3147b2eee
>
> Best regards,
> --
> Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] bitmap: remove _check_eq_u32_array
2025-02-07 20:14 ` [PATCH 1/3] bitmap: remove _check_eq_u32_array Tamir Duberstein
2025-02-08 9:07 ` David Gow
@ 2025-02-08 18:00 ` Yury Norov
2025-02-10 5:22 ` Muhammad Usama Anjum
2 siblings, 0 replies; 27+ messages in thread
From: Yury Norov @ 2025-02-08 18:00 UTC (permalink / raw)
To: Tamir Duberstein
Cc: David Gow, John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rasmus Villemoes, Shuah Khan,
Kees Cook, Muhammad Usama Anjum, linux-kernel, linux-m68k,
linuxppc-dev, linux-kselftest
On Fri, Feb 07, 2025 at 03:14:02PM -0500, Tamir Duberstein wrote:
> This has been unused since commit 3aa56885e516 ("bitmap: replace
> bitmap_{from,to}_u32array") in 2018. Remove it to avoid the need to port
> it to KUnit in this series.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
OK, 7 years is enough to drop it. Adding in bitmap-for-next for
testing.
Thanks,
Yury
> ---
> lib/test_bitmap.c | 28 ----------------------------
> 1 file changed, 28 deletions(-)
>
> diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
> index 65a75d58ed9e..c83829ef557f 100644
> --- a/lib/test_bitmap.c
> +++ b/lib/test_bitmap.c
> @@ -100,34 +100,6 @@ __check_eq_pbl(const char *srcfile, unsigned int line,
> return true;
> }
>
> -static bool __init
> -__check_eq_u32_array(const char *srcfile, unsigned int line,
> - const u32 *exp_arr, unsigned int exp_len,
> - const u32 *arr, unsigned int len) __used;
> -static bool __init
> -__check_eq_u32_array(const char *srcfile, unsigned int line,
> - const u32 *exp_arr, unsigned int exp_len,
> - const u32 *arr, unsigned int len)
> -{
> - if (exp_len != len) {
> - pr_warn("[%s:%u] array length differ: expected %u, got %u\n",
> - srcfile, line,
> - exp_len, len);
> - return false;
> - }
> -
> - if (memcmp(exp_arr, arr, len*sizeof(*arr))) {
> - pr_warn("[%s:%u] array contents differ\n", srcfile, line);
> - print_hex_dump(KERN_WARNING, " exp: ", DUMP_PREFIX_OFFSET,
> - 32, 4, exp_arr, exp_len*sizeof(*exp_arr), false);
> - print_hex_dump(KERN_WARNING, " got: ", DUMP_PREFIX_OFFSET,
> - 32, 4, arr, len*sizeof(*arr), false);
> - return false;
> - }
> -
> - return true;
> -}
> -
> static bool __init __check_eq_clump8(const char *srcfile, unsigned int line,
> const unsigned int offset,
> const unsigned int size,
>
> --
> 2.48.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-08 17:53 ` Yury Norov
@ 2025-02-08 18:42 ` Tamir Duberstein
2025-02-10 7:54 ` Geert Uytterhoeven
1 sibling, 0 replies; 27+ messages in thread
From: Tamir Duberstein @ 2025-02-08 18:42 UTC (permalink / raw)
To: Yury Norov
Cc: David Gow, John Hubbard, Andrew Morton, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rasmus Villemoes, Shuah Khan,
Kees Cook, Muhammad Usama Anjum, linux-kernel, linux-m68k,
linuxppc-dev, linux-kselftest
On Sat, Feb 8, 2025 at 12:53 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> [...]
>
> Take over means that you'd at least add the Co-developed-by tag.
I didn't use their code - the thing being "taken over" is the work of
having these debates with the maintainers.
> [...]
>
> KUNIT is disabled in defconfig, at least on x86_64. It is also disabled
> on my Ubuntu 24.04 machine. If I take your patches, I'll be unable to
> boot-test bitmaps. Even worse, I'll be unable to build the standalone
> test from sources as a module and load it later.
>
> Or I misunderstand it, and there's a way to build some particular KUNIT
> test without enabling KUNIT in config and/or re-compiling the whole kernel?
> Please teach me, if so
>
> Unless you give me a way to build and run the test in true
> production environment, I'm not going with KUNITs. Sorry.
This is a question for David -- I don't know if this is possible.
> [...]
>
> This is my evidence: sometimes people report performance or whatever
> issues on their systems, suspecting bitmaps guilty. I ask them to run
> the bitmap or find_bit test to narrow the problem. Sometimes I need to
> test a hardware I have no access to, and I have to (kindly!) ask people
> to build a small test and run it. I don't want to ask them to rebuild
> the whole kernel, or even to build something else.
>
> https://lore.kernel.org/all/YuWk3titnOiQACzC@yury-laptop/
This is compelling evidence, and it was not previously raised. Thank you.
I notice that two things are true about the performance test part of
test_bitmap.c:
- It's a minority of the code in the file (48 lines out of 1462).
- There are no assertions in it.
Do you also find value in running the testing portion on other
people's machines, to which you don't have access?
> [...]
>
> Nice summary for the discussion. Unfortunately you missed my concerns.
> Which are:
>
> Pros:
> - Now we switch to KUNITs because KUNITs are so good
>
> Cons:
> - Wipes git history;
I was very careful to minimize churn, and the result is 249 lines on
which I'd now own the blame (228 with `-w`). Still, it's a valid con.
> - Bloats the test's source code;
The test is 74 lines shorter after this series.
> - Adds dependencies;
> - Doesn't run on most popular distros and defconfig;
Yep, I understand your concerns much better now - and I'm grateful for
your having taken the time to explain and show receipts. Still, I
wonder if we can get the best of both worlds - either by finding what
you need in KUnit, or by moving the testing bit to KUnit and keeping
the performance bit where it is.
Thanks.
Tamir
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] bitmap: remove _check_eq_u32_array
2025-02-07 20:14 ` [PATCH 1/3] bitmap: remove _check_eq_u32_array Tamir Duberstein
2025-02-08 9:07 ` David Gow
2025-02-08 18:00 ` Yury Norov
@ 2025-02-10 5:22 ` Muhammad Usama Anjum
2 siblings, 0 replies; 27+ messages in thread
From: Muhammad Usama Anjum @ 2025-02-10 5:22 UTC (permalink / raw)
To: Tamir Duberstein, David Gow, John Hubbard, Andrew Morton,
Geert Uytterhoeven, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao, Yury Norov,
Rasmus Villemoes, Shuah Khan, Kees Cook
Cc: Usama.Anjum, linux-kernel, linux-m68k, linuxppc-dev,
linux-kselftest
On 2/8/25 1:14 AM, Tamir Duberstein wrote:
> This has been unused since commit 3aa56885e516 ("bitmap: replace
> bitmap_{from,to}_u32array") in 2018. Remove it to avoid the need to port
> it to KUnit in this series.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> lib/test_bitmap.c | 28 ----------------------------
> 1 file changed, 28 deletions(-)
>
> diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
> index 65a75d58ed9e..c83829ef557f 100644
> --- a/lib/test_bitmap.c
> +++ b/lib/test_bitmap.c
> @@ -100,34 +100,6 @@ __check_eq_pbl(const char *srcfile, unsigned int line,
> return true;
> }
>
> -static bool __init
> -__check_eq_u32_array(const char *srcfile, unsigned int line,
> - const u32 *exp_arr, unsigned int exp_len,
> - const u32 *arr, unsigned int len) __used;
> -static bool __init
> -__check_eq_u32_array(const char *srcfile, unsigned int line,
> - const u32 *exp_arr, unsigned int exp_len,
> - const u32 *arr, unsigned int len)
> -{
> - if (exp_len != len) {
> - pr_warn("[%s:%u] array length differ: expected %u, got %u\n",
> - srcfile, line,
> - exp_len, len);
> - return false;
> - }
> -
> - if (memcmp(exp_arr, arr, len*sizeof(*arr))) {
> - pr_warn("[%s:%u] array contents differ\n", srcfile, line);
> - print_hex_dump(KERN_WARNING, " exp: ", DUMP_PREFIX_OFFSET,
> - 32, 4, exp_arr, exp_len*sizeof(*exp_arr), false);
> - print_hex_dump(KERN_WARNING, " got: ", DUMP_PREFIX_OFFSET,
> - 32, 4, arr, len*sizeof(*arr), false);
> - return false;
> - }
> -
> - return true;
> -}
> -
> static bool __init __check_eq_clump8(const char *srcfile, unsigned int line,
> const unsigned int offset,
> const unsigned int size,
>
--
BR,
Muhammad Usama Anjum
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] bitmap: convert self-test to KUnit
2025-02-07 20:14 ` [PATCH 2/3] bitmap: convert self-test to KUnit Tamir Duberstein
2025-02-08 9:07 ` David Gow
@ 2025-02-10 5:22 ` Muhammad Usama Anjum
2025-02-10 7:42 ` Geert Uytterhoeven
2 siblings, 0 replies; 27+ messages in thread
From: Muhammad Usama Anjum @ 2025-02-10 5:22 UTC (permalink / raw)
To: Tamir Duberstein, David Gow, John Hubbard, Andrew Morton,
Geert Uytterhoeven, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao, Yury Norov,
Rasmus Villemoes, Shuah Khan, Kees Cook
Cc: Usama.Anjum, linux-kernel, linux-m68k, linuxppc-dev,
linux-kselftest
On 2/8/25 1:14 AM, Tamir Duberstein wrote:
> Convert the bitmap() self-test to a KUnit test.
>
> In the interest of keeping the patch reasonably-sized this doesn't
> refactor the tests into proper parameterized tests - it's all one big
> test case.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Thanks for the conversion.
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> MAINTAINERS | 2 +-
> arch/m68k/configs/amiga_defconfig | 1 -
> arch/m68k/configs/apollo_defconfig | 1 -
> arch/m68k/configs/atari_defconfig | 1 -
> arch/m68k/configs/bvme6000_defconfig | 1 -
> arch/m68k/configs/hp300_defconfig | 1 -
> arch/m68k/configs/mac_defconfig | 1 -
> arch/m68k/configs/multi_defconfig | 1 -
> arch/m68k/configs/mvme147_defconfig | 1 -
> arch/m68k/configs/mvme16x_defconfig | 1 -
> arch/m68k/configs/q40_defconfig | 1 -
> arch/m68k/configs/sun3_defconfig | 1 -
> arch/m68k/configs/sun3x_defconfig | 1 -
> arch/powerpc/configs/ppc64_defconfig | 1 -
> lib/Kconfig.debug | 24 ++-
> lib/Makefile | 2 +-
> lib/{test_bitmap.c => bitmap_kunit.c} | 322 +++++++++++++---------------------
> tools/testing/selftests/lib/bitmap.sh | 3 -
> tools/testing/selftests/lib/config | 1 -
> 19 files changed, 145 insertions(+), 222 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 896a307fa065..9824d4053748 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4016,11 +4016,11 @@ F: include/linux/nodemask_types.h
> F: include/vdso/bits.h
> F: lib/bitmap-str.c
> F: lib/bitmap.c
> +F: lib/bitmap_kunit.c
> F: lib/cpumask.c
> F: lib/cpumask_kunit.c
> F: lib/find_bit.c
> F: lib/find_bit_benchmark.c
> -F: lib/test_bitmap.c
> F: tools/include/linux/bitfield.h
> F: tools/include/linux/bitmap.h
> F: tools/include/linux/bits.h
> diff --git a/arch/m68k/configs/amiga_defconfig b/arch/m68k/configs/amiga_defconfig
> index dbf2ea561c85..3c9d7b58cb8a 100644
> --- a/arch/m68k/configs/amiga_defconfig
> +++ b/arch/m68k/configs/amiga_defconfig
> @@ -624,7 +624,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/apollo_defconfig b/arch/m68k/configs/apollo_defconfig
> index b0fd199cc0a4..b94c87a7cbdb 100644
> --- a/arch/m68k/configs/apollo_defconfig
> +++ b/arch/m68k/configs/apollo_defconfig
> @@ -581,7 +581,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/atari_defconfig b/arch/m68k/configs/atari_defconfig
> index bb5b2d3b6c10..823ba96c4486 100644
> --- a/arch/m68k/configs/atari_defconfig
> +++ b/arch/m68k/configs/atari_defconfig
> @@ -601,7 +601,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/bvme6000_defconfig b/arch/m68k/configs/bvme6000_defconfig
> index 8315a13bab73..0fa985129200 100644
> --- a/arch/m68k/configs/bvme6000_defconfig
> +++ b/arch/m68k/configs/bvme6000_defconfig
> @@ -573,7 +573,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/hp300_defconfig b/arch/m68k/configs/hp300_defconfig
> index 350370657e5f..75ed6eb547c6 100644
> --- a/arch/m68k/configs/hp300_defconfig
> +++ b/arch/m68k/configs/hp300_defconfig
> @@ -583,7 +583,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/mac_defconfig b/arch/m68k/configs/mac_defconfig
> index f942b4755702..149c398f80a8 100644
> --- a/arch/m68k/configs/mac_defconfig
> +++ b/arch/m68k/configs/mac_defconfig
> @@ -600,7 +600,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/multi_defconfig b/arch/m68k/configs/multi_defconfig
> index b1eaad02efab..34e6eaa47a18 100644
> --- a/arch/m68k/configs/multi_defconfig
> +++ b/arch/m68k/configs/multi_defconfig
> @@ -687,7 +687,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/mvme147_defconfig b/arch/m68k/configs/mvme147_defconfig
> index 6309a4442bb3..9e9f8883b38d 100644
> --- a/arch/m68k/configs/mvme147_defconfig
> +++ b/arch/m68k/configs/mvme147_defconfig
> @@ -573,7 +573,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/mvme16x_defconfig b/arch/m68k/configs/mvme16x_defconfig
> index 3feb0731f814..5ed08e78c4fd 100644
> --- a/arch/m68k/configs/mvme16x_defconfig
> +++ b/arch/m68k/configs/mvme16x_defconfig
> @@ -574,7 +574,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/q40_defconfig b/arch/m68k/configs/q40_defconfig
> index ea04b1b0da7d..4e0eea94a6a0 100644
> --- a/arch/m68k/configs/q40_defconfig
> +++ b/arch/m68k/configs/q40_defconfig
> @@ -590,7 +590,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/sun3_defconfig b/arch/m68k/configs/sun3_defconfig
> index f52d9af92153..690e2156b5f7 100644
> --- a/arch/m68k/configs/sun3_defconfig
> +++ b/arch/m68k/configs/sun3_defconfig
> @@ -570,7 +570,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/m68k/configs/sun3x_defconfig b/arch/m68k/configs/sun3x_defconfig
> index f348447824da..71486fd428da 100644
> --- a/arch/m68k/configs/sun3x_defconfig
> +++ b/arch/m68k/configs/sun3x_defconfig
> @@ -571,7 +571,6 @@ CONFIG_TEST_HEXDUMP=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
> index 465eb96c755e..cfd235d90c95 100644
> --- a/arch/powerpc/configs/ppc64_defconfig
> +++ b/arch/powerpc/configs/ppc64_defconfig
> @@ -446,7 +446,6 @@ CONFIG_TEST_STRING_HELPERS=m
> CONFIG_TEST_KSTRTOX=m
> CONFIG_TEST_PRINTF=m
> CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
> CONFIG_TEST_UUID=m
> CONFIG_TEST_XARRAY=m
> CONFIG_TEST_MAPLE_TREE=m
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 1af972a92d06..fd3dcb0677b5 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2427,6 +2427,23 @@ config ASYNC_RAID6_TEST
> config TEST_HEXDUMP
> tristate "Test functions located in the hexdump module at runtime"
>
> +config BITMAP_KUNIT_TEST
> + tristate "KUnit test bitmap_*() family of functions at runtime" if !KUNIT_ALL_TESTS
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + Enable this option to test the bitmap functions at boot.
> +
> + KUnit tests run during boot and output the results to the debug log
> + in TAP format (http://testanything.org/). Only useful for kernel devs
> + running the KUnit test harness, and not intended for inclusion into a
> + production build.
> +
> + For more information on KUnit and unit tests in general please refer
> + to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> + If unsure, say N.
> +
> config STRING_KUNIT_TEST
> tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS
> depends on KUNIT
> @@ -2446,13 +2463,6 @@ config TEST_PRINTF
> config TEST_SCANF
> tristate "Test scanf() family of functions at runtime"
>
> -config TEST_BITMAP
> - tristate "Test bitmap_*() family of functions at runtime"
> - help
> - Enable this option to test the bitmap functions at boot.
> -
> - If unsure, say N.
> -
> config TEST_UUID
> tristate "Test functions located in the uuid module at runtime"
>
> diff --git a/lib/Makefile b/lib/Makefile
> index d5cfc7afbbb8..d735e1b70606 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -87,7 +87,7 @@ obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
> obj-$(CONFIG_TEST_PRINTF) += test_printf.o
> obj-$(CONFIG_TEST_SCANF) += test_scanf.o
>
> -obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
> +obj-$(CONFIG_BITMAP_KUNIT_TEST) += bitmap_kunit.o
> ifeq ($(CONFIG_CC_IS_CLANG)$(CONFIG_KASAN),yy)
> # FIXME: Clang breaks test_bitmap_const_eval when KASAN and GCOV are enabled
> GCOV_PROFILE_test_bitmap.o := n
> diff --git a/lib/test_bitmap.c b/lib/bitmap_kunit.c
> similarity index 83%
> rename from lib/test_bitmap.c
> rename to lib/bitmap_kunit.c
> index c83829ef557f..0605228288d6 100644
> --- a/lib/test_bitmap.c
> +++ b/lib/bitmap_kunit.c
> @@ -3,10 +3,8 @@
> * Test cases for bitmap API.
> */
>
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> +#include <kunit/test.h>
> #include <linux/bitmap.h>
> -#include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/printk.h>
> @@ -14,16 +12,17 @@
> #include <linux/string.h>
> #include <linux/uaccess.h>
>
> -#include "../tools/testing/selftests/kselftest_module.h"
> -
> #define EXP1_IN_BITS (sizeof(exp1) * 8)
>
> -KSTM_MODULE_GLOBALS();
> +static char pbl_buffer[PAGE_SIZE];
> +static char print_buf[PAGE_SIZE * 2];
> +
> +static struct kunit *kunittest;
>
> -static char pbl_buffer[PAGE_SIZE] __initdata;
> -static char print_buf[PAGE_SIZE * 2] __initdata;
> +#define tc_err(fmt, ...) \
> + KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
>
> -static const unsigned long exp1[] __initconst = {
> +static const unsigned long exp1[] = {
> BITMAP_FROM_U64(1),
> BITMAP_FROM_U64(2),
> BITMAP_FROM_U64(0x0000ffff),
> @@ -41,130 +40,63 @@ static const unsigned long exp1[] __initconst = {
> BITMAP_FROM_U64(0x80000000),
> };
>
> -static const unsigned long exp2[] __initconst = {
> +static const unsigned long exp2[] = {
> BITMAP_FROM_U64(0x3333333311111111ULL),
> BITMAP_FROM_U64(0xffffffff77777777ULL),
> };
>
> /* Fibonacci sequence */
> -static const unsigned long exp2_to_exp3_mask[] __initconst = {
> +static const unsigned long exp2_to_exp3_mask[] = {
> BITMAP_FROM_U64(0x008000020020212eULL),
> };
> /* exp3_0_1 = (exp2[0] & ~exp2_to_exp3_mask) | (exp2[1] & exp2_to_exp3_mask) */
> -static const unsigned long exp3_0_1[] __initconst = {
> +static const unsigned long exp3_0_1[] = {
> BITMAP_FROM_U64(0x33b3333311313137ULL),
> };
> /* exp3_1_0 = (exp2[1] & ~exp2_to_exp3_mask) | (exp2[0] & exp2_to_exp3_mask) */
> -static const unsigned long exp3_1_0[] __initconst = {
> +static const unsigned long exp3_1_0[] = {
> BITMAP_FROM_U64(0xff7fffff77575751ULL),
> };
>
> -static bool __init
> -__check_eq_ulong(const char *srcfile, unsigned int line,
> - const unsigned long exp_ulong, unsigned long x)
> -{
> - if (exp_ulong != x) {
> - pr_err("[%s:%u] expected %lu, got %lu\n",
> - srcfile, line, exp_ulong, x);
> - return false;
> - }
> - return true;
> -}
> -
> -static bool __init
> -__check_eq_bitmap(const char *srcfile, unsigned int line,
> - const unsigned long *exp_bmap, const unsigned long *bmap,
> - unsigned int nbits)
> -{
> - if (!bitmap_equal(exp_bmap, bmap, nbits)) {
> - pr_warn("[%s:%u] bitmaps contents differ: expected \"%*pbl\", got \"%*pbl\"\n",
> - srcfile, line,
> - nbits, exp_bmap, nbits, bmap);
> - return false;
> - }
> - return true;
> -}
> -
> -static bool __init
> -__check_eq_pbl(const char *srcfile, unsigned int line,
> - const char *expected_pbl,
> - const unsigned long *bitmap, unsigned int nbits)
> -{
> - snprintf(pbl_buffer, sizeof(pbl_buffer), "%*pbl", nbits, bitmap);
> - if (strcmp(expected_pbl, pbl_buffer)) {
> - pr_warn("[%s:%u] expected \"%s\", got \"%s\"\n",
> - srcfile, line,
> - expected_pbl, pbl_buffer);
> - return false;
> - }
> - return true;
> -}
> -
> -static bool __init __check_eq_clump8(const char *srcfile, unsigned int line,
> - const unsigned int offset,
> - const unsigned int size,
> - const unsigned char *const clump_exp,
> - const unsigned long *const clump)
> -{
> - unsigned long exp;
> -
> - if (offset >= size) {
> - pr_warn("[%s:%u] bit offset for clump out-of-bounds: expected less than %u, got %u\n",
> - srcfile, line, size, offset);
> - return false;
> - }
> -
> - exp = clump_exp[offset / 8];
> - if (!exp) {
> - pr_warn("[%s:%u] bit offset for zero clump: expected nonzero clump, got bit offset %u with clump value 0",
> - srcfile, line, offset);
> - return false;
> - }
> -
> - if (*clump != exp) {
> - pr_warn("[%s:%u] expected clump value of 0x%lX, got clump value of 0x%lX",
> - srcfile, line, exp, *clump);
> - return false;
> - }
> -
> - return true;
> -}
> -
> -static bool __init
> -__check_eq_str(const char *srcfile, unsigned int line,
> - const char *exp_str, const char *str,
> - unsigned int len)
> -{
> - bool eq;
> -
> - eq = strncmp(exp_str, str, len) == 0;
> - if (!eq)
> - pr_err("[%s:%u] expected %s, got %s\n", srcfile, line, exp_str, str);
> -
> - return eq;
> -}
> -
> -#define __expect_eq(suffix, ...) \
> - ({ \
> - int result = 0; \
> - total_tests++; \
> - if (!__check_eq_ ## suffix(__FILE__, __LINE__, \
> - ##__VA_ARGS__)) { \
> - failed_tests++; \
> - result = 1; \
> +#define expect_eq_ulong(exp_ulong, x) KUNIT_EXPECT_EQ(kunittest, exp_ulong, x)
> +
> +#define expect_eq_bitmap(exp_bmap, bmap, nbits) \
> + KUNIT_EXPECT_TRUE_MSG(kunittest, bitmap_equal(exp_bmap, bmap, nbits), \
> + "bitmaps contents differ: expected \"%*pbl\", got \"%*pbl\"", \
> + nbits, exp_bmap, nbits, bmap)
> +
> +#define expect_eq_pbl(expected_pbl, bitmap, nbits) do { \
> + { \
> + snprintf(pbl_buffer, sizeof(pbl_buffer), "%*pbl", nbits, bitmap); \
> + KUNIT_EXPECT_STREQ(kunittest, expected_pbl, pbl_buffer); \
> + } \
> + } while (0)
> +
> +#define expect_eq_clump8(offset, size, clump_exp, clump) do { \
> + { \
> + unsigned long exp; \
> + \
> + KUNIT_EXPECT_LT_MSG(kunittest, offset, size, \
> + "bit offset for clump out-of-bounds"); \
> + \
> + exp = clump_exp[offset / 8]; \
> + KUNIT_EXPECT_NE_MSG(kunittest, exp, 0, \
> + "bit offset %u for zero clump", offset); \
> + \
> + KUNIT_EXPECT_EQ(kunittest, *clump, exp); \
> + } \
> + } while (0)
> +
> +#define expect_eq_str(exp_str, str, len) \
> + { \
> + if (strncmp(exp_str, str, len) != 0) { \
> + tc_err("expected %s, got %s", exp_str, str); \
> } \
> - result; \
> - })
> + }
>
> -#define expect_eq_ulong(...) __expect_eq(ulong, ##__VA_ARGS__)
> #define expect_eq_uint(x, y) expect_eq_ulong((unsigned int)(x), (unsigned int)(y))
> -#define expect_eq_bitmap(...) __expect_eq(bitmap, ##__VA_ARGS__)
> -#define expect_eq_pbl(...) __expect_eq(pbl, ##__VA_ARGS__)
> -#define expect_eq_u32_array(...) __expect_eq(u32_array, ##__VA_ARGS__)
> -#define expect_eq_clump8(...) __expect_eq(clump8, ##__VA_ARGS__)
> -#define expect_eq_str(...) __expect_eq(str, ##__VA_ARGS__)
>
> -static void __init test_zero_clear(void)
> +static void test_zero_clear(void)
> {
> DECLARE_BITMAP(bmap, 1024);
>
> @@ -193,7 +125,7 @@ static void __init test_zero_clear(void)
> expect_eq_pbl("", bmap, 1024);
> }
>
> -static void __init test_find_nth_bit(void)
> +static void test_find_nth_bit(void)
> {
> unsigned long b, bit, cnt = 0;
> DECLARE_BITMAP(bmap, 64 * 3);
> @@ -234,7 +166,7 @@ static void __init test_find_nth_bit(void)
> }
> }
>
> -static void __init test_fill_set(void)
> +static void test_fill_set(void)
> {
> DECLARE_BITMAP(bmap, 1024);
>
> @@ -263,7 +195,7 @@ static void __init test_fill_set(void)
> expect_eq_pbl("0-1023", bmap, 1024);
> }
>
> -static void __init test_copy(void)
> +static void test_copy(void)
> {
> DECLARE_BITMAP(bmap1, 1024);
> DECLARE_BITMAP(bmap2, 1024);
> @@ -302,7 +234,7 @@ static void __init test_copy(void)
> expect_eq_pbl("0-108,128-1023", bmap2, 1024);
> }
>
> -static void __init test_bitmap_region(void)
> +static void test_bitmap_region(void)
> {
> int pos, order;
>
> @@ -327,7 +259,7 @@ static void __init test_bitmap_region(void)
>
> #define EXP2_IN_BITS (sizeof(exp2) * 8)
>
> -static void __init test_replace(void)
> +static void test_replace(void)
> {
> unsigned int nbits = 64;
> unsigned int nlongs = DIV_ROUND_UP(nbits, BITS_PER_LONG);
> @@ -352,23 +284,23 @@ static void __init test_replace(void)
> expect_eq_bitmap(bmap, exp3_1_0, nbits);
> }
>
> -static const unsigned long sg_mask[] __initconst = {
> +static const unsigned long sg_mask[] = {
> BITMAP_FROM_U64(0x000000000000035aULL),
> };
>
> -static const unsigned long sg_src[] __initconst = {
> +static const unsigned long sg_src[] = {
> BITMAP_FROM_U64(0x0000000000000667ULL),
> };
>
> -static const unsigned long sg_gather_exp[] __initconst = {
> +static const unsigned long sg_gather_exp[] = {
> BITMAP_FROM_U64(0x0000000000000029ULL),
> };
>
> -static const unsigned long sg_scatter_exp[] __initconst = {
> +static const unsigned long sg_scatter_exp[] = {
> BITMAP_FROM_U64(0x000000000000021aULL),
> };
>
> -static void __init test_bitmap_sg(void)
> +static void test_bitmap_sg(void)
> {
> unsigned int nbits = 64;
> DECLARE_BITMAP(bmap_gather, 100);
> @@ -404,7 +336,7 @@ struct test_bitmap_parselist{
> const int flags;
> };
>
> -static const struct test_bitmap_parselist parselist_tests[] __initconst = {
> +static const struct test_bitmap_parselist parselist_tests[] = {
> #define step (sizeof(u64) / sizeof(unsigned long))
>
> {0, "0", &exp1[0], 8, 0},
> @@ -489,7 +421,7 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
>
> };
>
> -static void __init test_bitmap_parselist(void)
> +static void test_bitmap_parselist(void)
> {
> int i;
> int err;
> @@ -504,30 +436,28 @@ static void __init test_bitmap_parselist(void)
> time = ktime_get() - time;
>
> if (err != ptest.errno) {
> - pr_err("parselist: %d: input is %s, errno is %d, expected %d\n",
> + tc_err("parselist: %d: input is %s, errno is %d, expected %d",
> i, ptest.in, err, ptest.errno);
> - failed_tests++;
> continue;
> }
>
> if (!err && ptest.expected
> && !__bitmap_equal(bmap, ptest.expected, ptest.nbits)) {
> - pr_err("parselist: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
> + tc_err("parselist: %d: input is %s, result is 0x%lx, expected 0x%lx",
> i, ptest.in, bmap[0],
> *ptest.expected);
> - failed_tests++;
> continue;
> }
>
> if (ptest.flags & PARSE_TIME)
> - pr_info("parselist: %d: input is '%s' OK, Time: %llu\n",
> + kunit_info(kunittest, "parselist: %d: input is '%s' OK, Time: %llu",
> i, ptest.in, time);
>
> #undef ptest
> }
> }
>
> -static void __init test_bitmap_printlist(void)
> +static void test_bitmap_printlist(void)
> {
> unsigned long *bmap = kmalloc(PAGE_SIZE, GFP_KERNEL);
> char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> @@ -548,37 +478,35 @@ static void __init test_bitmap_printlist(void)
> time = ktime_get() - time;
>
> if (ret != slen + 1) {
> - pr_err("bitmap_print_to_pagebuf: result is %d, expected %d\n", ret, slen);
> - failed_tests++;
> + tc_err("bitmap_print_to_pagebuf: result is %d, expected %d", ret, slen);
> goto out;
> }
>
> if (strncmp(buf, expected, slen)) {
> - pr_err("bitmap_print_to_pagebuf: result is %s, expected %s\n", buf, expected);
> - failed_tests++;
> + tc_err("bitmap_print_to_pagebuf: result is %s, expected %s", buf, expected);
> goto out;
> }
>
> - pr_info("bitmap_print_to_pagebuf: input is '%s', Time: %llu\n", buf, time);
> + kunit_info(kunittest, "bitmap_print_to_pagebuf: input is '%s', Time: %llu", buf, time);
> out:
> kfree(buf);
> kfree(bmap);
> }
>
> -static const unsigned long parse_test[] __initconst = {
> +static const unsigned long parse_test[] = {
> BITMAP_FROM_U64(0),
> BITMAP_FROM_U64(1),
> BITMAP_FROM_U64(0xdeadbeef),
> BITMAP_FROM_U64(0x100000000ULL),
> };
>
> -static const unsigned long parse_test2[] __initconst = {
> +static const unsigned long parse_test2[] = {
> BITMAP_FROM_U64(0x100000000ULL), BITMAP_FROM_U64(0xdeadbeef),
> BITMAP_FROM_U64(0x100000000ULL), BITMAP_FROM_U64(0xbaadf00ddeadbeef),
> BITMAP_FROM_U64(0x100000000ULL), BITMAP_FROM_U64(0x0badf00ddeadbeef),
> };
>
> -static const struct test_bitmap_parselist parse_tests[] __initconst = {
> +static const struct test_bitmap_parselist parse_tests[] = {
> {0, "", &parse_test[0 * step], 32, 0},
> {0, " ", &parse_test[0 * step], 32, 0},
> {0, "0", &parse_test[0 * step], 32, 0},
> @@ -605,7 +533,7 @@ static const struct test_bitmap_parselist parse_tests[] __initconst = {
> #undef step
> };
>
> -static void __init test_bitmap_parse(void)
> +static void test_bitmap_parse(void)
> {
> int i;
> int err;
> @@ -621,28 +549,26 @@ static void __init test_bitmap_parse(void)
> time = ktime_get() - time;
>
> if (err != test.errno) {
> - pr_err("parse: %d: input is %s, errno is %d, expected %d\n",
> + tc_err("parse: %d: input is %s, errno is %d, expected %d",
> i, test.in, err, test.errno);
> - failed_tests++;
> continue;
> }
>
> if (!err && test.expected
> && !__bitmap_equal(bmap, test.expected, test.nbits)) {
> - pr_err("parse: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
> + tc_err("parse: %d: input is %s, result is 0x%lx, expected 0x%lx",
> i, test.in, bmap[0],
> *test.expected);
> - failed_tests++;
> continue;
> }
>
> if (test.flags & PARSE_TIME)
> - pr_info("parse: %d: input is '%s' OK, Time: %llu\n",
> + kunit_info(kunittest, "parse: %d: input is '%s' OK, Time: %llu",
> i, test.in, time);
> }
> }
>
> -static void __init test_bitmap_arr32(void)
> +static void test_bitmap_arr32(void)
> {
> unsigned int nbits, next_bit;
> u32 arr[EXP1_IN_BITS / 32];
> @@ -658,10 +584,8 @@ static void __init test_bitmap_arr32(void)
> next_bit = find_next_bit(bmap2,
> round_up(nbits, BITS_PER_LONG), nbits);
> if (next_bit < round_up(nbits, BITS_PER_LONG)) {
> - pr_err("bitmap_copy_arr32(nbits == %d:"
> - " tail is not safely cleared: %d\n",
> + tc_err("bitmap_copy_arr32(nbits == %d: tail is not safely cleared: %d",
> nbits, next_bit);
> - failed_tests++;
> }
>
> if (nbits < EXP1_IN_BITS - 32)
> @@ -670,7 +594,7 @@ static void __init test_bitmap_arr32(void)
> }
> }
>
> -static void __init test_bitmap_arr64(void)
> +static void test_bitmap_arr64(void)
> {
> unsigned int nbits, next_bit;
> u64 arr[EXP1_IN_BITS / 64];
> @@ -686,17 +610,15 @@ static void __init test_bitmap_arr64(void)
>
> next_bit = find_next_bit(bmap2, round_up(nbits, BITS_PER_LONG), nbits);
> if (next_bit < round_up(nbits, BITS_PER_LONG)) {
> - pr_err("bitmap_copy_arr64(nbits == %d:"
> - " tail is not safely cleared: %d\n", nbits, next_bit);
> - failed_tests++;
> + tc_err("bitmap_copy_arr64(nbits == %d: tail is not safely cleared: %d",
> + nbits, next_bit);
> }
>
> if ((nbits % 64) &&
> (arr[(nbits - 1) / 64] & ~GENMASK_ULL((nbits - 1) % 64, 0))) {
> - pr_err("bitmap_to_arr64(nbits == %d): tail is not safely cleared: 0x%016llx (must be 0x%016llx)\n",
> + tc_err("bitmap_to_arr64(nbits == %d): tail is not safely cleared: 0x%016llx (must be 0x%016llx)",
> nbits, arr[(nbits - 1) / 64],
> GENMASK_ULL((nbits - 1) % 64, 0));
> - failed_tests++;
> }
>
> if (nbits < EXP1_IN_BITS - 64)
> @@ -704,7 +626,7 @@ static void __init test_bitmap_arr64(void)
> }
> }
>
> -static void noinline __init test_mem_optimisations(void)
> +static noinline void test_mem_optimisations(void)
> {
> DECLARE_BITMAP(bmap1, 1024);
> DECLARE_BITMAP(bmap2, 1024);
> @@ -718,30 +640,25 @@ static void noinline __init test_mem_optimisations(void)
> bitmap_set(bmap1, start, nbits);
> __bitmap_set(bmap2, start, nbits);
> if (!bitmap_equal(bmap1, bmap2, 1024)) {
> - printk("set not equal %d %d\n", start, nbits);
> - failed_tests++;
> + tc_err("set not equal %d %d", start, nbits);
> }
> if (!__bitmap_equal(bmap1, bmap2, 1024)) {
> - printk("set not __equal %d %d\n", start, nbits);
> - failed_tests++;
> + tc_err("set not __equal %d %d", start, nbits);
> }
>
> bitmap_clear(bmap1, start, nbits);
> __bitmap_clear(bmap2, start, nbits);
> if (!bitmap_equal(bmap1, bmap2, 1024)) {
> - printk("clear not equal %d %d\n", start, nbits);
> - failed_tests++;
> + tc_err("clear not equal %d %d", start, nbits);
> }
> if (!__bitmap_equal(bmap1, bmap2, 1024)) {
> - printk("clear not __equal %d %d\n", start,
> - nbits);
> - failed_tests++;
> + tc_err("clear not __equal %d %d", start, nbits);
> }
> }
> }
> }
>
> -static const unsigned char clump_exp[] __initconst = {
> +static const unsigned char clump_exp[] = {
> 0x01, /* 1 bit set */
> 0x02, /* non-edge 1 bit set */
> 0x00, /* zero bits set */
> @@ -752,7 +669,7 @@ static const unsigned char clump_exp[] __initconst = {
> 0x05, /* non-adjacent 2 bits set */
> };
>
> -static void __init test_for_each_set_clump8(void)
> +static void test_for_each_set_clump8(void)
> {
> #define CLUMP_EXP_NUMBITS 64
> DECLARE_BITMAP(bits, CLUMP_EXP_NUMBITS);
> @@ -774,7 +691,7 @@ static void __init test_for_each_set_clump8(void)
> expect_eq_clump8(start, CLUMP_EXP_NUMBITS, clump_exp, &clump);
> }
>
> -static void __init test_for_each_set_bit_wrap(void)
> +static void test_for_each_set_bit_wrap(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -799,7 +716,7 @@ static void __init test_for_each_set_bit_wrap(void)
> }
> }
>
> -static void __init test_for_each_set_bit(void)
> +static void test_for_each_set_bit(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -821,7 +738,7 @@ static void __init test_for_each_set_bit(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void __init test_for_each_set_bit_from(void)
> +static void test_for_each_set_bit_from(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -851,7 +768,7 @@ static void __init test_for_each_set_bit_from(void)
> }
> }
>
> -static void __init test_for_each_clear_bit(void)
> +static void test_for_each_clear_bit(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -873,7 +790,7 @@ static void __init test_for_each_clear_bit(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void __init test_for_each_clear_bit_from(void)
> +static void test_for_each_clear_bit_from(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -903,7 +820,7 @@ static void __init test_for_each_clear_bit_from(void)
> }
> }
>
> -static void __init test_for_each_set_bitrange(void)
> +static void test_for_each_set_bitrange(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -925,7 +842,7 @@ static void __init test_for_each_set_bitrange(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void __init test_for_each_clear_bitrange(void)
> +static void test_for_each_clear_bitrange(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -947,7 +864,7 @@ static void __init test_for_each_clear_bitrange(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void __init test_for_each_set_bitrange_from(void)
> +static void test_for_each_set_bitrange_from(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -977,7 +894,7 @@ static void __init test_for_each_set_bitrange_from(void)
> }
> }
>
> -static void __init test_for_each_clear_bitrange_from(void)
> +static void test_for_each_clear_bitrange_from(void)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -1048,7 +965,7 @@ static struct test_bitmap_cut test_cut[] = {
> },
> };
>
> -static void __init test_bitmap_cut(void)
> +static void test_bitmap_cut(void)
> {
> unsigned long b[5], *in = &b[1], *out = &b[0]; /* Partial overlap */
> int i;
> @@ -1071,14 +988,14 @@ struct test_bitmap_print {
> const char *list;
> };
>
> -static const unsigned long small_bitmap[] __initconst = {
> +static const unsigned long small_bitmap[] = {
> BITMAP_FROM_U64(0x3333333311111111ULL),
> };
>
> -static const char small_mask[] __initconst = "33333333,11111111\n";
> -static const char small_list[] __initconst = "0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61\n";
> +static const char small_mask[] = "33333333,11111111\n";
> +static const char small_list[] = "0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61\n";
>
> -static const unsigned long large_bitmap[] __initconst = {
> +static const unsigned long large_bitmap[] = {
> BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
> BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
> BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
> @@ -1101,7 +1018,7 @@ static const unsigned long large_bitmap[] __initconst = {
> BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
> };
>
> -static const char large_mask[] __initconst = "33333333,11111111,33333333,11111111,"
> +static const char large_mask[] = "33333333,11111111,33333333,11111111,"
> "33333333,11111111,33333333,11111111,"
> "33333333,11111111,33333333,11111111,"
> "33333333,11111111,33333333,11111111,"
> @@ -1122,7 +1039,7 @@ static const char large_mask[] __initconst = "33333333,11111111,33333333,1111111
> "33333333,11111111,33333333,11111111,"
> "33333333,11111111,33333333,11111111\n";
>
> -static const char large_list[] __initconst = /* more than 4KB */
> +static const char large_list[] = /* more than 4KB */
> "0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61,64,68,72,76,80,84,88,92,96-97,100-101,104-1"
> "05,108-109,112-113,116-117,120-121,124-125,128,132,136,140,144,148,152,156,160-161,164-165,168-169,172-173,176-1"
> "77,180-181,184-185,188-189,192,196,200,204,208,212,216,220,224-225,228-229,232-233,236-237,240-241,244-245,248-2"
> @@ -1164,12 +1081,12 @@ static const char large_list[] __initconst = /* more than 4KB */
> "2489,2492-2493,2496,2500,2504,2508,2512,2516,2520,2524,2528-2529,2532-2533,2536-2537,2540-2541,2544-2545,2548-25"
> "49,2552-2553,2556-2557\n";
>
> -static const struct test_bitmap_print test_print[] __initconst = {
> +static const struct test_bitmap_print test_print[] = {
> { small_bitmap, sizeof(small_bitmap) * BITS_PER_BYTE, small_mask, small_list },
> { large_bitmap, sizeof(large_bitmap) * BITS_PER_BYTE, large_mask, large_list },
> };
>
> -static void __init test_bitmap_print_buf(void)
> +static void test_bitmap_print_buf(void)
> {
> int i;
>
> @@ -1201,7 +1118,7 @@ static void __init test_bitmap_print_buf(void)
> * FIXME: Clang breaks compile-time evaluations when KASAN and GCOV are enabled.
> * To workaround it, GCOV is force-disabled in Makefile for this configuration.
> */
> -static void __init test_bitmap_const_eval(void)
> +static void test_bitmap_const_eval(void)
> {
> DECLARE_BITMAP(bitmap, BITS_PER_LONG);
> unsigned long initvar = BIT(2);
> @@ -1269,7 +1186,7 @@ static void __init test_bitmap_const_eval(void)
> /*
> * Helper function to test bitmap_write() overwriting the chosen byte pattern.
> */
> -static void __init test_bitmap_write_helper(const char *pattern)
> +static void test_bitmap_write_helper(const char *pattern)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> DECLARE_BITMAP(exp_bitmap, TEST_BIT_LEN);
> @@ -1323,7 +1240,7 @@ static void __init test_bitmap_write_helper(const char *pattern)
> }
> }
>
> -static void __init test_bitmap_read_write(void)
> +static void test_bitmap_read_write(void)
> {
> unsigned char *pattern[3] = {"", "all:1/2", "all"};
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> @@ -1372,7 +1289,7 @@ static void __init test_bitmap_read_write(void)
> test_bitmap_write_helper(pattern[pi]);
> }
>
> -static void __init test_bitmap_read_perf(void)
> +static void test_bitmap_read_perf(void)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> unsigned int cnt, nbits, i;
> @@ -1395,10 +1312,10 @@ static void __init test_bitmap_read_perf(void)
> }
> }
> time = ktime_get() - time;
> - pr_info("Time spent in %s:\t%llu\n", __func__, time);
> + kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
> }
>
> -static void __init test_bitmap_write_perf(void)
> +static void test_bitmap_write_perf(void)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> unsigned int cnt, nbits, i;
> @@ -1417,13 +1334,15 @@ static void __init test_bitmap_write_perf(void)
> }
> }
> time = ktime_get() - time;
> - pr_info("Time spent in %s:\t%llu\n", __func__, time);
> + kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
> }
>
> #undef TEST_BIT_LEN
>
> -static void __init selftest(void)
> +static void bitmap_test(struct kunit *test)
> {
> + kunittest = test;
> +
> test_zero_clear();
> test_fill_set();
> test_copy();
> @@ -1456,7 +1375,18 @@ static void __init selftest(void)
> test_for_each_set_bit_wrap();
> }
>
> -KSTM_MODULE_LOADERS(test_bitmap);
> +static struct kunit_case bitmap_test_cases[] = {
> + KUNIT_CASE(bitmap_test),
> + {}
> +};
> +
> +static struct kunit_suite bitmap_test_suite = {
> + .name = "bitmap",
> + .test_cases = bitmap_test_cases,
> +};
> +
> +kunit_test_suite(bitmap_test_suite);
> +
> MODULE_AUTHOR("david decotigny <david.decotigny@googlers.com>");
> MODULE_DESCRIPTION("Test cases for bitmap API");
> MODULE_LICENSE("GPL");
> diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh
> deleted file mode 100755
> index 00a416fbc0ef..000000000000
> --- a/tools/testing/selftests/lib/bitmap.sh
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -$(dirname $0)/../kselftest/module.sh "bitmap" test_bitmap
> diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config
> index dc15aba8d0a3..8817520d1f42 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
>
--
BR,
Muhammad Usama Anjum
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] bitmap: break kunit into test cases
2025-02-07 20:14 ` [PATCH 3/3] bitmap: break kunit into test cases Tamir Duberstein
2025-02-08 9:07 ` David Gow
@ 2025-02-10 5:23 ` Muhammad Usama Anjum
1 sibling, 0 replies; 27+ messages in thread
From: Muhammad Usama Anjum @ 2025-02-10 5:23 UTC (permalink / raw)
To: Tamir Duberstein, David Gow, John Hubbard, Andrew Morton,
Geert Uytterhoeven, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao, Yury Norov,
Rasmus Villemoes, Shuah Khan, Kees Cook
Cc: Usama.Anjum, linux-kernel, linux-m68k, linuxppc-dev,
linux-kselftest
On 2/8/25 1:14 AM, Tamir Duberstein wrote:
> Move some tests into `bitmap_test_cases` and parameterize
> `test_bitmap_print_buf`. This gives us nicer output in the event of a
> failure.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> lib/bitmap_kunit.c | 182 ++++++++++++++++++++++++++---------------------------
> 1 file changed, 89 insertions(+), 93 deletions(-)
>
> diff --git a/lib/bitmap_kunit.c b/lib/bitmap_kunit.c
> index 0605228288d6..f7b90f6d5f49 100644
> --- a/lib/bitmap_kunit.c
> +++ b/lib/bitmap_kunit.c
> @@ -17,8 +17,6 @@
> static char pbl_buffer[PAGE_SIZE];
> static char print_buf[PAGE_SIZE * 2];
>
> -static struct kunit *kunittest;
> -
> #define tc_err(fmt, ...) \
> KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
>
> @@ -96,7 +94,7 @@ static const unsigned long exp3_1_0[] = {
>
> #define expect_eq_uint(x, y) expect_eq_ulong((unsigned int)(x), (unsigned int)(y))
>
> -static void test_zero_clear(void)
> +static void test_zero_clear(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bmap, 1024);
>
> @@ -125,7 +123,7 @@ static void test_zero_clear(void)
> expect_eq_pbl("", bmap, 1024);
> }
>
> -static void test_find_nth_bit(void)
> +static void test_find_nth_bit(struct kunit *kunittest)
> {
> unsigned long b, bit, cnt = 0;
> DECLARE_BITMAP(bmap, 64 * 3);
> @@ -166,7 +164,7 @@ static void test_find_nth_bit(void)
> }
> }
>
> -static void test_fill_set(void)
> +static void test_fill_set(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bmap, 1024);
>
> @@ -195,7 +193,7 @@ static void test_fill_set(void)
> expect_eq_pbl("0-1023", bmap, 1024);
> }
>
> -static void test_copy(void)
> +static void test_copy(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bmap1, 1024);
> DECLARE_BITMAP(bmap2, 1024);
> @@ -234,7 +232,7 @@ static void test_copy(void)
> expect_eq_pbl("0-108,128-1023", bmap2, 1024);
> }
>
> -static void test_bitmap_region(void)
> +static void test_bitmap_region(struct kunit *kunittest)
> {
> int pos, order;
>
> @@ -259,7 +257,7 @@ static void test_bitmap_region(void)
>
> #define EXP2_IN_BITS (sizeof(exp2) * 8)
>
> -static void test_replace(void)
> +static void test_replace(struct kunit *kunittest)
> {
> unsigned int nbits = 64;
> unsigned int nlongs = DIV_ROUND_UP(nbits, BITS_PER_LONG);
> @@ -300,7 +298,7 @@ static const unsigned long sg_scatter_exp[] = {
> BITMAP_FROM_U64(0x000000000000021aULL),
> };
>
> -static void test_bitmap_sg(void)
> +static void test_bitmap_sg(struct kunit *kunittest)
> {
> unsigned int nbits = 64;
> DECLARE_BITMAP(bmap_gather, 100);
> @@ -421,7 +419,7 @@ static const struct test_bitmap_parselist parselist_tests[] = {
>
> };
>
> -static void test_bitmap_parselist(void)
> +static void test_bitmap_parselist(struct kunit *kunittest)
> {
> int i;
> int err;
> @@ -457,7 +455,7 @@ static void test_bitmap_parselist(void)
> }
> }
>
> -static void test_bitmap_printlist(void)
> +static void test_bitmap_printlist(struct kunit *kunittest)
> {
> unsigned long *bmap = kmalloc(PAGE_SIZE, GFP_KERNEL);
> char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> @@ -533,7 +531,7 @@ static const struct test_bitmap_parselist parse_tests[] = {
> #undef step
> };
>
> -static void test_bitmap_parse(void)
> +static void test_bitmap_parse(struct kunit *kunittest)
> {
> int i;
> int err;
> @@ -568,7 +566,7 @@ static void test_bitmap_parse(void)
> }
> }
>
> -static void test_bitmap_arr32(void)
> +static void test_bitmap_arr32(struct kunit *kunittest)
> {
> unsigned int nbits, next_bit;
> u32 arr[EXP1_IN_BITS / 32];
> @@ -594,7 +592,7 @@ static void test_bitmap_arr32(void)
> }
> }
>
> -static void test_bitmap_arr64(void)
> +static void test_bitmap_arr64(struct kunit *kunittest)
> {
> unsigned int nbits, next_bit;
> u64 arr[EXP1_IN_BITS / 64];
> @@ -626,7 +624,7 @@ static void test_bitmap_arr64(void)
> }
> }
>
> -static noinline void test_mem_optimisations(void)
> +static noinline void test_mem_optimisations(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bmap1, 1024);
> DECLARE_BITMAP(bmap2, 1024);
> @@ -669,7 +667,7 @@ static const unsigned char clump_exp[] = {
> 0x05, /* non-adjacent 2 bits set */
> };
>
> -static void test_for_each_set_clump8(void)
> +static void test_for_each_set_clump8(struct kunit *kunittest)
> {
> #define CLUMP_EXP_NUMBITS 64
> DECLARE_BITMAP(bits, CLUMP_EXP_NUMBITS);
> @@ -691,7 +689,7 @@ static void test_for_each_set_clump8(void)
> expect_eq_clump8(start, CLUMP_EXP_NUMBITS, clump_exp, &clump);
> }
>
> -static void test_for_each_set_bit_wrap(void)
> +static void test_for_each_set_bit_wrap(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -716,7 +714,7 @@ static void test_for_each_set_bit_wrap(void)
> }
> }
>
> -static void test_for_each_set_bit(void)
> +static void test_for_each_set_bit(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -738,7 +736,7 @@ static void test_for_each_set_bit(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void test_for_each_set_bit_from(void)
> +static void test_for_each_set_bit_from(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -768,7 +766,7 @@ static void test_for_each_set_bit_from(void)
> }
> }
>
> -static void test_for_each_clear_bit(void)
> +static void test_for_each_clear_bit(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -790,7 +788,7 @@ static void test_for_each_clear_bit(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void test_for_each_clear_bit_from(void)
> +static void test_for_each_clear_bit_from(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -820,7 +818,7 @@ static void test_for_each_clear_bit_from(void)
> }
> }
>
> -static void test_for_each_set_bitrange(void)
> +static void test_for_each_set_bitrange(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -842,7 +840,7 @@ static void test_for_each_set_bitrange(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void test_for_each_clear_bitrange(void)
> +static void test_for_each_clear_bitrange(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -864,7 +862,7 @@ static void test_for_each_clear_bitrange(void)
> expect_eq_bitmap(orig, copy, 500);
> }
>
> -static void test_for_each_set_bitrange_from(void)
> +static void test_for_each_set_bitrange_from(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -894,7 +892,7 @@ static void test_for_each_set_bitrange_from(void)
> }
> }
>
> -static void test_for_each_clear_bitrange_from(void)
> +static void test_for_each_clear_bitrange_from(struct kunit *kunittest)
> {
> DECLARE_BITMAP(orig, 500);
> DECLARE_BITMAP(copy, 500);
> @@ -965,7 +963,7 @@ static struct test_bitmap_cut test_cut[] = {
> },
> };
>
> -static void test_bitmap_cut(void)
> +static void test_bitmap_cut(struct kunit *kunittest)
> {
> unsigned long b[5], *in = &b[1], *out = &b[0]; /* Partial overlap */
> int i;
> @@ -986,8 +984,14 @@ struct test_bitmap_print {
> unsigned long nbits;
> const char *mask;
> const char *list;
> + const char *name;
> };
>
> +static void param_to_desc(const struct test_bitmap_print *param, char *desc)
> +{
> + strscpy(desc, param->name, KUNIT_PARAM_DESC_SIZE);
> +}
> +
> static const unsigned long small_bitmap[] = {
> BITMAP_FROM_U64(0x3333333311111111ULL),
> };
> @@ -1082,35 +1086,34 @@ static const char large_list[] = /* more than 4KB */
> "49,2552-2553,2556-2557\n";
>
> static const struct test_bitmap_print test_print[] = {
> - { small_bitmap, sizeof(small_bitmap) * BITS_PER_BYTE, small_mask, small_list },
> - { large_bitmap, sizeof(large_bitmap) * BITS_PER_BYTE, large_mask, large_list },
> + { small_bitmap, sizeof(small_bitmap) * BITS_PER_BYTE, small_mask, small_list, "small" },
> + { large_bitmap, sizeof(large_bitmap) * BITS_PER_BYTE, large_mask, large_list, "large" },
> };
>
> -static void test_bitmap_print_buf(void)
> +KUNIT_ARRAY_PARAM(test_print, test_print, param_to_desc);
> +
> +static void test_bitmap_print_buf(struct kunit *kunittest)
> {
> - int i;
> + int n;
>
> - for (i = 0; i < ARRAY_SIZE(test_print); i++) {
> - const struct test_bitmap_print *t = &test_print[i];
> - int n;
> + const struct test_bitmap_print *t = kunittest->param_value;
>
> - n = bitmap_print_bitmask_to_buf(print_buf, t->bitmap, t->nbits,
> - 0, 2 * PAGE_SIZE);
> - expect_eq_uint(strlen(t->mask) + 1, n);
> - expect_eq_str(t->mask, print_buf, n);
> + n = bitmap_print_bitmask_to_buf(print_buf, t->bitmap, t->nbits,
> + 0, 2 * PAGE_SIZE);
> + expect_eq_uint(strlen(t->mask) + 1, n);
> + expect_eq_str(t->mask, print_buf, n);
>
> + n = bitmap_print_list_to_buf(print_buf, t->bitmap, t->nbits,
> + 0, 2 * PAGE_SIZE);
> + expect_eq_uint(strlen(t->list) + 1, n);
> + expect_eq_str(t->list, print_buf, n);
> +
> + /* test by non-zero offset */
> + if (strlen(t->list) > PAGE_SIZE) {
> n = bitmap_print_list_to_buf(print_buf, t->bitmap, t->nbits,
> - 0, 2 * PAGE_SIZE);
> - expect_eq_uint(strlen(t->list) + 1, n);
> - expect_eq_str(t->list, print_buf, n);
> -
> - /* test by non-zero offset */
> - if (strlen(t->list) > PAGE_SIZE) {
> - n = bitmap_print_list_to_buf(print_buf, t->bitmap, t->nbits,
> - PAGE_SIZE, PAGE_SIZE);
> - expect_eq_uint(strlen(t->list) + 1 - PAGE_SIZE, n);
> - expect_eq_str(t->list + PAGE_SIZE, print_buf, n);
> - }
> + PAGE_SIZE, PAGE_SIZE);
> + expect_eq_uint(strlen(t->list) + 1 - PAGE_SIZE, n);
> + expect_eq_str(t->list + PAGE_SIZE, print_buf, n);
> }
> }
>
> @@ -1118,7 +1121,7 @@ static void test_bitmap_print_buf(void)
> * FIXME: Clang breaks compile-time evaluations when KASAN and GCOV are enabled.
> * To workaround it, GCOV is force-disabled in Makefile for this configuration.
> */
> -static void test_bitmap_const_eval(void)
> +static void test_bitmap_const_eval(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bitmap, BITS_PER_LONG);
> unsigned long initvar = BIT(2);
> @@ -1186,7 +1189,7 @@ static void test_bitmap_const_eval(void)
> /*
> * Helper function to test bitmap_write() overwriting the chosen byte pattern.
> */
> -static void test_bitmap_write_helper(const char *pattern)
> +static void test_bitmap_write_helper(struct kunit *kunittest, const char *pattern)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> DECLARE_BITMAP(exp_bitmap, TEST_BIT_LEN);
> @@ -1240,7 +1243,7 @@ static void test_bitmap_write_helper(const char *pattern)
> }
> }
>
> -static void test_bitmap_read_write(void)
> +static void test_bitmap_read_write(struct kunit *kunittest)
> {
> unsigned char *pattern[3] = {"", "all:1/2", "all"};
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> @@ -1286,10 +1289,10 @@ static void test_bitmap_read_write(void)
> }
>
> for (pi = 0; pi < ARRAY_SIZE(pattern); pi++)
> - test_bitmap_write_helper(pattern[pi]);
> + test_bitmap_write_helper(kunittest, pattern[pi]);
> }
>
> -static void test_bitmap_read_perf(void)
> +static void test_bitmap_read_perf(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> unsigned int cnt, nbits, i;
> @@ -1312,10 +1315,10 @@ static void test_bitmap_read_perf(void)
> }
> }
> time = ktime_get() - time;
> - kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
> + kunit_info(kunittest, "Time spent:\t%llu\n", time);
> }
>
> -static void test_bitmap_write_perf(void)
> +static void test_bitmap_write_perf(struct kunit *kunittest)
> {
> DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
> unsigned int cnt, nbits, i;
> @@ -1334,49 +1337,42 @@ static void test_bitmap_write_perf(void)
> }
> }
> time = ktime_get() - time;
> - kunit_info(kunittest, "Time spent in %s:\t%llu\n", __func__, time);
> + kunit_info(kunittest, "Time spent:\t%llu\n", time);
> }
>
> #undef TEST_BIT_LEN
>
> -static void bitmap_test(struct kunit *test)
> -{
> - kunittest = test;
> -
> - test_zero_clear();
> - test_fill_set();
> - test_copy();
> - test_bitmap_region();
> - test_replace();
> - test_bitmap_sg();
> - test_bitmap_arr32();
> - test_bitmap_arr64();
> - test_bitmap_parse();
> - test_bitmap_parselist();
> - test_bitmap_printlist();
> - test_mem_optimisations();
> - test_bitmap_cut();
> - test_bitmap_print_buf();
> - test_bitmap_const_eval();
> - test_bitmap_read_write();
> - test_bitmap_read_perf();
> - test_bitmap_write_perf();
> -
> - test_find_nth_bit();
> - test_for_each_set_bit();
> - test_for_each_set_bit_from();
> - test_for_each_clear_bit();
> - test_for_each_clear_bit_from();
> - test_for_each_set_bitrange();
> - test_for_each_clear_bitrange();
> - test_for_each_set_bitrange_from();
> - test_for_each_clear_bitrange_from();
> - test_for_each_set_clump8();
> - test_for_each_set_bit_wrap();
> -}
> -
> static struct kunit_case bitmap_test_cases[] = {
> - KUNIT_CASE(bitmap_test),
> + KUNIT_CASE(test_zero_clear),
> + KUNIT_CASE(test_fill_set),
> + KUNIT_CASE(test_copy),
> + KUNIT_CASE(test_bitmap_region),
> + KUNIT_CASE(test_replace),
> + KUNIT_CASE(test_bitmap_sg),
> + KUNIT_CASE(test_bitmap_arr32),
> + KUNIT_CASE(test_bitmap_arr64),
> + KUNIT_CASE(test_bitmap_parse),
> + KUNIT_CASE(test_bitmap_parselist),
> + KUNIT_CASE(test_bitmap_printlist),
> + KUNIT_CASE(test_mem_optimisations),
> + KUNIT_CASE(test_bitmap_cut),
> + KUNIT_CASE_PARAM(test_bitmap_print_buf, test_print_gen_params),
> + KUNIT_CASE(test_bitmap_const_eval),
> + KUNIT_CASE(test_bitmap_read_write),
> + KUNIT_CASE(test_bitmap_read_perf),
> + KUNIT_CASE(test_bitmap_write_perf),
> +
> + KUNIT_CASE(test_find_nth_bit),
> + KUNIT_CASE(test_for_each_set_bit),
> + KUNIT_CASE(test_for_each_set_bit_from),
> + KUNIT_CASE(test_for_each_clear_bit),
> + KUNIT_CASE(test_for_each_clear_bit_from),
> + KUNIT_CASE(test_for_each_set_bitrange),
> + KUNIT_CASE(test_for_each_clear_bitrange),
> + KUNIT_CASE(test_for_each_set_bitrange_from),
> + KUNIT_CASE(test_for_each_clear_bitrange_from),
> + KUNIT_CASE(test_for_each_set_clump8),
> + KUNIT_CASE(test_for_each_set_bit_wrap),
> {}
> };
>
>
--
BR,
Muhammad Usama Anjum
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] bitmap: convert self-test to KUnit
2025-02-07 20:14 ` [PATCH 2/3] bitmap: convert self-test to KUnit Tamir Duberstein
2025-02-08 9:07 ` David Gow
2025-02-10 5:22 ` Muhammad Usama Anjum
@ 2025-02-10 7:42 ` Geert Uytterhoeven
2 siblings, 0 replies; 27+ messages in thread
From: Geert Uytterhoeven @ 2025-02-10 7:42 UTC (permalink / raw)
To: Tamir Duberstein
Cc: David Gow, John Hubbard, Andrew Morton, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Yury Norov, Rasmus Villemoes, Shuah Khan, Kees Cook,
Muhammad Usama Anjum, linux-kernel, linux-m68k, linuxppc-dev,
linux-kselftest
On Fri, 7 Feb 2025 at 21:14, Tamir Duberstein <tamird@gmail.com> wrote:
> Convert the bitmap() self-test to a KUnit test.
>
> In the interest of keeping the patch reasonably-sized this doesn't
> refactor the tests into proper parameterized tests - it's all one big
> test case.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> arch/m68k/configs/amiga_defconfig | 1 -
> arch/m68k/configs/apollo_defconfig | 1 -
> arch/m68k/configs/atari_defconfig | 1 -
> arch/m68k/configs/bvme6000_defconfig | 1 -
> arch/m68k/configs/hp300_defconfig | 1 -
> arch/m68k/configs/mac_defconfig | 1 -
> arch/m68k/configs/multi_defconfig | 1 -
> arch/m68k/configs/mvme147_defconfig | 1 -
> arch/m68k/configs/mvme16x_defconfig | 1 -
> arch/m68k/configs/q40_defconfig | 1 -
> arch/m68k/configs/sun3_defconfig | 1 -
> arch/m68k/configs/sun3x_defconfig | 1 -
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> # m68k
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-08 17:53 ` Yury Norov
2025-02-08 18:42 ` Tamir Duberstein
@ 2025-02-10 7:54 ` Geert Uytterhoeven
2025-02-10 19:35 ` distro support for CONFIG_KUNIT: " John Hubbard
1 sibling, 1 reply; 27+ messages in thread
From: Geert Uytterhoeven @ 2025-02-10 7:54 UTC (permalink / raw)
To: Yury Norov
Cc: Tamir Duberstein, David Gow, John Hubbard, Andrew Morton,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rasmus Villemoes, Shuah Khan,
Kees Cook, Muhammad Usama Anjum, linux-kernel, linux-m68k,
linuxppc-dev, linux-kselftest
Hi Yuri,
On Sat, 8 Feb 2025 at 18:53, Yury Norov <yury.norov@gmail.com> wrote:
> On Fri, Feb 07, 2025 at 03:14:01PM -0500, Tamir Duberstein wrote:
> > On 7/27/24 12:35 AM, Shuah Khan wrote:
> > > Please make sure you aren't taking away the ability to run these tests during
> > > boot.
> > >
> > > It doesn't make sense to convert every single test especially when it
> > > is intended to be run during boot without dependencies - not as a kunit test
> > > but a regression test during boot.
> > >
> > > bitmap is one example - pay attention to the config help test - bitmap
> > > one clearly states it runs regression testing during boot. Any test that
> > > says that isn't a candidate for conversion.
> > >
> > > I am going to nack any such conversions.
> >
> > The crux of the argument seems to be that the config help text is taken
> > to describe the author's intent with the fragment "at boot". I think
IMO, "at boot" is a misnomer, as most tests can be either builtin
or modular.
> KUNIT is disabled in defconfig, at least on x86_64. It is also disabled
> on my Ubuntu 24.04 machine. If I take your patches, I'll be unable to
I think distros should start setting CONFIG_KUNIT=m.
> boot-test bitmaps. Even worse, I'll be unable to build the standalone
> test from sources as a module and load it later.
If you could build the standalone test from sources as a module,
surely you can build the converted standalone test and KUNIT itself as
modules, and load both of them later?
> Or I misunderstand it, and there's a way to build some particular KUNIT
> test without enabling KUNIT in config and/or re-compiling the whole kernel?
> Please teach me, if so
>
> Unless you give me a way to build and run the test in true
> production environment, I'm not going with KUNITs. Sorry.
FTR, this is why I've been advocating for making all tests modular, and
for not letting any test select (possibly unwanted) extra functionality.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 27+ messages in thread
* distro support for CONFIG_KUNIT: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-10 7:54 ` Geert Uytterhoeven
@ 2025-02-10 19:35 ` John Hubbard
2025-02-10 19:46 ` David Hildenbrand
2025-02-10 20:20 ` Yury Norov
0 siblings, 2 replies; 27+ messages in thread
From: John Hubbard @ 2025-02-10 19:35 UTC (permalink / raw)
To: Geert Uytterhoeven, Yury Norov
Cc: Tamir Duberstein, David Gow, Andrew Morton, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Rasmus Villemoes, Shuah Khan, Kees Cook, Muhammad Usama Anjum,
linux-kernel, linux-m68k, linuxppc-dev, linux-kselftest,
Brad Figg, David Hildenbrand, Michal Hocko, Jason Gunthorpe
On 2/9/25 11:54 PM, Geert Uytterhoeven wrote:
> On Sat, 8 Feb 2025 at 18:53, Yury Norov <yury.norov@gmail.com> wrote:
>> On Fri, Feb 07, 2025 at 03:14:01PM -0500, Tamir Duberstein wrote:
>>> On 7/27/24 12:35 AM, Shuah Khan wrote:
...
>>> The crux of the argument seems to be that the config help text is taken
>>> to describe the author's intent with the fragment "at boot". I think
>
> IMO, "at boot" is a misnomer, as most tests can be either builtin
> or modular.
Right.
>
>> KUNIT is disabled in defconfig, at least on x86_64. It is also disabled
>> on my Ubuntu 24.04 machine. If I take your patches, I'll be unable to
OK so I just bought a shiny new test machine, and installed one of the
big name distros on it, hoping they've moved ahead and bought into the kunit
story...
$ grep KUNIT /boot/config-6.8.0-52-generic
# CONFIG_KUNIT is not set
...gagghh! No such luck. One more data point, in support of Yuri's complaint. :)
>
> I think distros should start setting CONFIG_KUNIT=m.
Yes they should! kunit really does have important advantages for many use
cases, including bitmaps here, and "CONFIG_KUNIT is not set" is the main
obstacle.
Let me add a few people to Cc who might be able to influence some distros.
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: distro support for CONFIG_KUNIT: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-10 19:35 ` distro support for CONFIG_KUNIT: " John Hubbard
@ 2025-02-10 19:46 ` David Hildenbrand
2025-02-10 23:10 ` Nico Pache
2025-02-10 20:20 ` Yury Norov
1 sibling, 1 reply; 27+ messages in thread
From: David Hildenbrand @ 2025-02-10 19:46 UTC (permalink / raw)
To: John Hubbard, Geert Uytterhoeven, Yury Norov
Cc: Tamir Duberstein, David Gow, Andrew Morton, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Rasmus Villemoes, Shuah Khan, Kees Cook, Muhammad Usama Anjum,
linux-kernel, linux-m68k, linuxppc-dev, linux-kselftest,
Brad Figg, Michal Hocko, Jason Gunthorpe, Nico Pache
On 10.02.25 20:35, John Hubbard wrote:
> On 2/9/25 11:54 PM, Geert Uytterhoeven wrote:
>> On Sat, 8 Feb 2025 at 18:53, Yury Norov <yury.norov@gmail.com> wrote:
>>> On Fri, Feb 07, 2025 at 03:14:01PM -0500, Tamir Duberstein wrote:
>>>> On 7/27/24 12:35 AM, Shuah Khan wrote:
> ...
>>>> The crux of the argument seems to be that the config help text is taken
>>>> to describe the author's intent with the fragment "at boot". I think
>>
>> IMO, "at boot" is a misnomer, as most tests can be either builtin
>> or modular.
>
> Right.
>
>>
>>> KUNIT is disabled in defconfig, at least on x86_64. It is also disabled
>>> on my Ubuntu 24.04 machine. If I take your patches, I'll be unable to
>
> OK so I just bought a shiny new test machine, and installed one of the
> big name distros on it, hoping they've moved ahead and bought into the kunit
> story...
>
> $ grep KUNIT /boot/config-6.8.0-52-generic
> # CONFIG_KUNIT is not set
>
> ...gagghh! No such luck. One more data point, in support of Yuri's complaint. :)
>
>>
>> I think distros should start setting CONFIG_KUNIT=m.
>
> Yes they should! kunit really does have important advantages for many use
> cases, including bitmaps here, and "CONFIG_KUNIT is not set" is the main
> obstacle.
> > Let me add a few people to Cc who might be able to influence some
distros.
>
> thanks,
Fedora has it.
CS-10 has it (-> RHEL-10):
redhat/configs/common/generic/CONFIG_KUNIT:CONFIG_KUNIT=m
https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-10/-/blob/main/redhat/configs/common/generic/CONFIG_KUNIT?ref_type=heads
CS-9 has it (-> RHEL-9):
redhat/configs/common/generic/CONFIG_KUNIT:CONFIG_KUNIT=m
https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/blob/main/redhat/configs/common/generic/CONFIG_KUNIT?ref_type=heads
So I think from the RH side everything is properly set?
Let me CC Nico, he did some KUNIT work in the past.
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: distro support for CONFIG_KUNIT: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-10 19:35 ` distro support for CONFIG_KUNIT: " John Hubbard
2025-02-10 19:46 ` David Hildenbrand
@ 2025-02-10 20:20 ` Yury Norov
2025-02-10 22:01 ` Tamir Duberstein
2025-02-11 7:51 ` David Gow
1 sibling, 2 replies; 27+ messages in thread
From: Yury Norov @ 2025-02-10 20:20 UTC (permalink / raw)
To: John Hubbard
Cc: Geert Uytterhoeven, Tamir Duberstein, David Gow, Andrew Morton,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rasmus Villemoes, Shuah Khan,
Kees Cook, Muhammad Usama Anjum, linux-kernel, linux-m68k,
linuxppc-dev, linux-kselftest, Brad Figg, David Hildenbrand,
Michal Hocko, Jason Gunthorpe
On Mon, Feb 10, 2025 at 11:35:48AM -0800, John Hubbard wrote:
> On 2/9/25 11:54 PM, Geert Uytterhoeven wrote:
> > On Sat, 8 Feb 2025 at 18:53, Yury Norov <yury.norov@gmail.com> wrote:
> > > On Fri, Feb 07, 2025 at 03:14:01PM -0500, Tamir Duberstein wrote:
> > > > On 7/27/24 12:35 AM, Shuah Khan wrote:
> ...
> > > > The crux of the argument seems to be that the config help text is taken
> > > > to describe the author's intent with the fragment "at boot". I think
> >
> > IMO, "at boot" is a misnomer, as most tests can be either builtin
> > or modular.
>
> Right.
>
> >
> > > KUNIT is disabled in defconfig, at least on x86_64. It is also disabled
> > > on my Ubuntu 24.04 machine. If I take your patches, I'll be unable to
>
> OK so I just bought a shiny new test machine, and installed one of the
> big name distros on it, hoping they've moved ahead and bought into the kunit
> story...
>
> $ grep KUNIT /boot/config-6.8.0-52-generic
> # CONFIG_KUNIT is not set
>
> ...gagghh! No such luck. One more data point, in support of Yuri's complaint. :)
>
> >
> > I think distros should start setting CONFIG_KUNIT=m.
>
> Yes they should! kunit really does have important advantages for many use
> cases, including bitmaps here, and "CONFIG_KUNIT is not set" is the main
> obstacle.
Hi John, Geert, Tamir,
Can you please explain in details which advantages KUNIT brings to
the test_bitmap.c, find_bit_benchmark.c and other low-level tests?
I'm not strongly against moving under KUNIT's hat, but I do:
- respect commitment of my contributors, so I don't want to wipe git
history for no serious reason;
- respect time of my testers, so no extra dependencies;
- respect time of reviewers.
Tamir,
If it comes to v2, can you please begin your series with an exhaustive
and clear answer to the following questions:
- What do the tests miss now?
- What do _you_ need from the tests? Describe your test scenario.
- How exactly KUNIT helps you testing bitmaps and friends better?
- Is there a way to meet your needs with a less invasive approach,
particularly without run-time dependencies?
Thanks,
Yury
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: distro support for CONFIG_KUNIT: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-10 20:20 ` Yury Norov
@ 2025-02-10 22:01 ` Tamir Duberstein
2025-02-11 7:51 ` David Gow
1 sibling, 0 replies; 27+ messages in thread
From: Tamir Duberstein @ 2025-02-10 22:01 UTC (permalink / raw)
To: Yury Norov
Cc: John Hubbard, Geert Uytterhoeven, David Gow, Andrew Morton,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rasmus Villemoes, Shuah Khan,
Kees Cook, Muhammad Usama Anjum, linux-kernel, linux-m68k,
linuxppc-dev, linux-kselftest, Brad Figg, David Hildenbrand,
Michal Hocko, Jason Gunthorpe
On Mon, Feb 10, 2025 at 3:20 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> On Mon, Feb 10, 2025 at 11:35:48AM -0800, John Hubbard wrote:
> > On 2/9/25 11:54 PM, Geert Uytterhoeven wrote:
> > > On Sat, 8 Feb 2025 at 18:53, Yury Norov <yury.norov@gmail.com> wrote:
> > > > On Fri, Feb 07, 2025 at 03:14:01PM -0500, Tamir Duberstein wrote:
> > > > > On 7/27/24 12:35 AM, Shuah Khan wrote:
> > ...
> > > > > The crux of the argument seems to be that the config help text is taken
> > > > > to describe the author's intent with the fragment "at boot". I think
> > >
> > > IMO, "at boot" is a misnomer, as most tests can be either builtin
> > > or modular.
> >
> > Right.
> >
> > >
> > > > KUNIT is disabled in defconfig, at least on x86_64. It is also disabled
> > > > on my Ubuntu 24.04 machine. If I take your patches, I'll be unable to
> >
> > OK so I just bought a shiny new test machine, and installed one of the
> > big name distros on it, hoping they've moved ahead and bought into the kunit
> > story...
> >
> > $ grep KUNIT /boot/config-6.8.0-52-generic
> > # CONFIG_KUNIT is not set
> >
> > ...gagghh! No such luck. One more data point, in support of Yuri's complaint. :)
> >
> > >
> > > I think distros should start setting CONFIG_KUNIT=m.
> >
> > Yes they should! kunit really does have important advantages for many use
> > cases, including bitmaps here, and "CONFIG_KUNIT is not set" is the main
> > obstacle.
>
> Hi John, Geert, Tamir,
>
> Can you please explain in details which advantages KUNIT brings to
> the test_bitmap.c, find_bit_benchmark.c and other low-level tests?
I can try, but I'm not the expert, and David Gow can probably elaborate further.
As I understand it the main benefit of KUnit is standardization and
speed (and standardization _is_ speed). KUnit makes it very easy for
me, a person who has not previously contributed to any of the bitmap
code, to run those tests, and it requires zero configuration, it all
just works. It's basically just `tools/testing/kunit/kunit.py run
bitmap`, and I get the test results in a human-readable format. The
same benefit applies on the author side: test facilities are
standardized, so once you get to know the tools, all the tests start
to look the same: you can jump in and contribute without having to
first learn the so-called local "testing framework".
The important part is that this all applies to ~all other tests
written in KUnit. I can even run them *all* trivially:
`tools/testing/kunit/kunit.py run`.
Anecdotally I've also noticed there are bots running those KUnit tests
e.g. see https://lore.kernel.org/all/20250207-blackholedev-kunit-convert-v1-1-8ef0dc1ff881@gmail.com/
where a test I converted was immediately flagged by a robot as having
dubious type coercion.
None of these are must-haves, they are just (to me) a nice way to make
the kernel more approachable for new contributors.
As for pure benchmarks like
find_bit_benchmark.c and
test_bitmap.c::`test_bitmap_{read,write}_perf` specifically: I believe
the benefits are super limited or even negative: AFAIK KUnit is
designed to generally suppress output (in the userspace reporter, not
in the kernel) unless a test fails, so I wouldn't hurry to use it for
these.
> I'm not strongly against moving under KUNIT's hat, but I do:
> - respect commitment of my contributors, so I don't want to wipe git
> history for no serious reason;
> - respect time of my testers, so no extra dependencies;
> - respect time of reviewers.
These are valid concerns. Certainly the testing case is the most
compelling and folks are clearly interested in lowering those
barriers. I don't have any influence in this area, but I am grateful
to John for starting the conversation.
As I mentioned in the previous thread: I think we could keep
`test_bitmap_{read,write}_perf` in test_bitmap.c and get the best of
both worlds. WDYT?
> Tamir,
>
> If it comes to v2, can you please begin your series with an exhaustive
> and clear answer to the following questions:
> - What do the tests miss now?
> - What do _you_ need from the tests? Describe your test scenario.
> - How exactly KUNIT helps you testing bitmaps and friends better?
> - Is there a way to meet your needs with a less invasive approach,
> particularly without run-time dependencies?
Hopefully I've answered these above. I can include some of it in a v2,
but perhaps the general pitch for KUnit is better placed in
documentation or slides from a conference?
Cheers.
Tamir
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: distro support for CONFIG_KUNIT: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-10 19:46 ` David Hildenbrand
@ 2025-02-10 23:10 ` Nico Pache
2025-02-10 23:41 ` John Hubbard
0 siblings, 1 reply; 27+ messages in thread
From: Nico Pache @ 2025-02-10 23:10 UTC (permalink / raw)
To: David Hildenbrand, John Hubbard, Geert Uytterhoeven, Yury Norov
Cc: Tamir Duberstein, David Gow, Andrew Morton, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Rasmus Villemoes, Shuah Khan, Kees Cook, Muhammad Usama Anjum,
linux-kernel, linux-m68k, linuxppc-dev, linux-kselftest,
Brad Figg, Michal Hocko, Jason Gunthorpe
On Mon, Feb 10, 2025 at 12:46 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 10.02.25 20:35, John Hubbard wrote:
> > On 2/9/25 11:54 PM, Geert Uytterhoeven wrote:
> >> On Sat, 8 Feb 2025 at 18:53, Yury Norov <yury.norov@gmail.com> wrote:
> >>> On Fri, Feb 07, 2025 at 03:14:01PM -0500, Tamir Duberstein wrote:
> >>>> On 7/27/24 12:35 AM, Shuah Khan wrote:
> > ...
> >>>> The crux of the argument seems to be that the config help text is taken
> >>>> to describe the author's intent with the fragment "at boot". I think
> >>
> >> IMO, "at boot" is a misnomer, as most tests can be either builtin
> >> or modular.
> >
> > Right.
> >
> >>
> >>> KUNIT is disabled in defconfig, at least on x86_64. It is also disabled
> >>> on my Ubuntu 24.04 machine. If I take your patches, I'll be unable to
> >
> > OK so I just bought a shiny new test machine, and installed one of the
> > big name distros on it, hoping they've moved ahead and bought into the kunit
> > story...
> >
> > $ grep KUNIT /boot/config-6.8.0-52-generic
> > # CONFIG_KUNIT is not set
> >
> > ...gagghh! No such luck. One more data point, in support of Yuri's complaint. :)
> >
> >>
> >> I think distros should start setting CONFIG_KUNIT=m.
> >
> > Yes they should! kunit really does have important advantages for many use
> > cases, including bitmaps here, and "CONFIG_KUNIT is not set" is the main
> > obstacle.
> > > Let me add a few people to Cc who might be able to influence some
> distros.
> >
> > thanks,
>
>
> Fedora has it.
>
> CS-10 has it (-> RHEL-10):
> redhat/configs/common/generic/CONFIG_KUNIT:CONFIG_KUNIT=m
>
> https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-10/-/blob/main/redhat/configs/common/generic/CONFIG_KUNIT?ref_type=heads
>
> CS-9 has it (-> RHEL-9):
> redhat/configs/common/generic/CONFIG_KUNIT:CONFIG_KUNIT=m
>
> https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/blob/main/redhat/configs/common/generic/CONFIG_KUNIT?ref_type=heads
>
> So I think from the RH side everything is properly set?
>
> Let me CC Nico, he did some KUNIT work in the past.
Yeah that is correct! I enabled KUNIT in our environments a few years
ago. We enable it as a module and use our own wrapper to exercise the
code. For RHEL and Centos these kunit modules are only shipped
internally for testing; However fedora-rawhide makes these modules
available in the kernel-modules-internal package.
To test this you can follow this to install rawhide-vm:
https://developer.fedoraproject.org/tools/virt-builder/about.html
then inside the vm:
yum install kernel-modules-internal
add the kunit.enable=1 to the cmdline
reboot vm
modprobe kunit
modprobe <test_name>
Hopefully that helps!
-- Nico
>
>
> --
> Cheers,
>
> David / dhildenb
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: distro support for CONFIG_KUNIT: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-10 23:10 ` Nico Pache
@ 2025-02-10 23:41 ` John Hubbard
0 siblings, 0 replies; 27+ messages in thread
From: John Hubbard @ 2025-02-10 23:41 UTC (permalink / raw)
To: Nico Pache, David Hildenbrand, Geert Uytterhoeven, Yury Norov
Cc: Tamir Duberstein, David Gow, Andrew Morton, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Rasmus Villemoes, Shuah Khan, Kees Cook, Muhammad Usama Anjum,
linux-kernel, linux-m68k, linuxppc-dev, linux-kselftest,
Brad Figg, Michal Hocko, Jason Gunthorpe
On 2/10/25 3:10 PM, Nico Pache wrote:
>>> ...
>> Fedora has it.
>>
>> CS-10 has it (-> RHEL-10):
>> redhat/configs/common/generic/CONFIG_KUNIT:CONFIG_KUNIT=m
>>
>> https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-10/-/blob/main/redhat/configs/common/generic/CONFIG_KUNIT?ref_type=heads
>>
>> CS-9 has it (-> RHEL-9):
>> redhat/configs/common/generic/CONFIG_KUNIT:CONFIG_KUNIT=m
>>
>> https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/blob/main/redhat/configs/common/generic/CONFIG_KUNIT?ref_type=heads
>>
>> So I think from the RH side everything is properly set?
>>
>> Let me CC Nico, he did some KUNIT work in the past.
>
> Yeah that is correct! I enabled KUNIT in our environments a few years
> ago. We enable it as a module and use our own wrapper to exercise the
> code. For RHEL and Centos these kunit modules are only shipped
> internally for testing; However fedora-rawhide makes these modules
> available in the kernel-modules-internal package.
>
> To test this you can follow this to install rawhide-vm:
> https://developer.fedoraproject.org/tools/virt-builder/about.html
> then inside the vm:
> yum install kernel-modules-internal
> add the kunit.enable=1 to the cmdline
> reboot vm
> modprobe kunit
> modprobe <test_name>
>
> Hopefully that helps!
> -- Nico
Great news, thanks for the quick answers for Red Hat. "Already done" is as
good as it gets, for this kind of question. :)
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: distro support for CONFIG_KUNIT: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-10 20:20 ` Yury Norov
2025-02-10 22:01 ` Tamir Duberstein
@ 2025-02-11 7:51 ` David Gow
2025-03-21 16:53 ` Tamir Duberstein
1 sibling, 1 reply; 27+ messages in thread
From: David Gow @ 2025-02-11 7:51 UTC (permalink / raw)
To: Yury Norov
Cc: John Hubbard, Geert Uytterhoeven, Tamir Duberstein, Andrew Morton,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rasmus Villemoes, Shuah Khan,
Kees Cook, Muhammad Usama Anjum, linux-kernel, linux-m68k,
linuxppc-dev, linux-kselftest, Brad Figg, David Hildenbrand,
Michal Hocko, Jason Gunthorpe
On Tue, 11 Feb 2025 at 04:20, Yury Norov <yury.norov@gmail.com> wrote:
>
> On Mon, Feb 10, 2025 at 11:35:48AM -0800, John Hubbard wrote:
> > On 2/9/25 11:54 PM, Geert Uytterhoeven wrote:
> > > On Sat, 8 Feb 2025 at 18:53, Yury Norov <yury.norov@gmail.com> wrote:
> > > > On Fri, Feb 07, 2025 at 03:14:01PM -0500, Tamir Duberstein wrote:
> > > > > On 7/27/24 12:35 AM, Shuah Khan wrote:
> > ...
> > > > > The crux of the argument seems to be that the config help text is taken
> > > > > to describe the author's intent with the fragment "at boot". I think
> > >
> > > IMO, "at boot" is a misnomer, as most tests can be either builtin
> > > or modular.
> >
> > Right.
> >
> > >
> > > > KUNIT is disabled in defconfig, at least on x86_64. It is also disabled
> > > > on my Ubuntu 24.04 machine. If I take your patches, I'll be unable to
> >
> > OK so I just bought a shiny new test machine, and installed one of the
> > big name distros on it, hoping they've moved ahead and bought into the kunit
> > story...
> >
> > $ grep KUNIT /boot/config-6.8.0-52-generic
> > # CONFIG_KUNIT is not set
> >
> > ...gagghh! No such luck. One more data point, in support of Yuri's complaint. :)
> >
> > >
> > > I think distros should start setting CONFIG_KUNIT=m.
> >
> > Yes they should! kunit really does have important advantages for many use
> > cases, including bitmaps here, and "CONFIG_KUNIT is not set" is the main
> > obstacle.
>
> Hi John, Geert, Tamir,
>
> Can you please explain in details which advantages KUNIT brings to
> the test_bitmap.c, find_bit_benchmark.c and other low-level tests?
>
First, I'd agree with Tamir and others who've pointed out that KUnit
is not really intended for benchmarks, so I definitely don't want to
oversell it for find_bit_benchmark.c and others (though it has been
used for benchmarks in a few cases, it's not perfect).
The main advantages of KUnit are that:
- it's already used for a bunch of lib/ tests (so quite familiar to a
number of contributors/users),
- tests can be run at boot without needing any userspace at all (if built-in),
- KUnit tests are already being automatically run as regression tests,
and it's relatively easy to run "all tests" associated with a config
- there's a whole bunch of tooling which automates configuring,
building, and running a kernel with these tests, across several
architectures
(e.g., passing --arch arm64 or --arch m68k to automatically build
and run the tests in QEMU for those architectures)
- there's tooling to nicely print, format, and summarise the results
of any number of suites.
- and, as a result of the above, it's very quick and easy to run a
test (or whole bunch of tests) in an isolated environment.
KUnit also has very few runtime dependencies from a developer
point-of-view (python, optionally QEMU).
It is, however, less focused on the "debug a running system" case,
hence the requirement for CONFIG_KUNIT to be enabled, and the tooling
largely being focused on the "run all of the tests in UML or a VM"
case.
> I'm not strongly against moving under KUNIT's hat, but I do:
> - respect commitment of my contributors, so I don't want to wipe git
> history for no serious reason;
It should be possible to preserve the git history during a port,
though it admittedly will have a bunch of porting changes which make
it more difficult to navigate with git blame. But it should at least
be better than a total wipe.
> - respect time of my testers, so no extra dependencies;
I think KUnit is a bit of a mixed bag here. In many cases, KUnit tests
can be run pretty quickly, either because the tester's environment
already has CONFIG_KUNIT enabled, or possibly under some kind of
virtualization with the tooling (either QEMU, or on x86,
User-Mode-Linux). But, as mentioned above, I'd not consider it ideal
for performance testing.
I am very sympathetic to not wanting to add a dependency here. I think
KUnit is a pretty minimal dependency as far as they go (it selects
CONFIG_GLOB and nothing else, needs no userspace, and even the tooling
only really requires python on top of the basic toolchain), but it's
definitely a further dependency for the
'test-the-currently-running-kernel' use-case.
> - respect time of reviewers.
This is an area where I think KUnit could be an advantage, as it'd be
easy for reviewers to simply run
./tools/testing/kunit/kunit.py run bitmap
as a spot check on the code. And if they want to test on other
architectures, the --arch option can be used to do so quickly.
(And, of course, it can still be loaded as a module (or even built-in)
on a physical system or other VM, albeit with the requirement for
CONFIG_KUNIT.)
As for distros enabling CONFIG_KUNIT by default, as mentioned, my
understanding is that the Fedora/Red Hat distros are doing it, as are
some branches of Android, but I doubt anyone else is. This makes
sense: enabling CONFIG_KUNIT isn't free (though the overhead should be
very low if no test is running, just a few static branches and an
extra check in the module loader), and there's always been some
reticence (which I've shared) in enabling test features on production
kernels. So it's definitely something I was recommending against a few
years ago.
However, we've definitely made sure that CONFIG_KUNIT is less
invasive, with things like hooks being kept behind static branches,
and made the decision to have _running_ a test taint the kernel, so it
should be relatively safe to just have the framework enabled (and even
safe to have it loaded), and pretty obvious when a test is run. We've
also got a bunch of extra options to disable running tests
automatically when they're built-in/loaded, both at runtime and
compile time.
So I'd definitely be happy with other distros choosing to enable
CONFIG_KUNIT by default now: I think it's much safer than it was, and
you won't be going against the grain. But equally, I'd understand
having it disabled in production: the cost, while very small, is
nonzero.
Cheers,
-- David
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: distro support for CONFIG_KUNIT: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-02-11 7:51 ` David Gow
@ 2025-03-21 16:53 ` Tamir Duberstein
2025-03-21 18:32 ` Yury Norov
0 siblings, 1 reply; 27+ messages in thread
From: Tamir Duberstein @ 2025-03-21 16:53 UTC (permalink / raw)
To: David Gow
Cc: Yury Norov, John Hubbard, Geert Uytterhoeven, Andrew Morton,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rasmus Villemoes, Shuah Khan,
Kees Cook, Muhammad Usama Anjum, linux-kernel, linux-m68k,
linuxppc-dev, linux-kselftest, Brad Figg, David Hildenbrand,
Michal Hocko, Jason Gunthorpe
Hi all, now that the printf and scanf series have been taken via kees'
tree[0] and sent in for v6.15-rc1[1], I wonder if we'd like to revisit
this discussion.
As I understand it, the primary objections to moving bitmap to KUnit were:
- Unclear benefits.
- Source churn.
- Extra dependencies for benchmarks.
Hopefully David's enumeration of the benefits of KUnit was compelling.
Regarding source churn: it is inevitable, but I did pay attention to
this and minimized the diff where possible.
The last point is trickiest, because KUnit doesn't have first-class
benchmark support, but nor is there a blessed benchmark facility in
the kernel generally. I'd prefer not to tie this series to distros
enabling KUNIT_CONFIG by default, which will take $time.
I think the most sensible thing we can do - if we accept that KUnit
has benefits to offer - is to split test_bitmap.c into
benchmark_bitmap.c and bitmap_kunit.c.
Please let me know your thoughts.
Tamir
[0] https://web.git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=move-lib-kunit-v6.15-rc1
[1] https://lore.kernel.org/all/202503170842.FFEE75351@keescook/
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: distro support for CONFIG_KUNIT: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-03-21 16:53 ` Tamir Duberstein
@ 2025-03-21 18:32 ` Yury Norov
2025-03-21 19:47 ` Tamir Duberstein
0 siblings, 1 reply; 27+ messages in thread
From: Yury Norov @ 2025-03-21 18:32 UTC (permalink / raw)
To: Tamir Duberstein
Cc: David Gow, John Hubbard, Geert Uytterhoeven, Andrew Morton,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rasmus Villemoes, Shuah Khan,
Kees Cook, Muhammad Usama Anjum, linux-kernel, linux-m68k,
linuxppc-dev, linux-kselftest, Brad Figg, David Hildenbrand,
Michal Hocko, Jason Gunthorpe
On Fri, Mar 21, 2025 at 12:53:36PM -0400, Tamir Duberstein wrote:
> Hi all, now that the printf and scanf series have been taken via kees'
> tree[0] and sent in for v6.15-rc1[1], I wonder if we'd like to revisit
> this discussion.
>
> As I understand it, the primary objections to moving bitmap to KUnit were:
> - Unclear benefits.
> - Source churn.
> - Extra dependencies for benchmarks.
>
> Hopefully David's enumeration of the benefits of KUnit was compelling.
> Regarding source churn: it is inevitable, but I did pay attention to
> this and minimized the diff where possible.
>
> The last point is trickiest, because KUnit doesn't have first-class
> benchmark support, but nor is there a blessed benchmark facility in
> the kernel generally. I'd prefer not to tie this series to distros
> enabling KUNIT_CONFIG by default, which will take $time.
>
> I think the most sensible thing we can do - if we accept that KUnit
> has benefits to offer - is to split test_bitmap.c into
> benchmark_bitmap.c and bitmap_kunit.c.
>
> Please let me know your thoughts.
Sure, no problem.
I asked you to answer to 4 very simple and specific questions. You
didn't answer any of them. David sent a lengthy email that doesn't
address them, either.
None of you guys submitted anything to bitmaps - neither in library,
nor in tests. Your opinion about what is good for bitmap development
and what's not is purely theoretical.
Real contributors never concerned about current testing model.
I think that you don't care about bitmaps. If bitmaps testing will get
broken one day, or more complicated, you will not come to help. If I'm
wrong and you are willing to contribute, you're warmly welcome! I always
encourage people to increase testing coverage.
If you'd like to add new cases to existing tests - I'll be happy. If
you'd like to add completely new tests based on KUNITs or whatever
else - I'll be happy just as well.
Thanks,
Yury
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: distro support for CONFIG_KUNIT: [PATCH 0/3] bitmap: convert self-test to KUnit
2025-03-21 18:32 ` Yury Norov
@ 2025-03-21 19:47 ` Tamir Duberstein
0 siblings, 0 replies; 27+ messages in thread
From: Tamir Duberstein @ 2025-03-21 19:47 UTC (permalink / raw)
To: Yury Norov
Cc: David Gow, John Hubbard, Geert Uytterhoeven, Andrew Morton,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rasmus Villemoes, Shuah Khan,
Kees Cook, Muhammad Usama Anjum, linux-kernel, linux-m68k,
linuxppc-dev, linux-kselftest, Brad Figg, David Hildenbrand,
Michal Hocko, Jason Gunthorpe
On Fri, Mar 21, 2025 at 2:32 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> On Fri, Mar 21, 2025 at 12:53:36PM -0400, Tamir Duberstein wrote:
> > Hi all, now that the printf and scanf series have been taken via kees'
> > tree[0] and sent in for v6.15-rc1[1], I wonder if we'd like to revisit
> > this discussion.
> >
> > As I understand it, the primary objections to moving bitmap to KUnit were:
> > - Unclear benefits.
> > - Source churn.
> > - Extra dependencies for benchmarks.
> >
> > Hopefully David's enumeration of the benefits of KUnit was compelling.
> > Regarding source churn: it is inevitable, but I did pay attention to
> > this and minimized the diff where possible.
> >
> > The last point is trickiest, because KUnit doesn't have first-class
> > benchmark support, but nor is there a blessed benchmark facility in
> > the kernel generally. I'd prefer not to tie this series to distros
> > enabling KUNIT_CONFIG by default, which will take $time.
> >
> > I think the most sensible thing we can do - if we accept that KUnit
> > has benefits to offer - is to split test_bitmap.c into
> > benchmark_bitmap.c and bitmap_kunit.c.
> >
> > Please let me know your thoughts.
>
> Sure, no problem.
>
> I asked you to answer to 4 very simple and specific questions. You
> didn't answer any of them. David sent a lengthy email that doesn't
> address them, either.
OK, that's fair I suppose. Let me try and address them now:
> - What do the tests miss now?
The tests do not _miss_ anything. They are just inconvenient to run,
particularly from automation, because they do not report success in a
way that is trivially understood by automation. In other words, I'm
not aware of something trivial I can run that will exit 0 if and only
if the bitmap tests pass.
> - What do _you_ need from the tests? Describe your test scenario.
I want kernel tests to be easier to run, and for more of them to be
run by existing automation such as LKP[0]. I know for sure that KUnit
tests are automatically run by LKP because other tests I converted to
KUnit subsequently had warnings reported by LKP.
> - How exactly KUNIT helps you testing bitmaps and friends better?
KUnit reports test results in a standard protocol (KTAP) that is
machine-friendly. It comes with userspace tools that understand this
protocol and produce useful exit codes, as well as human-friendly
output.
> - Is there a way to meet your needs with a less invasive approach,
> particularly without run-time dependencies?
I'm not aware of such a way, but if you know of one, I would be very
interested to learn.
> None of you guys submitted anything to bitmaps - neither in library,
> nor in tests. Your opinion about what is good for bitmap development
> and what's not is purely theoretical.
>
> Real contributors never concerned about current testing model.
>
> I think that you don't care about bitmaps. If bitmaps testing will get
> broken one day, or more complicated, you will not come to help. If I'm
> wrong and you are willing to contribute, you're warmly welcome! I always
> encourage people to increase testing coverage.
>
> If you'd like to add new cases to existing tests - I'll be happy. If
> you'd like to add completely new tests based on KUNITs or whatever
> else - I'll be happy just as well.
I can't speak for David, but you are right about me; I do not have an
interest in bitmap in particular. My interest is in kernel testing
generally, which I hope I have adequately explained above. As for my
willingness to help people obtain and keep good workflows, well,
you're welcome to examine my history in OSS. I've contributed to
dozens of projects, many for far longer than my professional goals
required.
Let's keep talking.
Tamir
[0] https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2025-03-21 19:48 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-07 20:14 [PATCH 0/3] bitmap: convert self-test to KUnit Tamir Duberstein
2025-02-07 20:14 ` [PATCH 1/3] bitmap: remove _check_eq_u32_array Tamir Duberstein
2025-02-08 9:07 ` David Gow
2025-02-08 18:00 ` Yury Norov
2025-02-10 5:22 ` Muhammad Usama Anjum
2025-02-07 20:14 ` [PATCH 2/3] bitmap: convert self-test to KUnit Tamir Duberstein
2025-02-08 9:07 ` David Gow
2025-02-10 5:22 ` Muhammad Usama Anjum
2025-02-10 7:42 ` Geert Uytterhoeven
2025-02-07 20:14 ` [PATCH 3/3] bitmap: break kunit into test cases Tamir Duberstein
2025-02-08 9:07 ` David Gow
2025-02-08 12:33 ` Tamir Duberstein
2025-02-10 5:23 ` Muhammad Usama Anjum
2025-02-08 9:07 ` [PATCH 0/3] bitmap: convert self-test to KUnit David Gow
2025-02-08 17:53 ` Yury Norov
2025-02-08 18:42 ` Tamir Duberstein
2025-02-10 7:54 ` Geert Uytterhoeven
2025-02-10 19:35 ` distro support for CONFIG_KUNIT: " John Hubbard
2025-02-10 19:46 ` David Hildenbrand
2025-02-10 23:10 ` Nico Pache
2025-02-10 23:41 ` John Hubbard
2025-02-10 20:20 ` Yury Norov
2025-02-10 22:01 ` Tamir Duberstein
2025-02-11 7:51 ` David Gow
2025-03-21 16:53 ` Tamir Duberstein
2025-03-21 18:32 ` Yury Norov
2025-03-21 19:47 ` Tamir Duberstein
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).