* [PATCH v1 0/5] cpumask: KUnit test suite fixes and improvements
@ 2022-08-09 18:08 Sander Vanheule
2022-08-09 18:08 ` [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test Sander Vanheule
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Sander Vanheule @ 2022-08-09 18:08 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes
Cc: linux-kernel, Maíra Canal, David Gow, Sander Vanheule
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").
[1] https://lore.kernel.org/all/85217b5de6d62257313ad7fde3e1969421acad75.1659077534.git.sander@svanheule.net/
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.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test
2022-08-09 18:08 [PATCH v1 0/5] cpumask: KUnit test suite fixes and improvements Sander Vanheule
@ 2022-08-09 18:08 ` Sander Vanheule
2022-08-09 22:26 ` Maíra Canal
2022-08-10 4:06 ` David Gow
2022-08-09 18:08 ` [PATCH v1 2/5] lib/test_cpumask: fix cpu_possible_mask last test Sander Vanheule
` (3 subsequent siblings)
4 siblings, 2 replies; 17+ messages in thread
From: Sander Vanheule @ 2022-08-09 18:08 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes
Cc: linux-kernel, Maíra Canal, David Gow, Sander Vanheule
cpu_possible_mask is not necessarily completely filled. That means
running a check on cpumask_full() doesn't make sense, so drop the test.
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>
Cc: David Gow <davidgow@google.com>
---
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.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 2/5] lib/test_cpumask: fix cpu_possible_mask last test
2022-08-09 18:08 [PATCH v1 0/5] cpumask: KUnit test suite fixes and improvements Sander Vanheule
2022-08-09 18:08 ` [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test Sander Vanheule
@ 2022-08-09 18:08 ` Sander Vanheule
2022-08-09 22:27 ` Maíra Canal
2022-08-10 4:06 ` David Gow
2022-08-09 18:08 ` [PATCH v1 3/5] lib/test_cpumask: follow KUnit style guidelines Sander Vanheule
` (2 subsequent siblings)
4 siblings, 2 replies; 17+ messages in thread
From: Sander Vanheule @ 2022-08-09 18:08 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes
Cc: linux-kernel, Maíra Canal, David Gow, Sander Vanheule
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>
Cc: 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)
--
2.37.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 3/5] lib/test_cpumask: follow KUnit style guidelines
2022-08-09 18:08 [PATCH v1 0/5] cpumask: KUnit test suite fixes and improvements Sander Vanheule
2022-08-09 18:08 ` [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test Sander Vanheule
2022-08-09 18:08 ` [PATCH v1 2/5] lib/test_cpumask: fix cpu_possible_mask last test Sander Vanheule
@ 2022-08-09 18:08 ` Sander Vanheule
2022-08-09 22:29 ` Maíra Canal
2022-08-10 4:06 ` David Gow
2022-08-09 18:08 ` [PATCH v1 4/5] lib/cpumask_kunit: log mask contents Sander Vanheule
2022-08-09 18:08 ` [PATCH v1 5/5] lib/cpumask_kunit: add tests file to MAINTAINERS Sander Vanheule
4 siblings, 2 replies; 17+ messages in thread
From: Sander Vanheule @ 2022-08-09 18:08 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes
Cc: linux-kernel, Maíra Canal, David Gow, Sander Vanheule
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>
---
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 c95212141928..22eafcd39b51 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -59,6 +59,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
@@ -99,7 +100,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
--
2.37.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 4/5] lib/cpumask_kunit: log mask contents
2022-08-09 18:08 [PATCH v1 0/5] cpumask: KUnit test suite fixes and improvements Sander Vanheule
` (2 preceding siblings ...)
2022-08-09 18:08 ` [PATCH v1 3/5] lib/test_cpumask: follow KUnit style guidelines Sander Vanheule
@ 2022-08-09 18:08 ` Sander Vanheule
2022-08-10 4:07 ` David Gow
2022-08-09 18:08 ` [PATCH v1 5/5] lib/cpumask_kunit: add tests file to MAINTAINERS Sander Vanheule
4 siblings, 1 reply; 17+ messages in thread
From: Sander Vanheule @ 2022-08-09 18:08 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes
Cc: linux-kernel, Maíra Canal, David Gow, Sander Vanheule
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=Vi4tpQ@mail.gmail.com/
Suggested-by: David Gow <davidgow@google.com>
Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
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;
}
--
2.37.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 5/5] lib/cpumask_kunit: add tests file to MAINTAINERS
2022-08-09 18:08 [PATCH v1 0/5] cpumask: KUnit test suite fixes and improvements Sander Vanheule
` (3 preceding siblings ...)
2022-08-09 18:08 ` [PATCH v1 4/5] lib/cpumask_kunit: log mask contents Sander Vanheule
@ 2022-08-09 18:08 ` Sander Vanheule
2022-08-10 4:06 ` David Gow
4 siblings, 1 reply; 17+ messages in thread
From: Sander Vanheule @ 2022-08-09 18:08 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes
Cc: linux-kernel, Maíra Canal, David Gow, Sander Vanheule
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>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 868bbf31603d..21ff272c2c10 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3601,6 +3601,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
--
2.37.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test
2022-08-09 18:08 ` [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test Sander Vanheule
@ 2022-08-09 22:26 ` Maíra Canal
2022-08-10 8:18 ` Sander Vanheule
2022-08-10 4:06 ` David Gow
1 sibling, 1 reply; 17+ messages in thread
From: Maíra Canal @ 2022-08-09 22:26 UTC (permalink / raw)
To: Sander Vanheule, Yury Norov, Andy Shevchenko, Rasmus Villemoes
Cc: linux-kernel, David Gow
On 8/9/22 15:08, Sander Vanheule wrote:
> cpu_possible_mask is not necessarily completely filled. That means
> running a check on cpumask_full() doesn't make sense, so drop the test.
>
> 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>
> Cc: David Gow <davidgow@google.com>
> ---
> 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));
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 2/5] lib/test_cpumask: fix cpu_possible_mask last test
2022-08-09 18:08 ` [PATCH v1 2/5] lib/test_cpumask: fix cpu_possible_mask last test Sander Vanheule
@ 2022-08-09 22:27 ` Maíra Canal
2022-08-10 4:06 ` David Gow
1 sibling, 0 replies; 17+ messages in thread
From: Maíra Canal @ 2022-08-09 22:27 UTC (permalink / raw)
To: Sander Vanheule, Yury Norov, Andy Shevchenko, Rasmus Villemoes
Cc: linux-kernel, David Gow
On 8/9/22 15:08, Sander Vanheule wrote:
> 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>
> Cc: 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)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 3/5] lib/test_cpumask: follow KUnit style guidelines
2022-08-09 18:08 ` [PATCH v1 3/5] lib/test_cpumask: follow KUnit style guidelines Sander Vanheule
@ 2022-08-09 22:29 ` Maíra Canal
2022-08-10 4:06 ` David Gow
1 sibling, 0 replies; 17+ messages in thread
From: Maíra Canal @ 2022-08-09 22:29 UTC (permalink / raw)
To: Sander Vanheule, Yury Norov, Andy Shevchenko, Rasmus Villemoes
Cc: linux-kernel, David Gow
On 8/9/22 15:08, Sander Vanheule wrote:
> 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>
> ---
> 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 c95212141928..22eafcd39b51 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -59,6 +59,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
> @@ -99,7 +100,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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 5/5] lib/cpumask_kunit: add tests file to MAINTAINERS
2022-08-09 18:08 ` [PATCH v1 5/5] lib/cpumask_kunit: add tests file to MAINTAINERS Sander Vanheule
@ 2022-08-10 4:06 ` David Gow
0 siblings, 0 replies; 17+ messages in thread
From: David Gow @ 2022-08-10 4:06 UTC (permalink / raw)
To: Sander Vanheule
Cc: Yury Norov, Andy Shevchenko, Rasmus Villemoes,
Linux Kernel Mailing List, Maíra Canal
On Wed, Aug 10, 2022 at 2:09 AM Sander Vanheule <sander@svanheule.net> wrote:
>
> 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>
Cheers,
-- David
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 868bbf31603d..21ff272c2c10 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3601,6 +3601,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
> --
> 2.37.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test
2022-08-09 18:08 ` [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test Sander Vanheule
2022-08-09 22:26 ` Maíra Canal
@ 2022-08-10 4:06 ` David Gow
2022-08-10 8:45 ` Sander Vanheule
1 sibling, 1 reply; 17+ messages in thread
From: David Gow @ 2022-08-10 4:06 UTC (permalink / raw)
To: Sander Vanheule
Cc: Yury Norov, Andy Shevchenko, Rasmus Villemoes,
Linux Kernel Mailing List, Maíra Canal
On Wed, Aug 10, 2022 at 2:09 AM Sander Vanheule <sander@svanheule.net> wrote:
>
> cpu_possible_mask is not necessarily completely filled. That means
> running a check on cpumask_full() doesn't make sense, so drop the test.
>
> 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>
> Cc: David Gow <davidgow@google.com>
> ---
Looks good to me. It'd maybe be worth noting _why_ cpu_possible_mask
is not always filled (i.e., that the number of available CPUs might
not match the maximum number of CPUs the kernel is built to support),
but it's probably not worth doing a new version of the patch series
just for that.
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> 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.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 2/5] lib/test_cpumask: fix cpu_possible_mask last test
2022-08-09 18:08 ` [PATCH v1 2/5] lib/test_cpumask: fix cpu_possible_mask last test Sander Vanheule
2022-08-09 22:27 ` Maíra Canal
@ 2022-08-10 4:06 ` David Gow
1 sibling, 0 replies; 17+ messages in thread
From: David Gow @ 2022-08-10 4:06 UTC (permalink / raw)
To: Sander Vanheule
Cc: Yury Norov, Andy Shevchenko, Rasmus Villemoes,
Linux Kernel Mailing List, Maíra Canal
On Wed, Aug 10, 2022 at 2:09 AM Sander Vanheule <sander@svanheule.net> wrote:
>
> 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>
> Cc: David Gow <davidgow@google.com>
> ---
Much better, thanks.
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> 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)
> --
> 2.37.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 3/5] lib/test_cpumask: follow KUnit style guidelines
2022-08-09 18:08 ` [PATCH v1 3/5] lib/test_cpumask: follow KUnit style guidelines Sander Vanheule
2022-08-09 22:29 ` Maíra Canal
@ 2022-08-10 4:06 ` David Gow
1 sibling, 0 replies; 17+ messages in thread
From: David Gow @ 2022-08-10 4:06 UTC (permalink / raw)
To: Sander Vanheule
Cc: Yury Norov, Andy Shevchenko, Rasmus Villemoes,
Linux Kernel Mailing List, Maíra Canal
On Wed, Aug 10, 2022 at 2:09 AM Sander Vanheule <sander@svanheule.net> wrote:
>
> 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>
> ---
Thanks: it's definitely nicer to have this match the KUnit style.
If renaming causes problems, we could live without it, but I'd prefer
this to go through as-is.
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 c95212141928..22eafcd39b51 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -59,6 +59,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
> @@ -99,7 +100,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
> --
> 2.37.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 4/5] lib/cpumask_kunit: log mask contents
2022-08-09 18:08 ` [PATCH v1 4/5] lib/cpumask_kunit: log mask contents Sander Vanheule
@ 2022-08-10 4:07 ` David Gow
0 siblings, 0 replies; 17+ messages in thread
From: David Gow @ 2022-08-10 4:07 UTC (permalink / raw)
To: Sander Vanheule
Cc: Yury Norov, Andy Shevchenko, Rasmus Villemoes,
Linux Kernel Mailing List, Maíra Canal
On Wed, Aug 10, 2022 at 2:09 AM Sander Vanheule <sander@svanheule.net> 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=Vi4tpQ@mail.gmail.com/
> Suggested-by: David Gow <davidgow@google.com>
> Signed-off-by: Sander Vanheule <sander@svanheule.net>
> ---
Thanks!
Another option would be to only print this on test failure, but it's
fine as-is as well.
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> 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;
> }
>
> --
> 2.37.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test
2022-08-09 22:26 ` Maíra Canal
@ 2022-08-10 8:18 ` Sander Vanheule
0 siblings, 0 replies; 17+ messages in thread
From: Sander Vanheule @ 2022-08-10 8:18 UTC (permalink / raw)
To: Maíra Canal, Yury Norov, Andy Shevchenko, Rasmus Villemoes
Cc: linux-kernel, David Gow
Hi Maíra,
On Tue, 2022-08-09 at 19:26 -0300, Maíra Canal wrote:
> On 8/9/22 15:08, Sander Vanheule wrote:
> > cpu_possible_mask is not necessarily completely filled. That means
> > running a check on cpumask_full() doesn't make sense, so drop the test.
> >
> > 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>
Thanks for testing again, and sorry for the trouble!
Best,
Sander
>
> > Cc: David Gow <davidgow@google.com>
> > ---
> > 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));
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test
2022-08-10 4:06 ` David Gow
@ 2022-08-10 8:45 ` Sander Vanheule
2022-08-10 10:51 ` David Gow
0 siblings, 1 reply; 17+ messages in thread
From: Sander Vanheule @ 2022-08-10 8:45 UTC (permalink / raw)
To: David Gow
Cc: Yury Norov, Andy Shevchenko, Rasmus Villemoes,
Linux Kernel Mailing List, Maíra Canal
Hi David,
On Wed, 2022-08-10 at 12:06 +0800, David Gow wrote:
> On Wed, Aug 10, 2022 at 2:09 AM Sander Vanheule <sander@svanheule.net> wrote:
> >
> > cpu_possible_mask is not necessarily completely filled. That means
> > running a check on cpumask_full() doesn't make sense, so drop the test.
> >
> > 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>
> > Cc: David Gow <davidgow@google.com>
> > ---
>
> Looks good to me. It'd maybe be worth noting _why_ cpu_possible_mask
> is not always filled (i.e., that the number of available CPUs might
> not match the maximum number of CPUs the kernel is built to support),
> but it's probably not worth doing a new version of the patch series
> just for that.
>
> Reviewed-by: David Gow <davidgow@google.com>
Thanks for the reviews!
Perhaps the commit message could be replaced by:
"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) may return false for valid system
configurations."
Best,
Sander
>
> Cheers,
> -- David
>
>
> > 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.1
> >
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test
2022-08-10 8:45 ` Sander Vanheule
@ 2022-08-10 10:51 ` David Gow
0 siblings, 0 replies; 17+ messages in thread
From: David Gow @ 2022-08-10 10:51 UTC (permalink / raw)
To: Sander Vanheule
Cc: Yury Norov, Andy Shevchenko, Rasmus Villemoes,
Linux Kernel Mailing List, Maíra Canal
On Wed, Aug 10, 2022 at 4:45 PM Sander Vanheule <sander@svanheule.net> wrote:
>
> Hi David,
>
> On Wed, 2022-08-10 at 12:06 +0800, David Gow wrote:
> > On Wed, Aug 10, 2022 at 2:09 AM Sander Vanheule <sander@svanheule.net> wrote:
> > >
> > > cpu_possible_mask is not necessarily completely filled. That means
> > > running a check on cpumask_full() doesn't make sense, so drop the test.
> > >
> > > 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>
> > > Cc: David Gow <davidgow@google.com>
> > > ---
> >
> > Looks good to me. It'd maybe be worth noting _why_ cpu_possible_mask
> > is not always filled (i.e., that the number of available CPUs might
> > not match the maximum number of CPUs the kernel is built to support),
> > but it's probably not worth doing a new version of the patch series
> > just for that.
> >
> > Reviewed-by: David Gow <davidgow@google.com>
>
> Thanks for the reviews!
>
> Perhaps the commit message could be replaced by:
>
> "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) may return false for valid system
> configurations."
>
Sounds good to me! Thanks!
-- David
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-08-10 10:51 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-09 18:08 [PATCH v1 0/5] cpumask: KUnit test suite fixes and improvements Sander Vanheule
2022-08-09 18:08 ` [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test Sander Vanheule
2022-08-09 22:26 ` Maíra Canal
2022-08-10 8:18 ` Sander Vanheule
2022-08-10 4:06 ` David Gow
2022-08-10 8:45 ` Sander Vanheule
2022-08-10 10:51 ` David Gow
2022-08-09 18:08 ` [PATCH v1 2/5] lib/test_cpumask: fix cpu_possible_mask last test Sander Vanheule
2022-08-09 22:27 ` Maíra Canal
2022-08-10 4:06 ` David Gow
2022-08-09 18:08 ` [PATCH v1 3/5] lib/test_cpumask: follow KUnit style guidelines Sander Vanheule
2022-08-09 22:29 ` Maíra Canal
2022-08-10 4:06 ` David Gow
2022-08-09 18:08 ` [PATCH v1 4/5] lib/cpumask_kunit: log mask contents Sander Vanheule
2022-08-10 4:07 ` David Gow
2022-08-09 18:08 ` [PATCH v1 5/5] lib/cpumask_kunit: add tests file to MAINTAINERS Sander Vanheule
2022-08-10 4:06 ` David Gow
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).