This series fixes the reported issues, and implements the suggested improvements, for the version of the cpumask tests [1] that was merged with commit c41e8866c28c ("lib/test: introduce cpumask KUnit test suite").
These changes include fixes for the tests, and better alignment with the KUnit style guidelines.
[1] https://lore.kernel.org/all/85217b5de6d62257313ad7fde3e1969421acad75.1659077...
Changes since v2: Link: https://lore.kernel.org/lkml/cover.1661007338.git.sander@svanheule.net/ - Update commit message of "lib/test_cpumask: drop cpu_possible_mask full test" - Use *_MSG() macros to only print mask contents on failure
Changes since v1: Link: https://lore.kernel.org/lkml/cover.1660068429.git.sander@svanheule.net/ - Collect tags - Rewrite commit message of "lib/test_cpumask: drop cpu_possible_mask full test" - Also CC KUnit mailing list
Sander Vanheule (5): lib/test_cpumask: drop cpu_possible_mask full test lib/test_cpumask: fix cpu_possible_mask last test lib/test_cpumask: follow KUnit style guidelines lib/cpumask_kunit: log mask contents lib/cpumask_kunit: add tests file to MAINTAINERS
MAINTAINERS | 1 + lib/Kconfig.debug | 7 +++- lib/Makefile | 2 +- lib/{test_cpumask.c => cpumask_kunit.c} | 52 +++++++++++++++---------- 4 files changed, 38 insertions(+), 24 deletions(-) rename lib/{test_cpumask.c => cpumask_kunit.c} (58%)
When the number of CPUs that can possibly be brought online is known at boot time, e.g. when HOTPLUG is disabled, nr_cpu_ids may be smaller than NR_CPUS. In that case, cpu_possible_mask would not be completely filled, and cpumask_full(cpu_possible_mask) can return false for valid system configurations.
Without this test, cpu_possible_mask contents are still constrained by a check on cpumask_weight(), as well as tests in test_cpumask_first(), test_cpumask_last(), test_cpumask_next(), and test_cpumask_iterators().
Fixes: c41e8866c28c ("lib/test: introduce cpumask KUnit test suite") Link: https://lore.kernel.org/lkml/346cb279-8e75-24b0-7d12-9803f2b41c73@riseup.net... Reported-by: Maíra Canal mairacanal@riseup.net Signed-off-by: Sander Vanheule sander@svanheule.net Tested-by: Maíra Canal mairacanal@riseup.net Reviewed-by: David Gow davidgow@google.com --- Changes in v3: - Update commit message to list remaining tests on cpu_possible_mask
lib/test_cpumask.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/lib/test_cpumask.c b/lib/test_cpumask.c index a31a1622f1f6..4ebf9f5805f3 100644 --- a/lib/test_cpumask.c +++ b/lib/test_cpumask.c @@ -54,7 +54,6 @@ 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));
Since cpumask_first() on the cpu_possible_mask must return at most nr_cpu_ids - 1 for a valid result, cpumask_last() cannot return anything larger than this value. As test_cpumask_weight() also verifies that the total weight of cpu_possible_mask must equal nr_cpu_ids, the last bit set in this mask must be at nr_cpu_ids - 1.
Fixes: c41e8866c28c ("lib/test: introduce cpumask KUnit test suite") Link: https://lore.kernel.org/lkml/346cb279-8e75-24b0-7d12-9803f2b41c73@riseup.net... Reported-by: Maíra Canal mairacanal@riseup.net Signed-off-by: Sander Vanheule sander@svanheule.net Tested-by: Maíra Canal mairacanal@riseup.net Reviewed-by: David Gow davidgow@google.com Acked-by: Yury Norov yury.norov@gmail.com --- lib/test_cpumask.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/test_cpumask.c b/lib/test_cpumask.c index 4ebf9f5805f3..4d353614d853 100644 --- a/lib/test_cpumask.c +++ b/lib/test_cpumask.c @@ -73,7 +73,7 @@ static void test_cpumask_first(struct kunit *test) 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)); + KUNIT_EXPECT_EQ(test, nr_cpu_ids - 1, cpumask_last(cpu_possible_mask)); }
static void test_cpumask_next(struct kunit *test)
The cpumask test suite doesn't follow the KUnit style guidelines, as laid out in Documentation/dev-tools/kunit/style.rst. The file is renamed to lib/cpumask_kunit.c to clearly distinguish it from other, non-KUnit, tests.
Link: https://lore.kernel.org/lkml/346cb279-8e75-24b0-7d12-9803f2b41c73@riseup.net... Suggested-by: Maíra Canal mairacanal@riseup.net Signed-off-by: Sander Vanheule sander@svanheule.net Reviewed-by: Maíra Canal mairacanal@riseup.net Reviewed-by: David Gow davidgow@google.com Acked-by: Yury Norov yury.norov@gmail.com --- lib/Kconfig.debug | 7 +++++-- lib/Makefile | 2 +- lib/{test_cpumask.c => cpumask_kunit.c} | 0 3 files changed, 6 insertions(+), 3 deletions(-) rename lib/{test_cpumask.c => cpumask_kunit.c} (100%)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 072e4b289c13..bcbe60d6c80c 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2029,13 +2029,16 @@ 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 +config CPUMASK_KUNIT_TEST + tristate "KUnit test for cpumask" 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.
+ For more information on KUnit and unit tests in general, please refer + to the KUnit documentation in Documentation/dev-tools/kunit/. + If unsure, say N.
config TEST_LIST_SORT diff --git a/lib/Makefile b/lib/Makefile index 5927d7fa0806..ffabc30a27d4 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -60,6 +60,7 @@ obj-$(CONFIG_TEST_BPF) += test_bpf.o obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o obj-$(CONFIG_TEST_BITOPS) += test_bitops.o CFLAGS_test_bitops.o += -Werror +obj-$(CONFIG_CPUMASK_KUNIT_TEST) += cpumask_kunit.o obj-$(CONFIG_TEST_SYSCTL) += test_sysctl.o obj-$(CONFIG_TEST_SIPHASH) += test_siphash.o obj-$(CONFIG_HASH_KUNIT_TEST) += test_hash.o @@ -100,7 +101,6 @@ 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/cpumask_kunit.c similarity index 100% rename from lib/test_cpumask.c rename to lib/cpumask_kunit.c
For extra context, log the contents of the masks under test. This should help with finding out why a certain test fails.
Link: https://lore.kernel.org/lkml/CABVgOSkPXBc-PWk1zBZRQ_Tt+Sz1ruFHBj3ixojymZF=Vi... Suggested-by: David Gow davidgow@google.com Signed-off-by: Sander Vanheule sander@svanheule.net --- Changes in v3: Instead of unconditionally printing the mask contents before each test with kunit_info(), use *_MSG() macros to only print on failure. Drop David's Reviewed-by since the patch is completely rewritten.
lib/cpumask_kunit.c | 51 +++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 20 deletions(-)
diff --git a/lib/cpumask_kunit.c b/lib/cpumask_kunit.c index 4d353614d853..ecbeec72221e 100644 --- a/lib/cpumask_kunit.c +++ b/lib/cpumask_kunit.c @@ -9,6 +9,10 @@ #include <linux/cpu.h> #include <linux/cpumask.h>
+#define MASK_MSG(m) \ + "%s contains %sCPUs %*pbl", #m, (cpumask_weight(m) ? "" : "no "), \ + nr_cpumask_bits, cpumask_bits(m) + #define EXPECT_FOR_EACH_CPU_EQ(test, mask) \ do { \ const cpumask_t *m = (mask); \ @@ -16,7 +20,7 @@ int cpu, iter = 0; \ for_each_cpu(cpu, m) \ iter++; \ - KUNIT_EXPECT_EQ((test), mask_weight, iter); \ + KUNIT_EXPECT_EQ_MSG((test), mask_weight, iter, MASK_MSG(mask)); \ } while (0)
#define EXPECT_FOR_EACH_CPU_NOT_EQ(test, mask) \ @@ -26,7 +30,7 @@ int cpu, iter = 0; \ for_each_cpu_not(cpu, m) \ iter++; \ - KUNIT_EXPECT_EQ((test), nr_cpu_ids - mask_weight, iter); \ + KUNIT_EXPECT_EQ_MSG((test), nr_cpu_ids - mask_weight, iter, MASK_MSG(mask)); \ } while (0)
#define EXPECT_FOR_EACH_CPU_WRAP_EQ(test, mask) \ @@ -36,7 +40,7 @@ int cpu, iter = 0; \ for_each_cpu_wrap(cpu, m, nr_cpu_ids / 2) \ iter++; \ - KUNIT_EXPECT_EQ((test), mask_weight, iter); \ + KUNIT_EXPECT_EQ_MSG((test), mask_weight, iter, MASK_MSG(mask)); \ } while (0)
#define EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, name) \ @@ -45,7 +49,7 @@ int cpu, iter = 0; \ for_each_##name##_cpu(cpu) \ iter++; \ - KUNIT_EXPECT_EQ((test), mask_weight, iter); \ + KUNIT_EXPECT_EQ_MSG((test), mask_weight, iter, MASK_MSG(cpu_##name##_mask)); \ } while (0)
static cpumask_t mask_empty; @@ -53,36 +57,43 @@ 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(&mask_all)); + KUNIT_EXPECT_TRUE_MSG(test, cpumask_empty(&mask_empty), MASK_MSG(&mask_empty)); + KUNIT_EXPECT_TRUE_MSG(test, cpumask_full(&mask_all), MASK_MSG(&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)); + KUNIT_EXPECT_EQ_MSG(test, 0, cpumask_weight(&mask_empty), MASK_MSG(&mask_empty)); + KUNIT_EXPECT_EQ_MSG(test, nr_cpu_ids, cpumask_weight(cpu_possible_mask), + MASK_MSG(cpu_possible_mask)); + KUNIT_EXPECT_EQ_MSG(test, nr_cpumask_bits, cpumask_weight(&mask_all), MASK_MSG(&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_LE_MSG(test, nr_cpu_ids, cpumask_first(&mask_empty), MASK_MSG(&mask_empty)); + KUNIT_EXPECT_EQ_MSG(test, 0, cpumask_first(cpu_possible_mask), MASK_MSG(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)); + KUNIT_EXPECT_EQ_MSG(test, 0, cpumask_first_zero(&mask_empty), MASK_MSG(&mask_empty)); + KUNIT_EXPECT_LE_MSG(test, nr_cpu_ids, cpumask_first_zero(cpu_possible_mask), + MASK_MSG(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_cpu_ids - 1, cpumask_last(cpu_possible_mask)); + KUNIT_EXPECT_LE_MSG(test, nr_cpumask_bits, cpumask_last(&mask_empty), + MASK_MSG(&mask_empty)); + KUNIT_EXPECT_EQ_MSG(test, nr_cpu_ids - 1, cpumask_last(cpu_possible_mask), + MASK_MSG(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)); + KUNIT_EXPECT_EQ_MSG(test, 0, cpumask_next_zero(-1, &mask_empty), MASK_MSG(&mask_empty)); + KUNIT_EXPECT_LE_MSG(test, nr_cpu_ids, cpumask_next_zero(-1, cpu_possible_mask), + MASK_MSG(cpu_possible_mask)); + + KUNIT_EXPECT_LE_MSG(test, nr_cpu_ids, cpumask_next(-1, &mask_empty), + MASK_MSG(&mask_empty)); + KUNIT_EXPECT_EQ_MSG(test, 0, cpumask_next(-1, cpu_possible_mask), + MASK_MSG(cpu_possible_mask)); }
static void test_cpumask_iterators(struct kunit *test)
cpumask related files are listed under the BITMAP API section, so the file with tests for cpumask should be added to that list.
Signed-off-by: Sander Vanheule sander@svanheule.net Reviewed-by: David Gow davidgow@google.com Acked-by: Yury Norov yury.norov@gmail.com --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS index f512b430c7cb..0f41174be0d3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3612,6 +3612,7 @@ F: include/linux/find.h F: include/linux/nodemask.h 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
On Tue, Aug 23, 2022 at 08:12:17AM +0200, Sander Vanheule wrote:
This series fixes the reported issues, and implements the suggested improvements, for the version of the cpumask tests [1] that was merged with commit c41e8866c28c ("lib/test: introduce cpumask KUnit test suite").
These changes include fixes for the tests, and better alignment with the KUnit style guidelines.
Applied in bitmap-for-next.
Thanks, Yury
[1] https://lore.kernel.org/all/85217b5de6d62257313ad7fde3e1969421acad75.1659077...
Changes since v2: Link: https://lore.kernel.org/lkml/cover.1661007338.git.sander@svanheule.net/
- Update commit message of "lib/test_cpumask: drop cpu_possible_mask full test"
- Use *_MSG() macros to only print mask contents on failure
Changes since v1: Link: https://lore.kernel.org/lkml/cover.1660068429.git.sander@svanheule.net/
- Collect tags
- Rewrite commit message of "lib/test_cpumask: drop cpu_possible_mask full test"
- Also CC KUnit mailing list
Sander Vanheule (5): lib/test_cpumask: drop cpu_possible_mask full test lib/test_cpumask: fix cpu_possible_mask last test lib/test_cpumask: follow KUnit style guidelines lib/cpumask_kunit: log mask contents lib/cpumask_kunit: add tests file to MAINTAINERS
MAINTAINERS | 1 + lib/Kconfig.debug | 7 +++- lib/Makefile | 2 +- lib/{test_cpumask.c => cpumask_kunit.c} | 52 +++++++++++++++---------- 4 files changed, 38 insertions(+), 24 deletions(-) rename lib/{test_cpumask.c => cpumask_kunit.c} (58%)
-- 2.37.2
linux-kselftest-mirror@lists.linaro.org
-
Sander Vanheule -
Yury Norov