Re: [PATCH] kunit: memcpy: Split slow memcpy tests into MEMCPY_SLOW_KUNIT_TEST

From: Daniel Latypov
Date: Tue Jan 10 2023 - 15:38:29 EST


On Mon, Jan 9, 2023 at 11:51 PM Vlastimil Babka <vbabka@xxxxxxx> 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@xxxxxxxxxxxx> 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