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

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

[Audra Mitchell]

On architectures with separate user address space, such as s390 or
those without an MMU, the call to __access_ok will return true.
The soft-dirty test attempts to check if the PAGEMAP_SCAN feature
is supported by providing an invalid address and expecting
__access_ok to return false, thus throwing an EFAULT error on return.
Because of this assumption, this check will always fail for the
architectures aforementioned. Update the supported check to handle
the return being zero for these types of cases.

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

diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
index d954bf91afd5..3bb7d322101c 100644
--- a/tools/testing/selftests/mm/vm_util.c
+++ b/tools/testing/selftests/mm/vm_util.c
@@ -77,10 +77,8 @@ static bool pagemap_scan_supported(int fd, char *start)
 
 	/* 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;
+	supported = (ret == 0) || (errno == EFAULT);
 
 	return supported;
 }
-- 
2.52.0

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

[David Hildenbrand (Arm)]

On 2/18/26 19:42, Audra Mitchell wrote:
> On architectures with separate user address space, such as s390 or
> those without an MMU, the call to __access_ok will return true.

Where is this __access_ok() you mention here? Somewhere in
fs/proc/task_mmu.c?

> The soft-dirty test attempts to check if the PAGEMAP_SCAN feature
> is supported by providing an invalid address and expecting
> __access_ok to return false, thus throwing an EFAULT error on return.

Where in the soft-dirty test is that triggered?

I'm wondering whether the soft-dirty test should be adjusted, but I did
not yet understand from where this behavior is triggered.

> Because of this assumption, this check will always fail for the
> architectures aforementioned. Update the supported check to handle
> the return being zero for these types of cases.

It would be great to include the output of an actual test case
(soft-dirty?) that is affected by this before/after your change (likely
on s390x where it seems to trigger).

Do we have a Fixes: tag?

> 
> Signed-off-by: Audra Mitchell <audra@redhat.com>
> ---
>  tools/testing/selftests/mm/vm_util.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> index d954bf91afd5..3bb7d322101c 100644
> --- a/tools/testing/selftests/mm/vm_util.c
> +++ b/tools/testing/selftests/mm/vm_util.c
> @@ -77,10 +77,8 @@ static bool pagemap_scan_supported(int fd, char *start)
>  
>  	/* 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;
> +	supported = (ret == 0) || (errno == EFAULT);
>  
>  	return supported;
>  }


-- 
Cheers,

David

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

[Audra Mitchell]

Sorry! I missed this email so never responded!

On Tue, Feb 24, 2026 at 05:15:14PM +0100, David Hildenbrand (Arm) wrote:
> On 2/18/26 19:42, Audra Mitchell wrote:
> > On architectures with separate user address space, such as s390 or
> > those without an MMU, the call to __access_ok will return true.
> 
> Where is this __access_ok() you mention here? Somewhere in
> fs/proc/task_mmu.c?
>
> Where in the soft-dirty test is that triggered?
> 
> I'm wondering whether the soft-dirty test should be adjusted, but I did
> not yet understand from where this behavior is triggered.

The problem arises when we are checking to see what features/categories are
supported. The call chain for the soft-dirty program goes:

  main()
    ->test_simple()
      ->pagemap_is_softdirty()
        ->page_entry_is()
          ->pagemap_scan_supported()
            ->__pagemap_scan_get_categories()
              ->ioctl()
  
We enter the kernel with an ioctl, expecting to have an EFAULT returned (see
the comment from pagemap_scan_get_categories():
    
          /* Provide an invalid address in order to trigger EFAULT. */
        ret = __pagemap_scan_get_categories(fd, start, (struct page_region *) ~0UL);

Once we enter the kernel, we will check the arguments passed which includes the
call to access_ok: 

  do_pagemap_cmd()
    ->do_pagemap_scan()
      ->pagemap_scan_get_args()
        ->access_ok()

Here is the path within pagemap_scan_get_args where we expect to fail return
the EFAULT:

        if (arg->vec && !access_ok((void __user *)(long)arg->vec,
                                   size_mul(arg->vec_len, sizeof(struct page_region))))
                return -EFAULT;
                
However, if CONFIG_ALTERNATE_USER_ADDRESS_SPACE is enabled or if CONFIG_MMU is
NOT enabled, then we just return true:

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

The intent appears to be just getting the categories available to us and
verifying that we have the feature available for testing. However, this corner
case means the soft-dirty test will fail with the following:

  # --------------------
  # 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
  1..1
  
Since the intent is just to validate that the features are available to us for
testing, I think we can just modify the check so that we don't fail if we
return 0.
  
Let me know what you think, or if you have more questions!

> Do we have a Fixes: tag?

I always hesistate to add a Fixes tag on situations like this since this is a
corner case that was not considered by the original author. If we need a
fixes tag, then it would be:

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

Thanks a bunch!
-- Audra Mitchell

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

[David Hildenbrand (Arm)]

On 3/17/26 16:08, Audra Mitchell wrote:
> Sorry! I missed this email so never responded!
> 
> On Tue, Feb 24, 2026 at 05:15:14PM +0100, David Hildenbrand (Arm) wrote:
>> On 2/18/26 19:42, Audra Mitchell wrote:
>>> On architectures with separate user address space, such as s390 or
>>> those without an MMU, the call to __access_ok will return true.
>>
>> Where is this __access_ok() you mention here? Somewhere in
>> fs/proc/task_mmu.c?
>>
>> Where in the soft-dirty test is that triggered?
>>
>> I'm wondering whether the soft-dirty test should be adjusted, but I did
>> not yet understand from where this behavior is triggered.
> 
> The problem arises when we are checking to see what features/categories are
> supported. The call chain for the soft-dirty program goes:
> 
>   main()
>     ->test_simple()
>       ->pagemap_is_softdirty()
>         ->page_entry_is()
>           ->pagemap_scan_supported()
>             ->__pagemap_scan_get_categories()
>               ->ioctl()
>   
> We enter the kernel with an ioctl, expecting to have an EFAULT returned (see
> the comment from pagemap_scan_get_categories():
>     
>           /* Provide an invalid address in order to trigger EFAULT. */
>         ret = __pagemap_scan_get_categories(fd, start, (struct page_region *) ~0UL);
> 
> Once we enter the kernel, we will check the arguments passed which includes the
> call to access_ok: 
> 
>   do_pagemap_cmd()
>     ->do_pagemap_scan()
>       ->pagemap_scan_get_args()
>         ->access_ok()
> 
> Here is the path within pagemap_scan_get_args where we expect to fail return
> the EFAULT:
> 
>         if (arg->vec && !access_ok((void __user *)(long)arg->vec,
>                                    size_mul(arg->vec_len, sizeof(struct page_region))))
>                 return -EFAULT;
>                 
> However, if CONFIG_ALTERNATE_USER_ADDRESS_SPACE is enabled or if CONFIG_MMU is
> NOT enabled, then we just return true:
> 
>         if (IS_ENABLED(CONFIG_ALTERNATE_USER_ADDRESS_SPACE) ||
>             !IS_ENABLED(CONFIG_MMU))
>                 return true;
> 
> The intent appears to be just getting the categories available to us and
> verifying that we have the feature available for testing. However, this corner
> case means the soft-dirty test will fail with the following:
> 

Thanks for the information, we should clarify that in the patch description.

>   # --------------------
>   # 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
>   1..1
>   
> Since the intent is just to validate that the features are available to us for
> testing, I think we can just modify the check so that we don't fail if we
> return 0.
>   
> Let me know what you think, or if you have more questions!

What about simply testing for success on a test area, wouldn't that be more reliable
and clearer?

diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
index a6d4ff7dfdc0..489a8d4d915d 100644
--- a/tools/testing/selftests/mm/vm_util.c
+++ b/tools/testing/selftests/mm/vm_util.c
@@ -67,21 +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 +95,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.43.0


> 
>> Do we have a Fixes: tag?
> 
> I always hesistate to add a Fixes tag on situations like this since this is a
> corner case that was not considered by the original author. If we need a
> fixes tag, then it would be:
> 
> Fixes: 600bca580579 ("selftests/mm: check that PAGEMAP_SCAN returns correct categories")

Yes, please add that. We nowadays also add proper Fixes tags for tests.

-- 
Cheers,

David

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

[Audra Mitchell]

> What about simply testing for success on a test area, wouldn't that be more reliable
> and clearer?

Hey I tested your version of this patch and it works fine on s390 and x86_64.
I have no issue with this version at all!

Would you prefer to push this fix since you wrote it or do you want me to push
a v2 and give you credit?

> > Fixes: 600bca580579 ("selftests/mm: check that PAGEMAP_SCAN returns correct categories")
> 
> Yes, please add that. We nowadays also add proper Fixes tags for tests.

Sure thing!

-- Audra

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

[David Hildenbrand (Arm)]

On 3/19/26 19:59, Audra Mitchell wrote:
>> What about simply testing for success on a test area, wouldn't that be more reliable
>> and clearer?
> 
> Hey I tested your version of this patch and it works fine on s390 and x86_64.
> I have no issue with this version at all!
> 
> Would you prefer to push this fix since you wrote it or do you want me to push
> a v2 and give you credit?

The credit is all yours :) Please polish the patch description to make
it clearer why the current check is problematic.

-- 
Cheers,

David

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

[Audra Mitchell]

Sending V2:

Changes are:
1. Cleared up commit message
2. Adjusted the patch to reflect David's recommendations

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

[Audra Mitchell]

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

[Andrew Morton]

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.

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

[David Hildenbrand (Arm)]

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

[Andrew Morton]

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

[Andrew Morton]

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

[Audra Mitchell]

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

[David Hildenbrand (Arm)]

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

[Lorenzo Stoakes (Oracle)]

On Wed, Mar 18, 2026 at 09:17:41AM +0100, David Hildenbrand (Arm) wrote:
> On 3/17/26 16:08, Audra Mitchell wrote:
> > Sorry! I missed this email so never responded!
> >
> > On Tue, Feb 24, 2026 at 05:15:14PM +0100, David Hildenbrand (Arm) wrote:
> >> On 2/18/26 19:42, Audra Mitchell wrote:
> >>> On architectures with separate user address space, such as s390 or
> >>> those without an MMU, the call to __access_ok will return true.
> >>
> >> Where is this __access_ok() you mention here? Somewhere in
> >> fs/proc/task_mmu.c?
> >>
> >> Where in the soft-dirty test is that triggered?
> >>
> >> I'm wondering whether the soft-dirty test should be adjusted, but I did
> >> not yet understand from where this behavior is triggered.
> >
> > The problem arises when we are checking to see what features/categories are
> > supported. The call chain for the soft-dirty program goes:
> >
> >   main()
> >     ->test_simple()
> >       ->pagemap_is_softdirty()
> >         ->page_entry_is()
> >           ->pagemap_scan_supported()
> >             ->__pagemap_scan_get_categories()
> >               ->ioctl()
> >
> > We enter the kernel with an ioctl, expecting to have an EFAULT returned (see
> > the comment from pagemap_scan_get_categories():
> >
> >           /* Provide an invalid address in order to trigger EFAULT. */
> >         ret = __pagemap_scan_get_categories(fd, start, (struct page_region *) ~0UL);
> >
> > Once we enter the kernel, we will check the arguments passed which includes the
> > call to access_ok:
> >
> >   do_pagemap_cmd()
> >     ->do_pagemap_scan()
> >       ->pagemap_scan_get_args()
> >         ->access_ok()
> >
> > Here is the path within pagemap_scan_get_args where we expect to fail return
> > the EFAULT:
> >
> >         if (arg->vec && !access_ok((void __user *)(long)arg->vec,
> >                                    size_mul(arg->vec_len, sizeof(struct page_region))))
> >                 return -EFAULT;
> >
> > However, if CONFIG_ALTERNATE_USER_ADDRESS_SPACE is enabled or if CONFIG_MMU is
> > NOT enabled, then we just return true:
> >
> >         if (IS_ENABLED(CONFIG_ALTERNATE_USER_ADDRESS_SPACE) ||
> >             !IS_ENABLED(CONFIG_MMU))
> >                 return true;
> >
> > The intent appears to be just getting the categories available to us and
> > verifying that we have the feature available for testing. However, this corner
> > case means the soft-dirty test will fail with the following:
> >
>
> Thanks for the information, we should clarify that in the patch description.
>
> >   # --------------------
> >   # 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
> >   1..1
> >
> > Since the intent is just to validate that the features are available to us for
> > testing, I think we can just modify the check so that we don't fail if we
> > return 0.
> >
> > Let me know what you think, or if you have more questions!
>
> What about simply testing for success on a test area, wouldn't that be more reliable
> and clearer?
>
> diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> index a6d4ff7dfdc0..489a8d4d915d 100644
> --- a/tools/testing/selftests/mm/vm_util.c
> +++ b/tools/testing/selftests/mm/vm_util.c
> @@ -67,21 +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 +95,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.43.0
>
>
> >
> >> Do we have a Fixes: tag?
> >
> > I always hesistate to add a Fixes tag on situations like this since this is a
> > corner case that was not considered by the original author. If we need a
> > fixes tag, then it would be:
> >
> > Fixes: 600bca580579 ("selftests/mm: check that PAGEMAP_SCAN returns correct categories")
>
> Yes, please add that. We nowadays also add proper Fixes tags for tests.
>
> --
> Cheers,
>
> David

Audra - to be clear this is discussion about mm process not your patch
specifically.

OK again I'm starting to think we just shouldn't support fix-patches at all any
more.

This is an example  of a change being done in a fix-patch that's _really_
causing issues.

Because this has now caused mayhem in mm-unstable and the 'kinda stable-ish'
branch now won't compile any self tests.

The fix in [0] on Chris Down's test series was for too many args to this
function (the patch changing this should have been rebased on mm-unstable and
changed Chris's caller there).

But now since this patch above ^ got yanked, that 'fix' has stayed in place and
now no mm self tests compile.

And now we see [1], hilariously.

[0]:https://lore.kernel.org/linux-mm/20260320195751.5b08b3e32ca835c3451d7bcd@linux-foundation.org/
[1]:https://lore.kernel.org/linux-mm/202603271115.uE9vpppi-lkp@intel.com/

This kind of massive levels of confusion and 'I am just trying to run some self
tests on what-should-be-for-next' is just not helpful...

I think we need a for-next branch that actually consists of stuff we genuinely
mean to take (i.e. review has settled) instead of 'literally everything because
we move stuff from mm-new unconditionally'.

Anyway we should revert the fix in [0] because it's broken now.

Cheers, Lorenzo

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

[Lorenzo Stoakes (Oracle)]

On Fri, Mar 27, 2026 at 10:52:24AM +0000, Lorenzo Stoakes (Oracle) wrote:
> Audra - to be clear this is discussion about mm process not your patch
> specifically.
>
> OK again I'm starting to think we just shouldn't support fix-patches at all any
> more.
>
> This is an example  of a change being done in a fix-patch that's _really_
> causing issues.
>
> Because this has now caused mayhem in mm-unstable and the 'kinda stable-ish'
> branch now won't compile any self tests.
>
> The fix in [0] on Chris Down's test series was for too many args to this
> function (the patch changing this should have been rebased on mm-unstable and
> changed Chris's caller there).
>
> But now since this patch above ^ got yanked, that 'fix' has stayed in place and
> now no mm self tests compile.
>
> And now we see [1], hilariously.
>
> [0]:https://lore.kernel.org/linux-mm/20260320195751.5b08b3e32ca835c3451d7bcd@linux-foundation.org/
> [1]:https://lore.kernel.org/linux-mm/202603271115.uE9vpppi-lkp@intel.com/
>
> This kind of massive levels of confusion and 'I am just trying to run some self
> tests on what-should-be-for-next' is just not helpful...
>
> I think we need a for-next branch that actually consists of stuff we genuinely
> mean to take (i.e. review has settled) instead of 'literally everything because
> we move stuff from mm-new unconditionally'.
>
> Anyway we should revert the fix in [0] because it's broken now.
>
> Cheers, Lorenzo

BTW, even _at_ the fix-patch commit hash (28edd9c5a25c) the test builds fail.

So I wonder if it might be good to at least do build checks of the kernel and
also tests (tools/testing/selftests/mm and tools/testing/vma) before applying a
patch?

As that would have caught this right away. Breaking the build should be
considered a no-no.

Cheers, Lorenzo

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

[Lorenzo Stoakes (Oracle)]

On Fri, Mar 27, 2026 at 10:58:30AM +0000, Lorenzo Stoakes (Oracle) wrote:
> BTW, even _at_ the fix-patch commit hash (28edd9c5a25c) the test builds fail.

Ugh sorry wrong commit :) should be 9652b82ca9632.

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

[Lorenzo Stoakes (Oracle)]

On Fri, Mar 27, 2026 at 10:52:24AM +0000, Lorenzo Stoakes (Oracle) wrote:
> OK again I'm starting to think we just shouldn't support fix-patches at all any
> more.
>
> This is an example  of a change being done in a fix-patch that's _really_
> causing issues.

I didn't see there was a v2 in-reply-to here sorry.

So in this case less fix patch more:

1. We shouldn't automatically take stuff to mm-unstable until it's settled

2. We should have clear separate branches for stuff-for-7.1 and stuff-for-7.2

3. We should build test stuff (!)

4. We should root cause the source of problems and update the thing that cause.

Part of the problem here is that mm-unstable is a totally unreliable base. Not
as bad as mm-new, but it's forgiveable that Audra didn't see Chris Down's patch
there because who knows when that went in etc.

if we had a for-7.1 git branch that only got stuff that had settled we'd not
have this issue.

And we could have a for-7.2 git branch for stuff that's arrived late.

We could have:

mm-untested (was mm-new)
mm-under-review (was mm-unstable) -> THIS doesn't go to -next
mm-for-7.1 (was mm-stable)        -> THIS goes to -next
mm-for-7.2 (fresh and new!)       -> THIS only goes to -next next cycle

And workflow could be:

-> patch comes in
go straight to mm-untested - we are undiscerning
[ make sure series gets build tested + self tests + etc ]
goes to mm-under-review
WEEKLY:
Stuff that has sub-M signoff and seen no further issues -> for-7.1 (i.e. next release)
if it's <= rc4 and no other reasons to hold off

OR for stuff that doesn't fit the rc4/no other reason criteria
-> for-7.2 if signoff and no issues

If things get reviewed and respun

-> back to mm-untested until tests pass no matter tags
-> (if stable) can jump right to for-7.1/for-7.2 if signed off already. if not then mm-under-review

This way we reduce chance of late yank, give reasonable rebase target of
for-[release you are interested in], documents and simplifies the process.

Thoughts?...

Cheers, Lorenzo