| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CABVgOSkPXBc-PWk1zBZRQ_Tt+Sz1ruFHBj3ixojymZF=Vi4tpQ@mail.gmail.com>
Date: Wed, 20 Jul 2022 13:24:38 +0800
From: David Gow <davidgow@...gle.com>
To: Maíra Canal <mairacanal@...eup.net>
Cc: sander@...nheule.net, Andrew Morton <akpm@...ux-foundation.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
bp@...en8.de, dave.hansen@...ux.intel.com,
Marco Elver <elver@...gle.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, hpa@...or.com,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>, tglx@...utronix.de,
vschneid@...hat.com, x86@...nel.org, yury.norov@...il.com,
Brendan Higgins <brendanhiggins@...gle.com>
Subject: Re: [PATCH v4 3/5] lib/test: Introduce cpumask KUnit test suite
On Wed, Jul 20, 2022 at 5:31 AM Maíra Canal <mairacanal@...eup.net> wrote:
>
> > Add a basic suite of tests for cpumask, providing some tests for empty
> > and completely filled cpumasks.
> >
> > Signed-off-by: Sander Vanheule <sander@...nheule.net>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
>
> The tests test_cpumask_weight and test_cpumask_last are failing on all
> architectures, as can be seen on [1]. Also this test doesn't follow the
> standard style for KUnit tests [2].
>
> [1]
> https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20220718/testrun/10865066/suite/kunit/tests/
> [2] https://docs.kernel.org/dev-tools/kunit/style.html
>
> CC: Brendan Higgins <brendanhiggins@...gle.com>
> CC: David Gow <davidgow@...gle.com>
>
> Best Regards,
> - Maíra Canal
>
Hmm... this test passes on the default kunit_tool configs for UML and
x86_64, which are all without SMP.
It looks like the flaw is that, if CONFIG_NR_CPUS is greater than the
actual number of CPUs present, then the cpu_possible_mask (correctly)
won't be full.
I'm not sure what the right fix is: but removing the checks for
cpu_possible_mask being full is probably the way to go. Unless we want
to plumb through some actual detail about the underlying system and
check against that, it doesn't make sense. (Or, we could generate an
artificial "possilbe_mask" which is always full, and test the cpu
against that. But we sort-of already do that with mask_all anyway.)
So, my recommendation for a fix would be:
- Get rid of "KUNIT_EXPECT_TRUE(test, cpumask_full(cpu_possible_mask));"
- Replace "KUNIT_EXPECT_EQ(test, nr_cpumask_bits - 1,
cpumask_last(cpu_possible_mask));" with a KUNIT_EXPECT_GE()
- _Maybe_ add some debug logging with the cpumask value being checked,
as it's a pain to tell from the expectation failure messages. e.g.,
kunit_info(test, "cpu_possible_mask = '%*pb[l]'\n",
cpumask_pr_args(cpu_possible_mask));
Cheers,
-- David
> > ---
> >
> > Notes:
> > Changes since v3:
> > - Test for_each_cpu*() over empty mask and cpu_possible_mask
> > - Add Andy's Reviewed-by
> > - Use num_{online,present,possible}_cpus() for builtin masks
> > - Guard against CPU hotplugging during test for dynamic builtin CPU masks
> >
> > Changes since v2:
> > - Rework for_each_* test macros, as suggested by Yury
> >
> > Changes since v1:
> > - New patch
> >
> > lib/Kconfig.debug | 9 +++
> > lib/Makefile | 1 +
> > lib/test_cpumask.c | 138 +++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 148 insertions(+)
> > create mode 100644 lib/test_cpumask.c
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 2e24db4bff19..04aaa20d50f9 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -2021,6 +2021,15 @@ 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
> > + depends on KUNIT
> > + default KUNIT_ALL_TESTS
> > + help
> > + Enable to turn on cpumask tests, running at boot or module load time.
> > +
> > + If unsure, say N.
> > +
> > config TEST_LIST_SORT
> > tristate "Linked list sorting test" if !KUNIT_ALL_TESTS
> > depends on KUNIT
> > diff --git a/lib/Makefile b/lib/Makefile
> > index bcc7e8ea0cde..de3e47453fe8 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -99,6 +99,7 @@ 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/test_cpumask.c
> > new file mode 100644
> > index 000000000000..a31a1622f1f6
> > --- /dev/null
> > +++ b/lib/test_cpumask.c
> > @@ -0,0 +1,138 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * KUnit tests for cpumask.
> > + *
> > + * Author: Sander Vanheule <sander@...nheule.net>
> > + */
> > +
> > +#include <kunit/test.h>
> > +#include <linux/cpu.h>
> > +#include <linux/cpumask.h>
> > +
> > +#define EXPECT_FOR_EACH_CPU_EQ(test, mask) \
> > + do { \
> > + const cpumask_t *m = (mask); \
> > + int mask_weight = cpumask_weight(m); \
> > + int cpu, iter = 0; \
> > + for_each_cpu(cpu, m) \
> > + iter++; \
> > + KUNIT_EXPECT_EQ((test), mask_weight, iter); \
> > + } while (0)
> > +
> > +#define EXPECT_FOR_EACH_CPU_NOT_EQ(test, mask) \
> > + do { \
> > + const cpumask_t *m = (mask); \
> > + int mask_weight = cpumask_weight(m); \
> > + int cpu, iter = 0; \
> > + for_each_cpu_not(cpu, m) \
> > + iter++; \
> > + KUNIT_EXPECT_EQ((test), nr_cpu_ids - mask_weight, iter); \
> > + } while (0)
> > +
> > +#define EXPECT_FOR_EACH_CPU_WRAP_EQ(test, mask) \
> > + do { \
> > + const cpumask_t *m = (mask); \
> > + int mask_weight = cpumask_weight(m); \
> > + int cpu, iter = 0; \
> > + for_each_cpu_wrap(cpu, m, nr_cpu_ids / 2) \
> > + iter++; \
> > + KUNIT_EXPECT_EQ((test), mask_weight, iter); \
> > + } while (0)
> > +
> > +#define EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, name) \
> > + do { \
> > + int mask_weight = num_##name##_cpus(); \
> > + int cpu, iter = 0; \
> > + for_each_##name##_cpu(cpu) \
> > + iter++; \
> > + KUNIT_EXPECT_EQ((test), mask_weight, iter); \
> > + } while (0)
> > +
> > +static cpumask_t mask_empty;
> > +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));
> > + KUNIT_EXPECT_EQ(test, nr_cpu_ids, cpumask_weight(cpu_possible_mask));
> > + KUNIT_EXPECT_EQ(test, nr_cpumask_bits, cpumask_weight(&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_EQ(test, 0, cpumask_first_zero(&mask_empty));
> > + KUNIT_EXPECT_LE(test, nr_cpu_ids, cpumask_first_zero(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_cpumask_bits - 1, cpumask_last(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));
> > +}
> > +
> > +static void test_cpumask_iterators(struct kunit *test)
> > +{
> > + EXPECT_FOR_EACH_CPU_EQ(test, &mask_empty);
> > + EXPECT_FOR_EACH_CPU_NOT_EQ(test, &mask_empty);
> > + EXPECT_FOR_EACH_CPU_WRAP_EQ(test, &mask_empty);
> > +
> > + EXPECT_FOR_EACH_CPU_EQ(test, cpu_possible_mask);
> > + EXPECT_FOR_EACH_CPU_NOT_EQ(test, cpu_possible_mask);
> > + EXPECT_FOR_EACH_CPU_WRAP_EQ(test, cpu_possible_mask);
> > +}
> > +
> > +static void test_cpumask_iterators_builtin(struct kunit *test)
> > +{
> > + EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, possible);
> > +
> > + /* Ensure the dynamic masks are stable while running the tests */
> > + cpu_hotplug_disable();
> > +
> > + EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, online);
> > + EXPECT_FOR_EACH_CPU_BUILTIN_EQ(test, present);
> > +
> > + cpu_hotplug_enable();
> > +}
> > +
> > +static int test_cpumask_init(struct kunit *test)
> > +{
> > + cpumask_clear(&mask_empty);
> > + cpumask_setall(&mask_all);
> > +
> > + return 0;
> > +}
> > +
> > +static struct kunit_case test_cpumask_cases[] = {
> > + KUNIT_CASE(test_cpumask_weight),
> > + KUNIT_CASE(test_cpumask_first),
> > + KUNIT_CASE(test_cpumask_last),
> > + KUNIT_CASE(test_cpumask_next),
> > + KUNIT_CASE(test_cpumask_iterators),
> > + KUNIT_CASE(test_cpumask_iterators_builtin),
> > + {}
> > +};
> > +
> > +static struct kunit_suite test_cpumask_suite = {
> > + .name = "cpumask",
> > + .init = test_cpumask_init,
> > + .test_cases = test_cpumask_cases,
> > +};
> > +kunit_test_suite(test_cpumask_suite);
> > +
> > +MODULE_LICENSE("GPL");
> > --
> > 2.36.1
>
>
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4003 bytes)
Powered by blists - more mailing lists