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 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} | 13 +++++++++++-- 4 files changed, 18 insertions(+), 5 deletions(-) rename lib/{test_cpumask.c => cpumask_kunit.c} (90%)
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.
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 v2: Rewrite commit message to explain why this test is wrong
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));
On Sat, Aug 20, 2022 at 05:03:09PM +0200, Sander Vanheule wrote:
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.
It doesn't mean we can just give up. You can check validity of possible cpumask like this: KUNIT_EXPECT_EQ(test, nr_cpu_ids, cpumask_first_zero(&mask_all)) KUNIT_EXPECT_EQ(test, NR_CPUS, cpumask_first(&mask_all))
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 v2: Rewrite commit message to explain why this test is wrong
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)); -- 2.37.2
Hi Yury,
On Sat, 2022-08-20 at 14:35 -0700, Yury Norov wrote:
On Sat, Aug 20, 2022 at 05:03:09PM +0200, Sander Vanheule wrote:
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.
It doesn't mean we can just give up. You can check validity of possible cpumask like this: KUNIT_EXPECT_EQ(test, nr_cpu_ids, cpumask_first_zero(&mask_all)) KUNIT_EXPECT_EQ(test, NR_CPUS, cpumask_first(&mask_all))
Did you mean cpu_possible_mask, or mask_all?
For cpu_possible_mask, these tests are in test_cpumask_first(), albeit under a slightly different form. Together with the tests in test_cpumask_weight() and test_cpumask_last(), cpu_possible_mask is already one of the more constrained masks.
For mask_all, the mask is filled up with nr_cpumask_bits <= NR_CPUS. I could add cpumask_first(), cpumask_first_zero(), and cpumask_last() tests though.
More tests could be also added for cpu_all_mask, since this does have all NR_CPUS bits set, but I think that belongs in a separate patch.
I think the extra mask_all and cpu_all_mask test are out of scope for this patch, but they could be added in another patch (for 6.1).
Best, Sander
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 v2: Rewrite commit message to explain why this test is wrong
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)); -- 2.37.2
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 --- 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 --- 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 Reviewed-by: David Gow davidgow@google.com --- lib/cpumask_kunit.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/lib/cpumask_kunit.c b/lib/cpumask_kunit.c index 4d353614d853..0f8059a5e93b 100644 --- a/lib/cpumask_kunit.c +++ b/lib/cpumask_kunit.c @@ -51,6 +51,10 @@ static cpumask_t mask_empty; static cpumask_t mask_all;
+#define STR_MASK(m) #m +#define TEST_CPUMASK_PRINT(test, mask) \ + kunit_info(test, "%s = '%*pbl'\n", STR_MASK(mask), nr_cpumask_bits, cpumask_bits(mask)) + static void test_cpumask_weight(struct kunit *test) { KUNIT_EXPECT_TRUE(test, cpumask_empty(&mask_empty)); @@ -103,6 +107,9 @@ static void test_cpumask_iterators_builtin(struct kunit *test) /* Ensure the dynamic masks are stable while running the tests */ cpu_hotplug_disable();
+ TEST_CPUMASK_PRINT(test, cpu_online_mask); + TEST_CPUMASK_PRINT(test, cpu_present_mask); + EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, online); EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, present);
@@ -114,6 +121,9 @@ static int test_cpumask_init(struct kunit *test) cpumask_clear(&mask_empty); cpumask_setall(&mask_all);
+ TEST_CPUMASK_PRINT(test, &mask_all); + TEST_CPUMASK_PRINT(test, cpu_possible_mask); + return 0; }
On Sat, Aug 20, 2022 at 05:03:12PM +0200, Sander Vanheule wrote:
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 Reviewed-by: David Gow davidgow@google.com
lib/cpumask_kunit.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/lib/cpumask_kunit.c b/lib/cpumask_kunit.c index 4d353614d853..0f8059a5e93b 100644 --- a/lib/cpumask_kunit.c +++ b/lib/cpumask_kunit.c @@ -51,6 +51,10 @@ static cpumask_t mask_empty; static cpumask_t mask_all; +#define STR_MASK(m) #m +#define TEST_CPUMASK_PRINT(test, mask) \
- kunit_info(test, "%s = '%*pbl'\n", STR_MASK(mask), nr_cpumask_bits, cpumask_bits(mask))
static void test_cpumask_weight(struct kunit *test) { KUNIT_EXPECT_TRUE(test, cpumask_empty(&mask_empty)); @@ -103,6 +107,9 @@ static void test_cpumask_iterators_builtin(struct kunit *test) /* Ensure the dynamic masks are stable while running the tests */ cpu_hotplug_disable();
- TEST_CPUMASK_PRINT(test, cpu_online_mask);
- TEST_CPUMASK_PRINT(test, cpu_present_mask);
- EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, online); EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, present);
@@ -114,6 +121,9 @@ static int test_cpumask_init(struct kunit *test) cpumask_clear(&mask_empty); cpumask_setall(&mask_all);
- TEST_CPUMASK_PRINT(test, &mask_all);
- TEST_CPUMASK_PRINT(test, cpu_possible_mask);
It sort of breaks the rule of silence. Can you make this print conditional on a test failure? If everything is OK, who wants to look into details?
return 0; } -- 2.37.2
On Sat, 2022-08-20 at 14:46 -0700, Yury Norov wrote:
On Sat, Aug 20, 2022 at 05:03:12PM +0200, Sander Vanheule wrote:
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 Reviewed-by: David Gow davidgow@google.com
lib/cpumask_kunit.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/lib/cpumask_kunit.c b/lib/cpumask_kunit.c index 4d353614d853..0f8059a5e93b 100644 --- a/lib/cpumask_kunit.c +++ b/lib/cpumask_kunit.c @@ -51,6 +51,10 @@ static cpumask_t mask_empty; static cpumask_t mask_all; +#define STR_MASK(m) #m +#define TEST_CPUMASK_PRINT(test, mask) \ + kunit_info(test, "%s = '%*pbl'\n", STR_MASK(mask), nr_cpumask_bits, cpumask_bits(mask))
static void test_cpumask_weight(struct kunit *test) { KUNIT_EXPECT_TRUE(test, cpumask_empty(&mask_empty)); @@ -103,6 +107,9 @@ static void test_cpumask_iterators_builtin(struct kunit *test) /* Ensure the dynamic masks are stable while running the tests */ cpu_hotplug_disable(); + TEST_CPUMASK_PRINT(test, cpu_online_mask); + TEST_CPUMASK_PRINT(test, cpu_present_mask);
EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, online); EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, present); @@ -114,6 +121,9 @@ static int test_cpumask_init(struct kunit *test) cpumask_clear(&mask_empty); cpumask_setall(&mask_all); + TEST_CPUMASK_PRINT(test, &mask_all); + TEST_CPUMASK_PRINT(test, cpu_possible_mask);
It sort of breaks the rule of silence. Can you make this print conditional on a test failure? If everything is OK, who wants to look into details?
I will change the macros to the _MSG versions, and log the mask there.
I implemented this with kunit_info() as David proposed. AFAICT I can't call kunit_info() only when the test fails, because the EXPECT_ macros don't return any result.
Best, Sander
return 0; } -- 2.37.2
On 8/21/22 10:13, Sander Vanheule wrote:
On Sat, 2022-08-20 at 14:46 -0700, Yury Norov wrote:
On Sat, Aug 20, 2022 at 05:03:12PM +0200, Sander Vanheule wrote:
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 Reviewed-by: David Gow davidgow@google.com
lib/cpumask_kunit.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/lib/cpumask_kunit.c b/lib/cpumask_kunit.c index 4d353614d853..0f8059a5e93b 100644 --- a/lib/cpumask_kunit.c +++ b/lib/cpumask_kunit.c @@ -51,6 +51,10 @@ static cpumask_t mask_empty; static cpumask_t mask_all; +#define STR_MASK(m) #m +#define TEST_CPUMASK_PRINT(test, mask) \ + kunit_info(test, "%s = '%*pbl'\n", STR_MASK(mask), nr_cpumask_bits, cpumask_bits(mask))
static void test_cpumask_weight(struct kunit *test) { KUNIT_EXPECT_TRUE(test, cpumask_empty(&mask_empty)); @@ -103,6 +107,9 @@ static void test_cpumask_iterators_builtin(struct kunit *test) /* Ensure the dynamic masks are stable while running the tests */ cpu_hotplug_disable(); + TEST_CPUMASK_PRINT(test, cpu_online_mask); + TEST_CPUMASK_PRINT(test, cpu_present_mask);
EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, online); EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, present); @@ -114,6 +121,9 @@ static int test_cpumask_init(struct kunit *test) cpumask_clear(&mask_empty); cpumask_setall(&mask_all); + TEST_CPUMASK_PRINT(test, &mask_all); + TEST_CPUMASK_PRINT(test, cpu_possible_mask);
It sort of breaks the rule of silence. Can you make this print conditional on a test failure? If everything is OK, who wants to look into details?
I will change the macros to the _MSG versions, and log the mask there.
I implemented this with kunit_info() as David proposed. AFAICT I can't call kunit_info() only when the test fails, because the EXPECT_ macros don't return any result.
Maybe you can use KUNIT_EXPECT_EQ_MSG to print a more detailed error and avoid printing the info when the test doesn't fail.
Best Regards, - Maíra Canal
Best, Sander
return 0; } -- 2.37.2
Hi Maíra,
On Sun, 2022-08-21 at 11:02 -0300, Maíra Canal wrote:
On 8/21/22 10:13, Sander Vanheule wrote:
On Sat, 2022-08-20 at 14:46 -0700, Yury Norov wrote:
On Sat, Aug 20, 2022 at 05:03:12PM +0200, Sander Vanheule wrote:
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 Reviewed-by: David Gow davidgow@google.com
lib/cpumask_kunit.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/lib/cpumask_kunit.c b/lib/cpumask_kunit.c index 4d353614d853..0f8059a5e93b 100644 --- a/lib/cpumask_kunit.c +++ b/lib/cpumask_kunit.c @@ -51,6 +51,10 @@ static cpumask_t mask_empty; static cpumask_t mask_all; +#define STR_MASK(m) #m +#define TEST_CPUMASK_PRINT(test, mask) \ + kunit_info(test, "%s = '%*pbl'\n", STR_MASK(mask), nr_cpumask_bits, cpumask_bits(mask))
static void test_cpumask_weight(struct kunit *test) { KUNIT_EXPECT_TRUE(test, cpumask_empty(&mask_empty)); @@ -103,6 +107,9 @@ static void test_cpumask_iterators_builtin(struct kunit *test) /* Ensure the dynamic masks are stable while running the tests */ cpu_hotplug_disable(); + TEST_CPUMASK_PRINT(test, cpu_online_mask); + TEST_CPUMASK_PRINT(test, cpu_present_mask);
EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, online); EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, present); @@ -114,6 +121,9 @@ static int test_cpumask_init(struct kunit *test) cpumask_clear(&mask_empty); cpumask_setall(&mask_all); + TEST_CPUMASK_PRINT(test, &mask_all); + TEST_CPUMASK_PRINT(test, cpu_possible_mask);
It sort of breaks the rule of silence. Can you make this print conditional on a test failure? If everything is OK, who wants to look into details?
I will change the macros to the _MSG versions, and log the mask there.
I implemented this with kunit_info() as David proposed. AFAICT I can't call kunit_info() only when the test fails, because the EXPECT_ macros don't return any result.
Maybe you can use KUNIT_EXPECT_EQ_MSG to print a more detailed error and avoid printing the info when the test doesn't fail.
Yes, this is what I currently have for use with the _MSG() variants of the macros:
+#define MASK_MSG(m) \ + "%s contains %sCPUs %*pbl", #m, (cpumask_weight(m) ? "" : "no "), nr_cpumask_bits, cpumask_bits(m) +
For example, with (bogus) KUNIT_EXPECT_TRUE_MSG(test, ..., MASK_MSG(mask)) this becomes (trimmed):
$ ./tools/testing/kunit/kunit.py run --build_dir=build-um cpumask [...] [18:15:33] # test_cpumask_weight: EXPECTATION FAILED at lib/cpumask_kunit.c:60 [18:15:33] Expected cpumask_empty(((struct cpumask *)(1 ? (cpu_all_bits) : (void *)sizeof(__check_is_bitmap(cpu_all_bits))))) to be true, but is false [18:15:33] [18:15:33] cpu_all_mask contains CPUs 0 [18:15:33] # test_cpumask_weight: EXPECTATION FAILED at lib/cpumask_kunit.c:61 [18:15:33] Expected cpumask_full(&mask_empty) to be true, but is false [18:15:33] [18:15:33] &mask_empty contains no CPUs [18:15:33] not ok 1 - test_cpumask_weight [18:15:33] [FAILED] test_cpumask_weight [...]
Or on a real system: [ 1.246805] # test_cpumask_weight: EXPECTATION FAILED at lib/cpumask_kunit.c:59 [ 1.246805] Expected cpumask_empty(((struct cpumask *)(1 ? (cpu_all_bits) : (void *)sizeof(__check_is_bitmap(cpu_all_bits))))) to be true, but is false [ 1.246805] [ 1.246805] cpu_all_mask contains CPUs 0-1 [ 1.249756] not ok 1 - test_cpumask_weight
I will send an updated series tomorrow, in case David or others have more more comments.
Best, Sander
cpumask related files are listed under the BITMAP API section, so file with the tests for cpumask should be added to that list.
Signed-off-by: Sander Vanheule sander@svanheule.net Reviewed-by: David Gow davidgow@google.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 Sat, Aug 20, 2022 at 05:03:08PM +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.
I wrote a couple comments, but the series looks OK to me in general. So for 2, 3 and 5: Acked-by: Yury Norov yury.norov@gmail.com
It's named as 'fix', but it fixes a test, and the kernel code itself looks correct. So, do you want to take it into 6.0-rc, or in 6.1?
I'm OK to do it this way or another, but for later -rc's it may look too noisy. And I'm not sure where to put a threshold.
Thanks, Yury
[1] https://lore.kernel.org/all/85217b5de6d62257313ad7fde3e1969421acad75.1659077...
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} | 13 +++++++++++-- 4 files changed, 18 insertions(+), 5 deletions(-) rename lib/{test_cpumask.c => cpumask_kunit.c} (90%)
-- 2.37.2
Hi Yury,
On Sat, 2022-08-20 at 15:06 -0700, Yury Norov wrote:
On Sat, Aug 20, 2022 at 05:03:08PM +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.
I wrote a couple comments, but the series looks OK to me in general. So for 2, 3 and 5: Acked-by: Yury Norov yury.norov@gmail.com
It's named as 'fix', but it fixes a test, and the kernel code itself looks correct. So, do you want to take it into 6.0-rc, or in 6.1?
I'm OK to do it this way or another, but for later -rc's it may look too noisy. And I'm not sure where to put a threshold.
Broken tests are worse than no tests IMHO, so I would at least like patches 1 and 2 to be merged for 6.0-rc. I don't want people to end up with false positives, like Maíra did, for an entire release cycle.
Preferably I would also like to see 3 in 6.0-rc, so no renames will be needed in 6.1 anymore. Not that I expect anything to depend on this symbol (or filename) by then, but I feel it's better not to risk that by waiting for 6.1.
Patches 4 and 5 can go with 6.1, as far as I'm concerned. Especially as the mask logging patch (4) may need some work still.
Best, Sander
linux-kselftest-mirror@lists.linaro.org
-
Maíra Canal -
Sander Vanheule -
Yury Norov