[PATCH v1 0/5] cpumask: KUnit test suite fixes and improvements

[PATCH v1 0/5] cpumask: KUnit test suite fixes and improvements

[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

[PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test

[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

Re: [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test

[Maíra Canal]

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));

Re: [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test

[Sander Vanheule]

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));

Re: [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test

[David Gow]

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
>

Re: [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test

[Sander Vanheule]

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
> > 

Re: [PATCH v1 1/5] lib/test_cpumask: drop cpu_possible_mask full test

[David Gow]

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

[PATCH v1 2/5] lib/test_cpumask: fix cpu_possible_mask last test

[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

Re: [PATCH v1 2/5] lib/test_cpumask: fix cpu_possible_mask last test

[Maíra Canal]

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)

Re: [PATCH v1 2/5] lib/test_cpumask: fix cpu_possible_mask last test

[David Gow]

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
>

[PATCH v1 3/5] lib/test_cpumask: follow KUnit style guidelines

[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

Re: [PATCH v1 3/5] lib/test_cpumask: follow KUnit style guidelines

[Maíra Canal]

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

Re: [PATCH v1 3/5] lib/test_cpumask: follow KUnit style guidelines

[David Gow]

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
>

[PATCH v1 4/5] lib/cpumask_kunit: log mask contents

[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

Re: [PATCH v1 4/5] lib/cpumask_kunit: log mask contents

[David Gow]

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
>

[PATCH v1 5/5] lib/cpumask_kunit: add tests file to MAINTAINERS

[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

Re: [PATCH v1 5/5] lib/cpumask_kunit: add tests file to MAINTAINERS

[David Gow]

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
>