Re: [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support

[David Gow]

On Wed, May 4, 2022 at 3:09 PM Marco Elver <elver@xxxxxxxxxx> wrote:
>
> Use the newly added suite_{init,exit} support for suite-wide init and
> cleanup. This avoids the unsupported method by which the test used to do
> suite-wide init and cleanup (avoiding issues such as missing TAP
> headers, and possible future conflicts).
>
> Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
> ---
> This patch should go on the -kselftest/kunit branch, where this new
> support currently lives, including a similar change to the KFENCE test.
> ---

Thanks! This is working for me. I ran it as a builtin using kunit_tool
under (I had to add an x86_64-smp architecture), then use:
./tools/testing/kunit/kunit.py run --arch=x86_64-smp
--kconfig_add=CONFIG_KCSAN=y --kconfig_add=CONFIG_DEBUG_KERNEL=y
--timeout 900 'kcsan'

To add the x86_64 smp architecture, I added a file
./tools/testing/kunit/qemu_configs/x86_64-smp.py, which was a copy of
x86_64.py but with 'CONFIG_SMP=y' added to XXXX and '-smp 16' added to
YYYY.
It took about 10 minutes on my system, so the default 5 minute timeout
definitely wasn't enough.

(It's maybe worth noting that kunit_tool's output is pretty ugly when
this isn't running on an SMP system, as the skipped subtests -- plus
the "no tests run" errors -- take up a lot of space on the screen.
That's possibly something we should consider when we look further into
how the kunit_tool NO_TEST result works. Not really related to this
change (or even this test) though.)

No complaints about the patch: I'm just really glad to see things
migrate off custom init/exit code!

Reviewed-by: David Gow <davidgow@xxxxxxxxxx>

-- David

>  kernel/kcsan/kcsan_test.c | 31 +++++++++++++------------------
>  1 file changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/kernel/kcsan/kcsan_test.c b/kernel/kcsan/kcsan_test.c
> index a36fca063a73..59560b5e1d9c 100644
> --- a/kernel/kcsan/kcsan_test.c
> +++ b/kernel/kcsan/kcsan_test.c
> @@ -1565,14 +1565,6 @@ static void test_exit(struct kunit *test)
>         torture_cleanup_end();
>  }
>
> -static struct kunit_suite kcsan_test_suite = {
> -       .name = "kcsan",
> -       .test_cases = kcsan_test_cases,
> -       .init = test_init,
> -       .exit = test_exit,
> -};
> -static struct kunit_suite *kcsan_test_suites[] = { &kcsan_test_suite, NULL };
> -
>  __no_kcsan
>  static void register_tracepoints(struct tracepoint *tp, void *ignore)
>  {
> @@ -1588,11 +1580,7 @@ static void unregister_tracepoints(struct tracepoint *tp, void *ignore)
>                 tracepoint_probe_unregister(tp, probe_console, NULL);
>  }
>
> -/*
> - * We only want to do tracepoints setup and teardown once, therefore we have to
> - * customize the init and exit functions and cannot rely on kunit_test_suite().
> - */
> -static int __init kcsan_test_init(void)
> +static int kcsan_suite_init(struct kunit_suite *suite)
>  {
>         /*
>          * Because we want to be able to build the test as a module, we need to
> @@ -1600,18 +1588,25 @@ static int __init kcsan_test_init(void)
>          * won't work here.
>          */
>         for_each_kernel_tracepoint(register_tracepoints, NULL);
> -       return __kunit_test_suites_init(kcsan_test_suites);
> +       return 0;
>  }
>
> -static void kcsan_test_exit(void)
> +static void kcsan_suite_exit(struct kunit_suite *suite)
>  {
> -       __kunit_test_suites_exit(kcsan_test_suites);
>         for_each_kernel_tracepoint(unregister_tracepoints, NULL);
>         tracepoint_synchronize_unregister();
>  }
>
> -late_initcall_sync(kcsan_test_init);
> -module_exit(kcsan_test_exit);
> +static struct kunit_suite kcsan_test_suite = {
> +       .name = "kcsan",
> +       .test_cases = kcsan_test_cases,
> +       .init = test_init,
> +       .exit = test_exit,
> +       .suite_init = kcsan_suite_init,
> +       .suite_exit = kcsan_suite_exit,
> +};
> +
> +kunit_test_suites(&kcsan_test_suite);
>
>  MODULE_LICENSE("GPL v2");
>  MODULE_AUTHOR("Marco Elver <elver@xxxxxxxxxx>");
> --
> 2.36.0.464.gb9c8b46e94-goog
>