2022-06-05 09:52:49

by Yury Norov

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] lib/test: Introduce cpumask KUnit test suite

On Sat, Jun 04, 2022 at 07:15:56PM +0200, Sander Vanheule wrote:
> Add a basic suite of tests for cpumask, providing some tests for empty
> and completely filled cpumasks.
>
> Signed-off-by: Sander Vanheule <[email protected]>

The test must go after fix, so that it's doesn't cause regressions
while bisecting.

> ---
> lib/Kconfig.debug | 9 ++++
> lib/Makefile | 1 +
> lib/test_cpumask.c | 115 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 125 insertions(+)
> create mode 100644 lib/test_cpumask.c

[..]

> +#define FOR_EACH_ITER_EQ(_test, _iter, _expect, _loop) \
> + do { \
> + (_iter) = 0; \
> + _loop \
> + (_iter)++; \
> + KUNIT_EXPECT_EQ((_test), (_expect), (_iter)); \
> + } while (0)

This one is harder to use than it should be. Maybe like this? (not tested,
just an idea)

#define TEST_FOR_EACH_CPU_EQ(test, mask) \
do { \
cpumask_t *m = (mask); \
int iter = 0, cpu; \
for_each_cpu(cpu, m) \
iter++; \
KUNIT_EXPECT_EQ((test), cpumask_weight(m), iter); \
} while (0)

static void test_cpumask_iterators(struct kunit *test)
{
TEST_FOR_EACH_CPU(test, &mask_empty);
...
}

Similarly for NOT and WRAP.

Thanks,
Yury

> +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)
> +{
> + unsigned int iterations;
> + int cpu;
> +
> + FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(&mask_empty),
> + for_each_cpu(cpu, &mask_empty));
> + FOR_EACH_ITER_EQ(test, iterations, nr_cpu_ids - cpumask_weight(&mask_empty),
> + for_each_cpu_not(cpu, &mask_empty));
> + FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(&mask_empty),
> + for_each_cpu_wrap(cpu, &mask_empty, nr_cpu_ids / 2));
> +
> + FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_possible_mask),
> + for_each_cpu(cpu, cpu_possible_mask));
> + FOR_EACH_ITER_EQ(test, iterations, nr_cpu_ids - cpumask_weight(cpu_possible_mask),
> + for_each_cpu_not(cpu, cpu_possible_mask));
> + FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_possible_mask),
> + for_each_cpu_wrap(cpu, cpu_possible_mask, nr_cpu_ids / 2));
> +}
> +
> +static void test_cpumask_iterators_builtin(struct kunit *test)
> +{
> + unsigned int iterations;
> + int cpu;
> +
> + FOR_EACH_ITER_EQ(test, iterations, nr_cpu_ids,
> + for_each_possible_cpu(cpu));
> + FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_online_mask),
> + for_each_online_cpu(cpu));
> + FOR_EACH_ITER_EQ(test, iterations, cpumask_weight(cpu_present_mask),
> + for_each_present_cpu(cpu));
> +}
> +
> +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


2022-06-06 03:56:28

by Sander Vanheule

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] lib/test: Introduce cpumask KUnit test suite

Hi Yury,

On Sat, 2022-06-04 at 12:31 -0700, Yury Norov wrote:
> On Sat, Jun 04, 2022 at 07:15:56PM +0200, Sander Vanheule wrote:
> > Add a basic suite of tests for cpumask, providing some tests for empty
> > and completely filled cpumasks.
> >
> > Signed-off-by: Sander Vanheule <[email protected]>
>
> The test must go after fix, so that it's doesn't cause regressions
> while bisecting.

OK, I'll change the order of the patches.

>
> > ---
> >  lib/Kconfig.debug  |   9 ++++
> >  lib/Makefile       |   1 +
> >  lib/test_cpumask.c | 115 +++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 125 insertions(+)
> >  create mode 100644 lib/test_cpumask.c
>
> [..]
>
> > +#define FOR_EACH_ITER_EQ(_test, _iter, _expect, _loop)         \
> > +       do {                                                    \
> > +               (_iter) = 0;                                    \
> > +               _loop                                           \
> > +                       (_iter)++;                              \
> > +               KUNIT_EXPECT_EQ((_test), (_expect), (_iter));   \
> > +       } while (0)
>
> This one is harder to use than it should be.

Perhaps I tried to hard to make one macro to cover all the cases.

> Maybe like this? (not tested,
> just an idea)
>
> #define TEST_FOR_EACH_CPU_EQ(test, mask)                                \
>         do {                                                            \
>                 cpumask_t *m = (mask);                                  \
>                 int iter = 0, cpu;                                      \
>                 for_each_cpu(cpu, m)                                    \
>                         iter++;                                         \
>                 KUNIT_EXPECT_EQ((test), cpumask_weight(m), iter);       \
>         } while (0)
>
> static void test_cpumask_iterators(struct kunit *test)
> {
>         TEST_FOR_EACH_CPU(test, &mask_empty);
>         ...
> }
>  
> Similarly for NOT and WRAP.

Thanks for the suggestion, I forgot we don't have to declare all variables at the start of the
function. I'll send an update with a few more specific macros.

Best,
Sander