Add a new QEMU config for kunit_tool, x86_64-smp, which provides an 8-cpu SMP setup. No other kunit_tool configurations provide an SMP setup, so this is the best bet for testing things like KCSAN, which require a multicore/multi-cpu system.
The choice of 8 CPUs is pretty arbitrary: it's enough to get tests like KCSAN to run with a nontrivial number of worker threads, while still working relatively quickly on older machines.
Signed-off-by: David Gow davidgow@google.com ---
This is based off the discussion in: https://groups.google.com/g/kasan-dev/c/A7XzC2pXRC8
--- tools/testing/kunit/qemu_configs/x86_64-smp.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 tools/testing/kunit/qemu_configs/x86_64-smp.py
diff --git a/tools/testing/kunit/qemu_configs/x86_64-smp.py b/tools/testing/kunit/qemu_configs/x86_64-smp.py new file mode 100644 index 000000000000..a95623f5f8b7 --- /dev/null +++ b/tools/testing/kunit/qemu_configs/x86_64-smp.py @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: GPL-2.0 +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', '8'])
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
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
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
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
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
On Thu, May 19, 2022 at 6:24 AM Marco Elver elver@google.com wrote:
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. ;-)
David decided to drop the parent patch (the new QEMU config) now --qemu_args was merged into the kunit tree. Did we want a standalone v2 of this patch?
Based on Marco's comments, we'd change: * drop CONFIG_KCSAN_STRICT=y per this comment [1] * drop CONFIG_KCSAN_WEAK_MEMORY per previous comments Then for --qemu_args changes: * add CONFIG_SMP=y explicitly to this file * update the comment to show to include --qemu_args="-smp 8"
Does this sound right?
[1] Note: there's also patches in kunit now so you could do --kconfig_add=CONFIG_KCSAN_STRICT=n to explicitly disable it. This wasn't possible before. Does that change what we want for the default?
Daniel
On Thu, 14 Jul 2022 at 22:23, Daniel Latypov dlatypov@google.com wrote:
On Thu, May 19, 2022 at 6:24 AM Marco Elver elver@google.com wrote:
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. ;-)
David decided to drop the parent patch (the new QEMU config) now --qemu_args was merged into the kunit tree. Did we want a standalone v2 of this patch?
Based on Marco's comments, we'd change:
- drop CONFIG_KCSAN_STRICT=y per this comment [1]
- drop CONFIG_KCSAN_WEAK_MEMORY per previous comments
Then for --qemu_args changes:
- add CONFIG_SMP=y explicitly to this file
- update the comment to show to include --qemu_args="-smp 8"
Does this sound right?
Yes, sounds good to me, and thanks for remembering this. I'd prefer a close-to-default config.
[1] Note: there's also patches in kunit now so you could do --kconfig_add=CONFIG_KCSAN_STRICT=n to explicitly disable it. This wasn't possible before. Does that change what we want for the default?
I'd just have KCSAN_STRICT=n by default, and if desired it can be added per kconfig_add just the same way.
Thanks, -- Marco
On Thu, Jul 14, 2022 at 2:41 PM Marco Elver elver@google.com wrote:
On Thu, 14 Jul 2022 at 22:23, Daniel Latypov dlatypov@google.com wrote:
On Thu, May 19, 2022 at 6:24 AM Marco Elver elver@google.com wrote:
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. ;-)
David decided to drop the parent patch (the new QEMU config) now --qemu_args was merged into the kunit tree. Did we want a standalone v2 of this patch?
Based on Marco's comments, we'd change:
- drop CONFIG_KCSAN_STRICT=y per this comment [1]
- drop CONFIG_KCSAN_WEAK_MEMORY per previous comments
Then for --qemu_args changes:
- add CONFIG_SMP=y explicitly to this file
- update the comment to show to include --qemu_args="-smp 8"
Does this sound right?
Yes, sounds good to me, and thanks for remembering this. I'd prefer a close-to-default config.
[1] Note: there's also patches in kunit now so you could do --kconfig_add=CONFIG_KCSAN_STRICT=n to explicitly disable it. This wasn't possible before. Does that change what we want for the default?
I'd just have KCSAN_STRICT=n by default, and if desired it can be added per kconfig_add just the same way.
Ack. So concretely, so then a final result like this?
$ cat kernel/kcsan/.kunitconfig # 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 --qemu_args='-smp 8'
CONFIG_KUNIT=y
CONFIG_DEBUG_KERNEL=y
CONFIG_KCSAN=y CONFIG_KCSAN_KUNIT_TEST=y
# Need some level of concurrency to test a concurrency sanitizer. CONFIG_SMP=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
On Thu, Jul 14, 2022 at 4:45 PM Daniel Latypov dlatypov@google.com wrote:
Ack. So concretely, so then a final result like this?
$ cat kernel/kcsan/.kunitconfig # 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:
Oops, this bit would need to be updated to something like:
# Under kunit_tool, this can be done by using --qemu_args:
# ./tools/testing/kunit/kunit.py run --kunitconfig=kernel/kcsan --arch=x86_64 --qemu_args='-smp 8'
CONFIG_KUNIT=y
CONFIG_DEBUG_KERNEL=y
CONFIG_KCSAN=y CONFIG_KCSAN_KUNIT_TEST=y
# Need some level of concurrency to test a concurrency sanitizer. CONFIG_SMP=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
On Fri, Jul 15, 2022 at 7:48 AM Daniel Latypov dlatypov@google.com wrote:
On Thu, Jul 14, 2022 at 4:45 PM Daniel Latypov dlatypov@google.com wrote:
Ack. So concretely, so then a final result like this?
$ cat kernel/kcsan/.kunitconfig # 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:
Oops, this bit would need to be updated to something like:
# Under kunit_tool, this can be done by using --qemu_args:
# ./tools/testing/kunit/kunit.py run --kunitconfig=kernel/kcsan --arch=x86_64 --qemu_args='-smp 8'
CONFIG_KUNIT=y
CONFIG_DEBUG_KERNEL=y
CONFIG_KCSAN=y CONFIG_KCSAN_KUNIT_TEST=y
# Need some level of concurrency to test a concurrency sanitizer. CONFIG_SMP=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
Thanks everyone. I've sent out a v2 with just this patch here: https://lore.kernel.org/linux-kselftest/20220715064052.2673958-1-davidgow@go...
I expect we'll take it in via the KUnit branch, as it's most useful with the --qemu_args option.
Cheers, -- David
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@go...
+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
On Wed, May 18, 2022 at 3:32 AM David Gow davidgow@google.com 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
Ack, but I think Marco settled on removing CONFIG_KCSAN_STRICT=y and CONFIG_KCSAN_WEAK_MEMORY=y.
Acked-by: Brendan Higgins brendanhiggins@google.com
On Wed, May 18, 2022 at 03:32PM +0800, 'David Gow' via KUnit Development wrote:
Add a new QEMU config for kunit_tool, x86_64-smp, which provides an 8-cpu SMP setup. No other kunit_tool configurations provide an SMP setup, so this is the best bet for testing things like KCSAN, which require a multicore/multi-cpu system.
The choice of 8 CPUs is pretty arbitrary: it's enough to get tests like KCSAN to run with a nontrivial number of worker threads, while still working relatively quickly on older machines.
Signed-off-by: David Gow davidgow@google.com
Acked-by: Marco Elver elver@google.com
This is based off the discussion in: https://groups.google.com/g/kasan-dev/c/A7XzC2pXRC8
tools/testing/kunit/qemu_configs/x86_64-smp.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 tools/testing/kunit/qemu_configs/x86_64-smp.py
diff --git a/tools/testing/kunit/qemu_configs/x86_64-smp.py b/tools/testing/kunit/qemu_configs/x86_64-smp.py new file mode 100644 index 000000000000..a95623f5f8b7 --- /dev/null +++ b/tools/testing/kunit/qemu_configs/x86_64-smp.py @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: GPL-2.0 +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', '8'])-- 2.36.0.550.gb090851708-goog
-- You received this message because you are subscribed to the Google Groups "KUnit Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20220518073232.526443-1-davidgow....
On Wed, May 18, 2022 at 12:32 AM 'David Gow' via KUnit Development kunit-dev@googlegroups.com wrote:
Add a new QEMU config for kunit_tool, x86_64-smp, which provides an 8-cpu SMP setup. No other kunit_tool configurations provide an SMP setup, so this is the best bet for testing things like KCSAN, which require a multicore/multi-cpu system.
The choice of 8 CPUs is pretty arbitrary: it's enough to get tests like KCSAN to run with a nontrivial number of worker threads, while still working relatively quickly on older machines.
Since it's arbitrary, I somewhat prefer the idea of leaving up entirely to the caller i.e. $ kunit.py run --kconfig_add=CONFIG_SMP=y --qemu_args '-smp 8'
We could add CONFIG_SMP=y to the default qemu_configs/*.py and do $ kunit.py run --qemu_args '-smp 8' but I'd prefer the first, even if it is more verbose.
Marco, does this seem reasonable from your perspective?
I think that a new --qemu_args would be generically useful for adhoc use and light enough that people won't need to add qemu_configs much. E.g. I can see people wanting multiple NUMA nodes, a specific -cpu, and so on.
On Wed, 18 May 2022 at 17:31, Daniel Latypov dlatypov@google.com wrote:
On Wed, May 18, 2022 at 12:32 AM 'David Gow' via KUnit Development kunit-dev@googlegroups.com wrote:
Add a new QEMU config for kunit_tool, x86_64-smp, which provides an 8-cpu SMP setup. No other kunit_tool configurations provide an SMP setup, so this is the best bet for testing things like KCSAN, which require a multicore/multi-cpu system.
The choice of 8 CPUs is pretty arbitrary: it's enough to get tests like KCSAN to run with a nontrivial number of worker threads, while still working relatively quickly on older machines.
Since it's arbitrary, I somewhat prefer the idea of leaving up entirely to the caller i.e. $ kunit.py run --kconfig_add=CONFIG_SMP=y --qemu_args '-smp 8'
We could add CONFIG_SMP=y to the default qemu_configs/*.py and do $ kunit.py run --qemu_args '-smp 8' but I'd prefer the first, even if it is more verbose.
Marco, does this seem reasonable from your perspective?
Either way works. But I wouldn't mind a sane default though, where that default can be overridden with custom number of CPUs.
I think that a new --qemu_args would be generically useful for adhoc use and light enough that people won't need to add qemu_configs much. E.g. I can see people wanting multiple NUMA nodes, a specific -cpu, and so on.
On Wed, May 18, 2022 at 8:36 AM Marco Elver elver@google.com wrote:
On Wed, 18 May 2022 at 17:31, Daniel Latypov dlatypov@google.com wrote:
On Wed, May 18, 2022 at 12:32 AM 'David Gow' via KUnit Development kunit-dev@googlegroups.com wrote:
Add a new QEMU config for kunit_tool, x86_64-smp, which provides an 8-cpu SMP setup. No other kunit_tool configurations provide an SMP setup, so this is the best bet for testing things like KCSAN, which require a multicore/multi-cpu system.
The choice of 8 CPUs is pretty arbitrary: it's enough to get tests like KCSAN to run with a nontrivial number of worker threads, while still working relatively quickly on older machines.
Since it's arbitrary, I somewhat prefer the idea of leaving up entirely to the caller i.e. $ kunit.py run --kconfig_add=CONFIG_SMP=y --qemu_args '-smp 8'
We could add CONFIG_SMP=y to the default qemu_configs/*.py and do $ kunit.py run --qemu_args '-smp 8' but I'd prefer the first, even if it is more verbose.
Marco, does this seem reasonable from your perspective?
Either way works. But I wouldn't mind a sane default though, where that default can be overridden with custom number of CPUs.
Ack. Let me clean up what I have for --qemu_args and send it out for discussion.
One downside I see to adding more qemu_configs is that --arch now becomes more kunit-specific. Before, a user could assume "oh, it's just what I pass in to make ARCH=...". This new "--arch=x86_64-smp" violates that. I don't personally see it being that confusing, but I still worry.
On Wed, May 18, 2022 at 8:39 AM Daniel Latypov dlatypov@google.com wrote:
Either way works. But I wouldn't mind a sane default though, where that default can be overridden with custom number of CPUs.
Ack. Let me clean up what I have for --qemu_args and send it out for discussion.
Sent out as https://lore.kernel.org/linux-kselftest/20220518170124.2849497-1-dlatypov@go...
On Wed, May 18, 2022 at 11:36 PM Marco Elver elver@google.com wrote:
On Wed, 18 May 2022 at 17:31, Daniel Latypov dlatypov@google.com wrote:
On Wed, May 18, 2022 at 12:32 AM 'David Gow' via KUnit Development kunit-dev@googlegroups.com wrote:
Add a new QEMU config for kunit_tool, x86_64-smp, which provides an 8-cpu SMP setup. No other kunit_tool configurations provide an SMP setup, so this is the best bet for testing things like KCSAN, which require a multicore/multi-cpu system.
The choice of 8 CPUs is pretty arbitrary: it's enough to get tests like KCSAN to run with a nontrivial number of worker threads, while still working relatively quickly on older machines.
Since it's arbitrary, I somewhat prefer the idea of leaving up entirely to the caller i.e. $ kunit.py run --kconfig_add=CONFIG_SMP=y --qemu_args '-smp 8'
We could add CONFIG_SMP=y to the default qemu_configs/*.py and do $ kunit.py run --qemu_args '-smp 8' but I'd prefer the first, even if it is more verbose.
Marco, does this seem reasonable from your perspective?
Either way works. But I wouldn't mind a sane default though, where that default can be overridden with custom number of CPUs.
I tend to agree that having both would be nice: I think there are enough useful "machine configs" that trying to maintain, e.g, a 1:1 mapping with kernel architectures is going to leave a bunch of things on the table, particularly as we add more tests for, e.g., drivers and specific CPU models.
The problem, of course, is that the --kconfig_add flags don't allow us to override anything explicitly stated in either the kunitconfig or qemu_config (and I imagine there could be problems with --qemu_config, too).
I think that a new --qemu_args would be generically useful for adhoc use and light enough that people won't need to add qemu_configs much. E.g. I can see people wanting multiple NUMA nodes, a specific -cpu, and so on.
On Thu, May 19, 2022 at 6:15 AM David Gow davidgow@google.com wrote:
I tend to agree that having both would be nice: I think there are enough useful "machine configs" that trying to maintain, e.g, a 1:1 mapping with kernel architectures is going to leave a bunch of things on the table, particularly as we add more tests for, e.g., drivers and specific CPU models.
I agree that we don't necessarily need to maintain a 1:1 mapping. But I feel like we should have a pretty convincing reason for doing so, e.g. support for a CPU that requires we add in a bunch of kconfigs.
This particular one feels simple enough to me. Given we already have to put specific instructions in the kcsan/.kunitconfig, I don't know if there's much of a difference in cost between these two commands
$ ./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 --kconfig_add CONFIG_SMP=y --qemu_args "-smp 8"
I've generally learned to prefer more explicit commands like the second, even if they're quite a bit longer. But I have the following biases * I use FZF heavily, so I don't re-type long commands much * I'm the person who proposed --kconfig_add and --qemu_args, so of course I'd think the longer form is easy to understand. so I'm not in a position to object to this change.
Changing topics: Users can overwrite the '-smp 8' here via --qemu_args [1], so I'm much less worried about hard-coding any specific value in this file anymore. And given that, I think a more "natural" value for this file would be "-smp 2". I think anything that needs more than that should explicitly should --qemu_args.
Thoughts?
[1] tested with --qemu_args='-smp 4' --qemu_args='-smp 8' and I see the following in the test.log smpboot: Allowing 8 CPUs, 0 hotplug CPUs so QEMU respects the last value passed in, as expected.
The problem, of course, is that the --kconfig_add flags don't allow us to override anything explicitly stated in either the kunitconfig or qemu_config (and I imagine there could be problems with --qemu_config, too).
This patch would fix that. https://lore.kernel.org/linux-kselftest/20220519164512.3180360-1-dlatypov@go...
It introduces an overwriting priority of * --kconfig_add * kunitconfig / --kunitconfig * qemu_config
On Thu, May 19, 2022 at 1:11 PM Daniel Latypov dlatypov@google.com wrote:
On Thu, May 19, 2022 at 6:15 AM David Gow davidgow@google.com wrote:
I tend to agree that having both would be nice: I think there are enough useful "machine configs" that trying to maintain, e.g, a 1:1 mapping with kernel architectures is going to leave a bunch of things on the table, particularly as we add more tests for, e.g., drivers and specific CPU models.
I agree that we don't necessarily need to maintain a 1:1 mapping. But I feel like we should have a pretty convincing reason for doing so, e.g. support for a CPU that requires we add in a bunch of kconfigs.
Agreed. That being said, if we have a good convention for archs that are not in arch/, then it should be OK. The biggest thing is that all archs passed into ARCH=, if supported, should have a default with the same value for kunittool; as long as that is the case, I don't think anyone will get confused.
This particular one feels simple enough to me. Given we already have to put specific instructions in the kcsan/.kunitconfig, I don't know if there's much of a difference in cost between these two commands
$ ./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 --kconfig_add CONFIG_SMP=y --qemu_args "-smp 8"
Also agree.
I've generally learned to prefer more explicit commands like the second, even if they're quite a bit longer.
I agree, but I think I learned this from you :-)
But I have the following biases
- I use FZF heavily, so I don't re-type long commands much
Same.
- I'm the person who proposed --kconfig_add and --qemu_args, so of
course I'd think the longer form is easy to understand. so I'm not in a position to object to this change.
Yeah, I think I am a bit biased on this too, but I don't terribly care one way or the other.
Changing topics: Users can overwrite the '-smp 8' here via --qemu_args [1], so I'm much less worried about hard-coding any specific value in this file anymore. And given that, I think a more "natural" value for this file would be "-smp 2". I think anything that needs more than that should explicitly should --qemu_args.
Thoughts?
If we have time, we could bring this topic up at LPC?
[1] tested with --qemu_args='-smp 4' --qemu_args='-smp 8' and I see the following in the test.log smpboot: Allowing 8 CPUs, 0 hotplug CPUs so QEMU respects the last value passed in, as expected.
The problem, of course, is that the --kconfig_add flags don't allow us to override anything explicitly stated in either the kunitconfig or qemu_config (and I imagine there could be problems with --qemu_config, too).
This patch would fix that. https://lore.kernel.org/linux-kselftest/20220519164512.3180360-1-dlatypov@go...
It introduces an overwriting priority of
- --kconfig_add
- kunitconfig / --kunitconfig
- qemu_config
On Wed, May 18, 2022 at 3:32 AM 'David Gow' via KUnit Development kunit-dev@googlegroups.com wrote:
Add a new QEMU config for kunit_tool, x86_64-smp, which provides an 8-cpu SMP setup. No other kunit_tool configurations provide an SMP setup, so this is the best bet for testing things like KCSAN, which require a multicore/multi-cpu system.
The choice of 8 CPUs is pretty arbitrary: it's enough to get tests like KCSAN to run with a nontrivial number of worker threads, while still working relatively quickly on older machines.
Signed-off-by: David Gow davidgow@google.com
I know there is some discussion on this patch, but I think this patch is good as implemented; we could always delete this config if we change our policies later.
Reviewed-by: Brendan Higgins brendanhiggins@google.com
linux-kselftest-mirror@lists.linaro.org
-
Brendan Higgins -
Daniel Latypov -
David Gow -
Marco Elver