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

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

[Peter Xu]

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>
---
 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");
+		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

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

[Axel Rasmussen]

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
>

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

[Peter Xu]

On Thu, Jun 26, 2025 at 10:32:20AM -0700, Axel Rasmussen wrote:
> 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>

Thanks, Axel.

> 
> 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.

IIUC the major complain from others are the pollutions in the dmesg.  And
yes, it's also slow and IIUC it's because of the messages piped, at least
when I ran it in VMs with a serial console.

> 
> > ---
> >  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.

Right..  This is currently in mm-unstable.  Andrew, would you mind take
below as a fixup?  TIA!

===8<===

commit 810f5674e1b990775600ffca533dbf75505adbef (HEAD -> test-poison)
Author: Peter Xu <peterx@redhat.com>
Date:   Thu Jun 26 14:22:21 2025 -0400

    fixup! selftests/mm: reduce uffd-unit-test poison test to minimum
    
    Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tools/testing/selftests/mm/uffd-unit-tests.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
index 39b3fd1b7bf2..50501b38e34e 100644
--- a/tools/testing/selftests/mm/uffd-unit-tests.c
+++ b/tools/testing/selftests/mm/uffd-unit-tests.c
@@ -1040,7 +1040,7 @@ static void uffd_poison_test(uffd_test_args_t *targs)
        unsigned long nr, poison_pages = UFFD_POISON_TEST_NPAGES;
 
        if (nr_pages < poison_pages) {
-               uffd_test_skip("Too less pages for POISON test");
+               uffd_test_skip("Too few pages for POISON test");
                return;
        }

-- 
Peter Xu