+Cc rest of kunit from MAINTAINERS
On 1/7/23 11:55, Geert Uytterhoeven wrote:
Hi Kees,
On Sat, Jan 7, 2023 at 5:02 AM Kees Cook keescook@chromium.org wrote:
Since the long memcpy tests may stall a system for tens of seconds in virtualized architecture environments, split those tests off under CONFIG_MEMCPY_SLOW_KUNIT_TEST so they can be separately disabled.
Reported-by: Guenter Roeck linux@roeck-us.net Link: https://lore.kernel.org/lkml/20221226195206.GA2626419@roeck-us.net Cc: Andrew Morton akpm@linux-foundation.org Cc: Nathan Chancellor nathan@kernel.org Cc: linux-hardening@vger.kernel.org Signed-off-by: Kees Cook keescook@chromium.org
Thanks for your patch!
--- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2621,6 +2621,15 @@ config MEMCPY_KUNIT_TEST
If unsure, say N.
+config MEMCPY_SLOW_KUNIT_TEST
tristate "Include exhaustive memcpy tests" if !KUNIT_ALL_TESTS
Why the tristate?
depends on MEMCPY_KUNIT_TEST
default KUNIT_ALL_TESTS
help
Some memcpy tests are quite exhaustive in checking for overlaps
and bit ranges. These can be very slow, so they are split out
as a separate config.
config IS_SIGNED_TYPE_KUNIT_TEST tristate "Test is_signed_type() macro" if !KUNIT_ALL_TESTS depends on KUNIT diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c index 89128551448d..cc1f36335a9b 100644 --- a/lib/memcpy_kunit.c +++ b/lib/memcpy_kunit.c @@ -307,8 +307,12 @@ static void set_random_nonzero(struct kunit *test, u8 *byte) } }
-static void init_large(struct kunit *test) +static int init_large(struct kunit *test) {
if (!IS_ENABLED(CONFIG_MEMCPY_SLOW_KUNIT_TEST)) {
kunit_skip(test, "Slow test skipped. Enable with CONFIG_MEMCPY_SLOW_KUNIT_TEST=y");
So I can't make the slower tests available for when I need them, but not run them by default?
Indeed it seems weird to tie this to a config option without runtime override.
I guess that's why you made MEMCPY_SLOW_KUNIT_TEST tristate originally, to have a separate module with the slow tests?
On the other hand I can imagine requiring a separate module for slow tests would lead to more churn - IIUC there would need to be two files instead of memcpy_kunit.c, possibly a duplicated boilerplate code (or another shared .c file).
So the idea is to have a generic way to mark some tests as slow and a way to opt-in/opt-out for those when running the tests. Maybe KUnit folks already have such mechanism or have an idea how to implement that.
return -EBUSY;
} /* Get many bit patterns. */ get_random_bytes(large_src, ARRAY_SIZE(large_src));
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Mon, Jan 9, 2023 at 11:51 PM Vlastimil Babka vbabka@suse.cz wrote:
+Cc rest of kunit from MAINTAINERS
On 1/7/23 11:55, Geert Uytterhoeven wrote:
Hi Kees,
On Sat, Jan 7, 2023 at 5:02 AM Kees Cook keescook@chromium.org wrote:
Since the long memcpy tests may stall a system for tens of seconds in virtualized architecture environments, split those tests off under CONFIG_MEMCPY_SLOW_KUNIT_TEST so they can be separately disabled.
<snip>
-static void init_large(struct kunit *test) +static int init_large(struct kunit *test) {
if (!IS_ENABLED(CONFIG_MEMCPY_SLOW_KUNIT_TEST)) {
kunit_skip(test, "Slow test skipped. Enable with CONFIG_MEMCPY_SLOW_KUNIT_TEST=y");
So I can't make the slower tests available for when I need them, but not run them by default?
Indeed it seems weird to tie this to a config option without runtime override.
I guess that's why you made MEMCPY_SLOW_KUNIT_TEST tristate originally, to have a separate module with the slow tests?
On the other hand I can imagine requiring a separate module for slow tests would lead to more churn - IIUC there would need to be two files instead of memcpy_kunit.c, possibly a duplicated boilerplate code (or another shared .c file).
So the idea is to have a generic way to mark some tests as slow and a way to opt-in/opt-out for those when running the tests. Maybe KUnit folks already have such mechanism or have an idea how to implement that.
There is no mechanism atm, and we'd still need to figure it out so it'll be a while. So I think a patch like this makes sense in the short-term.
This is definitely something we've always thought would be useful eventually. See this TODO which has been there since day 1 ;) https://elixir.bootlin.com/linux/latest/source/lib/kunit/try-catch.c#L36
It just felt like it would be premature to come up with something when basically all the tests up until now ran ~instantly.
Throwing out some rough implementation ideas: I was thinking the granularity for these timeout annotations would be at the suite-level. If we go with that, then I guess the intended flow is to group slow tests into their own suite and mark them as such.
Then maybe we'd have some runtime way of disabling/enabling "long" tests, like a cmdline opt. E.g. you'd pass `kunit.max_test_size=30` to exclude tests longer than 30 seconds.
Daniel
linux-kselftest-mirror@lists.linaro.org