[PATCH v2 0/4] cpumask: Fix invalid uniprocessor assumptions

[PATCH v2 0/4] cpumask: Fix invalid uniprocessor assumptions

[Sander Vanheule]

On uniprocessor builds, it is currently assumed that any cpumask will
contain the single CPU: cpu0. This assumption is used to provide
optimised implementations.

The current assumption also appears to be wrong, by ignoring the fact
that users can provide empty cpumask-s. This can result in bugs as
explained in [1].

This series introduces some basic tests, and updates the optimisations
for uniprocessor builds.

[1] https://lore.kernel.org/all/20220530082552.46113-1-sander@svanheule.net/

Changes since v1:
Link: https://lore.kernel.org/all/cover.1654201862.git.sander@svanheule.net/
  - Place tests in lib/test_cpumask.c
  - Drop the modified UP code in favor of the generic SMP implementation
  - Update declaration of cpumask_next_wrap()

Sander Vanheule (4):
  lib/test: Introduce cpumask KUnit test suite
  cpumask: Fix invalid uniprocessor mask assumption
  cpumask: Add UP optimised for_each_*_cpu versions
  cpumask: Update cpumask_next_wrap() signature

 include/linux/cpumask.h |  89 +++----------------------------
 lib/Kconfig.debug       |   9 ++++
 lib/Makefile            |   4 +-
 lib/test_cpumask.c      | 115 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 134 insertions(+), 83 deletions(-)
 create mode 100644 lib/test_cpumask.c

-- 
2.36.1

[PATCH v2 1/4] lib/test: Introduce cpumask KUnit test suite

[Sander Vanheule]

Add a basic suite of tests for cpumask, providing some tests for empty
and completely filled cpumasks.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 lib/Kconfig.debug  |   9 ++++
 lib/Makefile       |   1 +
 lib/test_cpumask.c | 115 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 125 insertions(+)
 create mode 100644 lib/test_cpumask.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index b8cc65d22169..85f2eb5c0b07 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2100,6 +2100,15 @@ config LKDTM
 	Documentation on how to use the module can be found in
 	Documentation/fault-injection/provoke-crashes.rst
 
+config TEST_CPUMASK
+	tristate "cpumask tests" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  Enable to turn on cpumask tests, running at boot or module load time.
+
+	  If unsure, say N.
+
 config TEST_LIST_SORT
 	tristate "Linked list sorting test" if !KUNIT_ALL_TESTS
 	depends on KUNIT
diff --git a/lib/Makefile b/lib/Makefile
index 89fcae891361..81f944cd74ae 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_TEST_HMM) += test_hmm.o
 obj-$(CONFIG_TEST_FREE_PAGES) += test_free_pages.o
 obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
 obj-$(CONFIG_TEST_REF_TRACKER) += test_ref_tracker.o
+obj-$(CONFIG_TEST_CPUMASK) += test_cpumask.o
 CFLAGS_test_fprobe.o += $(CC_FLAGS_FTRACE)
 obj-$(CONFIG_FPROBE_SANITY_TEST) += test_fprobe.o
 #
diff --git a/lib/test_cpumask.c b/lib/test_cpumask.c
new file mode 100644
index 000000000000..3b69fcb6730e
--- /dev/null
+++ b/lib/test_cpumask.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * KUnit tests for cpumask.
+ *
+ * Author: Sander Vanheule <sander@svanheule.net>
+ */
+
+#include <kunit/test.h>
+#include <linux/cpumask.h>
+
+#define FOR_EACH_ITER_EQ(_test, _iter, _expect, _loop)		\
+	do {							\
+		(_iter) = 0;					\
+		_loop						\
+			(_iter)++;				\
+		KUNIT_EXPECT_EQ((_test), (_expect), (_iter));	\
+	} while (0)
+
+static cpumask_t mask_empty;
+static cpumask_t mask_all;
+
+static void test_cpumask_weight(struct kunit *test)
+{
+	KUNIT_EXPECT_TRUE(test, cpumask_empty(&mask_empty));
+	KUNIT_EXPECT_TRUE(test, cpumask_full(cpu_possible_mask));
+	KUNIT_EXPECT_TRUE(test, cpumask_full(&mask_all));
+
+	KUNIT_EXPECT_EQ(test, 0, cpumask_weight(&mask_empty));
+	KUNIT_EXPECT_EQ(test, nr_cpu_ids, cpumask_weight(cpu_possible_mask));
+	KUNIT_EXPECT_EQ(test, nr_cpumask_bits, cpumask_weight(&mask_all));
+}
+
+static void test_cpumask_first(struct kunit *test)
+{
+	KUNIT_EXPECT_LE(test, nr_cpu_ids, cpumask_first(&mask_empty));
+	KUNIT_EXPECT_EQ(test, 0, cpumask_first(cpu_possible_mask));
+
+	KUNIT_EXPECT_EQ(test, 0, cpumask_first_zero(&mask_empty));
+	KUNIT_EXPECT_LE(test, nr_cpu_ids, cpumask_first_zero(cpu_possible_mask));
+}
+
+static void test_cpumask_last(struct kunit *test)
+{
+	KUNIT_EXPECT_LE(test, nr_cpumask_bits, cpumask_last(&mask_empty));
+	KUNIT_EXPECT_EQ(test, nr_cpumask_bits - 1, cpumask_last(cpu_possible_mask));
+}
+
+static void test_cpumask_next(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, 0, cpumask_next_zero(-1, &mask_empty));
+	KUNIT_EXPECT_LE(test, nr_cpu_ids, cpumask_next_zero(-1, cpu_possible_mask));
+
+	KUNIT_EXPECT_LE(test, nr_cpu_ids, cpumask_next(-1, &mask_empty));
+	KUNIT_EXPECT_EQ(test, 0, cpumask_next(-1, cpu_possible_mask));
+}
+
+static void test_cpumask_iterators(struct kunit *test)
+{
+	unsigned int iterations;
+	int cpu;
+
+	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(&mask_empty),
+			for_each_cpu(cpu, &mask_empty));
+	FOR_EACH_ITER_EQ(test, iterations, nr_cpu_ids - cpumask_weight(&mask_empty),
+			for_each_cpu_not(cpu, &mask_empty));
+	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(&mask_empty),
+			for_each_cpu_wrap(cpu, &mask_empty, nr_cpu_ids / 2));
+
+	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_possible_mask),
+			for_each_cpu(cpu, cpu_possible_mask));
+	FOR_EACH_ITER_EQ(test, iterations, nr_cpu_ids - cpumask_weight(cpu_possible_mask),
+			for_each_cpu_not(cpu, cpu_possible_mask));
+	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_possible_mask),
+			for_each_cpu_wrap(cpu, cpu_possible_mask, nr_cpu_ids / 2));
+}
+
+static void test_cpumask_iterators_builtin(struct kunit *test)
+{
+	unsigned int iterations;
+	int cpu;
+
+	FOR_EACH_ITER_EQ(test, iterations, nr_cpu_ids,
+		for_each_possible_cpu(cpu));
+	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_online_mask),
+		for_each_online_cpu(cpu));
+	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_present_mask),
+		for_each_present_cpu(cpu));
+}
+
+static int test_cpumask_init(struct kunit *test)
+{
+	cpumask_clear(&mask_empty);
+	cpumask_setall(&mask_all);
+
+	return 0;
+}
+
+static struct kunit_case test_cpumask_cases[] = {
+	KUNIT_CASE(test_cpumask_weight),
+	KUNIT_CASE(test_cpumask_first),
+	KUNIT_CASE(test_cpumask_last),
+	KUNIT_CASE(test_cpumask_next),
+	KUNIT_CASE(test_cpumask_iterators),
+	KUNIT_CASE(test_cpumask_iterators_builtin),
+	{}
+};
+
+static struct kunit_suite test_cpumask_suite = {
+	.name = "cpumask",
+	.init = test_cpumask_init,
+	.test_cases = test_cpumask_cases,
+};
+kunit_test_suite(test_cpumask_suite);
+
+MODULE_LICENSE("GPL");
-- 
2.36.1

[PATCH v2 2/4] cpumask: Fix invalid uniprocessor mask assumption

[Sander Vanheule]

On uniprocessor builds, any CPU mask is assumed to contain exactly one
CPU (cpu0). This assumption ignores the existence of empty masks, for
which the optimised implementations result in incorrect behaviour, and
causes the tests to fail:

[...]     # Subtest: cpumask
[...]     1..6
[...]     ok 1 - test_cpumask_weight
[...]     # test_cpumask_first: ASSERTION FAILED at lib/test_cpumask.c:35
[...]     Expected 1U <= cpumask_first(&mask_empty), but
[...]         1U == 1
[...]         cpumask_first(&mask_empty) == 0
[...]     not ok 2 - test_cpumask_first
[...]     # test_cpumask_last: ASSERTION FAILED at lib/test_cpumask.c:44
[...]     Expected ((unsigned int)1) <= cpumask_last(&mask_empty), but
[...]         ((unsigned int)1) == 1
[...]         cpumask_last(&mask_empty) == 0
[...]     not ok 3 - test_cpumask_last
[...]     # test_cpumask_next: ASSERTION FAILED at lib/test_cpumask.c:51
[...]     Expected 1U <= cpumask_next_zero(-1, ((const struct cpumask *)&__cpu_possible_mask)), but
[...]         1U == 1
[...]         cpumask_next_zero(-1, ((const struct cpumask *)&__cpu_possible_mask)) == 0
[...]     not ok 4 - test_cpumask_next
[...]     # test_cpumask_iterators: EXPECTATION FAILED at lib/test_cpumask.c:62
[...]     Expected (cpumask_weight(&mask_empty)) == (iterations), but
[...]         (cpumask_weight(&mask_empty)) == 0
[...]         (iterations) == 1
[...]     # test_cpumask_iterators: EXPECTATION FAILED at lib/test_cpumask.c:66
[...]     Expected (cpumask_weight(&mask_empty)) == (iterations), but
[...]         (cpumask_weight(&mask_empty)) == 0
[...]         (iterations) == 1
[...]     # test_cpumask_iterators: EXPECTATION FAILED at lib/test_cpumask.c:71
[...]     Expected (1U - cpumask_weight(((const struct cpumask *)&__cpu_possible_mask))) == (iterations), but
[...]         (1U - cpumask_weight(((const struct cpumask *)&__cpu_possible_mask))) == 0
[...]         (iterations) == 1
[...]     not ok 5 - test_cpumask_iterators
[...]     ok 6 - test_cpumask_iterators_builtin

Drop the incorrectly optimised code and use the generic implementations
in all cases.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 include/linux/cpumask.h | 80 -----------------------------------------
 lib/Makefile            |  3 +-
 2 files changed, 1 insertion(+), 82 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index fe29ac7cc469..d6add0e29ef4 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -116,85 +116,6 @@ static __always_inline unsigned int cpumask_check(unsigned int cpu)
 	return cpu;
 }
 
-#if NR_CPUS == 1
-/* Uniprocessor.  Assume all masks are "1". */
-static inline unsigned int cpumask_first(const struct cpumask *srcp)
-{
-	return 0;
-}
-
-static inline unsigned int cpumask_first_zero(const struct cpumask *srcp)
-{
-	return 0;
-}
-
-static inline unsigned int cpumask_first_and(const struct cpumask *srcp1,
-					     const struct cpumask *srcp2)
-{
-	return 0;
-}
-
-static inline unsigned int cpumask_last(const struct cpumask *srcp)
-{
-	return 0;
-}
-
-/* Valid inputs for n are -1 and 0. */
-static inline unsigned int cpumask_next(int n, const struct cpumask *srcp)
-{
-	return n+1;
-}
-
-static inline unsigned int cpumask_next_zero(int n, const struct cpumask *srcp)
-{
-	return n+1;
-}
-
-static inline unsigned int cpumask_next_and(int n,
-					    const struct cpumask *srcp,
-					    const struct cpumask *andp)
-{
-	return n+1;
-}
-
-static inline unsigned int cpumask_next_wrap(int n, const struct cpumask *mask,
-					     int start, bool wrap)
-{
-	/* cpu0 unless stop condition, wrap and at cpu0, then nr_cpumask_bits */
-	return (wrap && n == 0);
-}
-
-/* cpu must be a valid cpu, ie 0, so there's no other choice. */
-static inline unsigned int cpumask_any_but(const struct cpumask *mask,
-					   unsigned int cpu)
-{
-	return 1;
-}
-
-static inline unsigned int cpumask_local_spread(unsigned int i, int node)
-{
-	return 0;
-}
-
-static inline int cpumask_any_and_distribute(const struct cpumask *src1p,
-					     const struct cpumask *src2p) {
-	return cpumask_first_and(src1p, src2p);
-}
-
-static inline int cpumask_any_distribute(const struct cpumask *srcp)
-{
-	return cpumask_first(srcp);
-}
-
-#define for_each_cpu(cpu, mask)			\
-	for ((cpu) = 0; (cpu) < 1; (cpu)++, (void)mask)
-#define for_each_cpu_not(cpu, mask)		\
-	for ((cpu) = 0; (cpu) < 1; (cpu)++, (void)mask)
-#define for_each_cpu_wrap(cpu, mask, start)	\
-	for ((cpu) = 0; (cpu) < 1; (cpu)++, (void)mask, (void)(start))
-#define for_each_cpu_and(cpu, mask1, mask2)	\
-	for ((cpu) = 0; (cpu) < 1; (cpu)++, (void)mask1, (void)mask2)
-#else
 /**
  * cpumask_first - get the first cpu in a cpumask
  * @srcp: the cpumask pointer
@@ -324,7 +245,6 @@ extern int cpumask_next_wrap(int n, const struct cpumask *mask, int start, bool
 	for ((cpu) = -1;						\
 		(cpu) = cpumask_next_and((cpu), (mask1), (mask2)),	\
 		(cpu) < nr_cpu_ids;)
-#endif /* SMP */
 
 #define CPU_BITS_NONE						\
 {								\
diff --git a/lib/Makefile b/lib/Makefile
index 81f944cd74ae..5abd7b2064f1 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -34,10 +34,9 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
 	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
 	 earlycpio.o seq_buf.o siphash.o dec_and_lock.o \
 	 nmi_backtrace.o nodemask.o win_minmax.o memcat_p.o \
-	 buildid.o
+	 buildid.o cpumask.o
 
 lib-$(CONFIG_PRINTK) += dump_stack.o
-lib-$(CONFIG_SMP) += cpumask.o
 
 lib-y	+= kobject.o klist.o
 obj-y	+= lockref.o
-- 
2.36.1

[PATCH v2 3/4] cpumask: Add UP optimised for_each_*_cpu versions

[Sander Vanheule]

On uniprocessor builds, the following loops will always run over a mask
that contains one enabled CPU (cpu0):
    - for_each_possible_cpu
    - for_each_online_cpu
    - for_each_present_cpu

Provide uniprocessor-specific macros for these loops, that always run
exactly once.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 include/linux/cpumask.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index d6add0e29ef4..7ccddbc27ac3 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -731,9 +731,16 @@ extern const DECLARE_BITMAP(cpu_all_bits, NR_CPUS);
 /* First bits of cpu_bit_bitmap are in fact unset. */
 #define cpu_none_mask to_cpumask(cpu_bit_bitmap[0])
 
+#if NR_CPUS == 1
+/* Uniprocessor: the possible/online/present masks are always "1" */
+#define for_each_possible_cpu(cpu)	for ((cpu) = 0; (cpu) < 1; (cpu)++)
+#define for_each_online_cpu(cpu)	for ((cpu) = 0; (cpu) < 1; (cpu)++)
+#define for_each_present_cpu(cpu)	for ((cpu) = 0; (cpu) < 1; (cpu)++)
+#else
 #define for_each_possible_cpu(cpu) for_each_cpu((cpu), cpu_possible_mask)
 #define for_each_online_cpu(cpu)   for_each_cpu((cpu), cpu_online_mask)
 #define for_each_present_cpu(cpu)  for_each_cpu((cpu), cpu_present_mask)
+#endif
 
 /* Wrappers for arch boot code to manipulate normally-constant masks */
 void init_cpu_present(const struct cpumask *src);
-- 
2.36.1

[PATCH v2 4/4] cpumask: Update cpumask_next_wrap() signature

[Sander Vanheule]

The extern specifier is not needed for this declaration, so drop it. The
function also depends only on the input parameters, and has no side
effects, so it can be marked __pure like other functions in cpumask.h

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 include/linux/cpumask.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 7ccddbc27ac3..f37ce00741a3 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -210,7 +210,7 @@ int cpumask_any_distribute(const struct cpumask *srcp);
 		(cpu) = cpumask_next_zero((cpu), (mask)),	\
 		(cpu) < nr_cpu_ids;)
 
-extern int cpumask_next_wrap(int n, const struct cpumask *mask, int start, bool wrap);
+int __pure cpumask_next_wrap(int n, const struct cpumask *mask, int start, bool wrap);
 
 /**
  * for_each_cpu_wrap - iterate over every cpu in a mask, starting at a specified location
-- 
2.36.1

Re: [PATCH v2 1/4] lib/test: Introduce cpumask KUnit test suite

[Yury Norov]

On Sat, Jun 04, 2022 at 07:15:56PM +0200, Sander Vanheule wrote:
> Add a basic suite of tests for cpumask, providing some tests for empty
> and completely filled cpumasks.
> 
> Signed-off-by: Sander Vanheule <sander@svanheule.net>

The test must go after fix, so that it's doesn't cause regressions
while bisecting.

> ---
>  lib/Kconfig.debug  |   9 ++++
>  lib/Makefile       |   1 +
>  lib/test_cpumask.c | 115 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 125 insertions(+)
>  create mode 100644 lib/test_cpumask.c

[..]

> +#define FOR_EACH_ITER_EQ(_test, _iter, _expect, _loop)		\
> +	do {							\
> +		(_iter) = 0;					\
> +		_loop						\
> +			(_iter)++;				\
> +		KUNIT_EXPECT_EQ((_test), (_expect), (_iter));	\
> +	} while (0)

This one is harder to use than it should be. Maybe like this? (not tested,
just an idea)

#define TEST_FOR_EACH_CPU_EQ(test, mask)                                \
	do {							        \
                cpumask_t *m = (mask);                                  \
                int iter = 0, cpu;                                      \
		for_each_cpu(cpu, m)		                        \
			iter++;			        	        \
		KUNIT_EXPECT_EQ((test), cpumask_weight(m), iter);	\
	} while (0)

static void test_cpumask_iterators(struct kunit *test)
{
        TEST_FOR_EACH_CPU(test, &mask_empty);
        ...
}
 
Similarly for NOT and WRAP.

Thanks,
Yury

> +static cpumask_t mask_empty;
> +static cpumask_t mask_all;
> +
> +static void test_cpumask_weight(struct kunit *test)
> +{
> +	KUNIT_EXPECT_TRUE(test, cpumask_empty(&mask_empty));
> +	KUNIT_EXPECT_TRUE(test, cpumask_full(cpu_possible_mask));
> +	KUNIT_EXPECT_TRUE(test, cpumask_full(&mask_all));
> +
> +	KUNIT_EXPECT_EQ(test, 0, cpumask_weight(&mask_empty));
> +	KUNIT_EXPECT_EQ(test, nr_cpu_ids, cpumask_weight(cpu_possible_mask));
> +	KUNIT_EXPECT_EQ(test, nr_cpumask_bits, cpumask_weight(&mask_all));
> +}
> +
> +static void test_cpumask_first(struct kunit *test)
> +{
> +	KUNIT_EXPECT_LE(test, nr_cpu_ids, cpumask_first(&mask_empty));
> +	KUNIT_EXPECT_EQ(test, 0, cpumask_first(cpu_possible_mask));
> +
> +	KUNIT_EXPECT_EQ(test, 0, cpumask_first_zero(&mask_empty));
> +	KUNIT_EXPECT_LE(test, nr_cpu_ids, cpumask_first_zero(cpu_possible_mask));
> +}
> +
> +static void test_cpumask_last(struct kunit *test)
> +{
> +	KUNIT_EXPECT_LE(test, nr_cpumask_bits, cpumask_last(&mask_empty));
> +	KUNIT_EXPECT_EQ(test, nr_cpumask_bits - 1, cpumask_last(cpu_possible_mask));
> +}
> +
> +static void test_cpumask_next(struct kunit *test)
> +{
> +	KUNIT_EXPECT_EQ(test, 0, cpumask_next_zero(-1, &mask_empty));
> +	KUNIT_EXPECT_LE(test, nr_cpu_ids, cpumask_next_zero(-1, cpu_possible_mask));
> +
> +	KUNIT_EXPECT_LE(test, nr_cpu_ids, cpumask_next(-1, &mask_empty));
> +	KUNIT_EXPECT_EQ(test, 0, cpumask_next(-1, cpu_possible_mask));
> +}
> +
> +static void test_cpumask_iterators(struct kunit *test)
> +{
> +	unsigned int iterations;
> +	int cpu;
> +
> +	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(&mask_empty),
> +			for_each_cpu(cpu, &mask_empty));
> +	FOR_EACH_ITER_EQ(test, iterations, nr_cpu_ids - cpumask_weight(&mask_empty),
> +			for_each_cpu_not(cpu, &mask_empty));
> +	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(&mask_empty),
> +			for_each_cpu_wrap(cpu, &mask_empty, nr_cpu_ids / 2));
> +
> +	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_possible_mask),
> +			for_each_cpu(cpu, cpu_possible_mask));
> +	FOR_EACH_ITER_EQ(test, iterations, nr_cpu_ids - cpumask_weight(cpu_possible_mask),
> +			for_each_cpu_not(cpu, cpu_possible_mask));
> +	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_possible_mask),
> +			for_each_cpu_wrap(cpu, cpu_possible_mask, nr_cpu_ids / 2));
> +}
> +
> +static void test_cpumask_iterators_builtin(struct kunit *test)
> +{
> +	unsigned int iterations;
> +	int cpu;
> +
> +	FOR_EACH_ITER_EQ(test, iterations, nr_cpu_ids,
> +		for_each_possible_cpu(cpu));
> +	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_online_mask),
> +		for_each_online_cpu(cpu));
> +	FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_present_mask),
> +		for_each_present_cpu(cpu));
> +}
> +
> +static int test_cpumask_init(struct kunit *test)
> +{
> +	cpumask_clear(&mask_empty);
> +	cpumask_setall(&mask_all);
> +
> +	return 0;
> +}
> +
> +static struct kunit_case test_cpumask_cases[] = {
> +	KUNIT_CASE(test_cpumask_weight),
> +	KUNIT_CASE(test_cpumask_first),
> +	KUNIT_CASE(test_cpumask_last),
> +	KUNIT_CASE(test_cpumask_next),
> +	KUNIT_CASE(test_cpumask_iterators),
> +	KUNIT_CASE(test_cpumask_iterators_builtin),
> +	{}
> +};
> +
> +static struct kunit_suite test_cpumask_suite = {
> +	.name = "cpumask",
> +	.init = test_cpumask_init,
> +	.test_cases = test_cpumask_cases,
> +};
> +kunit_test_suite(test_cpumask_suite);
> +
> +MODULE_LICENSE("GPL");
> -- 
> 2.36.1

Re: [PATCH v2 1/4] lib/test: Introduce cpumask KUnit test suite

[Sander Vanheule]

Hi Yury,

On Sat, 2022-06-04 at 12:31 -0700, Yury Norov wrote:
> On Sat, Jun 04, 2022 at 07:15:56PM +0200, Sander Vanheule wrote:
> > Add a basic suite of tests for cpumask, providing some tests for empty
> > and completely filled cpumasks.
> > 
> > Signed-off-by: Sander Vanheule <sander@svanheule.net>
> 
> The test must go after fix, so that it's doesn't cause regressions
> while bisecting.

OK, I'll change the order of the patches.

> 
> > ---
> >  lib/Kconfig.debug  |   9 ++++
> >  lib/Makefile       |   1 +
> >  lib/test_cpumask.c | 115 +++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 125 insertions(+)
> >  create mode 100644 lib/test_cpumask.c
> 
> [..]
> 
> > +#define FOR_EACH_ITER_EQ(_test, _iter, _expect, _loop)         \
> > +       do {                                                    \
> > +               (_iter) = 0;                                    \
> > +               _loop                                           \
> > +                       (_iter)++;                              \
> > +               KUNIT_EXPECT_EQ((_test), (_expect), (_iter));   \
> > +       } while (0)
> 
> This one is harder to use than it should be.

Perhaps I tried to hard to make one macro to cover all the cases.

>  Maybe like this? (not tested,
> just an idea)
> 
> #define TEST_FOR_EACH_CPU_EQ(test, mask)                                \
>         do {                                                            \
>                 cpumask_t *m = (mask);                                  \
>                 int iter = 0, cpu;                                      \
>                 for_each_cpu(cpu, m)                                    \
>                         iter++;                                         \
>                 KUNIT_EXPECT_EQ((test), cpumask_weight(m), iter);       \
>         } while (0)
> 
> static void test_cpumask_iterators(struct kunit *test)
> {
>         TEST_FOR_EACH_CPU(test, &mask_empty);
>         ...
> }
>  
> Similarly for NOT and WRAP.

Thanks for the suggestion, I forgot we don't have to declare all variables at the start of the
function. I'll send an update with a few more specific macros.

Best,
Sander