2022-08-23 06:22:16

by Sander Vanheule

[permalink] [raw]
Subject: [PATCH v3 0/5] cpumask: KUnit test suite fixes and improvements

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.1659077534.git.sander@svanheule.net/

Changes since v2:
Link: https://lore.kernel.org/lkml/[email protected]/
- 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/[email protected]/
- 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


2022-08-23 06:22:53

by Sander Vanheule

[permalink] [raw]
Subject: [PATCH v3 3/5] lib/test_cpumask: follow KUnit style guidelines

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/[email protected]/
Suggested-by: Maíra Canal <[email protected]>
Signed-off-by: Sander Vanheule <[email protected]>
Reviewed-by: Maíra Canal <[email protected]>
Reviewed-by: David Gow <[email protected]>
Acked-by: Yury Norov <[email protected]>
---
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
--
2.37.2

2022-08-23 06:23:24

by Sander Vanheule

[permalink] [raw]
Subject: [PATCH v3 4/5] lib/cpumask_kunit: log mask contents

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 <[email protected]>
Signed-off-by: Sander Vanheule <[email protected]>
---
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)
--
2.37.2

2022-08-23 06:23:25

by Sander Vanheule

[permalink] [raw]
Subject: [PATCH v3 1/5] lib/test_cpumask: drop cpu_possible_mask full test

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/[email protected]/
Reported-by: Maíra Canal <[email protected]>
Signed-off-by: Sander Vanheule <[email protected]>
Tested-by: Maíra Canal <[email protected]>
Reviewed-by: David Gow <[email protected]>
---
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));
--
2.37.2

2022-08-23 06:56:14

by Sander Vanheule

[permalink] [raw]
Subject: [PATCH v3 5/5] lib/cpumask_kunit: add tests file to MAINTAINERS

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 <[email protected]>
Reviewed-by: David Gow <[email protected]>
Acked-by: Yury Norov <[email protected]>
---
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
--
2.37.2

2022-08-23 07:13:20

by Sander Vanheule

[permalink] [raw]
Subject: [PATCH v3 2/5] lib/test_cpumask: fix cpu_possible_mask last test

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/[email protected]/
Reported-by: Maíra Canal <[email protected]>
Signed-off-by: Sander Vanheule <[email protected]>
Tested-by: Maíra Canal <[email protected]>
Reviewed-by: David Gow <[email protected]>
Acked-by: Yury Norov <[email protected]>
---
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.2

2022-08-24 16:23:34

by Yury Norov

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] cpumask: KUnit test suite fixes and improvements

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.1659077534.git.sander@svanheule.net/
>
> Changes since v2:
> Link: https://lore.kernel.org/lkml/[email protected]/
> - 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/[email protected]/
> - 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