[PATCH] selftests/mm: Fix soft-dirty kselftest supported check

Audra Mitchell posted 1 patch 1 week, 6 days ago
tools/testing/selftests/mm/vm_util.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
[PATCH] selftests/mm: Fix soft-dirty kselftest supported check
Posted by Audra Mitchell 1 week, 6 days ago
On architectures with separate user address space, such as s390 or
those without an MMU, the soft-dirty kselftest may fail when checking
to see if the feature is supported.

  # --------------------
  # running ./soft-dirty
  # --------------------
  # TAP version 13
  # 1..15
  # Bail out! PAGEMAP_SCAN succeeded unexpectedly
  # # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0
  # [FAIL]
  not ok 1 soft-dirty # exit=1
  # SUMMARY: PASS=0 SKIP=0 FAIL=1

The soft-dirty test will initate an ioctl with the PAGEMAP_SCAN flag with
an invalid address for the page_region. This is done intentionally to
have the ioctl return with an expected EFAULT and with the correct
categories returned.

However, on architectures with separate user address space, such as s390 or
those without an MMU, the call to __access_ok (used to validate the
variables provided with the ioctl) will always return true and we will not
fail as expected.

        if (IS_ENABLED(CONFIG_ALTERNATE_USER_ADDRESS_SPACE) ||
            !IS_ENABLED(CONFIG_MMU))
                return true;

Let's simplify the check for PAGEMAP_SCAN and provide a valid page_region
address so that we get a non-errno return if it is supported.

Fixes: 600bca580579 ("selftests/mm: check that PAGEMAP_SCAN returns correct categories")

Signed-off-by: Audra Mitchell <audra@redhat.com>
---
 tools/testing/selftests/mm/vm_util.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
index a6d4ff7dfdc0..82998406b335 100644
--- a/tools/testing/selftests/mm/vm_util.c
+++ b/tools/testing/selftests/mm/vm_util.c
@@ -67,20 +67,26 @@ static uint64_t pagemap_scan_get_categories(int fd, char *start)
 }
 
 /* `start` is any valid address. */
-static bool pagemap_scan_supported(int fd, char *start)
+static bool pagemap_scan_supported(int fd)
 {
+	const size_t pagesize = getpagesize();
 	static int supported = -1;
-	int ret;
+	struct page_region r;
+	void *test_area;
 
 	if (supported != -1)
 		return supported;
 
-	/* Provide an invalid address in order to trigger EFAULT. */
-	ret = __pagemap_scan_get_categories(fd, start, (struct page_region *) ~0UL);
-	if (ret == 0)
-		ksft_exit_fail_msg("PAGEMAP_SCAN succeeded unexpectedly\n");
-
-	supported = errno == EFAULT;
+	test_area = mmap(0, pagesize, PROT_READ | PROT_WRITE,
+			MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+	if (test_area == MAP_FAILED) {
+		ksft_print_msg("WARN: mmap() failed: %s\n", strerror(errno));
+		supported = 0;
+	} else {
+		supported = __pagemap_scan_get_categories(fd, test_area, &r) >= 0;
+		ksft_print_msg("errno: %d\n", errno);
+		munmap(test_area, pagesize);
+	}
 
 	return supported;
 }
@@ -90,7 +96,7 @@ static bool page_entry_is(int fd, char *start, char *desc,
 {
 	bool m = pagemap_get_entry(fd, start) & pagemap_flags;
 
-	if (pagemap_scan_supported(fd, start)) {
+	if (pagemap_scan_supported(fd)) {
 		bool s = pagemap_scan_get_categories(fd, start) & pagescan_flags;
 
 		if (m == s)
-- 
2.52.0
Re: [PATCH] selftests/mm: Fix soft-dirty kselftest supported check
Posted by David Hildenbrand (Arm) 1 week, 3 days ago
On 3/20/26 19:39, Audra Mitchell wrote:
> On architectures with separate user address space, such as s390 or
> those without an MMU, the soft-dirty kselftest may fail when checking
> to see if the feature is supported.
> 
>   # --------------------
>   # running ./soft-dirty
>   # --------------------
>   # TAP version 13
>   # 1..15
>   # Bail out! PAGEMAP_SCAN succeeded unexpectedly
>   # # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0
>   # [FAIL]
>   not ok 1 soft-dirty # exit=1
>   # SUMMARY: PASS=0 SKIP=0 FAIL=1
> 
> The soft-dirty test will initate an ioctl with the PAGEMAP_SCAN flag with
> an invalid address for the page_region. This is done intentionally to
> have the ioctl return with an expected EFAULT and with the correct
> categories returned.
> 
> However, on architectures with separate user address space, such as s390 or
> those without an MMU, the call to __access_ok (used to validate the
> variables provided with the ioctl) will always return true and we will not
> fail as expected.
> 
>         if (IS_ENABLED(CONFIG_ALTERNATE_USER_ADDRESS_SPACE) ||
>             !IS_ENABLED(CONFIG_MMU))
>                 return true;
> 
> Let's simplify the check for PAGEMAP_SCAN and provide a valid page_region
> address so that we get a non-errno return if it is supported.
> 
> Fixes: 600bca580579 ("selftests/mm: check that PAGEMAP_SCAN returns correct categories")
> 
> Signed-off-by: Audra Mitchell <audra@redhat.com>
> ---
>  tools/testing/selftests/mm/vm_util.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> index a6d4ff7dfdc0..82998406b335 100644
> --- a/tools/testing/selftests/mm/vm_util.c
> +++ b/tools/testing/selftests/mm/vm_util.c
> @@ -67,20 +67,26 @@ static uint64_t pagemap_scan_get_categories(int fd, char *start)
>  }
>  
>  /* `start` is any valid address. */
> -static bool pagemap_scan_supported(int fd, char *start)
> +static bool pagemap_scan_supported(int fd)
>  {
> +	const size_t pagesize = getpagesize();
>  	static int supported = -1;
> -	int ret;
> +	struct page_region r;
> +	void *test_area;
>  
>  	if (supported != -1)
>  		return supported;
>  
> -	/* Provide an invalid address in order to trigger EFAULT. */
> -	ret = __pagemap_scan_get_categories(fd, start, (struct page_region *) ~0UL);
> -	if (ret == 0)
> -		ksft_exit_fail_msg("PAGEMAP_SCAN succeeded unexpectedly\n");
> -
> -	supported = errno == EFAULT;
> +	test_area = mmap(0, pagesize, PROT_READ | PROT_WRITE,
> +			MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
> +	if (test_area == MAP_FAILED) {
> +		ksft_print_msg("WARN: mmap() failed: %s\n", strerror(errno));
> +		supported = 0;
> +	} else {
> +		supported = __pagemap_scan_get_categories(fd, test_area, &r) >= 0;

You have to cast it to a (long) first before comparing, like we do in
pagemap_scan_get_categories().


Maybe best written as

	long ret = __pagemap_scan_get_categories(fd, test_area, &r);

	if (ret >= 0)
		supported = 1;

> +		ksft_print_msg("errno: %d\n", errno);

I guess we should drop that, was mostly for testing.

Thanks!

-- 
Cheers,

David
Re: [PATCH] selftests/mm: Fix soft-dirty kselftest supported check
Posted by Andrew Morton 1 week, 2 days ago
On Mon, 23 Mar 2026 12:56:33 +0100 "David Hildenbrand (Arm)" <david@kernel.org> wrote:

> > +		supported = __pagemap_scan_get_categories(fd, test_area, &r) >= 0;
> 
> You have to cast it to a (long) first before comparing, like we do in
> pagemap_scan_get_categories().
> 
> 
> Maybe best written as
> 
> 	long ret = __pagemap_scan_get_categories(fd, test_area, &r);
> 
> 	if (ret >= 0)
> 		supported = 1;
> 
> > +		ksft_print_msg("errno: %d\n", errno);
> 
> I guess we should drop that, was mostly for testing.
Re: [PATCH] selftests/mm: Fix soft-dirty kselftest supported check
Posted by Andrew Morton 1 week, 2 days ago
On Tue, 24 Mar 2026 16:23:24 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Mon, 23 Mar 2026 12:56:33 +0100 "David Hildenbrand (Arm)" <david@kernel.org> wrote:
> 
> > > +		supported = __pagemap_scan_get_categories(fd, test_area, &r) >= 0;
> > 
> > You have to cast it to a (long) first before comparing, like we do in
> > pagemap_scan_get_categories().
> > 
> > 
> > Maybe best written as
> > 
> > 	long ret = __pagemap_scan_get_categories(fd, test_area, &r);
> > 
> > 	if (ret >= 0)
> > 		supported = 1;
> > 
> > > +		ksft_print_msg("errno: %d\n", errno);
> > 
> > I guess we should drop that, was mostly for testing.

oops, sorry, just learned the difference between "delete" and "send"

I was going to say "unclear why we're printing the errno from a
successful mmap()".
Re: [PATCH] selftests/mm: Fix soft-dirty kselftest supported check
Posted by Audra Mitchell 1 week, 1 day ago
On Tue, Mar 24, 2026 at 04:24:45PM -0700, Andrew Morton wrote:
> On Tue, 24 Mar 2026 16:23:24 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Mon, 23 Mar 2026 12:56:33 +0100 "David Hildenbrand (Arm)" <david@kernel.org> wrote:
> > 
> > > > +		supported = __pagemap_scan_get_categories(fd, test_area, &r) >= 0;
> > > 
> > > You have to cast it to a (long) first before comparing, like we do in
> > > pagemap_scan_get_categories().
> > > 
> > > 
> > > Maybe best written as
> > > 
> > > 	long ret = __pagemap_scan_get_categories(fd, test_area, &r);
> > > 
> > > 	if (ret >= 0)
> > > 		supported = 1;
> > > 
> > > > +		ksft_print_msg("errno: %d\n", errno);
> > > 
> > > I guess we should drop that, was mostly for testing.

Wouldn't it be prudent to also update __pagemap_scan_get_categories
return type to a long? Right now its uint64_t, but we expect a long
to be returned from the ioctl:

	static long do_pagemap_cmd(struct file *file, unsigned int cmd,
                           unsigned long arg)

I've made the other changes, but I'll wait to push the next version after
this feedback. Thanks!
Re: [PATCH] selftests/mm: Fix soft-dirty kselftest supported check
Posted by David Hildenbrand (Arm) 6 days, 18 hours ago
On 3/25/26 17:23, Audra Mitchell wrote:
> On Tue, Mar 24, 2026 at 04:24:45PM -0700, Andrew Morton wrote:
>> On Tue, 24 Mar 2026 16:23:24 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>>
>>>
> 
> Wouldn't it be prudent to also update __pagemap_scan_get_categories
> return type to a long? Right now its uint64_t, but we expect a long
> to be returned from the ioctl:
> 
> 	static long do_pagemap_cmd(struct file *file, unsigned int cmd,
>                            unsigned long arg)
> 
> I've made the other changes, but I'll wait to push the next version after
> this feedback. Thanks!

Yes, definitely.

I was under the impression that __pagemap_scan_get_categories() could
actually return some kind of a mask that would warrant the uint64_t. But
we really just return the result from the ioctl().

And ioctl() is defined to return an int. So you could just return an int
there.

-- 
Cheers,

David
Re: [PATCH] selftests/mm: Fix soft-dirty kselftest supported check
Posted by Andrew Morton 1 week, 6 days ago
On Fri, 20 Mar 2026 14:39:15 -0400 Audra Mitchell <audra@redhat.com> wrote:

> On architectures with separate user address space, such as s390 or
> those without an MMU, the soft-dirty kselftest may fail when checking
> to see if the feature is supported.
> 
>   # --------------------
>   # running ./soft-dirty
>   # --------------------
>   # TAP version 13
>   # 1..15
>   # Bail out! PAGEMAP_SCAN succeeded unexpectedly
>   # # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0
>   # [FAIL]
>   not ok 1 soft-dirty # exit=1
>   # SUMMARY: PASS=0 SKIP=0 FAIL=1
> 
> The soft-dirty test will initate an ioctl with the PAGEMAP_SCAN flag with
> an invalid address for the page_region. This is done intentionally to
> have the ioctl return with an expected EFAULT and with the correct
> categories returned.
> 
> However, on architectures with separate user address space, such as s390 or
> those without an MMU, the call to __access_ok (used to validate the
> variables provided with the ioctl) will always return true and we will not
> fail as expected.
> 
>         if (IS_ENABLED(CONFIG_ALTERNATE_USER_ADDRESS_SPACE) ||
>             !IS_ENABLED(CONFIG_MMU))
>                 return true;
> 
> Let's simplify the check for PAGEMAP_SCAN and provide a valid page_region
> address so that we get a non-errno return if it is supported.

AI review asked some questions about v1:
	https://sashiko.dev/#/patchset/20260320184010.759461-2-audra%40redhat.com

These might have been addressed in this version but without a
versioning tag and without a what-i-changed description I find it
hard(er) to tell.