Re: [PATCH] selftests/mm: Reduce uffd-unit-test poison test to minimum

[Axel Rasmussen <axelrasmussen@google.com>]

On Fri, Jun 20, 2025 at 8:01 AM Peter Xu <peterx@redhat.com> wrote:
>
> The test will still generate quite some unwanted MCE error messages to
> syslog.  There was old proposal ratelimiting the MCE messages from kernel,
> but that has risk of hiding real useful information on production systems.
>
> We can at least reduce the test to minimum to not over-pollute dmesg,
> however trying to not lose its coverage too much.
>
> Cc: Axel Rasmussen <axelrasmussen@google.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Besides a small nitpick you can take:

Reviewed-by: Axel Rasmussen <axelrasmussen@google.com>

Making the functional tests small makes sense to me, especially for
poisoning. Only reason to use a huge number of pages is if we're
trying to stress racy bugs or so, but really for that you'd want even
more pages / more threads / run for a longer time. It makes sense to
separate that use case out / maybe not run it by default, and leave
the functional tests small + fast.

> ---
>  tools/testing/selftests/mm/uffd-unit-tests.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
> index c73fd5d455c8..39b3fd1b7bf2 100644
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -1027,6 +1027,9 @@ static void uffd_poison_handle_fault(
>                 do_uffdio_poison(uffd, offset);
>  }
>
> +/* Make sure to cover odd/even, and minimum duplications */
> +#define  UFFD_POISON_TEST_NPAGES  4
> +
>  static void uffd_poison_test(uffd_test_args_t *targs)
>  {
>         pthread_t uffd_mon;
> @@ -1034,12 +1037,17 @@ static void uffd_poison_test(uffd_test_args_t *targs)
>         struct uffd_args args = { 0 };
>         struct sigaction act = { 0 };
>         unsigned long nr_sigbus = 0;
> -       unsigned long nr;
> +       unsigned long nr, poison_pages = UFFD_POISON_TEST_NPAGES;
> +
> +       if (nr_pages < poison_pages) {
> +               uffd_test_skip("Too less pages for POISON test");

I think "Too few pages for POISON test" is more grammatically correct.

> +               return;
> +       }
>
>         fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
>
> -       uffd_register_poison(uffd, area_dst, nr_pages * page_size);
> -       memset(area_src, 0, nr_pages * page_size);
> +       uffd_register_poison(uffd, area_dst, poison_pages * page_size);
> +       memset(area_src, 0, poison_pages * page_size);
>
>         args.handle_fault = uffd_poison_handle_fault;
>         if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
> @@ -1051,7 +1059,7 @@ static void uffd_poison_test(uffd_test_args_t *targs)
>         if (sigaction(SIGBUS, &act, 0))
>                 err("sigaction");
>
> -       for (nr = 0; nr < nr_pages; ++nr) {
> +       for (nr = 0; nr < poison_pages; ++nr) {
>                 unsigned long offset = nr * page_size;
>                 const char *bytes = (const char *) area_dst + offset;
>                 const char *i;
> @@ -1078,9 +1086,9 @@ static void uffd_poison_test(uffd_test_args_t *targs)
>         if (pthread_join(uffd_mon, NULL))
>                 err("pthread_join()");
>
> -       if (nr_sigbus != nr_pages / 2)
> +       if (nr_sigbus != poison_pages / 2)
>                 err("expected to receive %lu SIGBUS, actually received %lu",
> -                   nr_pages / 2, nr_sigbus);
> +                   poison_pages / 2, nr_sigbus);
>
>         uffd_test_pass();
>  }
> --
> 2.49.0
>