Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210922182541.1372400-1-elver@google.com>
In-Reply-To: <20210922182541.1372400-1-elver@google.com>
From:   David Gow <davidgow@google.com>
Date:   Fri, 24 Sep 2021 01:39:19 +0800
Message-ID: <CABVgOSmKTAQpMzFp6vd+t=ojTPXOT+heME210cq2NA0sMML==w@mail.gmail.com>
Subject: Re: [PATCH] kfence: test: use kunit_skip() to skip tests
To:     Marco Elver <elver@google.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Alexander Potapenko <glider@google.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        Aleksandr Nogikh <nogikh@google.com>,
        Taras Madan <tarasmadan@google.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux Memory Management List <linux-mm@kvack.org>,
        kasan-dev <kasan-dev@googlegroups.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Sep 23, 2021 at 2:26 AM Marco Elver <elver@google.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@google.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@google.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
>