linux-kernel - Re: [PATCH] kfence: test: use kunit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 Sep 2021 01:39:19 +0800
From:   David Gow <davidgow@...gle.com>
To:     Marco Elver <elver@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Potapenko <glider@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Aleksandr Nogikh <nogikh@...gle.com>,
        Taras Madan <tarasmadan@...gle.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Memory Management List <linux-mm@...ck.org>,
        kasan-dev <kasan-dev@...glegroups.com>
Subject: Re: [PATCH] kfence: test: use kunit_skip() to skip tests

On Thu, Sep 23, 2021 at 2:26 AM Marco Elver <elver@...gle.com> wrote:
>
> Use the new kunit_skip() to skip tests if requirements were not met. It
> makes it easier to see in KUnit's summary if there were skipped tests.
>
> Signed-off-by: Marco Elver <elver@...gle.com>
> ---

Thanks: I'm glad these features are proving useful. I've tested these
under qemu, and it works pretty well.

Certainly from the KUnit point of view, this is:
Reviewed-by: David Gow <davidgow@...gle.com>

(A couple of unrelated complaints about the kfence tests are that
TRACEPOINTS isn't selected by default, and that the manual
registering/unregistering of the tracepoints does break some of the
kunit tooling when several tests are built-in. That's something that
exists independently of this patch, though, and possibly requires some
KUnit changes to be fixed cleanly (kfence isn't the only thing to do
this). So not something to hold up this patch.)

Cheers,
-- David

>  mm/kfence/kfence_test.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/mm/kfence/kfence_test.c b/mm/kfence/kfence_test.c
> index f1690cf54199..695030c1fff8 100644
> --- a/mm/kfence/kfence_test.c
> +++ b/mm/kfence/kfence_test.c
> @@ -32,6 +32,11 @@
>  #define arch_kfence_test_address(addr) (addr)
>  #endif
>
> +#define KFENCE_TEST_REQUIRES(test, cond) do {                  \
> +       if (!(cond))                                            \
> +               kunit_skip((test), "Test requires: " #cond);    \
> +} while (0)
> +
>  /* Report as observed from console. */
>  static struct {
>         spinlock_t lock;
> @@ -555,8 +560,7 @@ static void test_init_on_free(struct kunit *test)
>         };
>         int i;
>
> -       if (!IS_ENABLED(CONFIG_INIT_ON_FREE_DEFAULT_ON))
> -               return;
> +       KFENCE_TEST_REQUIRES(test, IS_ENABLED(CONFIG_INIT_ON_FREE_DEFAULT_ON));
>         /* Assume it hasn't been disabled on command line. */
>
>         setup_test_cache(test, size, 0, NULL);
> @@ -603,10 +607,8 @@ static void test_gfpzero(struct kunit *test)
>         char *buf1, *buf2;
>         int i;
>
> -       if (CONFIG_KFENCE_SAMPLE_INTERVAL > 100) {
> -               kunit_warn(test, "skipping ... would take too long\n");
> -               return;
> -       }
> +       /* Skip if we think it'd take too long. */
> +       KFENCE_TEST_REQUIRES(test, CONFIG_KFENCE_SAMPLE_INTERVAL <= 100);
>
>         setup_test_cache(test, size, 0, NULL);
>         buf1 = test_alloc(test, size, GFP_KERNEL, ALLOCATE_ANY);
> --
> 2.33.0.464.g1972c5931b-goog
>