diff mbox series

[2/2] kcsan: test: Add a .kunitconfig to run KCSAN tests

Message ID 20220518073232.526443-2-davidgow@google.com (mailing list archive)
State New
Headers show
Series [1/2] kunit: tool: Add x86_64-smp architecture for SMP testing | expand

Commit Message

David Gow May 18, 2022, 7:32 a.m. UTC
Add a .kunitconfig file, which provides a default, working config for
running the KCSAN tests. Note that it needs to run on an SMP machine, so
to run under kunit_tool, the x86_64-smp qemu-based setup should be used:
./tools/testing/kunit/kunit.py run --arch=x86_64-smp --kunitconfig=kernel/kcsan

Signed-off-by: David Gow <davidgow@google.com>
---
 kernel/kcsan/.kunitconfig | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 kernel/kcsan/.kunitconfig

Comments

Marco Elver May 18, 2022, 9:21 a.m. UTC | #1
On Wed, May 18, 2022 at 03:32PM +0800, David Gow wrote:
> Add a .kunitconfig file, which provides a default, working config for
> running the KCSAN tests. Note that it needs to run on an SMP machine, so
> to run under kunit_tool, the x86_64-smp qemu-based setup should be used:
> ./tools/testing/kunit/kunit.py run --arch=x86_64-smp --kunitconfig=kernel/kcsan
> 
> Signed-off-by: David Gow <davidgow@google.com>

Reviewed-by: Marco Elver <elver@google.com>

Thanks for adding this.

> ---
>  kernel/kcsan/.kunitconfig | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 kernel/kcsan/.kunitconfig
> 
> diff --git a/kernel/kcsan/.kunitconfig b/kernel/kcsan/.kunitconfig
> new file mode 100644
> index 000000000000..a8a815b1eb73
> --- /dev/null
> +++ b/kernel/kcsan/.kunitconfig
> @@ -0,0 +1,20 @@
> +# Note that the KCSAN tests need to run on an SMP setup.
> +# Under kunit_tool, this can be done by using the x86_64-smp
> +# qemu-based architecture:
> +# ./tools/testing/kunit/kunit.py run --kunitconfig=kernel/kcsan --arch=x86_64-smp
> +
> +CONFIG_KUNIT=y
> +
> +CONFIG_DEBUG_KERNEL=y
> +
> +CONFIG_KCSAN=y
> +CONFIG_KCSAN_KUNIT_TEST=y
> +
> +# Needed for test_barrier_nothreads
> +CONFIG_KCSAN_STRICT=y
> +CONFIG_KCSAN_WEAK_MEMORY=y

Note, KCSAN_STRICT implies KCSAN_WEAK_MEMORY.

Also, a bunch of the test cases' outcomes depend on KCSAN's
"strictness". I think to cover the various combinations would be too
complex, but we can just settle on testing KCSAN_STRICT=y.

The end result is the same, but you could drop the
CONFIG_KCSAN_WEAK_MEMORY=y line, and let the latest KCSAN_STRICT
defaults decide (I don't expect them to change any time soon).

If you want it to be more explicit, it's also fine leaving the
CONFIG_KCSAN_WEAK_MEMORY=y line in.

> +# This prevents the test from timing out on many setups. Feel free to remove
> +# (or alter) this, in conjunction with setting a different test timeout with,
> +# for example, the --timeout kunit_tool option.
> +CONFIG_KCSAN_REPORT_ONCE_IN_MS=100
> -- 
> 2.36.0.550.gb090851708-goog
>
Daniel Latypov May 18, 2022, 5:12 p.m. UTC | #2
On Wed, May 18, 2022 at 12:32 AM David Gow <davidgow@google.com> wrote:
> diff --git a/kernel/kcsan/.kunitconfig b/kernel/kcsan/.kunitconfig
> new file mode 100644
> index 000000000000..a8a815b1eb73
> --- /dev/null
> +++ b/kernel/kcsan/.kunitconfig
> @@ -0,0 +1,20 @@
> +# Note that the KCSAN tests need to run on an SMP setup.
> +# Under kunit_tool, this can be done by using the x86_64-smp
> +# qemu-based architecture:
> +# ./tools/testing/kunit/kunit.py run --kunitconfig=kernel/kcsan --arch=x86_64-smp

Just noting here, if we go with --qemu_args [1], then we'd change this to
  --arch=x86_64 --qemu_args='-smp 8'
and then probably add
  CONFIG_SMP=y
to this file.

[1] https://lore.kernel.org/linux-kselftest/20220518170124.2849497-1-dlatypov@google.com

> +
> +CONFIG_KUNIT=y
> +
> +CONFIG_DEBUG_KERNEL=y
> +
> +CONFIG_KCSAN=y
> +CONFIG_KCSAN_KUNIT_TEST=y
> +
> +# Needed for test_barrier_nothreads
> +CONFIG_KCSAN_STRICT=y
> +CONFIG_KCSAN_WEAK_MEMORY=y
> +
> +# This prevents the test from timing out on many setups. Feel free to remove
> +# (or alter) this, in conjunction with setting a different test timeout with,
> +# for example, the --timeout kunit_tool option.
> +CONFIG_KCSAN_REPORT_ONCE_IN_MS=100

Tangent:

Ah this reminds me, unfortunately you can't use --kconfig_add to
overwrite this atm.
Right now, it'll just blindly try to append and then complain that one
of the two copies of the option is missing.

That might be a feature to look into.
Or at least, we can maybe give a better error message.

E.g. with the default kunitconfig, the error currently looks like
# Try to overwrite CONFIG_KUNIT_ALL_TESTS=y
$ ./tools/testing/kunit/kunit.py config --kconfig_add=CONFIG_KUNIT_ALL_TESTS=m
...
ERROR:root:Not all Kconfig options selected in kunitconfig were in the
generated .config.
This is probably due to unsatisfied dependencies.
Missing: CONFIG_KUNIT_ALL_TESTS=m
David Gow May 19, 2022, 1:08 p.m. UTC | #3
On Wed, May 18, 2022 at 5:21 PM Marco Elver <elver@google.com> wrote:
>
> On Wed, May 18, 2022 at 03:32PM +0800, David Gow wrote:
> > Add a .kunitconfig file, which provides a default, working config for
> > running the KCSAN tests. Note that it needs to run on an SMP machine, so
> > to run under kunit_tool, the x86_64-smp qemu-based setup should be used:
> > ./tools/testing/kunit/kunit.py run --arch=x86_64-smp --kunitconfig=kernel/kcsan
> >
> > Signed-off-by: David Gow <davidgow@google.com>
>
> Reviewed-by: Marco Elver <elver@google.com>
>
> Thanks for adding this.
>
> > ---
> >  kernel/kcsan/.kunitconfig | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >  create mode 100644 kernel/kcsan/.kunitconfig
> >
> > diff --git a/kernel/kcsan/.kunitconfig b/kernel/kcsan/.kunitconfig
> > new file mode 100644
> > index 000000000000..a8a815b1eb73
> > --- /dev/null
> > +++ b/kernel/kcsan/.kunitconfig
> > @@ -0,0 +1,20 @@
> > +# Note that the KCSAN tests need to run on an SMP setup.
> > +# Under kunit_tool, this can be done by using the x86_64-smp
> > +# qemu-based architecture:
> > +# ./tools/testing/kunit/kunit.py run --kunitconfig=kernel/kcsan --arch=x86_64-smp
> > +
> > +CONFIG_KUNIT=y
> > +
> > +CONFIG_DEBUG_KERNEL=y
> > +
> > +CONFIG_KCSAN=y
> > +CONFIG_KCSAN_KUNIT_TEST=y
> > +
> > +# Needed for test_barrier_nothreads
> > +CONFIG_KCSAN_STRICT=y
> > +CONFIG_KCSAN_WEAK_MEMORY=y
>
> Note, KCSAN_STRICT implies KCSAN_WEAK_MEMORY.
>
> Also, a bunch of the test cases' outcomes depend on KCSAN's
> "strictness". I think to cover the various combinations would be too
> complex, but we can just settle on testing KCSAN_STRICT=y.

It's definitely possible to either have multiple .kunitconfigs, each
of which could have slightly different setups, e.g.:
- kernel/kcsan/.kunitconfig (defualt)
- kernel/kcsan/strict.kunitconfig (passed explicitly when desired)

Equally, if we got rid of KCSAN_STRICT in the .kunitconfig, you could
override it with --kconfig_add, e.g.
-  ./tools/testing/kunit/kunit.py run --kunitconfig=kernel/kcsan
--arch=x86_64-smp
- ./tools/testing/kunit/kunit.py run --kunitconfig=kernel/kcsan
--arch=x86_64-smp --kconfig_add CONFIG_KSCAN_STRICT=y

> The end result is the same, but you could drop the
> CONFIG_KCSAN_WEAK_MEMORY=y line, and let the latest KCSAN_STRICT
> defaults decide (I don't expect them to change any time soon).
>
> If you want it to be more explicit, it's also fine leaving the
> CONFIG_KCSAN_WEAK_MEMORY=y line in.

Do you have a preference here? Or to get rid of both and default to
the non-strict version mentioned above?

>
> > +# This prevents the test from timing out on many setups. Feel free to remove
> > +# (or alter) this, in conjunction with setting a different test timeout with,
> > +# for example, the --timeout kunit_tool option.
> > +CONFIG_KCSAN_REPORT_ONCE_IN_MS=100
> > --
> > 2.36.0.550.gb090851708-goog
> >
Marco Elver May 19, 2022, 1:24 p.m. UTC | #4
On Thu, 19 May 2022 at 15:08, David Gow <davidgow@google.com> wrote:
>
> On Wed, May 18, 2022 at 5:21 PM Marco Elver <elver@google.com> wrote:
> >
> > On Wed, May 18, 2022 at 03:32PM +0800, David Gow wrote:
> > > Add a .kunitconfig file, which provides a default, working config for
> > > running the KCSAN tests. Note that it needs to run on an SMP machine, so
> > > to run under kunit_tool, the x86_64-smp qemu-based setup should be used:
> > > ./tools/testing/kunit/kunit.py run --arch=x86_64-smp --kunitconfig=kernel/kcsan
> > >
> > > Signed-off-by: David Gow <davidgow@google.com>
> >
> > Reviewed-by: Marco Elver <elver@google.com>
> >
> > Thanks for adding this.
> >
> > > ---
> > >  kernel/kcsan/.kunitconfig | 20 ++++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > >  create mode 100644 kernel/kcsan/.kunitconfig
> > >
> > > diff --git a/kernel/kcsan/.kunitconfig b/kernel/kcsan/.kunitconfig
> > > new file mode 100644
> > > index 000000000000..a8a815b1eb73
> > > --- /dev/null
> > > +++ b/kernel/kcsan/.kunitconfig
> > > @@ -0,0 +1,20 @@
> > > +# Note that the KCSAN tests need to run on an SMP setup.
> > > +# Under kunit_tool, this can be done by using the x86_64-smp
> > > +# qemu-based architecture:
> > > +# ./tools/testing/kunit/kunit.py run --kunitconfig=kernel/kcsan --arch=x86_64-smp
> > > +
> > > +CONFIG_KUNIT=y
> > > +
> > > +CONFIG_DEBUG_KERNEL=y
> > > +
> > > +CONFIG_KCSAN=y
> > > +CONFIG_KCSAN_KUNIT_TEST=y
> > > +
> > > +# Needed for test_barrier_nothreads
> > > +CONFIG_KCSAN_STRICT=y
> > > +CONFIG_KCSAN_WEAK_MEMORY=y
> >
> > Note, KCSAN_STRICT implies KCSAN_WEAK_MEMORY.
> >
> > Also, a bunch of the test cases' outcomes depend on KCSAN's
> > "strictness". I think to cover the various combinations would be too
> > complex, but we can just settle on testing KCSAN_STRICT=y.
>
> It's definitely possible to either have multiple .kunitconfigs, each
> of which could have slightly different setups, e.g.:
> - kernel/kcsan/.kunitconfig (defualt)
> - kernel/kcsan/strict.kunitconfig (passed explicitly when desired)
>
> Equally, if we got rid of KCSAN_STRICT in the .kunitconfig, you could
> override it with --kconfig_add, e.g.
> -  ./tools/testing/kunit/kunit.py run --kunitconfig=kernel/kcsan
> --arch=x86_64-smp
> - ./tools/testing/kunit/kunit.py run --kunitconfig=kernel/kcsan
> --arch=x86_64-smp --kconfig_add CONFIG_KSCAN_STRICT=y
>
> > The end result is the same, but you could drop the
> > CONFIG_KCSAN_WEAK_MEMORY=y line, and let the latest KCSAN_STRICT
> > defaults decide (I don't expect them to change any time soon).
> >
> > If you want it to be more explicit, it's also fine leaving the
> > CONFIG_KCSAN_WEAK_MEMORY=y line in.
>
> Do you have a preference here? Or to get rid of both and default to
> the non-strict version mentioned above?

I'd keep it simple for now, and remove both lines i.e. make non-strict
the default. It's easy to just run with --kconfig_add
CONFIG_KCSAN_STRICT=y, along with other variations. I know that
rcutoruture uses KCSAN_STRICT=y by default, so it's already getting
coverage there. ;-)

Thanks,
-- Marco
diff mbox series

Patch

diff --git a/kernel/kcsan/.kunitconfig b/kernel/kcsan/.kunitconfig
new file mode 100644
index 000000000000..a8a815b1eb73
--- /dev/null
+++ b/kernel/kcsan/.kunitconfig
@@ -0,0 +1,20 @@ 
+# Note that the KCSAN tests need to run on an SMP setup.
+# Under kunit_tool, this can be done by using the x86_64-smp
+# qemu-based architecture:
+# ./tools/testing/kunit/kunit.py run --kunitconfig=kernel/kcsan --arch=x86_64-smp
+
+CONFIG_KUNIT=y
+
+CONFIG_DEBUG_KERNEL=y
+
+CONFIG_KCSAN=y
+CONFIG_KCSAN_KUNIT_TEST=y
+
+# Needed for test_barrier_nothreads
+CONFIG_KCSAN_STRICT=y
+CONFIG_KCSAN_WEAK_MEMORY=y
+
+# This prevents the test from timing out on many setups. Feel free to remove
+# (or alter) this, in conjunction with setting a different test timeout with,
+# for example, the --timeout kunit_tool option.
+CONFIG_KCSAN_REPORT_ONCE_IN_MS=100