[PATCH 0/3] bitmap: Convert test_bitmap to kunit test

[PATCH 0/3] bitmap: Convert test_bitmap to kunit test

[Muhammad Usama Anjum]

In this series, test_bitmap is being converted to kunit test. Multiple
patches will make the review process smooth.

- Patch-1: Convert the tests in lib/test_bitmap.c to kunit
- Patch-2: Rename the lib/test_bitmap.c to lib/bitmap_kunit.c and other
           configuration options
- Patch-3: Remove the bitmap.sh selftest

Muhammad Usama Anjum (3):
  bitmap: convert test_bitmap to KUnit test
  bitmap: Rename module
  selftests: lib: remove test_bitmap

 MAINTAINERS                           |   2 +-
 lib/Kconfig.debug                     |  15 +-
 lib/Makefile                          |   2 +-
 lib/{test_bitmap.c => bitmap_kunit.c} | 624 ++++++++++++--------------
 tools/testing/selftests/lib/Makefile  |   2 +-
 tools/testing/selftests/lib/bitmap.sh |   3 -
 tools/testing/selftests/lib/config    |   1 -
 7 files changed, 295 insertions(+), 354 deletions(-)
 rename lib/{test_bitmap.c => bitmap_kunit.c} (70%)
 delete mode 100755 tools/testing/selftests/lib/bitmap.sh

-- 
2.39.2

[PATCH 1/3] bitmap: convert test_bitmap to KUnit test

[Muhammad Usama Anjum]

Convert the test_bitmap to Kunit test in-place (without moving the file
so that changes can be reviewed esaily). The test has been converted
from kselftest module helper functions to kunit. Following major changes
were done to achieve this:
- Convert the init logic
- Remove all __init* as kunit tests can be called multiple times
- Use KUNIT_ASSERT_* macros for determining results.

Cc: kees@kernel.org
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 lib/test_bitmap.c | 624 +++++++++++++++++++++-------------------------
 1 file changed, 284 insertions(+), 340 deletions(-)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 65a75d58ed9e4..41c82b8339ffc 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -13,17 +13,14 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/uaccess.h>
-
-#include "../tools/testing/selftests/kselftest_module.h"
+#include <kunit/test.h>
 
 #define EXP1_IN_BITS	(sizeof(exp1) * 8)
 
-KSTM_MODULE_GLOBALS();
-
-static char pbl_buffer[PAGE_SIZE] __initdata;
-static char print_buf[PAGE_SIZE * 2] __initdata;
+static char pbl_buffer[PAGE_SIZE];
+static char print_buf[PAGE_SIZE * 2];
 
-static const unsigned long exp1[] __initconst = {
+static const unsigned long exp1[] = {
 	BITMAP_FROM_U64(1),
 	BITMAP_FROM_U64(2),
 	BITMAP_FROM_U64(0x0000ffff),
@@ -41,70 +38,59 @@ 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,
+static void
+__check_eq_ulong(struct kunit *test, 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;
+	KUNIT_ASSERT_EQ_MSG(test, exp_ulong, x,
+			    "[%s:%u] expected %lu, got %lu\n",
+			    srcfile, line, exp_ulong, x);
 }
 
-static bool __init
-__check_eq_bitmap(const char *srcfile, unsigned int line,
+static void
+__check_eq_bitmap(struct kunit *test, 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;
+	KUNIT_ASSERT_TRUE_MSG(test, bitmap_equal(exp_bmap, bmap, nbits),
+			      "[%s:%u] bitmaps contents differ: expected \"%*pbl\", got \"%*pbl\"\n",
+			      srcfile, line, nbits, exp_bmap, nbits, bmap);
 }
 
-static bool __init
-__check_eq_pbl(const char *srcfile, unsigned int line,
+static void
+__check_eq_pbl(struct kunit *test, 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;
+	KUNIT_ASSERT_STREQ_MSG(test, expected_pbl, pbl_buffer,
+			       "[%s:%u] expected \"%s\", got \"%s\"\n",
+			       srcfile, line, expected_pbl, pbl_buffer);
 }
 
-static bool __init
+static bool
 __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
+static bool
 __check_eq_u32_array(const char *srcfile, unsigned int line,
 		     const u32 *exp_arr, unsigned int exp_len,
 		     const u32 *arr, unsigned int len)
@@ -128,100 +114,82 @@ __check_eq_u32_array(const char *srcfile, unsigned int line,
 	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)
+static void __check_eq_clump8(struct kunit *test, 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;
-	}
+	KUNIT_ASSERT_LE_MSG(test, offset, size,
+			    "[%s:%u] bit offset for clump out-of-bounds: expected less than %u, got %u\n",
+			    srcfile, line, size, offset);
 
 	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;
-	}
+	KUNIT_ASSERT_TRUE_MSG(test, exp,
+			      "[%s:%u] bit offset for zero clump: expected nonzero clump, got bit offset %u with clump value 0",
+			      srcfile, line, offset);
 
-	return true;
+	KUNIT_ASSERT_EQ_MSG(test, *clump, exp,
+			    "[%s:%u] expected clump value of 0x%lX, got clump value of 0x%lX",
+			    srcfile, line, exp, *clump);
 }
 
-static bool __init
-__check_eq_str(const char *srcfile, unsigned int line,
-		const char *exp_str, const char *str,
-		unsigned int len)
+static void
+__check_eq_str(struct kunit *test, const char *srcfile, unsigned int line,
+	       const char *exp_str, const char *str,
+	       unsigned int len)
 {
-	bool eq;
+	KUNIT_EXPECT_STREQ_MSG(test, exp_str, str,
+			       "[%s:%u] expected %s, got %s\n",
+			       srcfile, line, exp_str, str);
 
-	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, ...)					\
+#define __expect_eq(test, suffix, ...)					\
 	({								\
-		int result = 0;						\
-		total_tests++;						\
-		if (!__check_eq_ ## suffix(__FILE__, __LINE__,		\
-					   ##__VA_ARGS__)) {		\
-			failed_tests++;					\
-			result = 1;					\
-		}							\
-		result;							\
+		__check_eq_ ## suffix(test, __FILE__, __LINE__,		\
+					   ##__VA_ARGS__);		\
 	})
 
-#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__)
+#define expect_eq_ulong(test, ...)	__expect_eq(test, ulong, ##__VA_ARGS__)
+#define expect_eq_uint(test, x, y)	expect_eq_ulong(test, (unsigned int)(x), (unsigned int)(y))
+#define expect_eq_bitmap(test, ...)	__expect_eq(test, bitmap, ##__VA_ARGS__)
+#define expect_eq_pbl(test, ...)	__expect_eq(test, pbl, ##__VA_ARGS__)
+#define expect_eq_clump8(test, ...)	__expect_eq(test, clump8, ##__VA_ARGS__)
+#define expect_eq_str(test, ...)	__expect_eq(test, str, ##__VA_ARGS__)
 
-static void __init test_zero_clear(void)
+static void test_zero_clear(struct kunit *test)
 {
 	DECLARE_BITMAP(bmap, 1024);
 
 	/* Known way to set all bits */
 	memset(bmap, 0xff, 128);
 
-	expect_eq_pbl("0-22", bmap, 23);
-	expect_eq_pbl("0-1023", bmap, 1024);
+	expect_eq_pbl(test, "0-22", bmap, 23);
+	expect_eq_pbl(test, "0-1023", bmap, 1024);
 
 	/* single-word bitmaps */
 	bitmap_clear(bmap, 0, 9);
-	expect_eq_pbl("9-1023", bmap, 1024);
+	expect_eq_pbl(test, "9-1023", bmap, 1024);
 
 	bitmap_zero(bmap, 35);
-	expect_eq_pbl("64-1023", bmap, 1024);
+	expect_eq_pbl(test, "64-1023", bmap, 1024);
 
 	/* cross boundaries operations */
 	bitmap_clear(bmap, 79, 19);
-	expect_eq_pbl("64-78,98-1023", bmap, 1024);
+	expect_eq_pbl(test, "64-78,98-1023", bmap, 1024);
 
 	bitmap_zero(bmap, 115);
-	expect_eq_pbl("128-1023", bmap, 1024);
+	expect_eq_pbl(test, "128-1023", bmap, 1024);
 
 	/* Zeroing entire area */
 	bitmap_zero(bmap, 1024);
-	expect_eq_pbl("", bmap, 1024);
+	expect_eq_pbl(test, "", bmap, 1024);
 }
 
-static void __init test_find_nth_bit(void)
+static void test_find_nth_bit(struct kunit *test)
 {
 	unsigned long b, bit, cnt = 0;
 	DECLARE_BITMAP(bmap, 64 * 3);
@@ -236,62 +204,62 @@ static void __init test_find_nth_bit(void)
 	__set_bit(80, bmap);
 	__set_bit(123, bmap);
 
-	expect_eq_uint(10,  find_nth_bit(bmap, 64 * 3, 0));
-	expect_eq_uint(20,  find_nth_bit(bmap, 64 * 3, 1));
-	expect_eq_uint(30,  find_nth_bit(bmap, 64 * 3, 2));
-	expect_eq_uint(40,  find_nth_bit(bmap, 64 * 3, 3));
-	expect_eq_uint(50,  find_nth_bit(bmap, 64 * 3, 4));
-	expect_eq_uint(60,  find_nth_bit(bmap, 64 * 3, 5));
-	expect_eq_uint(80,  find_nth_bit(bmap, 64 * 3, 6));
-	expect_eq_uint(123, find_nth_bit(bmap, 64 * 3, 7));
-	expect_eq_uint(0,   !!(find_nth_bit(bmap, 64 * 3, 8) < 64 * 3));
-
-	expect_eq_uint(10,  find_nth_bit(bmap, 64 * 3 - 1, 0));
-	expect_eq_uint(20,  find_nth_bit(bmap, 64 * 3 - 1, 1));
-	expect_eq_uint(30,  find_nth_bit(bmap, 64 * 3 - 1, 2));
-	expect_eq_uint(40,  find_nth_bit(bmap, 64 * 3 - 1, 3));
-	expect_eq_uint(50,  find_nth_bit(bmap, 64 * 3 - 1, 4));
-	expect_eq_uint(60,  find_nth_bit(bmap, 64 * 3 - 1, 5));
-	expect_eq_uint(80,  find_nth_bit(bmap, 64 * 3 - 1, 6));
-	expect_eq_uint(123, find_nth_bit(bmap, 64 * 3 - 1, 7));
-	expect_eq_uint(0,   !!(find_nth_bit(bmap, 64 * 3 - 1, 8) < 64 * 3 - 1));
+	expect_eq_uint(test, 10,  find_nth_bit(bmap, 64 * 3, 0));
+	expect_eq_uint(test, 20,  find_nth_bit(bmap, 64 * 3, 1));
+	expect_eq_uint(test, 30,  find_nth_bit(bmap, 64 * 3, 2));
+	expect_eq_uint(test, 40,  find_nth_bit(bmap, 64 * 3, 3));
+	expect_eq_uint(test, 50,  find_nth_bit(bmap, 64 * 3, 4));
+	expect_eq_uint(test, 60,  find_nth_bit(bmap, 64 * 3, 5));
+	expect_eq_uint(test, 80,  find_nth_bit(bmap, 64 * 3, 6));
+	expect_eq_uint(test, 123, find_nth_bit(bmap, 64 * 3, 7));
+	expect_eq_uint(test, 0,   !!(find_nth_bit(bmap, 64 * 3, 8) < 64 * 3));
+
+	expect_eq_uint(test, 10,  find_nth_bit(bmap, 64 * 3 - 1, 0));
+	expect_eq_uint(test, 20,  find_nth_bit(bmap, 64 * 3 - 1, 1));
+	expect_eq_uint(test, 30,  find_nth_bit(bmap, 64 * 3 - 1, 2));
+	expect_eq_uint(test, 40,  find_nth_bit(bmap, 64 * 3 - 1, 3));
+	expect_eq_uint(test, 50,  find_nth_bit(bmap, 64 * 3 - 1, 4));
+	expect_eq_uint(test, 60,  find_nth_bit(bmap, 64 * 3 - 1, 5));
+	expect_eq_uint(test, 80,  find_nth_bit(bmap, 64 * 3 - 1, 6));
+	expect_eq_uint(test, 123, find_nth_bit(bmap, 64 * 3 - 1, 7));
+	expect_eq_uint(test, 0,   !!(find_nth_bit(bmap, 64 * 3 - 1, 8) < 64 * 3 - 1));
 
 	for_each_set_bit(bit, exp1, EXP1_IN_BITS) {
 		b = find_nth_bit(exp1, EXP1_IN_BITS, cnt++);
-		expect_eq_uint(b, bit);
+		expect_eq_uint(test, b, bit);
 	}
 }
 
-static void __init test_fill_set(void)
+static void test_fill_set(struct kunit *test)
 {
 	DECLARE_BITMAP(bmap, 1024);
 
 	/* Known way to clear all bits */
 	memset(bmap, 0x00, 128);
 
-	expect_eq_pbl("", bmap, 23);
-	expect_eq_pbl("", bmap, 1024);
+	expect_eq_pbl(test, "", bmap, 23);
+	expect_eq_pbl(test, "", bmap, 1024);
 
 	/* single-word bitmaps */
 	bitmap_set(bmap, 0, 9);
-	expect_eq_pbl("0-8", bmap, 1024);
+	expect_eq_pbl(test, "0-8", bmap, 1024);
 
 	bitmap_fill(bmap, 35);
-	expect_eq_pbl("0-63", bmap, 1024);
+	expect_eq_pbl(test, "0-63", bmap, 1024);
 
 	/* cross boundaries operations */
 	bitmap_set(bmap, 79, 19);
-	expect_eq_pbl("0-63,79-97", bmap, 1024);
+	expect_eq_pbl(test, "0-63,79-97", bmap, 1024);
 
 	bitmap_fill(bmap, 115);
-	expect_eq_pbl("0-127", bmap, 1024);
+	expect_eq_pbl(test, "0-127", bmap, 1024);
 
 	/* Zeroing entire area */
 	bitmap_fill(bmap, 1024);
-	expect_eq_pbl("0-1023", bmap, 1024);
+	expect_eq_pbl(test, "0-1023", bmap, 1024);
 }
 
-static void __init test_copy(void)
+static void test_copy(struct kunit *test)
 {
 	DECLARE_BITMAP(bmap1, 1024);
 	DECLARE_BITMAP(bmap2, 1024);
@@ -302,20 +270,20 @@ static void __init test_copy(void)
 	/* single-word bitmaps */
 	bitmap_set(bmap1, 0, 19);
 	bitmap_copy(bmap2, bmap1, 23);
-	expect_eq_pbl("0-18", bmap2, 1024);
+	expect_eq_pbl(test, "0-18", bmap2, 1024);
 
 	bitmap_set(bmap2, 0, 23);
 	bitmap_copy(bmap2, bmap1, 23);
-	expect_eq_pbl("0-18", bmap2, 1024);
+	expect_eq_pbl(test, "0-18", bmap2, 1024);
 
 	/* multi-word bitmaps */
 	bitmap_set(bmap1, 0, 109);
 	bitmap_copy(bmap2, bmap1, 1024);
-	expect_eq_pbl("0-108", bmap2, 1024);
+	expect_eq_pbl(test, "0-108", bmap2, 1024);
 
 	bitmap_fill(bmap2, 1024);
 	bitmap_copy(bmap2, bmap1, 1024);
-	expect_eq_pbl("0-108", bmap2, 1024);
+	expect_eq_pbl(test, "0-108", bmap2, 1024);
 
 	/* the following tests assume a 32- or 64-bit arch (even 128b
 	 * if we care)
@@ -323,14 +291,14 @@ static void __init test_copy(void)
 
 	bitmap_fill(bmap2, 1024);
 	bitmap_copy(bmap2, bmap1, 109);  /* ... but 0-padded til word length */
-	expect_eq_pbl("0-108,128-1023", bmap2, 1024);
+	expect_eq_pbl(test, "0-108,128-1023", bmap2, 1024);
 
 	bitmap_fill(bmap2, 1024);
 	bitmap_copy(bmap2, bmap1, 97);  /* ... but aligned on word length */
-	expect_eq_pbl("0-108,128-1023", bmap2, 1024);
+	expect_eq_pbl(test, "0-108,128-1023", bmap2, 1024);
 }
 
-static void __init test_bitmap_region(void)
+static void test_bitmap_region(struct kunit *test)
 {
 	int pos, order;
 
@@ -341,21 +309,21 @@ static void __init test_bitmap_region(void)
 	for (order = 0; order < 10; order++) {
 		pos = bitmap_find_free_region(bmap, 1000, order);
 		if (order == 0)
-			expect_eq_uint(pos, 0);
+			expect_eq_uint(test, pos, 0);
 		else
-			expect_eq_uint(pos, order < 9 ? BIT(order) : -ENOMEM);
+			expect_eq_uint(test, pos, order < 9 ? BIT(order) : -ENOMEM);
 	}
 
 	bitmap_release_region(bmap, 0, 0);
 	for (order = 1; order < 9; order++)
 		bitmap_release_region(bmap, BIT(order), order);
 
-	expect_eq_uint(bitmap_weight(bmap, 1000), 0);
+	expect_eq_uint(test, bitmap_weight(bmap, 1000), 0);
 }
 
 #define EXP2_IN_BITS	(sizeof(exp2) * 8)
 
-static void __init test_replace(void)
+static void test_replace(struct kunit *test)
 {
 	unsigned int nbits = 64;
 	unsigned int nlongs = DIV_ROUND_UP(nbits, BITS_PER_LONG);
@@ -365,38 +333,38 @@ static void __init test_replace(void)
 
 	bitmap_zero(bmap, 1024);
 	bitmap_replace(bmap, &exp2[0 * nlongs], &exp2[1 * nlongs], exp2_to_exp3_mask, nbits);
-	expect_eq_bitmap(bmap, exp3_0_1, nbits);
+	expect_eq_bitmap(test, bmap, exp3_0_1, nbits);
 
 	bitmap_zero(bmap, 1024);
 	bitmap_replace(bmap, &exp2[1 * nlongs], &exp2[0 * nlongs], exp2_to_exp3_mask, nbits);
-	expect_eq_bitmap(bmap, exp3_1_0, nbits);
+	expect_eq_bitmap(test, bmap, exp3_1_0, nbits);
 
 	bitmap_fill(bmap, 1024);
 	bitmap_replace(bmap, &exp2[0 * nlongs], &exp2[1 * nlongs], exp2_to_exp3_mask, nbits);
-	expect_eq_bitmap(bmap, exp3_0_1, nbits);
+	expect_eq_bitmap(test, bmap, exp3_0_1, nbits);
 
 	bitmap_fill(bmap, 1024);
 	bitmap_replace(bmap, &exp2[1 * nlongs], &exp2[0 * nlongs], exp2_to_exp3_mask, nbits);
-	expect_eq_bitmap(bmap, exp3_1_0, nbits);
+	expect_eq_bitmap(test, 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(struct kunit *test)
 {
 	unsigned int nbits = 64;
 	DECLARE_BITMAP(bmap_gather, 100);
@@ -407,18 +375,18 @@ static void __init test_bitmap_sg(void)
 	/* Simple gather call */
 	bitmap_zero(bmap_gather, 100);
 	bitmap_gather(bmap_gather, sg_src, sg_mask, nbits);
-	expect_eq_bitmap(sg_gather_exp, bmap_gather, nbits);
+	expect_eq_bitmap(test, sg_gather_exp, bmap_gather, nbits);
 
 	/* Simple scatter call */
 	bitmap_zero(bmap_scatter, 100);
 	bitmap_scatter(bmap_scatter, sg_src, sg_mask, nbits);
-	expect_eq_bitmap(sg_scatter_exp, bmap_scatter, nbits);
+	expect_eq_bitmap(test, sg_scatter_exp, bmap_scatter, nbits);
 
 	/* Scatter/gather relationship */
 	bitmap_zero(bmap_tmp, 100);
 	bitmap_gather(bmap_tmp, bmap_scatter, sg_mask, nbits);
 	bitmap_scatter(bmap_res, bmap_tmp, sg_mask, nbits);
-	expect_eq_bitmap(bmap_scatter, bmap_res, nbits);
+	expect_eq_bitmap(test, bmap_scatter, bmap_res, nbits);
 }
 
 #define PARSE_TIME	0x1
@@ -432,7 +400,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},
@@ -517,7 +485,7 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
 
 };
 
-static void __init test_bitmap_parselist(void)
+static void test_bitmap_parselist(struct kunit *test)
 {
 	int i;
 	int err;
@@ -531,21 +499,14 @@ static void __init test_bitmap_parselist(void)
 		err = bitmap_parselist(ptest.in, bmap, ptest.nbits);
 		time = ktime_get() - time;
 
-		if (err != ptest.errno) {
-			pr_err("parselist: %d: input is %s, errno is %d, expected %d\n",
-					i, ptest.in, err, ptest.errno);
-			failed_tests++;
-			continue;
-		}
+		KUNIT_ASSERT_EQ_MSG(test, err, ptest.errno,
+				    "parselist: %d: input is %s, errno is %d, expected %d\n",
+				    i, ptest.in, err, ptest.errno);
 
-		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",
-					i, ptest.in, bmap[0],
-					*ptest.expected);
-			failed_tests++;
-			continue;
-		}
+		KUNIT_ASSERT_FALSE_MSG(test, !err && ptest.expected &&
+				       !__bitmap_equal(bmap, ptest.expected, ptest.nbits),
+				       "parselist: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
+				       i, ptest.in, bmap[0], *ptest.expected);
 
 		if (ptest.flags & PARSE_TIME)
 			pr_info("parselist: %d: input is '%s' OK, Time: %llu\n",
@@ -555,7 +516,7 @@ static void __init test_bitmap_parselist(void)
 	}
 }
 
-static void __init test_bitmap_printlist(void)
+static void test_bitmap_printlist(struct kunit *test)
 {
 	unsigned long *bmap = kmalloc(PAGE_SIZE, GFP_KERNEL);
 	char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
@@ -575,17 +536,13 @@ static void __init test_bitmap_printlist(void)
 	ret = bitmap_print_to_pagebuf(true, buf, bmap, PAGE_SIZE * 8);
 	time = ktime_get() - time;
 
-	if (ret != slen + 1) {
-		pr_err("bitmap_print_to_pagebuf: result is %d, expected %d\n", ret, slen);
-		failed_tests++;
-		goto out;
-	}
+	KUNIT_ASSERT_EQ_MSG(test, ret, slen + 1,
+			    "bitmap_print_to_pagebuf: result is %d, expected %d\n",
+			    ret, slen);
 
-	if (strncmp(buf, expected, slen)) {
-		pr_err("bitmap_print_to_pagebuf: result is %s, expected %s\n", buf, expected);
-		failed_tests++;
-		goto out;
-	}
+	KUNIT_ASSERT_FALSE_MSG(test, strncmp(buf, expected, slen),
+			       "bitmap_print_to_pagebuf: result is %s, expected %s\n",
+			       buf, expected);
 
 	pr_info("bitmap_print_to_pagebuf: input is '%s', Time: %llu\n", buf, time);
 out:
@@ -593,20 +550,20 @@ static void __init test_bitmap_printlist(void)
 	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},
@@ -633,7 +590,7 @@ static const struct test_bitmap_parselist parse_tests[] __initconst = {
 #undef step
 };
 
-static void __init test_bitmap_parse(void)
+static void test_bitmap_parse(struct kunit *_test)
 {
 	int i;
 	int err;
@@ -648,21 +605,14 @@ static void __init test_bitmap_parse(void)
 		err = bitmap_parse(test.in, len, bmap, test.nbits);
 		time = ktime_get() - time;
 
-		if (err != test.errno) {
-			pr_err("parse: %d: input is %s, errno is %d, expected %d\n",
-					i, test.in, err, test.errno);
-			failed_tests++;
-			continue;
-		}
+		KUNIT_ASSERT_EQ_MSG(_test, err, test.errno,
+				    "parse: %d: input is %s, errno is %d, expected %d\n",
+				     i, test.in, err, test.errno);
 
-		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",
-					i, test.in, bmap[0],
-					*test.expected);
-			failed_tests++;
-			continue;
-		}
+		KUNIT_ASSERT_FALSE_MSG(_test, !err && test.expected &&
+				       !__bitmap_equal(bmap, test.expected, test.nbits),
+				       "parse: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
+					i, test.in, bmap[0], *test.expected);
 
 		if (test.flags & PARSE_TIME)
 			pr_info("parse: %d: input is '%s' OK, Time: %llu\n",
@@ -670,7 +620,7 @@ static void __init test_bitmap_parse(void)
 	}
 }
 
-static void __init test_bitmap_arr32(void)
+static void test_bitmap_arr32(struct kunit *test)
 {
 	unsigned int nbits, next_bit;
 	u32 arr[EXP1_IN_BITS / 32];
@@ -681,24 +631,22 @@ static void __init test_bitmap_arr32(void)
 	for (nbits = 0; nbits < EXP1_IN_BITS; ++nbits) {
 		bitmap_to_arr32(arr, exp1, nbits);
 		bitmap_from_arr32(bmap2, arr, nbits);
-		expect_eq_bitmap(bmap2, exp1, nbits);
+		expect_eq_bitmap(test, bmap2, exp1, nbits);
 
 		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",
-				nbits, next_bit);
-			failed_tests++;
-		}
+
+		KUNIT_ASSERT_GE_MSG(test, next_bit, round_up(nbits, BITS_PER_LONG),
+				    "bitmap_copy_arr32(nbits == %d: tail is not safely cleared: %d\n",
+				    nbits, next_bit);
 
 		if (nbits < EXP1_IN_BITS - 32)
-			expect_eq_uint(arr[DIV_ROUND_UP(nbits, 32)],
-								0xa5a5a5a5);
+			expect_eq_uint(test, arr[DIV_ROUND_UP(nbits, 32)],
+				       0xa5a5a5a5);
 	}
 }
 
-static void __init test_bitmap_arr64(void)
+static void test_bitmap_arr64(struct kunit *test)
 {
 	unsigned int nbits, next_bit;
 	u64 arr[EXP1_IN_BITS / 64];
@@ -710,29 +658,25 @@ static void __init test_bitmap_arr64(void)
 		memset(bmap2, 0xff, sizeof(arr));
 		bitmap_to_arr64(arr, exp1, nbits);
 		bitmap_from_arr64(bmap2, arr, nbits);
-		expect_eq_bitmap(bmap2, exp1, nbits);
+		expect_eq_bitmap(test, bmap2, exp1, nbits);
 
 		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++;
-		}
+		KUNIT_ASSERT_GE_MSG(test, next_bit, round_up(nbits, BITS_PER_LONG),
+				    "bitmap_copy_arr64(nbits == %d: tail is not safely cleared: %d\n",
+				    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",
-			       nbits, arr[(nbits - 1) / 64],
-			       GENMASK_ULL((nbits - 1) % 64, 0));
-			failed_tests++;
-		}
+		KUNIT_ASSERT_FALSE_MSG(test, (nbits % 64) &&
+				       (arr[(nbits - 1) / 64] & ~GENMASK_ULL((nbits - 1) % 64, 0)),
+				       "bitmap_to_arr64(nbits == %d): tail is not safely cleared: 0x%016llx (must be 0x%016llx)\n",
+				       nbits, arr[(nbits - 1) / 64],
+				       GENMASK_ULL((nbits - 1) % 64, 0));
 
 		if (nbits < EXP1_IN_BITS - 64)
-			expect_eq_uint(arr[DIV_ROUND_UP(nbits, 64)], 0xa5a5a5a5);
+			expect_eq_uint(test, arr[DIV_ROUND_UP(nbits, 64)], 0xa5a5a5a5);
 	}
 }
 
-static void noinline __init test_mem_optimisations(void)
+static noinline void test_mem_optimisations(struct kunit *test)
 {
 	DECLARE_BITMAP(bmap1, 1024);
 	DECLARE_BITMAP(bmap2, 1024);
@@ -745,31 +689,26 @@ 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++;
-			}
-			if (!__bitmap_equal(bmap1, bmap2, 1024)) {
-				printk("set not __equal %d %d\n", start, nbits);
-				failed_tests++;
-			}
+
+			KUNIT_ASSERT_TRUE_MSG(test, bitmap_equal(bmap1, bmap2, 1024),
+					      "set not equal %d %d\n", start, nbits);
+
+			KUNIT_ASSERT_TRUE_MSG(test, __bitmap_equal(bmap1, bmap2, 1024),
+					      "set not __equal %d %d\n", 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++;
-			}
-			if (!__bitmap_equal(bmap1, bmap2, 1024)) {
-				printk("clear not __equal %d %d\n", start,
-									nbits);
-				failed_tests++;
-			}
+
+			KUNIT_ASSERT_TRUE_MSG(test, bitmap_equal(bmap1, bmap2, 1024),
+					      "clear not equal %d %d\n", start, nbits);
+
+			KUNIT_ASSERT_TRUE_MSG(test, __bitmap_equal(bmap1, bmap2, 1024),
+					      "clear not __equal %d %d\n", 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 */
@@ -780,7 +719,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(struct kunit *test)
 {
 #define CLUMP_EXP_NUMBITS 64
 	DECLARE_BITMAP(bits, CLUMP_EXP_NUMBITS);
@@ -799,10 +738,10 @@ static void __init test_for_each_set_clump8(void)
 	bitmap_set(bits, 58, 1);	/* 0x05 - part 2 */
 
 	for_each_set_clump8(start, clump, bits, CLUMP_EXP_NUMBITS)
-		expect_eq_clump8(start, CLUMP_EXP_NUMBITS, clump_exp, &clump);
+		expect_eq_clump8(test, 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(struct kunit *test)
 {
 	DECLARE_BITMAP(orig, 500);
 	DECLARE_BITMAP(copy, 500);
@@ -823,11 +762,11 @@ static void __init test_for_each_set_bit_wrap(void)
 		for_each_set_bit_wrap(bit, orig, 500, wr)
 			bitmap_set(copy, bit, 1);
 
-		expect_eq_bitmap(orig, copy, 500);
+		expect_eq_bitmap(test, orig, copy, 500);
 	}
 }
 
-static void __init test_for_each_set_bit(void)
+static void test_for_each_set_bit(struct kunit *test)
 {
 	DECLARE_BITMAP(orig, 500);
 	DECLARE_BITMAP(copy, 500);
@@ -846,10 +785,10 @@ static void __init test_for_each_set_bit(void)
 	for_each_set_bit(bit, orig, 500)
 		bitmap_set(copy, bit, 1);
 
-	expect_eq_bitmap(orig, copy, 500);
+	expect_eq_bitmap(test, orig, copy, 500);
 }
 
-static void __init test_for_each_set_bit_from(void)
+static void test_for_each_set_bit_from(struct kunit *test)
 {
 	DECLARE_BITMAP(orig, 500);
 	DECLARE_BITMAP(copy, 500);
@@ -875,11 +814,11 @@ static void __init test_for_each_set_bit_from(void)
 
 		bitmap_copy(tmp, orig, 500);
 		bitmap_clear(tmp, 0, wr);
-		expect_eq_bitmap(tmp, copy, 500);
+		expect_eq_bitmap(test, tmp, copy, 500);
 	}
 }
 
-static void __init test_for_each_clear_bit(void)
+static void test_for_each_clear_bit(struct kunit *test)
 {
 	DECLARE_BITMAP(orig, 500);
 	DECLARE_BITMAP(copy, 500);
@@ -898,10 +837,10 @@ static void __init test_for_each_clear_bit(void)
 	for_each_clear_bit(bit, orig, 500)
 		bitmap_clear(copy, bit, 1);
 
-	expect_eq_bitmap(orig, copy, 500);
+	expect_eq_bitmap(test, orig, copy, 500);
 }
 
-static void __init test_for_each_clear_bit_from(void)
+static void test_for_each_clear_bit_from(struct kunit *test)
 {
 	DECLARE_BITMAP(orig, 500);
 	DECLARE_BITMAP(copy, 500);
@@ -927,11 +866,11 @@ static void __init test_for_each_clear_bit_from(void)
 
 		bitmap_copy(tmp, orig, 500);
 		bitmap_set(tmp, 0, wr);
-		expect_eq_bitmap(tmp, copy, 500);
+		expect_eq_bitmap(test, tmp, copy, 500);
 	}
 }
 
-static void __init test_for_each_set_bitrange(void)
+static void test_for_each_set_bitrange(struct kunit *test)
 {
 	DECLARE_BITMAP(orig, 500);
 	DECLARE_BITMAP(copy, 500);
@@ -950,10 +889,10 @@ static void __init test_for_each_set_bitrange(void)
 	for_each_set_bitrange(s, e, orig, 500)
 		bitmap_set(copy, s, e-s);
 
-	expect_eq_bitmap(orig, copy, 500);
+	expect_eq_bitmap(test, orig, copy, 500);
 }
 
-static void __init test_for_each_clear_bitrange(void)
+static void test_for_each_clear_bitrange(struct kunit *test)
 {
 	DECLARE_BITMAP(orig, 500);
 	DECLARE_BITMAP(copy, 500);
@@ -972,10 +911,10 @@ static void __init test_for_each_clear_bitrange(void)
 	for_each_clear_bitrange(s, e, orig, 500)
 		bitmap_clear(copy, s, e-s);
 
-	expect_eq_bitmap(orig, copy, 500);
+	expect_eq_bitmap(test, orig, copy, 500);
 }
 
-static void __init test_for_each_set_bitrange_from(void)
+static void test_for_each_set_bitrange_from(struct kunit *test)
 {
 	DECLARE_BITMAP(orig, 500);
 	DECLARE_BITMAP(copy, 500);
@@ -1001,11 +940,11 @@ static void __init test_for_each_set_bitrange_from(void)
 
 		bitmap_copy(tmp, orig, 500);
 		bitmap_clear(tmp, 0, wr);
-		expect_eq_bitmap(tmp, copy, 500);
+		expect_eq_bitmap(test, tmp, copy, 500);
 	}
 }
 
-static void __init test_for_each_clear_bitrange_from(void)
+static void test_for_each_clear_bitrange_from(struct kunit *test)
 {
 	DECLARE_BITMAP(orig, 500);
 	DECLARE_BITMAP(copy, 500);
@@ -1031,7 +970,7 @@ static void __init test_for_each_clear_bitrange_from(void)
 
 		bitmap_copy(tmp, orig, 500);
 		bitmap_set(tmp, 0, wr);
-		expect_eq_bitmap(tmp, copy, 500);
+		expect_eq_bitmap(test, tmp, copy, 500);
 	}
 }
 
@@ -1076,7 +1015,7 @@ static struct test_bitmap_cut test_cut[] = {
 	},
 };
 
-static void __init test_bitmap_cut(void)
+static void test_bitmap_cut(struct kunit *test)
 {
 	unsigned long b[5], *in = &b[1], *out = &b[0];	/* Partial overlap */
 	int i;
@@ -1088,7 +1027,7 @@ static void __init test_bitmap_cut(void)
 
 		bitmap_cut(out, in, t->first, t->cut, t->nbits);
 
-		expect_eq_bitmap(t->expected, out, t->nbits);
+		expect_eq_bitmap(test, t->expected, out, t->nbits);
 	}
 }
 
@@ -1099,14 +1038,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),
@@ -1129,28 +1068,28 @@ 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,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111,"
-					"33333333,11111111,33333333,11111111\n";
-
-static const char large_list[] __initconst = /* more than 4KB */
+static const char large_mask[] = "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111,"
+				 "33333333,11111111,33333333,11111111\n";
+
+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"
@@ -1192,12 +1131,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(struct kunit *test)
 {
 	int i;
 
@@ -1207,20 +1146,20 @@ static void __init test_bitmap_print_buf(void)
 
 		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);
+		expect_eq_uint(test, strlen(t->mask) + 1, n);
+		expect_eq_str(test, 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);
+		expect_eq_uint(test, strlen(t->list) + 1, n);
+		expect_eq_str(test, 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);
+			expect_eq_uint(test, strlen(t->list) + 1 - PAGE_SIZE, n);
+			expect_eq_str(test, t->list + PAGE_SIZE, print_buf, n);
 		}
 	}
 }
@@ -1229,7 +1168,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(struct kunit *test)
 {
 	DECLARE_BITMAP(bitmap, BITS_PER_LONG);
 	unsigned long initvar = BIT(2);
@@ -1297,7 +1236,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(struct kunit *test, const char *pattern)
 {
 	DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
 	DECLARE_BITMAP(exp_bitmap, TEST_BIT_LEN);
@@ -1321,7 +1260,7 @@ static void __init test_bitmap_write_helper(const char *pattern)
 		for (bit = 0; bit <= 1; bit++) {
 			bitmap_write(bitmap, bit, i, 1);
 			__assign_bit(i, exp_bitmap, bit);
-			expect_eq_bitmap(exp_bitmap, bitmap,
+			expect_eq_bitmap(test, exp_bitmap, bitmap,
 					 TEST_BIT_LEN);
 		}
 	}
@@ -1331,7 +1270,7 @@ static void __init test_bitmap_write_helper(const char *pattern)
 	bitmap_copy(exp_bitmap, pat_bitmap, TEST_BIT_LEN);
 	for (i = 0; i < TEST_BIT_LEN; i++) {
 		bitmap_write(bitmap, ~0UL, i, 0);
-		expect_eq_bitmap(exp_bitmap, bitmap, TEST_BIT_LEN);
+		expect_eq_bitmap(test, exp_bitmap, bitmap, TEST_BIT_LEN);
 	}
 
 	for (nbits = BITS_PER_LONG; nbits >= 1; nbits--) {
@@ -1344,14 +1283,14 @@ static void __init test_bitmap_write_helper(const char *pattern)
 			for (n = 0; n < nbits; n++)
 				__assign_bit(i + n, exp_bitmap, w & BIT(n));
 			bitmap_write(bitmap, w, i, nbits);
-			expect_eq_bitmap(exp_bitmap, bitmap, TEST_BIT_LEN);
+			expect_eq_bitmap(test, exp_bitmap, bitmap, TEST_BIT_LEN);
 			r = bitmap_read(bitmap, i, nbits);
-			expect_eq_ulong(r, w);
+			expect_eq_ulong(test, r, w);
 		}
 	}
 }
 
-static void __init test_bitmap_read_write(void)
+static void test_bitmap_read_write(struct kunit *test)
 {
 	unsigned char *pattern[3] = {"", "all:1/2", "all"};
 	DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
@@ -1386,21 +1325,21 @@ static void __init test_bitmap_read_write(void)
 
 		bitmap_write(bitmap, 0b10101UL, i, 5);
 		val = bitmap_read(bitmap, i, 5);
-		expect_eq_ulong(0b10101UL, val);
+		expect_eq_ulong(test, 0b10101UL, val);
 
 		bitmap_write(bitmap, 0b101UL, i + 5, 3);
 		val = bitmap_read(bitmap, i + 5, 3);
-		expect_eq_ulong(0b101UL, val);
+		expect_eq_ulong(test, 0b101UL, val);
 
 		val = bitmap_read(bitmap, i, 8);
-		expect_eq_ulong(0b10110101UL, val);
+		expect_eq_ulong(test, 0b10110101UL, val);
 	}
 
 	for (pi = 0; pi < ARRAY_SIZE(pattern); pi++)
-		test_bitmap_write_helper(pattern[pi]);
+		test_bitmap_write_helper(test, pattern[pi]);
 }
 
-static void __init test_bitmap_read_perf(void)
+static void test_bitmap_read_perf(struct kunit *test)
 {
 	DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
 	unsigned int cnt, nbits, i;
@@ -1426,7 +1365,7 @@ static void __init test_bitmap_read_perf(void)
 	pr_info("Time spent in %s:\t%llu\n", __func__, time);
 }
 
-static void __init test_bitmap_write_perf(void)
+static void test_bitmap_write_perf(struct kunit *test)
 {
 	DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
 	unsigned int cnt, nbits, i;
@@ -1450,41 +1389,46 @@ static void __init test_bitmap_write_perf(void)
 
 #undef TEST_BIT_LEN
 
-static void __init selftest(void)
-{
-	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(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(test_bitmap_print_buf),
+	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),
+	{}
+};
+
+static struct kunit_suite bitmap_test_suite = {
+	.name = "bitmap",
+	.test_cases = bitmap_test_cases,
+};
 
-KSTM_MODULE_LOADERS(test_bitmap);
+kunit_test_suite(bitmap_test_suite);
 MODULE_AUTHOR("david decotigny <david.decotigny@googlers.com>");
 MODULE_DESCRIPTION("Test cases for bitmap API");
 MODULE_LICENSE("GPL");
-- 
2.39.2

[PATCH 2/3] bitmap: Rename module

[Muhammad Usama Anjum]

Rename module to bitmap_kunit and rename the configuration option
compliant with kunit framework.

Cc: kees@kernel.org
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 MAINTAINERS                           |  2 +-
 lib/Kconfig.debug                     | 15 ++++++++-------
 lib/Makefile                          |  2 +-
 lib/{test_bitmap.c => bitmap_kunit.c} |  0
 4 files changed, 10 insertions(+), 9 deletions(-)
 rename lib/{test_bitmap.c => bitmap_kunit.c} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 12b870712da4a..289b727344d64 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3814,13 +3814,13 @@ F:	include/linux/find.h
 F:	include/linux/nodemask.h
 F:	include/linux/nodemask_types.h
 F:	include/vdso/bits.h
+F:	lib/bitmap_kunit.c
 F:	lib/bitmap-str.c
 F:	lib/bitmap.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/lib/Kconfig.debug b/lib/Kconfig.debug
index a30c03a661726..6bb02990a73e7 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2420,13 +2420,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"
 
@@ -2813,6 +2806,14 @@ config USERCOPY_KUNIT_TEST
 	  on the copy_to/from_user infrastructure, making sure basic
 	  user/kernel boundary testing is working.
 
+config BITMAP_KUNIT_TEST
+	tristate "KUnit Test for bitmap_*() family of functions"
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the "bitmap_kunit" module that runs tests for
+	  bitmaps int the kernel making sure that there isn't any bug.
+
 config TEST_UDELAY
 	tristate "udelay test driver"
 	help
diff --git a/lib/Makefile b/lib/Makefile
index 322bb127b4dc6..37e7359a7065e 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -84,7 +84,6 @@ 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
 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
@@ -388,6 +387,7 @@ CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN)
 obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
 obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
 obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
+obj-$(CONFIG_BITMAP_KUNIT_TEST) += bitmap_kunit.o
 
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
 
diff --git a/lib/test_bitmap.c b/lib/bitmap_kunit.c
similarity index 100%
rename from lib/test_bitmap.c
rename to lib/bitmap_kunit.c
-- 
2.39.2

Re: [PATCH 2/3] bitmap: Rename module

[John Hubbard]

On 7/26/24 4:06 AM, Muhammad Usama Anjum wrote:
> Rename module to bitmap_kunit and rename the configuration option
> compliant with kunit framework.
> 
> Cc: kees@kernel.org
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>   MAINTAINERS                           |  2 +-
>   lib/Kconfig.debug                     | 15 ++++++++-------
>   lib/Makefile                          |  2 +-
>   lib/{test_bitmap.c => bitmap_kunit.c} |  0
>   4 files changed, 10 insertions(+), 9 deletions(-)
>   rename lib/{test_bitmap.c => bitmap_kunit.c} (100%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 12b870712da4a..289b727344d64 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3814,13 +3814,13 @@ F:	include/linux/find.h
>   F:	include/linux/nodemask.h
>   F:	include/linux/nodemask_types.h
>   F:	include/vdso/bits.h
> +F:	lib/bitmap_kunit.c
>   F:	lib/bitmap-str.c
>   F:	lib/bitmap.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

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

thanks,
-- 
John Hubbard
NVIDIA


>   F:	tools/include/linux/bitfield.h
>   F:	tools/include/linux/bitmap.h
>   F:	tools/include/linux/bits.h
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index a30c03a661726..6bb02990a73e7 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2420,13 +2420,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"
>   
> @@ -2813,6 +2806,14 @@ config USERCOPY_KUNIT_TEST
>   	  on the copy_to/from_user infrastructure, making sure basic
>   	  user/kernel boundary testing is working.
>   
> +config BITMAP_KUNIT_TEST
> +	tristate "KUnit Test for bitmap_*() family of functions"
> +	depends on KUNIT
> +	default KUNIT_ALL_TESTS
> +	help
> +	  This builds the "bitmap_kunit" module that runs tests for
> +	  bitmaps int the kernel making sure that there isn't any bug.
> +
>   config TEST_UDELAY
>   	tristate "udelay test driver"
>   	help
> diff --git a/lib/Makefile b/lib/Makefile
> index 322bb127b4dc6..37e7359a7065e 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -84,7 +84,6 @@ 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
>   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
> @@ -388,6 +387,7 @@ CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN)
>   obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
>   obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
>   obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
> +obj-$(CONFIG_BITMAP_KUNIT_TEST) += bitmap_kunit.o
>   
>   obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
>   
> diff --git a/lib/test_bitmap.c b/lib/bitmap_kunit.c
> similarity index 100%
> rename from lib/test_bitmap.c
> rename to lib/bitmap_kunit.c

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 2/3] bitmap: Rename module

[Muhammad Usama Anjum]

On 7/26/24 11:45 PM, John Hubbard wrote:
> On 7/26/24 4:06 AM, Muhammad Usama Anjum wrote:
>> Rename module to bitmap_kunit and rename the configuration option
>> compliant with kunit framework.
>>
>> Cc: kees@kernel.org
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>>   MAINTAINERS                           |  2 +-
>>   lib/Kconfig.debug                     | 15 ++++++++-------
>>   lib/Makefile                          |  2 +-
>>   lib/{test_bitmap.c => bitmap_kunit.c} |  0
>>   4 files changed, 10 insertions(+), 9 deletions(-)
>>   rename lib/{test_bitmap.c => bitmap_kunit.c} (100%)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 12b870712da4a..289b727344d64 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3814,13 +3814,13 @@ F:    include/linux/find.h
>>   F:    include/linux/nodemask.h
>>   F:    include/linux/nodemask_types.h
>>   F:    include/vdso/bits.h
>> +F:    lib/bitmap_kunit.c
>>   F:    lib/bitmap-str.c
>>   F:    lib/bitmap.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
> 
> 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?
Yes, Thank you for mentioning it. I'd missed the naming scheme. I'll fix it.

> 
> So:
> 
>     lib/tests/bitmap_kunit.c
> 
> 
> [1] https://lore.kernel.org/20240724201354.make.730-kees@kernel.org
> 
> thanks,

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 2/3] bitmap: Rename module

[Shuah Khan]

On 7/26/24 05:06, Muhammad Usama Anjum wrote:
> Rename module to bitmap_kunit and rename the configuration option
> compliant with kunit framework.
> 
> Cc: kees@kernel.org
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>   MAINTAINERS                           |  2 +-
>   lib/Kconfig.debug                     | 15 ++++++++-------
>   lib/Makefile                          |  2 +-
>   lib/{test_bitmap.c => bitmap_kunit.c} |  0
>   4 files changed, 10 insertions(+), 9 deletions(-)
>   rename lib/{test_bitmap.c => bitmap_kunit.c} (100%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 12b870712da4a..289b727344d64 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3814,13 +3814,13 @@ F:	include/linux/find.h
>   F:	include/linux/nodemask.h
>   F:	include/linux/nodemask_types.h
>   F:	include/vdso/bits.h
> +F:	lib/bitmap_kunit.c
>   F:	lib/bitmap-str.c
>   F:	lib/bitmap.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/lib/Kconfig.debug b/lib/Kconfig.debug
> index a30c03a661726..6bb02990a73e7 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2420,13 +2420,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.
> -

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.

thanks,
-- Shuah

Re: [PATCH 2/3] bitmap: Rename module

[Muhammad Usama Anjum]

On 7/27/24 12:24 AM, Shuah Khan wrote:
> On 7/26/24 05:06, Muhammad Usama Anjum wrote:
>> Rename module to bitmap_kunit and rename the configuration option
>> compliant with kunit framework.
>>
>> Cc: kees@kernel.org
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>>   MAINTAINERS                           |  2 +-
>>   lib/Kconfig.debug                     | 15 ++++++++-------
>>   lib/Makefile                          |  2 +-
>>   lib/{test_bitmap.c => bitmap_kunit.c} |  0
>>   4 files changed, 10 insertions(+), 9 deletions(-)
>>   rename lib/{test_bitmap.c => bitmap_kunit.c} (100%)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 12b870712da4a..289b727344d64 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3814,13 +3814,13 @@ F:    include/linux/find.h
>>   F:    include/linux/nodemask.h
>>   F:    include/linux/nodemask_types.h
>>   F:    include/vdso/bits.h
>> +F:    lib/bitmap_kunit.c
>>   F:    lib/bitmap-str.c
>>   F:    lib/bitmap.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/lib/Kconfig.debug b/lib/Kconfig.debug
>> index a30c03a661726..6bb02990a73e7 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -2420,13 +2420,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.
>> -
> 
> This change will take away the ability to run bitmap tests during
> boot on a non-kunit kernel.
Kees, what opinion do you have on this? [1] has all the discussion though
and my recent thoughts on why I sent this patch.

> 
> 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.
> 
> thanks,
> -- Shuah
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 2/3] bitmap: Rename module

[Yury Norov]

On Fri, Jul 26, 2024 at 04:06:57PM +0500, Muhammad Usama Anjum wrote:
> Rename module to bitmap_kunit and rename the configuration option
> compliant with kunit framework.

... , so those enabling bitmaps testing in their configs by setting
"CONFIG_TEST_BITMAP=y" will suddenly get it broken, and will likely
not realize it until something nasty will happen.

Sorry, NAK for config rename.
 
> Cc: kees@kernel.org
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>  MAINTAINERS                           |  2 +-
>  lib/Kconfig.debug                     | 15 ++++++++-------
>  lib/Makefile                          |  2 +-
>  lib/{test_bitmap.c => bitmap_kunit.c} |  0
>  4 files changed, 10 insertions(+), 9 deletions(-)
>  rename lib/{test_bitmap.c => bitmap_kunit.c} (100%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 12b870712da4a..289b727344d64 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3814,13 +3814,13 @@ F:	include/linux/find.h
>  F:	include/linux/nodemask.h
>  F:	include/linux/nodemask_types.h
>  F:	include/vdso/bits.h
> +F:	lib/bitmap_kunit.c
>  F:	lib/bitmap-str.c
>  F:	lib/bitmap.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/lib/Kconfig.debug b/lib/Kconfig.debug
> index a30c03a661726..6bb02990a73e7 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2420,13 +2420,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"
>  
> @@ -2813,6 +2806,14 @@ config USERCOPY_KUNIT_TEST
>  	  on the copy_to/from_user infrastructure, making sure basic
>  	  user/kernel boundary testing is working.
>  
> +config BITMAP_KUNIT_TEST
> +	tristate "KUnit Test for bitmap_*() family of functions"
> +	depends on KUNIT
> +	default KUNIT_ALL_TESTS
> +	help
> +	  This builds the "bitmap_kunit" module that runs tests for
> +	  bitmaps int the kernel making sure that there isn't any bug.
> +
>  config TEST_UDELAY
>  	tristate "udelay test driver"
>  	help
> diff --git a/lib/Makefile b/lib/Makefile
> index 322bb127b4dc6..37e7359a7065e 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -84,7 +84,6 @@ 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
>  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
> @@ -388,6 +387,7 @@ CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN)
>  obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
>  obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
>  obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
> +obj-$(CONFIG_BITMAP_KUNIT_TEST) += bitmap_kunit.o
>  
>  obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
>  
> diff --git a/lib/test_bitmap.c b/lib/bitmap_kunit.c
> similarity index 100%
> rename from lib/test_bitmap.c
> rename to lib/bitmap_kunit.c
> -- 
> 2.39.2

Re: [PATCH 2/3] bitmap: Rename module

[Muhammad Usama Anjum]

On 7/27/24 10:35 PM, Yury Norov wrote:
> On Fri, Jul 26, 2024 at 04:06:57PM +0500, Muhammad Usama Anjum wrote:
>> Rename module to bitmap_kunit and rename the configuration option
>> compliant with kunit framework.
> 
> ... , so those enabling bitmaps testing in their configs by setting
> "CONFIG_TEST_BITMAP=y" will suddenly get it broken, and will likely
> not realize it until something nasty will happen.
CONFIG_TEST_BITMAP was being enabled by the kselftest suite lib. The bitmap
test and its config option would disappear. The same test can be run by
just enabling KUNIT default config option:

KUNIT_ALL_TESTS=y enables this bitmap config by default.

> 
> Sorry, NAK for config rename.
>  
>> Cc: kees@kernel.org
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>>  MAINTAINERS                           |  2 +-
>>  lib/Kconfig.debug                     | 15 ++++++++-------
>>  lib/Makefile                          |  2 +-
>>  lib/{test_bitmap.c => bitmap_kunit.c} |  0
>>  4 files changed, 10 insertions(+), 9 deletions(-)
>>  rename lib/{test_bitmap.c => bitmap_kunit.c} (100%)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 12b870712da4a..289b727344d64 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3814,13 +3814,13 @@ F:	include/linux/find.h
>>  F:	include/linux/nodemask.h
>>  F:	include/linux/nodemask_types.h
>>  F:	include/vdso/bits.h
>> +F:	lib/bitmap_kunit.c
>>  F:	lib/bitmap-str.c
>>  F:	lib/bitmap.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/lib/Kconfig.debug b/lib/Kconfig.debug
>> index a30c03a661726..6bb02990a73e7 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -2420,13 +2420,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"
>>  
>> @@ -2813,6 +2806,14 @@ config USERCOPY_KUNIT_TEST
>>  	  on the copy_to/from_user infrastructure, making sure basic
>>  	  user/kernel boundary testing is working.
>>  
>> +config BITMAP_KUNIT_TEST
>> +	tristate "KUnit Test for bitmap_*() family of functions"
>> +	depends on KUNIT
>> +	default KUNIT_ALL_TESTS
>> +	help
>> +	  This builds the "bitmap_kunit" module that runs tests for
>> +	  bitmaps int the kernel making sure that there isn't any bug.
>> +
>>  config TEST_UDELAY
>>  	tristate "udelay test driver"
>>  	help
>> diff --git a/lib/Makefile b/lib/Makefile
>> index 322bb127b4dc6..37e7359a7065e 100644
>> --- a/lib/Makefile
>> +++ b/lib/Makefile
>> @@ -84,7 +84,6 @@ 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
>>  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
>> @@ -388,6 +387,7 @@ CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN)
>>  obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
>>  obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
>>  obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
>> +obj-$(CONFIG_BITMAP_KUNIT_TEST) += bitmap_kunit.o
>>  
>>  obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
>>  
>> diff --git a/lib/test_bitmap.c b/lib/bitmap_kunit.c
>> similarity index 100%
>> rename from lib/test_bitmap.c
>> rename to lib/bitmap_kunit.c
>> -- 
>> 2.39.2
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 2/3] bitmap: Rename module

[Randy Dunlap]

On 7/29/24 1:07 AM, Muhammad Usama Anjum wrote:
> On 7/27/24 10:35 PM, Yury Norov wrote:
>> On Fri, Jul 26, 2024 at 04:06:57PM +0500, Muhammad Usama Anjum wrote:
>>> Rename module to bitmap_kunit and rename the configuration option
>>> compliant with kunit framework.
>>
>> ... , so those enabling bitmaps testing in their configs by setting
>> "CONFIG_TEST_BITMAP=y" will suddenly get it broken, and will likely
>> not realize it until something nasty will happen.
> CONFIG_TEST_BITMAP was being enabled by the kselftest suite lib. The bitmap
> test and its config option would disappear. The same test can be run by
> just enabling KUNIT default config option:
> 
> KUNIT_ALL_TESTS=y enables this bitmap config by default.
> 
>>
>> Sorry, NAK for config rename.
>>  

I agree with Yury. Using KUNIT takes away test coverage for people who
are willing to run selftests but not use KUNIT.


-- 
~Randy

Re: [PATCH 2/3] bitmap: Rename module

[Muhammad Usama Anjum]

On 7/29/24 7:09 PM, Randy Dunlap wrote:
> 
> 
> On 7/29/24 1:07 AM, Muhammad Usama Anjum wrote:
>> On 7/27/24 10:35 PM, Yury Norov wrote:
>>> On Fri, Jul 26, 2024 at 04:06:57PM +0500, Muhammad Usama Anjum wrote:
>>>> Rename module to bitmap_kunit and rename the configuration option
>>>> compliant with kunit framework.
>>>
>>> ... , so those enabling bitmaps testing in their configs by setting
>>> "CONFIG_TEST_BITMAP=y" will suddenly get it broken, and will likely
>>> not realize it until something nasty will happen.
>> CONFIG_TEST_BITMAP was being enabled by the kselftest suite lib. The bitmap
>> test and its config option would disappear. The same test can be run by
>> just enabling KUNIT default config option:
>>
>> KUNIT_ALL_TESTS=y enables this bitmap config by default.
>>
>>>
>>> Sorry, NAK for config rename.
>>>  
> 
> I agree with Yury. Using KUNIT takes away test coverage for people who
> are willing to run selftests but not use KUNIT.
How is a kselftest useful when it doesn't even check results of the tests
and report failures when they happen?

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 2/3] bitmap: Rename module

[Randy Dunlap]

On 7/30/24 12:51 AM, Muhammad Usama Anjum wrote:
> On 7/29/24 7:09 PM, Randy Dunlap wrote:
>>
>>
>> On 7/29/24 1:07 AM, Muhammad Usama Anjum wrote:
>>> On 7/27/24 10:35 PM, Yury Norov wrote:
>>>> On Fri, Jul 26, 2024 at 04:06:57PM +0500, Muhammad Usama Anjum wrote:
>>>>> Rename module to bitmap_kunit and rename the configuration option
>>>>> compliant with kunit framework.
>>>>
>>>> ... , so those enabling bitmaps testing in their configs by setting
>>>> "CONFIG_TEST_BITMAP=y" will suddenly get it broken, and will likely
>>>> not realize it until something nasty will happen.
>>> CONFIG_TEST_BITMAP was being enabled by the kselftest suite lib. The bitmap
>>> test and its config option would disappear. The same test can be run by
>>> just enabling KUNIT default config option:
>>>
>>> KUNIT_ALL_TESTS=y enables this bitmap config by default.
>>>
>>>>
>>>> Sorry, NAK for config rename.
>>>>  
>>
>> I agree with Yury. Using KUNIT takes away test coverage for people who
>> are willing to run selftests but not use KUNIT.
> How is a kselftest useful when it doesn't even check results of the tests
> and report failures when they happen?

That should be an easy fix then.

-- 
~Randy

Re: [PATCH 2/3] bitmap: Rename module

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 2453 bytes --]

On Mon, 29 Jul 2024 at 22:09, Randy Dunlap <rdunlap@infradead.org> wrote:
>
>
>
> On 7/29/24 1:07 AM, Muhammad Usama Anjum wrote:
> > On 7/27/24 10:35 PM, Yury Norov wrote:
> >> On Fri, Jul 26, 2024 at 04:06:57PM +0500, Muhammad Usama Anjum wrote:
> >>> Rename module to bitmap_kunit and rename the configuration option
> >>> compliant with kunit framework.
> >>
> >> ... , so those enabling bitmaps testing in their configs by setting
> >> "CONFIG_TEST_BITMAP=y" will suddenly get it broken, and will likely
> >> not realize it until something nasty will happen.
> > CONFIG_TEST_BITMAP was being enabled by the kselftest suite lib. The bitmap
> > test and its config option would disappear. The same test can be run by
> > just enabling KUNIT default config option:
> >
> > KUNIT_ALL_TESTS=y enables this bitmap config by default.
> >
> >>
> >> Sorry, NAK for config rename.
> >>
>
> I agree with Yury. Using KUNIT takes away test coverage for people who
> are willing to run selftests but not use KUNIT.

I can see the point that renaming the config option is just churn, but
is there a reason people would run the bitmap selftest but be unable
or unwilling to use KUnit?

Beyond a brief period of adjustment (which could probably be made
quite minimal with a wrapper script or something), there shouldn't
really be any fundamental difference: KUnit tests can already run at
boot, be configured with a config option, and write output to the
kernel log. There's nothing really being taken away here, and the
bonus of having easier access to run the tests with KUnit's tooling
(or have them automatically run by systems which run KUnit tests)
would seem worthwhile to me, especially since it's optional. And
CONFIG_KUNIT shouldn't be heavy enough to cause problems.

Obviously I can't force people to use KUnit, but this is exactly the
sort of test which would fit KUnit well, and -- forgive me if I'm
missing something -- the only real argument against it I'm hearing is
"it's different". And while that's valid (as I said, churn for churn's
sake isn't great), none of the "people who are willing to run
selftests but not use KUnit" have given reasons why. Especially since
this is the sort of test (testing in-kernel functions) we're
encouraging people to write with KUnit in
Documentation/dev-tools/testing-overview.rst -- if there are good
reasons people are refusing to run these, maybe we need to fix those
or change the recommendation.

-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4014 bytes --]

Re: [PATCH 2/3] bitmap: Rename module

[Shuah Khan]

On 7/30/24 04:10, David Gow wrote:
> On Mon, 29 Jul 2024 at 22:09, Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>>
>>
>> On 7/29/24 1:07 AM, Muhammad Usama Anjum wrote:
>>> On 7/27/24 10:35 PM, Yury Norov wrote:
>>>> On Fri, Jul 26, 2024 at 04:06:57PM +0500, Muhammad Usama Anjum wrote:
>>>>> Rename module to bitmap_kunit and rename the configuration option
>>>>> compliant with kunit framework.
>>>>
>>>> ... , so those enabling bitmaps testing in their configs by setting
>>>> "CONFIG_TEST_BITMAP=y" will suddenly get it broken, and will likely
>>>> not realize it until something nasty will happen.
>>> CONFIG_TEST_BITMAP was being enabled by the kselftest suite lib. The bitmap
>>> test and its config option would disappear. The same test can be run by
>>> just enabling KUNIT default config option:
>>>
>>> KUNIT_ALL_TESTS=y enables this bitmap config by default.
>>>
>>>>
>>>> Sorry, NAK for config rename.
>>>>
>>
>> I agree with Yury. Using KUNIT takes away test coverage for people who
>> are willing to run selftests but not use KUNIT.

This is incorrect. There are selftest that are used to

- regression test a subsystem or a abi during boot
- spot check on a running system to debug and test by loading
   a test module.

> 
> I can see the point that renaming the config option is just churn, but
> is there a reason people would run the bitmap selftest but be unable
> or unwilling to use KUnit?
> 
> Beyond a brief period of adjustment (which could probably be made
> quite minimal with a wrapper script or something), there shouldn't
> really be any fundamental difference: KUnit tests can already run at
> boot, be configured with a config option, and write output to the
> kernel log. There's nothing really being taken away here, and the
> bonus of having easier access to run the tests with KUnit's tooling
> (or have them automatically run by systems which run KUnit tests)
> would seem worthwhile to me, especially since it's optional. And
> CONFIG_KUNIT shouldn't be heavy enough to cause problems.
> 
> Obviously I can't force people to use KUnit, but this is exactly the
> sort of test which would fit KUnit well, and -- forgive me if I'm
> missing something -- the only real argument against it I'm hearing is
> "it's different". And while that's valid (as I said, churn for churn's
> sake isn't great), none of the "people who are willing to run
> selftests but not use KUnit" have given reasons why. Especially since
> this is the sort of test (testing in-kernel functions) we're
> encouraging people to write with KUnit in
> Documentation/dev-tools/testing-overview.rst -- if there are good
> reasons people are refusing to run these, maybe we need to fix those
> or change the recommendation.

It isn't about kunit vs. kselftest. It is about overall kernel validation
and ability to test effectively by developers and users.

KUnit isn't ideal for cases where people would want to check a subsystem
on a running kernel - KUnit covers some use-cases and kselftest covers
others.

What happens if we are debugging a problem that requires us to debug on
a running system? Please don't go converting kselftest into kunit without
understanding how these frameworks are intended to be used.

Yes kselftest results need to be looked at. Write a parser which can
be improved. What you are doing is reducing the coverage and talking
away the ability to debug and test on running system.

Fix what needs to be fixed instead of deleting tests.

Think through the use-cases before advocating KUnit is suitable for
all testing needs and talking about being able to force or not force
people to use one or the other.

Reports aren't everything. The primary reason we have these tests is for
developers to be able to test. Reports can be improved and shouldn't
come at the expense of coverage and testing. Any patch that does that
will be NACKed.

thanks,
-- Shuah

Re: [PATCH 2/3] bitmap: Rename module

[Shuah Khan]

On 7/30/24 09:55, Shuah Khan wrote:
> On 7/30/24 04:10, David Gow wrote:
>> On Mon, 29 Jul 2024 at 22:09, Randy Dunlap <rdunlap@infradead.org> wrote:
>>>
>>>
>>>
>>> On 7/29/24 1:07 AM, Muhammad Usama Anjum wrote:
>>>> On 7/27/24 10:35 PM, Yury Norov wrote:
>>>>> On Fri, Jul 26, 2024 at 04:06:57PM +0500, Muhammad Usama Anjum wrote:
>>>>>> Rename module to bitmap_kunit and rename the configuration option
>>>>>> compliant with kunit framework.
>>>>>
>>>>> ... , so those enabling bitmaps testing in their configs by setting
>>>>> "CONFIG_TEST_BITMAP=y" will suddenly get it broken, and will likely
>>>>> not realize it until something nasty will happen.
>>>> CONFIG_TEST_BITMAP was being enabled by the kselftest suite lib. The bitmap
>>>> test and its config option would disappear. The same test can be run by
>>>> just enabling KUNIT default config option:
>>>>
>>>> KUNIT_ALL_TESTS=y enables this bitmap config by default.
>>>>
>>>>>
>>>>> Sorry, NAK for config rename.
>>>>>
>>>
>>> I agree with Yury. Using KUNIT takes away test coverage for people who
>>> are willing to run selftests but not use KUNIT.
> 
> This is incorrect. There are selftest that are used to
> 
> - regression test a subsystem or a abi during boot
> - spot check on a running system to debug and test by loading
>    a test module.
> 
>>
>> I can see the point that renaming the config option is just churn, but
>> is there a reason people would run the bitmap selftest but be unable
>> or unwilling to use KUnit?
>>
>> Beyond a brief period of adjustment (which could probably be made
>> quite minimal with a wrapper script or something), there shouldn't
>> really be any fundamental difference: KUnit tests can already run at
>> boot, be configured with a config option, and write output to the
>> kernel log. There's nothing really being taken away here, and the
>> bonus of having easier access to run the tests with KUnit's tooling
>> (or have them automatically run by systems which run KUnit tests)
>> would seem worthwhile to me, especially since it's optional. And
>> CONFIG_KUNIT shouldn't be heavy enough to cause problems.
>>

Shouldn't be is the operative word? This doesn't help people who
want run a run bitmap test on a running system. This is a wrong
direction to go to say all testing has to be done under kunit.

What happened to the effort to run selftests as is under KUnit? What
is the motivation to convert all tests to kunit instead of trying to
provide support to run kselftest under kunit environment?

We discussed this a few years ago as I recall. Let's work on that
instead of removing existing selftests and regressing current use-cases?

Can we look into providing:

1. running kselftest under kunit environment without changes
    as user space applications?
2. Leave kselftests alone so we don't weaken kernel testing


thanks,
-- Shuah

Re: [PATCH 2/3] bitmap: Rename module

[John Hubbard]

On 7/30/24 11:17 AM, Shuah Khan wrote:
> On 7/30/24 09:55, Shuah Khan wrote:
>> On 7/30/24 04:10, David Gow wrote:
>>> On Mon, 29 Jul 2024 at 22:09, Randy Dunlap <rdunlap@infradead.org> 
...
>>> I can see the point that renaming the config option is just churn, but
>>> is there a reason people would run the bitmap selftest but be unable
>>> or unwilling to use KUnit?
>>>
>>> Beyond a brief period of adjustment (which could probably be made
>>> quite minimal with a wrapper script or something), there shouldn't
>>> really be any fundamental difference: KUnit tests can already run at
>>> boot, be configured with a config option, and write output to the
>>> kernel log. There's nothing really being taken away here, and the
>>> bonus of having easier access to run the tests with KUnit's tooling
>>> (or have them automatically run by systems which run KUnit tests)
>>> would seem worthwhile to me, especially since it's optional. And
>>> CONFIG_KUNIT shouldn't be heavy enough to cause problems.
>>>
> 
> Shouldn't be is the operative word? This doesn't help people who
> want run a run bitmap test on a running system. This is a wrong
> direction to go to say all testing has to be done under kunit.
> 
> What happened to the effort to run selftests as is under KUnit? What
> is the motivation to convert all tests to kunit instead of trying to
> provide support to run kselftest under kunit environment?
> 
> We discussed this a few years ago as I recall. Let's work on that
> instead of removing existing selftests and regressing current use-cases?
> 
> Can we look into providing:
> 
> 1. running kselftest under kunit environment without changes
>     as user space applications?

Yes. I suggested this earlier: if something fits neatly into
a KUnit test, then with some additional work, it can also be
run from kselftest. Just supporting both would be very nice,
because people don't have to change anything about their testing
flow.


> 2. Leave kselftests alone so we don't weaken kernel testing

Or augment them as above, so that we don't weaken kernel testing,
yes.


thanks,

-- 
John Hubbard
NVIDIA

Re: [PATCH 2/3] bitmap: Rename module

[Yury Norov]

On Tue, Jul 30, 2024 at 06:10:55PM +0800, David Gow wrote:
> On Mon, 29 Jul 2024 at 22:09, Randy Dunlap <rdunlap@infradead.org> wrote:
> >
> >
> >
> > On 7/29/24 1:07 AM, Muhammad Usama Anjum wrote:
> > > On 7/27/24 10:35 PM, Yury Norov wrote:
> > >> On Fri, Jul 26, 2024 at 04:06:57PM +0500, Muhammad Usama Anjum wrote:
> > >>> Rename module to bitmap_kunit and rename the configuration option
> > >>> compliant with kunit framework.
> > >>
> > >> ... , so those enabling bitmaps testing in their configs by setting
> > >> "CONFIG_TEST_BITMAP=y" will suddenly get it broken, and will likely
> > >> not realize it until something nasty will happen.
> > > CONFIG_TEST_BITMAP was being enabled by the kselftest suite lib. The bitmap
> > > test and its config option would disappear. The same test can be run by
> > > just enabling KUNIT default config option:
> > >
> > > KUNIT_ALL_TESTS=y enables this bitmap config by default.
> > >
> > >>
> > >> Sorry, NAK for config rename.
> > >>
> >
> > I agree with Yury. Using KUNIT takes away test coverage for people who
> > are willing to run selftests but not use KUNIT.
> 
> I can see the point that renaming the config option is just churn, but
> is there a reason people would run the bitmap selftest but be unable
> or unwilling to use KUnit?
> 
> Beyond a brief period of adjustment (which could probably be made
> quite minimal with a wrapper script or something), there shouldn't
> really be any fundamental difference: KUnit tests can already run at
> boot, be configured with a config option, and write output to the
> kernel log. There's nothing really being taken away here, and the
> bonus of having easier access to run the tests with KUnit's tooling
> (or have them automatically run by systems which run KUnit tests)
> would seem worthwhile to me, especially since it's optional. And
> CONFIG_KUNIT shouldn't be heavy enough to cause problems.
> 
> Obviously I can't force people to use KUnit, but this is exactly the
> sort of test which would fit KUnit well, and -- forgive me if I'm
> missing something -- the only real argument against it I'm hearing is
> "it's different". And while that's valid (as I said, churn for churn's
> sake isn't great), none of the "people who are willing to run
> selftests but not use KUnit" have given reasons why. Especially since
> this is the sort of test (testing in-kernel functions) we're
> encouraging people to write with KUnit in
> Documentation/dev-tools/testing-overview.rst -- if there are good
> reasons people are refusing to run these, maybe we need to fix those
> or change the recommendation.

This doesn't work like this, and never did. Against every change of
that sort there's always a strong, valid and self-contained argument:
don't touch something that works. 

However, reviewers provided more than one reason against this rework.
Every person has their own reasoning. For me it's history wipe and
change of a method how we enable the test. For John, Shuah, Randy and
others there's a bunch of other reasons.

And my question is still unanswered: what exactly is getting better
with this switch to KUNIT, comparing to the old behavior?

From KUNIT development perspective, I'd look at this situation as an
opportunity to improve the framework. If people don't like such things,
I'd leave them alone with their habits and write some sort of
compatibility layer for KUNIT, such that you can integrate the test
that you like into your framework with no or minimal changes to the
original code.

Thanks,
Yury

[PATCH 3/3] selftests: lib: remove test_bitmap

[Muhammad Usama Anjum]

Remove the test_bitmap as it has been converted to kunit test.

Cc: kees@kernel.org
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/lib/Makefile  | 2 +-
 tools/testing/selftests/lib/bitmap.sh | 3 ---
 tools/testing/selftests/lib/config    | 1 -
 3 files changed, 1 insertion(+), 5 deletions(-)
 delete mode 100755 tools/testing/selftests/lib/bitmap.sh

diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile
index ee71fc99d5b51..386c5887c0d65 100644
--- a/tools/testing/selftests/lib/Makefile
+++ b/tools/testing/selftests/lib/Makefile
@@ -4,6 +4,6 @@
 # No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
 all:
 
-TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh scanf.sh strscpy.sh
+TEST_PROGS := printf.sh prime_numbers.sh scanf.sh strscpy.sh
 
 include ../lib.mk
diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh
deleted file mode 100755
index 00a416fbc0ef0..0000000000000
--- 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 645839b50b0a2..7d3b1de29d3d6 100644
--- a/tools/testing/selftests/lib/config
+++ b/tools/testing/selftests/lib/config
@@ -1,6 +1,5 @@
 CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
-CONFIG_TEST_BITMAP=m
 CONFIG_PRIME_NUMBERS=m
 CONFIG_TEST_STRSCPY=m
 CONFIG_TEST_BITOPS=m
-- 
2.39.2

Re: [PATCH 3/3] selftests: lib: remove test_bitmap

[Shuah Khan]

On 7/26/24 05:06, Muhammad Usama Anjum wrote:
> Remove the test_bitmap as it has been converted to kunit test.
> 

Care to give some commit information on this change?
This change will take the ability away to run bitmap tests during
boot - as it will now be dependent on kunit

Why are we making changing like this without thinking through?

> Cc: kees@kernel.org
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>   tools/testing/selftests/lib/Makefile  | 2 +-
>   tools/testing/selftests/lib/bitmap.sh | 3 ---
>   tools/testing/selftests/lib/config    | 1 -
>   3 files changed, 1 insertion(+), 5 deletions(-)
>   delete mode 100755 tools/testing/selftests/lib/bitmap.sh
> 
> diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile
> index ee71fc99d5b51..386c5887c0d65 100644
> --- a/tools/testing/selftests/lib/Makefile
> +++ b/tools/testing/selftests/lib/Makefile
> @@ -4,6 +4,6 @@
>   # No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
>   all:
>   
> -TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh scanf.sh strscpy.sh
> +TEST_PROGS := printf.sh prime_numbers.sh scanf.sh strscpy.sh
>   
>   include ../lib.mk
> diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh
> deleted file mode 100755
> index 00a416fbc0ef0..0000000000000
> --- 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 645839b50b0a2..7d3b1de29d3d6 100644
> --- a/tools/testing/selftests/lib/config
> +++ b/tools/testing/selftests/lib/config
> @@ -1,6 +1,5 @@
>   CONFIG_TEST_PRINTF=m
>   CONFIG_TEST_SCANF=m
> -CONFIG_TEST_BITMAP=m
>   CONFIG_PRIME_NUMBERS=m
>   CONFIG_TEST_STRSCPY=m
>   CONFIG_TEST_BITOPS=m

thanks,
-- Shuah

Re: [PATCH 0/3] bitmap: Convert test_bitmap to kunit test

[Shuah Khan]

On 7/26/24 05:06, Muhammad Usama Anjum wrote:
> In this series, test_bitmap is being converted to kunit test. Multiple
> patches will make the review process smooth.
> 
> - Patch-1: Convert the tests in lib/test_bitmap.c to kunit
> - Patch-2: Rename the lib/test_bitmap.c to lib/bitmap_kunit.c and other
>             configuration options
> - Patch-3: Remove the bitmap.sh selftest
> 
> Muhammad Usama Anjum (3):
>    bitmap: convert test_bitmap to KUnit test
>    bitmap: Rename module
>    selftests: lib: remove test_bitmap
> 
>   MAINTAINERS                           |   2 +-
>   lib/Kconfig.debug                     |  15 +-
>   lib/Makefile                          |   2 +-
>   lib/{test_bitmap.c => bitmap_kunit.c} | 624 ++++++++++++--------------
>   tools/testing/selftests/lib/Makefile  |   2 +-
>   tools/testing/selftests/lib/bitmap.sh |   3 -
>   tools/testing/selftests/lib/config    |   1 -
>   7 files changed, 295 insertions(+), 354 deletions(-)
>   rename lib/{test_bitmap.c => bitmap_kunit.c} (70%)
>   delete mode 100755 tools/testing/selftests/lib/bitmap.sh
> 

Can you tell me how this conversion helps?

It is removing the ability to run bitmap tests during boot.
It doesn't make sense to blindly convert all test under lib
to kunit - Nack on this change or any change that takes away
the ability to run tests and makes them dependent on kunit.

thanks,
-- Shuah

Re: [PATCH 0/3] bitmap: Convert test_bitmap to kunit test

[Yury Norov]

On Fri, Jul 26, 2024 at 01:26:48PM -0600, Shuah Khan wrote:
> On 7/26/24 05:06, Muhammad Usama Anjum wrote:
> > In this series, test_bitmap is being converted to kunit test. Multiple
> > patches will make the review process smooth.
> > 
> > - Patch-1: Convert the tests in lib/test_bitmap.c to kunit
> > - Patch-2: Rename the lib/test_bitmap.c to lib/bitmap_kunit.c and other
> >             configuration options
> > - Patch-3: Remove the bitmap.sh selftest
> > 
> > Muhammad Usama Anjum (3):
> >    bitmap: convert test_bitmap to KUnit test
> >    bitmap: Rename module
> >    selftests: lib: remove test_bitmap
> > 
> >   MAINTAINERS                           |   2 +-
> >   lib/Kconfig.debug                     |  15 +-
> >   lib/Makefile                          |   2 +-
> >   lib/{test_bitmap.c => bitmap_kunit.c} | 624 ++++++++++++--------------
> >   tools/testing/selftests/lib/Makefile  |   2 +-
> >   tools/testing/selftests/lib/bitmap.sh |   3 -
> >   tools/testing/selftests/lib/config    |   1 -
> >   7 files changed, 295 insertions(+), 354 deletions(-)
> >   rename lib/{test_bitmap.c => bitmap_kunit.c} (70%)
> >   delete mode 100755 tools/testing/selftests/lib/bitmap.sh
> > 
> 
> Can you tell me how this conversion helps?
> 
> It is removing the ability to run bitmap tests during boot.
> It doesn't make sense to blindly convert all test under lib
> to kunit - Nack on this change or any change that takes away
> the ability to run tests and makes them dependent on kunit.

Hi Muhammad,

In addition to Shuah's and John's reasoning. This patch wipes the
test history (git blame will point on you for most of the test),
breaks boot-time testing support, messes with config names and
usability, and drops kselftest support for ... exactly, what?

KUNIT engine here doesn't improve on readability, neither shorten
the test length, to my taste.

If you'd like to contribute to bitmaps testing - I'm all for that.
This is the very core and performance-sensitive piece of kernel,
and any extra-coverage is always welcome.

But I think the best way would be either adding new cases to the
existing test, or writing a new test, KUNIT-based, if you like.

Thanks,
Yury

Re: [PATCH 0/3] bitmap: Convert test_bitmap to kunit test

[Muhammad Usama Anjum]

On 7/27/24 11:10 PM, Yury Norov wrote:
> On Fri, Jul 26, 2024 at 01:26:48PM -0600, Shuah Khan wrote:
>> On 7/26/24 05:06, Muhammad Usama Anjum wrote:
>>> In this series, test_bitmap is being converted to kunit test. Multiple
>>> patches will make the review process smooth.
>>>
>>> - Patch-1: Convert the tests in lib/test_bitmap.c to kunit
>>> - Patch-2: Rename the lib/test_bitmap.c to lib/bitmap_kunit.c and other
>>>             configuration options
>>> - Patch-3: Remove the bitmap.sh selftest
>>>
>>> Muhammad Usama Anjum (3):
>>>    bitmap: convert test_bitmap to KUnit test
>>>    bitmap: Rename module
>>>    selftests: lib: remove test_bitmap
>>>
>>>   MAINTAINERS                           |   2 +-
>>>   lib/Kconfig.debug                     |  15 +-
>>>   lib/Makefile                          |   2 +-
>>>   lib/{test_bitmap.c => bitmap_kunit.c} | 624 ++++++++++++--------------
>>>   tools/testing/selftests/lib/Makefile  |   2 +-
>>>   tools/testing/selftests/lib/bitmap.sh |   3 -
>>>   tools/testing/selftests/lib/config    |   1 -
>>>   7 files changed, 295 insertions(+), 354 deletions(-)
>>>   rename lib/{test_bitmap.c => bitmap_kunit.c} (70%)
>>>   delete mode 100755 tools/testing/selftests/lib/bitmap.sh
>>>
>>
>> Can you tell me how this conversion helps?
>>
>> It is removing the ability to run bitmap tests during boot.
>> It doesn't make sense to blindly convert all test under lib
>> to kunit - Nack on this change or any change that takes away
>> the ability to run tests and makes them dependent on kunit.
> 
> Hi Muhammad,
> 
> In addition to Shuah's and John's reasoning. This patch wipes the
> test history (git blame will point on you for most of the test),
When files are renamed, their history isn't lost. We just need to use
--follow option with git log to get complete history[1].

> breaks boot-time testing support, messes with config names and
> usability, and drops kselftest support for ... exactly, what?
AFAIU the kselftest wasn't detected the test results that's why I started
thinking on which could be best way to detect if any failure happens in
this test. Triggering the test from kselftest doesn't grantee the test it
would pass every time until we check results. For this kind of in-kernel
testing, kunit is best suites. Please find earlier discussion [2].

> 
> KUNIT engine here doesn't improve on readability, neither shorten
> the test length, to my taste.
> 
> If you'd like to contribute to bitmaps testing - I'm all for that.
> This is the very core and performance-sensitive piece of kernel,
> and any extra-coverage is always welcome.
> 
> But I think the best way would be either adding new cases to the
> existing test, or writing a new test, KUNIT-based, if you like.
> 

[1] https://git-scm.com/docs/git-log#Documentation/git-log.txt---follow
[2]
https://lore.kernel.org/all/327831fb-47ab-4555-8f0b-19a8dbcaacd7@collabora.com

> Thanks,
> Yury
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 0/3] bitmap: Convert test_bitmap to kunit test

[Shuah Khan]

On 7/29/24 02:15, Muhammad Usama Anjum wrote:
> On 7/27/24 11:10 PM, Yury Norov wrote:
>> On Fri, Jul 26, 2024 at 01:26:48PM -0600, Shuah Khan wrote:
>>> On 7/26/24 05:06, Muhammad Usama Anjum wrote:
>>>> In this series, test_bitmap is being converted to kunit test. Multiple
>>>> patches will make the review process smooth.
>>>>
>>>> - Patch-1: Convert the tests in lib/test_bitmap.c to kunit
>>>> - Patch-2: Rename the lib/test_bitmap.c to lib/bitmap_kunit.c and other
>>>>              configuration options
>>>> - Patch-3: Remove the bitmap.sh selftest
>>>>
>>>> Muhammad Usama Anjum (3):
>>>>     bitmap: convert test_bitmap to KUnit test
>>>>     bitmap: Rename module
>>>>     selftests: lib: remove test_bitmap
>>>>
>>>>    MAINTAINERS                           |   2 +-
>>>>    lib/Kconfig.debug                     |  15 +-
>>>>    lib/Makefile                          |   2 +-
>>>>    lib/{test_bitmap.c => bitmap_kunit.c} | 624 ++++++++++++--------------
>>>>    tools/testing/selftests/lib/Makefile  |   2 +-
>>>>    tools/testing/selftests/lib/bitmap.sh |   3 -
>>>>    tools/testing/selftests/lib/config    |   1 -
>>>>    7 files changed, 295 insertions(+), 354 deletions(-)
>>>>    rename lib/{test_bitmap.c => bitmap_kunit.c} (70%)
>>>>    delete mode 100755 tools/testing/selftests/lib/bitmap.sh
>>>>
>>>
>>> Can you tell me how this conversion helps?
>>>
>>> It is removing the ability to run bitmap tests during boot.
>>> It doesn't make sense to blindly convert all test under lib
>>> to kunit - Nack on this change or any change that takes away
>>> the ability to run tests and makes them dependent on kunit.
>>
>> Hi Muhammad,
>>
>> In addition to Shuah's and John's reasoning. This patch wipes the
>> test history (git blame will point on you for most of the test),
> When files are renamed, their history isn't lost. We just need to use
> --follow option with git log to get complete history[1].
> 
>> breaks boot-time testing support, messes with config names and
>> usability, and drops kselftest support for ... exactly, what?
> AFAIU the kselftest wasn't detected the test results that's why I started
> thinking on which could be best way to detect if any failure happens in
> this test. Triggering the test from kselftest doesn't grantee the test it
> would pass every time until we check results. For this kind of in-kernel
> testing, kunit is best suites. Please find earlier discussion [2].
> 

KUnit isn't idea for cases where people would want to check a subsystem
on a running kernel - KUnit covers some use-cases and kselftest covers
others.

What happens if we are debugging a problem that requires us to debug on
a running system? Please don't go converting kselftest into kunit without
understanding how these are intended to be used.

Yes kselftest results need to be looked at. Write a parser which can
be improved. What you are doing is reducing the coverage and talking
away the ability to debug and test on running system.

Fix what needs to be fixed instead of deleting tests.

>>
>> KUNIT engine here doesn't improve on readability, neither shorten
>> the test length, to my taste.
>>
>> If you'd like to contribute to bitmaps testing - I'm all for that.
>> This is the very core and performance-sensitive piece of kernel,
>> and any extra-coverage is always welcome.
>>

+1 on this. Add new tests and look at the reports.

>> But I think the best way would be either adding new cases to the
>> existing test, or writing a new test, KUNIT-based, if you like.
>>
> 

+1

As I mentioned in my earlier message, I am going to nack all patches
that convert existing selftests to kunit such as this one.

thanks,
-- Shuah

Re: [PATCH 0/3] bitmap: Convert test_bitmap to kunit test

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 6819 bytes --]

On Tue, 30 Jul 2024 at 23:39, Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 7/29/24 02:15, Muhammad Usama Anjum wrote:
> > On 7/27/24 11:10 PM, Yury Norov wrote:
> >> On Fri, Jul 26, 2024 at 01:26:48PM -0600, Shuah Khan wrote:
> >>> On 7/26/24 05:06, Muhammad Usama Anjum wrote:
> >>>> In this series, test_bitmap is being converted to kunit test. Multiple
> >>>> patches will make the review process smooth.
> >>>>
> >>>> - Patch-1: Convert the tests in lib/test_bitmap.c to kunit
> >>>> - Patch-2: Rename the lib/test_bitmap.c to lib/bitmap_kunit.c and other
> >>>>              configuration options
> >>>> - Patch-3: Remove the bitmap.sh selftest
> >>>>
> >>>> Muhammad Usama Anjum (3):
> >>>>     bitmap: convert test_bitmap to KUnit test
> >>>>     bitmap: Rename module
> >>>>     selftests: lib: remove test_bitmap
> >>>>
> >>>>    MAINTAINERS                           |   2 +-
> >>>>    lib/Kconfig.debug                     |  15 +-
> >>>>    lib/Makefile                          |   2 +-
> >>>>    lib/{test_bitmap.c => bitmap_kunit.c} | 624 ++++++++++++--------------
> >>>>    tools/testing/selftests/lib/Makefile  |   2 +-
> >>>>    tools/testing/selftests/lib/bitmap.sh |   3 -
> >>>>    tools/testing/selftests/lib/config    |   1 -
> >>>>    7 files changed, 295 insertions(+), 354 deletions(-)
> >>>>    rename lib/{test_bitmap.c => bitmap_kunit.c} (70%)
> >>>>    delete mode 100755 tools/testing/selftests/lib/bitmap.sh
> >>>>
> >>>
> >>> Can you tell me how this conversion helps?
> >>>
> >>> It is removing the ability to run bitmap tests during boot.
> >>> It doesn't make sense to blindly convert all test under lib
> >>> to kunit - Nack on this change or any change that takes away
> >>> the ability to run tests and makes them dependent on kunit.
> >>
> >> Hi Muhammad,
> >>
> >> In addition to Shuah's and John's reasoning. This patch wipes the
> >> test history (git blame will point on you for most of the test),
> > When files are renamed, their history isn't lost. We just need to use
> > --follow option with git log to get complete history[1].
> >
> >> breaks boot-time testing support, messes with config names and
> >> usability, and drops kselftest support for ... exactly, what?
> > AFAIU the kselftest wasn't detected the test results that's why I started
> > thinking on which could be best way to detect if any failure happens in
> > this test. Triggering the test from kselftest doesn't grantee the test it
> > would pass every time until we check results. For this kind of in-kernel
> > testing, kunit is best suites. Please find earlier discussion [2].
> >
>
> KUnit isn't idea for cases where people would want to check a subsystem
> on a running kernel - KUnit covers some use-cases and kselftest covers
> others.

Thanks, Shuah. That clarifies a lot. So the use-case we're concerned about is:
- We have an existing running kernel, which has CONFIG_KUNIT=n.
- Something breaks, and we want to check (for example) test_bitmap.
- We can modprobe test_bitmap on the current kernel.

That's definitely something KUnit can't do at the moment (though the
workaround of always building with CONFIG_KUNIT=m is working for Red
Hat and Android, and seems the closest equivalent). Maybe we need to
work out a way for KUnit tests to handle this case more smoothly.
Because I don't think there's anything special about this test which
makes this use-case more likely than any of the other KUnit tests we
have.

> What happens if we are debugging a problem that requires us to debug on
> a running system? Please don't go converting kselftest into kunit without
> understanding how these are intended to be used.

I don't think it's clear that this _is_ how the test is intended to be
used: the config help text (and earlier messages in this thread) were
all about running tests at boot, which is distinct from "debugging a
problem on a running system". I think this is where I've been most
confused. We've been happy to assume the "testing at runtime" case is
against a kernel with test configs enabled, too.

When it comes to test_bitmap specifically, I can't find any difference
(save possibly for a couple of performance test functions) between it
and any of the other tests we've converted to or written with KUnit in
the past. I'd imagine (and correct me if I'm wrong) that it's pretty
rare for there to be an issue which will be caught by test_bitmap on a
running kernel, but not on a fresh boot / in a kernel with KUnit
enabled. So the "debug a running system" situation seems to me only
decrease coverage of some very rare situations. (Of course, that's
ignoring that running the module against the current kernel might be
most convenient, but that's a separate issue from coverage.)

> Yes kselftest results need to be looked at. Write a parser which can
> be improved. What you are doing is reducing the coverage and talking
> away the ability to debug and test on running system.
>
> Fix what needs to be fixed instead of deleting tests.
>
> >>
> >> KUNIT engine here doesn't improve on readability, neither shorten
> >> the test length, to my taste.
> >>
> >> If you'd like to contribute to bitmaps testing - I'm all for that.
> >> This is the very core and performance-sensitive piece of kernel,
> >> and any extra-coverage is always welcome.
> >>
>
> +1 on this. Add new tests and look at the reports.
>
> >> But I think the best way would be either adding new cases to the
> >> existing test, or writing a new test, KUNIT-based, if you like.
> >>
> >
>
> +1
>
> As I mentioned in my earlier message, I am going to nack all patches
> that convert existing selftests to kunit such as this one.

I should point out that we've already been porting a lot of tests in
lib/ to KUnit for years, sometimes with the help of the test author,
sometimes by encouraging it as a task for new contributors. And while
most of those are not actually selftests (though we may have ported
one or two), they differ from test_bitmap only in that there's a
selftest wrapper script. The authors and users of those tests have
thus far found this a worthwhile trade -- I've not heard any
complaints -- so I'd rather we leave this as a "pause" on new
conversions while we sort this out, rather than do something drastic
like revert existing conversion.

Personally, I'd still prefer these sorts of conversions to go ahead --
I think the benefits outweightthe costs -- but I can totally get
behind doing it on a case-per-case basis. I'd be disappointed if we
NACKed changes like this in the case where the authors and users of
that individual test would like it to be converted, though.

Regardless, I think there's more discussion to be had here, and I'll
look into what changes we can make either to KUnit or to the testing
documentation to make sure we're all on the same page.

-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4014 bytes --]

Re: [PATCH 0/3] bitmap: Convert test_bitmap to kunit test

[Muhammad Usama Anjum]

On 7/27/24 12:26 AM, Shuah Khan wrote:
> On 7/26/24 05:06, Muhammad Usama Anjum wrote:
>> In this series, test_bitmap is being converted to kunit test. Multiple
>> patches will make the review process smooth.
>>
>> - Patch-1: Convert the tests in lib/test_bitmap.c to kunit
>> - Patch-2: Rename the lib/test_bitmap.c to lib/bitmap_kunit.c and other
>>             configuration options
>> - Patch-3: Remove the bitmap.sh selftest
>>
>> Muhammad Usama Anjum (3):
>>    bitmap: convert test_bitmap to KUnit test
>>    bitmap: Rename module
>>    selftests: lib: remove test_bitmap
>>
>>   MAINTAINERS                           |   2 +-
>>   lib/Kconfig.debug                     |  15 +-
>>   lib/Makefile                          |   2 +-
>>   lib/{test_bitmap.c => bitmap_kunit.c} | 624 ++++++++++++--------------
>>   tools/testing/selftests/lib/Makefile  |   2 +-
>>   tools/testing/selftests/lib/bitmap.sh |   3 -
>>   tools/testing/selftests/lib/config    |   1 -
>>   7 files changed, 295 insertions(+), 354 deletions(-)
>>   rename lib/{test_bitmap.c => bitmap_kunit.c} (70%)
>>   delete mode 100755 tools/testing/selftests/lib/bitmap.sh
>>
> 
> Can you tell me how this conversion helps?
> 
> It is removing the ability to run bitmap tests during boot.
> It doesn't make sense to blindly convert all test under lib
> to kunit - Nack on this change or any change that takes away
> the ability to run tests and makes them dependent on kunit.
Let's discuss this on discussion thread [1].

[1]
https://lore.kernel.org/all/a3083ad4-e9dc-40da-bf57-8391bcd96a6c@collabora.com

> 
> thanks,
> -- Shuah

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 0/3] bitmap: Convert test_bitmap to kunit test

[Shuah Khan]

On 7/29/24 02:29, Muhammad Usama Anjum wrote:
> On 7/27/24 12:26 AM, Shuah Khan wrote:
>> On 7/26/24 05:06, Muhammad Usama Anjum wrote:
>>> In this series, test_bitmap is being converted to kunit test. Multiple
>>> patches will make the review process smooth.
>>>
>>> - Patch-1: Convert the tests in lib/test_bitmap.c to kunit
>>> - Patch-2: Rename the lib/test_bitmap.c to lib/bitmap_kunit.c and other
>>>              configuration options
>>> - Patch-3: Remove the bitmap.sh selftest
>>>
>>> Muhammad Usama Anjum (3):
>>>     bitmap: convert test_bitmap to KUnit test
>>>     bitmap: Rename module
>>>     selftests: lib: remove test_bitmap
>>>
>>>    MAINTAINERS                           |   2 +-
>>>    lib/Kconfig.debug                     |  15 +-
>>>    lib/Makefile                          |   2 +-
>>>    lib/{test_bitmap.c => bitmap_kunit.c} | 624 ++++++++++++--------------
>>>    tools/testing/selftests/lib/Makefile  |   2 +-
>>>    tools/testing/selftests/lib/bitmap.sh |   3 -
>>>    tools/testing/selftests/lib/config    |   1 -
>>>    7 files changed, 295 insertions(+), 354 deletions(-)
>>>    rename lib/{test_bitmap.c => bitmap_kunit.c} (70%)
>>>    delete mode 100755 tools/testing/selftests/lib/bitmap.sh
>>>
>>
>> Can you tell me how this conversion helps?
>>
>> It is removing the ability to run bitmap tests during boot.
>> It doesn't make sense to blindly convert all test under lib
>> to kunit - Nack on this change or any change that takes away
>> the ability to run tests and makes them dependent on kunit.
> Let's discuss this on discussion thread [1].

So - it doesn't mean that it is a done deal. Each patch will be
reviewed on individual basis. This test in particular clearly its
use-case right in the config which was deleted without understanding
it.

-config TEST_BITMAP
-	tristate "Test bitmap_*() family of functions at runtime"
-	help
-	  Enable this option to test the bitmap functions at boot.


This line above is the important piece of information which tells
you how the test is intended to be used.

1. You can enable this option and boot the kernel to check for regressions.
2. You can load the module on a running kernel to check for health.

Converting it to kunit drops support for these two use-cases which
are user-space regressions. You don't want to do that.
  
-
-	  If unsure, say N.
-
  config TEST_UUID
  	tristate "Test functions located in the uuid module at runtime"
  
@@ -2813,6 +2806,14 @@ config USERCOPY_KUNIT_TEST
  	  on the copy_to/from_user infrastructure, making sure basic
  	  user/kernel boundary testing is working.
  
+config BITMAP_KUNIT_TEST
+	tristate "KUnit Test for bitmap_*() family of functions"
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the "bitmap_kunit" module that runs tests for
+	  bitmaps int the kernel making sure that there isn't any bug.

And this isn't complete even. I don'ty see tristate in here.

I responded to the thread that started this flurry of conversion activity
stating that converting tests without thinking through the use-cases
isn't what we want to do.

Reports aren't everything. The primary reason we have these tests is for
developers to be able to test. Reports can be improved and shouldn't
come at the expense of coverage and testing. Any patch that does that
will be NACKed.

thanks,
-- Shuah

Re: [PATCH 0/3] bitmap: Convert test_bitmap to kunit test

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 4614 bytes --]

On Tue, 30 Jul 2024 at 23:49, Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 7/29/24 02:29, Muhammad Usama Anjum wrote:
> > On 7/27/24 12:26 AM, Shuah Khan wrote:
> >> On 7/26/24 05:06, Muhammad Usama Anjum wrote:
> >>> In this series, test_bitmap is being converted to kunit test. Multiple
> >>> patches will make the review process smooth.
> >>>
> >>> - Patch-1: Convert the tests in lib/test_bitmap.c to kunit
> >>> - Patch-2: Rename the lib/test_bitmap.c to lib/bitmap_kunit.c and other
> >>>              configuration options
> >>> - Patch-3: Remove the bitmap.sh selftest
> >>>
> >>> Muhammad Usama Anjum (3):
> >>>     bitmap: convert test_bitmap to KUnit test
> >>>     bitmap: Rename module
> >>>     selftests: lib: remove test_bitmap
> >>>
> >>>    MAINTAINERS                           |   2 +-
> >>>    lib/Kconfig.debug                     |  15 +-
> >>>    lib/Makefile                          |   2 +-
> >>>    lib/{test_bitmap.c => bitmap_kunit.c} | 624 ++++++++++++--------------
> >>>    tools/testing/selftests/lib/Makefile  |   2 +-
> >>>    tools/testing/selftests/lib/bitmap.sh |   3 -
> >>>    tools/testing/selftests/lib/config    |   1 -
> >>>    7 files changed, 295 insertions(+), 354 deletions(-)
> >>>    rename lib/{test_bitmap.c => bitmap_kunit.c} (70%)
> >>>    delete mode 100755 tools/testing/selftests/lib/bitmap.sh
> >>>
> >>
> >> Can you tell me how this conversion helps?
> >>
> >> It is removing the ability to run bitmap tests during boot.
> >> It doesn't make sense to blindly convert all test under lib
> >> to kunit - Nack on this change or any change that takes away
> >> the ability to run tests and makes them dependent on kunit.
> > Let's discuss this on discussion thread [1].
>
> So - it doesn't mean that it is a done deal. Each patch will be
> reviewed on individual basis. This test in particular clearly its
> use-case right in the config which was deleted without understanding
> it.
>
> -config TEST_BITMAP
> -       tristate "Test bitmap_*() family of functions at runtime"
> -       help
> -         Enable this option to test the bitmap functions at boot.
>
>
> This line above is the important piece of information which tells
> you how the test is intended to be used.
>
> 1. You can enable this option and boot the kernel to check for regressions.

Converting the test to KUnit _does not break this_. You can still
enable this option and boot the kernel to test for regressions if it's
a KUnit test.

> 2. You can load the module on a running kernel to check for health.

This is where the disagreement lies in my mind. While you can do this
with KUnit, the kernel does have to have been built with CONFIG_KUNIT
enabled (either built-in or as a module).

(Indeed, KUnit adds the ability to trigger a test at runtime even if
it's built-in, via debugfs, so it's possible to do so even if
CONFIG_MODULES is disabled. That's a pretty niche use-case, I admit,
but I'd guess that the bitmap functionailty breaking in a way which
disappears when CONFIG_KUNIT is enabled is as well.)

In general, though, my interpretation of "Test ... at runtime" in
config option descriptions has not precluded introducing the
CONFIG_KUNIT dependency. We already have several KUnit tests with that
in their description, several of which were ported to KUnit later.

>
> Converting it to kunit drops support for these two use-cases which
> are user-space regressions. You don't want to do that.
>
> -
> -         If unsure, say N.
> -
>   config TEST_UUID
>         tristate "Test functions located in the uuid module at runtime"
>
> @@ -2813,6 +2806,14 @@ config USERCOPY_KUNIT_TEST
>           on the copy_to/from_user infrastructure, making sure basic
>           user/kernel boundary testing is working.
>
> +config BITMAP_KUNIT_TEST
> +       tristate "KUnit Test for bitmap_*() family of functions"
> +       depends on KUNIT
> +       default KUNIT_ALL_TESTS
> +       help
> +         This builds the "bitmap_kunit" module that runs tests for
> +         bitmaps int the kernel making sure that there isn't any bug.
>
> And this isn't complete even. I don'ty see tristate in here.

(This looks like it's tristate to me?)

>
> I responded to the thread that started this flurry of conversion activity
> stating that converting tests without thinking through the use-cases
> isn't what we want to do.
>
> Reports aren't everything. The primary reason we have these tests is for
> developers to be able to test. Reports can be improved and shouldn't
> come at the expense of coverage and testing. Any patch that does that
> will be NACKed.
>
> thanks,
> -- Shuah

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4014 bytes --]

Re: [PATCH 0/3] bitmap: Convert test_bitmap to kunit test

[Shuah Khan]

On 7/30/24 21:06, David Gow wrote:
> On Tue, 30 Jul 2024 at 23:49, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 7/29/24 02:29, Muhammad Usama Anjum wrote:
>>> On 7/27/24 12:26 AM, Shuah Khan wrote:
>>>> On 7/26/24 05:06, Muhammad Usama Anjum wrote:
>>>>> In this series, test_bitmap is being converted to kunit test. Multiple
>>>>> patches will make the review process smooth.
>>>>>
>>>>> - Patch-1: Convert the tests in lib/test_bitmap.c to kunit
>>>>> - Patch-2: Rename the lib/test_bitmap.c to lib/bitmap_kunit.c and other
>>>>>               configuration options
>>>>> - Patch-3: Remove the bitmap.sh selftest
>>>>>
>>>>> Muhammad Usama Anjum (3):
>>>>>      bitmap: convert test_bitmap to KUnit test
>>>>>      bitmap: Rename module
>>>>>      selftests: lib: remove test_bitmap
>>>>>
>>>>>     MAINTAINERS                           |   2 +-
>>>>>     lib/Kconfig.debug                     |  15 +-
>>>>>     lib/Makefile                          |   2 +-
>>>>>     lib/{test_bitmap.c => bitmap_kunit.c} | 624 ++++++++++++--------------
>>>>>     tools/testing/selftests/lib/Makefile  |   2 +-
>>>>>     tools/testing/selftests/lib/bitmap.sh |   3 -
>>>>>     tools/testing/selftests/lib/config    |   1 -
>>>>>     7 files changed, 295 insertions(+), 354 deletions(-)
>>>>>     rename lib/{test_bitmap.c => bitmap_kunit.c} (70%)
>>>>>     delete mode 100755 tools/testing/selftests/lib/bitmap.sh
>>>>>
>>>>
>>>> Can you tell me how this conversion helps?
>>>>
>>>> It is removing the ability to run bitmap tests during boot.
>>>> It doesn't make sense to blindly convert all test under lib
>>>> to kunit - Nack on this change or any change that takes away
>>>> the ability to run tests and makes them dependent on kunit.
>>> Let's discuss this on discussion thread [1].
>>
>> So - it doesn't mean that it is a done deal. Each patch will be
>> reviewed on individual basis. This test in particular clearly its
>> use-case right in the config which was deleted without understanding
>> it.
>>
>> -config TEST_BITMAP
>> -       tristate "Test bitmap_*() family of functions at runtime"
>> -       help
>> -         Enable this option to test the bitmap functions at boot.
>>
>>
>> This line above is the important piece of information which tells
>> you how the test is intended to be used.
>>
>> 1. You can enable this option and boot the kernel to check for regressions.
> 
> Converting the test to KUnit _does not break this_. You can still
> enable this option and boot the kernel to test for regressions if it's
> a KUnit test.
> 
>> 2. You can load the module on a running kernel to check for health.
> 
> This is where the disagreement lies in my mind. While you can do this
> with KUnit, the kernel does have to have been built with CONFIG_KUNIT
> enabled (either built-in or as a module).

Right. It appears the confusion is coming in because "What's the problem
enabling CONFIG_KUNIT". The question is would you recommend enabling
CONFIG_KUNIT in distribution kernels - there is a confusion that
unit testing can be enabled in all environments.

The answer is no. Running tests should not require should not require
CONFIG_KUNIT - I would call this not disagreement, but not understanding
the role of unit (open box) vs closed box testing.

We want to users to be able to run as many tests possible without
needing to enable KUnit or other config options to verify kernels
they build. KUnit is a tool for kernel developers to verify - it
is a unit testing framework that is where it brings value.

It isn't a replacement for selftests and other tests. I am not happy
to seer tests that are supported as kselftests be changed to now
require KUNIT to be enabled.

thanks,
-- Shuah