linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support
@ 2022-05-04  7:09 Marco Elver
  2022-05-04 13:43 ` David Gow
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Marco Elver @ 2022-05-04  7:09 UTC (permalink / raw)
  To: elver
  Cc: kasan-dev, linux-kernel, Shuah Khan, Daniel Latypov, David Gow,
	Brendan Higgins

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@google.com>
---
This patch should go on the -kselftest/kunit branch, where this new
support currently lives, including a similar change to the KFENCE test.
---
 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@google.com>");
-- 
2.36.0.464.gb9c8b46e94-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support
  2022-05-04  7:09 [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support Marco Elver
@ 2022-05-04 13:43 ` David Gow
  2022-05-04 13:47   ` Marco Elver
  2022-05-04 16:13 ` Daniel Latypov
  2022-05-12 19:17 ` Brendan Higgins
  2 siblings, 1 reply; 8+ messages in thread
From: David Gow @ 2022-05-04 13:43 UTC (permalink / raw)
  To: Marco Elver
  Cc: kasan-dev, Linux Kernel Mailing List, Shuah Khan, Daniel Latypov,
	Brendan Higgins

On Wed, May 4, 2022 at 3:09 PM Marco Elver <elver@google.com> 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@google.com>
> ---
> 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@google.com>

-- 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@google.com>");
> --
> 2.36.0.464.gb9c8b46e94-goog
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support
  2022-05-04 13:43 ` David Gow
@ 2022-05-04 13:47   ` Marco Elver
  2022-05-04 13:54     ` David Gow
  0 siblings, 1 reply; 8+ messages in thread
From: Marco Elver @ 2022-05-04 13:47 UTC (permalink / raw)
  To: David Gow
  Cc: kasan-dev, Linux Kernel Mailing List, Shuah Khan, Daniel Latypov,
	Brendan Higgins

On Wed, 4 May 2022 at 15:43, David Gow <davidgow@google.com> wrote:
>
> On Wed, May 4, 2022 at 3:09 PM Marco Elver <elver@google.com> 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@google.com>
> > ---
> > 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.

The trick to reduce the KCSAN test time is to set
CONFIG_KCSAN_REPORT_ONCE_IN_MS=100 or lower. So should you consider a
special KUnit config, I'd add that.

> (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@google.com>

Thank you!

> -- 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@google.com>");
> > --
> > 2.36.0.464.gb9c8b46e94-goog
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support
  2022-05-04 13:47   ` Marco Elver
@ 2022-05-04 13:54     ` David Gow
  2022-05-04 13:58       ` Marco Elver
  2022-05-04 15:56       ` Daniel Latypov
  0 siblings, 2 replies; 8+ messages in thread
From: David Gow @ 2022-05-04 13:54 UTC (permalink / raw)
  To: Marco Elver
  Cc: kasan-dev, Linux Kernel Mailing List, Shuah Khan, Daniel Latypov,
	Brendan Higgins

On Wed, May 4, 2022 at 9:48 PM Marco Elver <elver@google.com> wrote:
>
> On Wed, 4 May 2022 at 15:43, David Gow <davidgow@google.com> wrote:
> >
> > On Wed, May 4, 2022 at 3:09 PM Marco Elver <elver@google.com> 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@google.com>
> > > ---
> > > 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.

(Whoops, forgot to copy this in properly: XXXX was 'kconfig' and YYYY
was 'extra_qemu_params'.)

The x86_64-smp.py file ends up looking like this:
---8<---
from ..qemu_config import QemuArchParams

QEMU_ARCH = QemuArchParams(linux_arch='x86_64',
                          kconfig='''
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_SMP=y
                          ''',
                          qemu_arch='x86_64',
                          kernel_path='arch/x86/boot/bzImage',
                          kernel_command_line='console=ttyS0',
                          extra_qemu_params=['-smp 16'])
---8<---
> > It took about 10 minutes on my system, so the default 5 minute timeout
> > definitely wasn't enough.
>
> The trick to reduce the KCSAN test time is to set
> CONFIG_KCSAN_REPORT_ONCE_IN_MS=100 or lower. So should you consider a
> special KUnit config, I'd add that.
>

Ah: it might be worth adding a dedicated kcsan .kunitconfig, in which
case this would be helpful. It'd also need the SMP qemu config above
before it's particularly useful, and 16 was a randomly-picked number
of CPUs -- not sure if there's a better default.

If you're likely to use it, though, we can definitely add it in. I'm
sure there'll eventually be other uses for an SMP config under
kunit_tool, too.

> > (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@google.com>
>
> Thank you!
>
> > -- 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@google.com>");
> > > --
> > > 2.36.0.464.gb9c8b46e94-goog
> > >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support
  2022-05-04 13:54     ` David Gow
@ 2022-05-04 13:58       ` Marco Elver
  2022-05-04 15:56       ` Daniel Latypov
  1 sibling, 0 replies; 8+ messages in thread
From: Marco Elver @ 2022-05-04 13:58 UTC (permalink / raw)
  To: David Gow
  Cc: kasan-dev, Linux Kernel Mailing List, Shuah Khan, Daniel Latypov,
	Brendan Higgins

On Wed, 4 May 2022 at 15:54, David Gow <davidgow@google.com> wrote:
>
> On Wed, May 4, 2022 at 9:48 PM Marco Elver <elver@google.com> wrote:
> >
> > On Wed, 4 May 2022 at 15:43, David Gow <davidgow@google.com> wrote:
> > >
> > > On Wed, May 4, 2022 at 3:09 PM Marco Elver <elver@google.com> 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@google.com>
> > > > ---
> > > > 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.
>
> (Whoops, forgot to copy this in properly: XXXX was 'kconfig' and YYYY
> was 'extra_qemu_params'.)
>
> The x86_64-smp.py file ends up looking like this:
> ---8<---
> from ..qemu_config import QemuArchParams
>
> QEMU_ARCH = QemuArchParams(linux_arch='x86_64',
>                           kconfig='''
> CONFIG_SERIAL_8250=y
> CONFIG_SERIAL_8250_CONSOLE=y
> CONFIG_SMP=y
>                           ''',
>                           qemu_arch='x86_64',
>                           kernel_path='arch/x86/boot/bzImage',
>                           kernel_command_line='console=ttyS0',
>                           extra_qemu_params=['-smp 16'])
> ---8<---
> > > It took about 10 minutes on my system, so the default 5 minute timeout
> > > definitely wasn't enough.
> >
> > The trick to reduce the KCSAN test time is to set
> > CONFIG_KCSAN_REPORT_ONCE_IN_MS=100 or lower. So should you consider a
> > special KUnit config, I'd add that.
> >
>
> Ah: it might be worth adding a dedicated kcsan .kunitconfig, in which
> case this would be helpful. It'd also need the SMP qemu config above
> before it's particularly useful, and 16 was a randomly-picked number
> of CPUs -- not sure if there's a better default.
>
> If you're likely to use it, though, we can definitely add it in. I'm
> sure there'll eventually be other uses for an SMP config under
> kunit_tool, too.

I currently have some other frankenscript to run it, but I wouldn't
mind just using kunit_tool to do so. So having real SMP support there
would be very useful.

Thanks,
-- Marco

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support
  2022-05-04 13:54     ` David Gow
  2022-05-04 13:58       ` Marco Elver
@ 2022-05-04 15:56       ` Daniel Latypov
  1 sibling, 0 replies; 8+ messages in thread
From: Daniel Latypov @ 2022-05-04 15:56 UTC (permalink / raw)
  To: David Gow
  Cc: Marco Elver, kasan-dev, Linux Kernel Mailing List, Shuah Khan,
	Brendan Higgins

On Wed, May 4, 2022 at 8:54 AM David Gow <davidgow@google.com> wrote:
>
> On Wed, May 4, 2022 at 9:48 PM Marco Elver <elver@google.com> wrote:
> >
> > On Wed, 4 May 2022 at 15:43, David Gow <davidgow@google.com> wrote:
> > >
> > > On Wed, May 4, 2022 at 3:09 PM Marco Elver <elver@google.com> 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@google.com>
> > > > ---
> > > > 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.
>
> (Whoops, forgot to copy this in properly: XXXX was 'kconfig' and YYYY
> was 'extra_qemu_params'.)
>
> The x86_64-smp.py file ends up looking like this:
> ---8<---
> from ..qemu_config import QemuArchParams
>
> QEMU_ARCH = QemuArchParams(linux_arch='x86_64',
>                           kconfig='''
> CONFIG_SERIAL_8250=y
> CONFIG_SERIAL_8250_CONSOLE=y
> CONFIG_SMP=y
>                           ''',
>                           qemu_arch='x86_64',
>                           kernel_path='arch/x86/boot/bzImage',
>                           kernel_command_line='console=ttyS0',
>                           extra_qemu_params=['-smp 16'])

You'd want ['-smp', '16'].
Otherwise this config will be broken by
https://lore.kernel.org/linux-kselftest/20220420203020.1412886-1-dlatypov@google.com/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support
  2022-05-04  7:09 [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support Marco Elver
  2022-05-04 13:43 ` David Gow
@ 2022-05-04 16:13 ` Daniel Latypov
  2022-05-12 19:17 ` Brendan Higgins
  2 siblings, 0 replies; 8+ messages in thread
From: Daniel Latypov @ 2022-05-04 16:13 UTC (permalink / raw)
  To: Marco Elver
  Cc: kasan-dev, linux-kernel, Shuah Khan, David Gow, Brendan Higgins

On Wed, May 4, 2022 at 2:09 AM Marco Elver <elver@google.com> 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@google.com>

Reviewed-by: Daniel Latypov <dlatypov@google.com>

Adding '-smp 16' and CONFIG_KCSAN_REPORT_ONCE_IN_MS=100 as you
suggested below, I was able to get it running under kunit.py with the
following results:
Testing complete. Passed: 168, Failed: 0, Crashed: 0, Skipped: 1, Errors: 0
Elapsed time: 92.642s total, 0.003s configuring, 4.592s building,
88.009s running

Nice!

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support
  2022-05-04  7:09 [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support Marco Elver
  2022-05-04 13:43 ` David Gow
  2022-05-04 16:13 ` Daniel Latypov
@ 2022-05-12 19:17 ` Brendan Higgins
  2 siblings, 0 replies; 8+ messages in thread
From: Brendan Higgins @ 2022-05-12 19:17 UTC (permalink / raw)
  To: Marco Elver
  Cc: kasan-dev, linux-kernel, Shuah Khan, Daniel Latypov, David Gow

On Wed, May 4, 2022 at 3:09 AM Marco Elver <elver@google.com> 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@google.com>

Acked-by: Brendan Higgins <brendanhiggins@google.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-05-12 19:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-04  7:09 [PATCH -kselftest/kunit] kcsan: test: use new suite_{init,exit} support Marco Elver
2022-05-04 13:43 ` David Gow
2022-05-04 13:47   ` Marco Elver
2022-05-04 13:54     ` David Gow
2022-05-04 13:58       ` Marco Elver
2022-05-04 15:56       ` Daniel Latypov
2022-05-04 16:13 ` Daniel Latypov
2022-05-12 19:17 ` Brendan Higgins

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).