Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230412163922.327282-1-peterx@redhat.com> <20230412164257.328375-1-peterx@redhat.com>
In-Reply-To: <20230412164257.328375-1-peterx@redhat.com>
From:   Axel Rasmussen <axelrasmussen@google.com>
Date:   Wed, 12 Apr 2023 12:47:39 -0700
Message-ID: <CAJHvVcgtAhEkqkYegHONCJz5uJoxRLRdm9Kp8DmkcnZhU=w=9g@mail.gmail.com>
Subject: Re: [PATCH v2 16/31] selftests/mm: UFFDIO_API test
To:     Peter Xu <peterx@redhat.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Nadav Amit <nadav.amit@gmail.com>,
        David Hildenbrand <david@redhat.com>,
        Mike Rapoport <rppt@linux.vnet.ibm.com>,
        Leonardo Bras Soares Passos <lsoaresp@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Apr 12, 2023 at 9:43=E2=80=AFAM Peter Xu <peterx@redhat.com> wrote:
>
> Add one simple test for UFFDIO_API.  With that, I also added a bunch of
> small but handy helpers along the way.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tools/testing/selftests/mm/uffd-unit-tests.c | 109 ++++++++++++++++++-
>  1 file changed, 108 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing=
/selftests/mm/uffd-unit-tests.c
> index 6857388783be..bb492c258486 100644
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -9,9 +9,116 @@
>
>  #ifdef __NR_userfaultfd
>
> +static void uffd_test_report(void)
> +{
> +       printf("Userfaults unit tests: pass=3D%u, skip=3D%u, fail=3D%u (t=
otal=3D%u)\n",
> +              ksft_get_pass_cnt(),
> +              ksft_get_xskip_cnt(),
> +              ksft_get_fail_cnt(),
> +              ksft_test_num());
> +}
> +
> +static void uffd_test_pass(void)
> +{
> +       printf("done\n");
> +       ksft_inc_pass_cnt();
> +}
> +
> +#define  uffd_test_start(...)  do {            \
> +               printf("Testing ");             \
> +               printf(__VA_ARGS__);            \
> +               printf("... ");                 \
> +               fflush(stdout);                 \
> +       } while (0)
> +
> +#define  uffd_test_fail(...)  do {             \
> +               printf("failed [reason: ");     \
> +               printf(__VA_ARGS__);            \
> +               printf("]\n");                  \
> +               ksft_inc_fail_cnt();            \
> +       } while (0)
> +
> +#define  uffd_test_skip(...)  do {             \
> +               printf("skipped [reason: ");    \
> +               printf(__VA_ARGS__);            \
> +               printf("]\n");                  \
> +               ksft_inc_xskip_cnt();           \
> +       } while (0)

Some of this is duplicating what's in kselftest_harness.h - e.g.
assertions, printing test summary, keeping track of pass/fail/skip
counts, etc.

I wonder how you feel about using the standard harness? E.g. is the
plan to do this (more straightforward?) refactor first, and then
switch later? Or is there some reason it can't be used?

> +
> +/*
> + * Returns 1 if specific userfaultfd supported, 0 otherwise.  Note, we'l=
l
> + * return 1 even if some test failed as long as uffd supported, because =
in
> + * that case we still want to proceed with the rest uffd unit tests.
> + */
> +static int test_uffd_api(bool use_dev)
> +{
> +       struct uffdio_api uffdio_api;
> +       int uffd;
> +
> +       uffd_test_start("UFFDIO_API (with %s)",
> +                       use_dev ? "/dev/userfaultfd" : "syscall");
> +
> +       if (use_dev)
> +               uffd =3D uffd_open_dev(UFFD_FLAGS);
> +       else
> +               uffd =3D uffd_open_sys(UFFD_FLAGS);
> +       if (uffd < 0) {
> +               uffd_test_skip("cannot open userfaultfd handle");
> +               return 0;
> +       }
> +
> +       /* Test wrong UFFD_API */
> +       uffdio_api.api =3D 0xab;
> +       uffdio_api.features =3D 0;
> +       if (ioctl(uffd, UFFDIO_API, &uffdio_api) =3D=3D 0) {
> +               uffd_test_fail("UFFDIO_API should fail with wrong api but=
 didn't");
> +               goto out;
> +       }
> +
> +       /* Test wrong feature bit */
> +       uffdio_api.api =3D UFFD_API;
> +       uffdio_api.features =3D BIT_ULL(63);
> +       if (ioctl(uffd, UFFDIO_API, &uffdio_api) =3D=3D 0) {
> +               uffd_test_fail("UFFDIO_API should fail with wrong feature=
 but didn't");
> +               goto out;
> +       }
> +
> +       /* Test normal UFFDIO_API */
> +       uffdio_api.api =3D UFFD_API;
> +       uffdio_api.features =3D 0;
> +       if (ioctl(uffd, UFFDIO_API, &uffdio_api)) {
> +               uffd_test_fail("UFFDIO_API should succeed but failed");
> +               goto out;
> +       }
> +
> +       /* Test double requests of UFFDIO_API with a random feature set *=
/
> +       uffdio_api.features =3D BIT_ULL(0);
> +       if (ioctl(uffd, UFFDIO_API, &uffdio_api) =3D=3D 0) {
> +               uffd_test_fail("UFFDIO_API should reject initialized uffd=
");
> +               goto out;
> +       }
> +
> +       uffd_test_pass();
> +out:
> +       close(uffd);
> +       /* We have a valid uffd handle */
> +       return 1;
> +}
> +
>  int main(int argc, char *argv[])
>  {
> -       return KSFT_PASS;
> +       int has_uffd;
> +
> +       has_uffd =3D test_uffd_api(false);
> +       has_uffd |=3D test_uffd_api(true);
> +
> +       if (!has_uffd) {
> +               printf("Userfaultfd not supported or unprivileged, skip a=
ll tests\n");
> +               exit(KSFT_SKIP);
> +       }
> +       uffd_test_report();
> +
> +       return ksft_get_fail_cnt() ? KSFT_FAIL : KSFT_PASS;
>  }
>
>  #else /* __NR_userfaultfd */
> --
> 2.39.1
>