[PATCH 0/4] Various mm kselftests improvements/fixes

[PATCH 0/4] Various mm kselftests improvements/fixes

[Kevin Brodsky]

A few improvements/fixes for the mm kselftests:

- Patch 1-2 extend support for more build configurations: out-of-tree
  $KDIR, cross-compilation, etc.

- Patch 3-4 fix issues in the pagemap_ioctl tests, most importantly that
  it does not report failures: ./run_kselftests.sh would report OK
  even if some pagemap_ioctl tests fail. That's probably why the issue
  in patch 3 went unnoticed.

---
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@kernel.org>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Shuah Khan <shuah@kernel.org>
---
Kevin Brodsky (4):
  selftests/mm: remove flaky header check
  selftests/mm: pass down full CC and CFLAGS to check_config.sh
  selftests/mm: fix faulting-in code in pagemap_ioctl test
  selftests/mm: fix exit code in pagemap_ioctl

 tools/testing/selftests/mm/Makefile        |  6 +-----
 tools/testing/selftests/mm/check_config.sh |  3 +--
 tools/testing/selftests/mm/pagemap_ioctl.c | 12 ++++++------
 3 files changed, 8 insertions(+), 13 deletions(-)


base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
-- 
2.51.2

[PATCH 1/4] selftests/mm: remove flaky header check

[Kevin Brodsky]

Commit 96ed62ea0298 ("mm: page_frag: fix a compile error when kernel
is not compiled") introduced a check to avoid attempting to build
the page_frag module if <linux/page_frag_cache.h> is missing.

Unfortunately this check only works if KDIR points to
/lib/modules/... or an in-tree kernel build. It always fails if KDIR
points to an out-of-tree build (i.e. when the kernel was built with
O=$KDIR make) because only generated headers are present under
$KDIR/include/ in that case.

<linux/page_frag_cache.h> was added more than a year ago (v6.13) so
we can probably live without that check.

Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Yunsheng Lin <linyunsheng@huawei.com>
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
 tools/testing/selftests/mm/Makefile | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index eaf9312097f7..aba51fcac752 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -46,12 +46,8 @@ CFLAGS += -U_FORTIFY_SOURCE
 
 KDIR ?= /lib/modules/$(shell uname -r)/build
 ifneq (,$(wildcard $(KDIR)/Module.symvers))
-ifneq (,$(wildcard $(KDIR)/include/linux/page_frag_cache.h))
 TEST_GEN_MODS_DIR := page_frag
 else
-PAGE_FRAG_WARNING = "missing page_frag_cache.h, please use a newer kernel"
-endif
-else
 PAGE_FRAG_WARNING = "missing Module.symvers, please have the kernel built first"
 endif
 
-- 
2.51.2

Re: [PATCH 1/4] selftests/mm: remove flaky header check

[Yunsheng Lin]

On 2025/12/16 22:26, Kevin Brodsky wrote:
> Commit 96ed62ea0298 ("mm: page_frag: fix a compile error when kernel
> is not compiled") introduced a check to avoid attempting to build
> the page_frag module if <linux/page_frag_cache.h> is missing.
> 
> Unfortunately this check only works if KDIR points to
> /lib/modules/... or an in-tree kernel build. It always fails if KDIR
> points to an out-of-tree build (i.e. when the kernel was built with
> O=$KDIR make) because only generated headers are present under
> $KDIR/include/ in that case.
> 
> <linux/page_frag_cache.h> was added more than a year ago (v6.13) so
> we can probably live without that check.

As some commercial OS still uses v6.6, I am wondering if we need that
check for a little longer, is it possible to do something like below to
avoid the flaky check?

@@ -46,7 +46,8 @@ CFLAGS += -U_FORTIFY_SOURCE

 KDIR ?= /lib/modules/$(shell uname -r)/build
 ifneq (,$(wildcard $(KDIR)/Module.symvers))
-ifneq (,$(wildcard $(KDIR)/include/linux/page_frag_cache.h))
+KSRC := $(shell readlink -f $(KDIR)/source 2>/dev/null || echo $(KDIR))
+ifneq (,$(wildcard $(KSRC)/include/linux/page_frag_cache.h))
 TEST_GEN_MODS_DIR := page_frag
 else
 PAGE_FRAG_WARNING = "missing page_frag_cache.h, please use a newer kernel"

> 
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Yunsheng Lin <linyunsheng@huawei.com>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>  tools/testing/selftests/mm/Makefile | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index eaf9312097f7..aba51fcac752 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -46,12 +46,8 @@ CFLAGS += -U_FORTIFY_SOURCE
>  
>  KDIR ?= /lib/modules/$(shell uname -r)/build
>  ifneq (,$(wildcard $(KDIR)/Module.symvers))
> -ifneq (,$(wildcard $(KDIR)/include/linux/page_frag_cache.h))
>  TEST_GEN_MODS_DIR := page_frag
>  else
> -PAGE_FRAG_WARNING = "missing page_frag_cache.h, please use a newer kernel"
> -endif
> -else
>  PAGE_FRAG_WARNING = "missing Module.symvers, please have the kernel built first"
>  endif
>  

Re: [PATCH 1/4] selftests/mm: remove flaky header check

[Kevin Brodsky]

On 17/12/2025 04:18, Yunsheng Lin wrote:
> On 2025/12/16 22:26, Kevin Brodsky wrote:
>> Commit 96ed62ea0298 ("mm: page_frag: fix a compile error when kernel
>> is not compiled") introduced a check to avoid attempting to build
>> the page_frag module if <linux/page_frag_cache.h> is missing.
>>
>> Unfortunately this check only works if KDIR points to
>> /lib/modules/... or an in-tree kernel build. It always fails if KDIR
>> points to an out-of-tree build (i.e. when the kernel was built with
>> O=$KDIR make) because only generated headers are present under
>> $KDIR/include/ in that case.
>>
>> <linux/page_frag_cache.h> was added more than a year ago (v6.13) so
>> we can probably live without that check.
> As some commercial OS still uses v6.6, I am wondering if we need that

Fair point, I hadn't considered that kselftests are supposed to be
buildable against older stable kernels.

> check for a little longer, is it possible to do something like below to
> avoid the flaky check?
>
> @@ -46,7 +46,8 @@ CFLAGS += -U_FORTIFY_SOURCE
>
>  KDIR ?= /lib/modules/$(shell uname -r)/build
>  ifneq (,$(wildcard $(KDIR)/Module.symvers))
> -ifneq (,$(wildcard $(KDIR)/include/linux/page_frag_cache.h))
> +KSRC := $(shell readlink -f $(KDIR)/source 2>/dev/null || echo $(KDIR))
> +ifneq (,$(wildcard $(KSRC)/include/linux/page_frag_cache.h))
>  TEST_GEN_MODS_DIR := page_frag
>  else
>  PAGE_FRAG_WARNING = "missing page_frag_cache.h, please use a newer kernel"

That seems reasonable, and it works for my out-of-tree setup.

Will do that in v2, shall I add your Suggested-by, or maybe Co-developed-by?

- Kevin

Re: [PATCH 1/4] selftests/mm: remove flaky header check

[Yunsheng Lin]

On 2025/12/17 17:58, Kevin Brodsky wrote:
> On 17/12/2025 04:18, Yunsheng Lin wrote:
>> On 2025/12/16 22:26, Kevin Brodsky wrote:
>>> Commit 96ed62ea0298 ("mm: page_frag: fix a compile error when kernel
>>> is not compiled") introduced a check to avoid attempting to build
>>> the page_frag module if <linux/page_frag_cache.h> is missing.
>>>
>>> Unfortunately this check only works if KDIR points to
>>> /lib/modules/... or an in-tree kernel build. It always fails if KDIR
>>> points to an out-of-tree build (i.e. when the kernel was built with
>>> O=$KDIR make) because only generated headers are present under
>>> $KDIR/include/ in that case.
>>>
>>> <linux/page_frag_cache.h> was added more than a year ago (v6.13) so
>>> we can probably live without that check.
>> As some commercial OS still uses v6.6, I am wondering if we need that
> 
> Fair point, I hadn't considered that kselftests are supposed to be
> buildable against older stable kernels.
> 
>> check for a little longer, is it possible to do something like below to
>> avoid the flaky check?
>>
>> @@ -46,7 +46,8 @@ CFLAGS += -U_FORTIFY_SOURCE
>>
>>  KDIR ?= /lib/modules/$(shell uname -r)/build
>>  ifneq (,$(wildcard $(KDIR)/Module.symvers))
>> -ifneq (,$(wildcard $(KDIR)/include/linux/page_frag_cache.h))
>> +KSRC := $(shell readlink -f $(KDIR)/source 2>/dev/null || echo $(KDIR))
>> +ifneq (,$(wildcard $(KSRC)/include/linux/page_frag_cache.h))
>>  TEST_GEN_MODS_DIR := page_frag
>>  else
>>  PAGE_FRAG_WARNING = "missing page_frag_cache.h, please use a newer kernel"
> 
> That seems reasonable, and it works for my out-of-tree setup.
> 
> Will do that in v2, shall I add your Suggested-by, or maybe Co-developed-by?

Yes if you want to go that direction.

Re: [PATCH 1/4] selftests/mm: remove flaky header check

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 864 bytes --]

On Tue, Dec 16, 2025 at 02:26:30PM +0000, Kevin Brodsky wrote:
> Commit 96ed62ea0298 ("mm: page_frag: fix a compile error when kernel
> is not compiled") introduced a check to avoid attempting to build
> the page_frag module if <linux/page_frag_cache.h> is missing.
> 
> Unfortunately this check only works if KDIR points to
> /lib/modules/... or an in-tree kernel build. It always fails if KDIR
> points to an out-of-tree build (i.e. when the kernel was built with
> O=$KDIR make) because only generated headers are present under
> $KDIR/include/ in that case.
> 
> <linux/page_frag_cache.h> was added more than a year ago (v6.13) so
> we can probably live without that check.

More generally building selftests with random older kernel versions
isn't really something that's expected to be robust:

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/4] selftests/mm: remove flaky header check

[Kevin Brodsky]

On 17/12/2025 11:04, Mark Brown wrote:
> On Tue, Dec 16, 2025 at 02:26:30PM +0000, Kevin Brodsky wrote:
>> Commit 96ed62ea0298 ("mm: page_frag: fix a compile error when kernel
>> is not compiled") introduced a check to avoid attempting to build
>> the page_frag module if <linux/page_frag_cache.h> is missing.
>>
>> Unfortunately this check only works if KDIR points to
>> /lib/modules/... or an in-tree kernel build. It always fails if KDIR
>> points to an out-of-tree build (i.e. when the kernel was built with
>> O=$KDIR make) because only generated headers are present under
>> $KDIR/include/ in that case.
>>
>> <linux/page_frag_cache.h> was added more than a year ago (v6.13) so
>> we can probably live without that check.
> More generally building selftests with random older kernel versions
> isn't really something that's expected to be robust:

I suppose that Documentation/dev-tools/kselftest.rst talks about
*running* against older kernels, not *building* against them. That said,
we are dealing with an out-of-tree kernel module here, so the two are
essentially the same... Yunsheng suggested an updated check that I think
is reasonable, maybe it is a reasonable compromise?

- Kevin

Re: [PATCH 1/4] selftests/mm: remove flaky header check

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

On Thu, Dec 18, 2025 at 02:24:10PM +0100, Kevin Brodsky wrote:
> On 17/12/2025 11:04, Mark Brown wrote:

> > More generally building selftests with random older kernel versions
> > isn't really something that's expected to be robust:

> I suppose that Documentation/dev-tools/kselftest.rst talks about
> *running* against older kernels, not *building* against them. That said,

Yeah, running is fairly normal but huge swathes of the selftests won't
build without current kernel headers and it's not an especially useful
use of time to support that.

> we are dealing with an out-of-tree kernel module here, so the two are
> essentially the same... Yunsheng suggested an updated check that I think
> is reasonable, maybe it is a reasonable compromise?

Well, there's also the selection of KDIR which for some reason defaults
to the installed kernel so we get:

  $ make -C tools/testing/selftests LLVM=1 ARCH=arm64 TARGETS=mm

  Warning: missing page_frag_cache.h, please use a newer kernel. page_frag test will be skipped.

Your changelog says it'll work for an in tree build but I can't figure
out how to do that (using the top level Makefile to recurse doesn't seem
to DTRT either).  Having looked at this more I think the problem here is
that the selection of KDIR is wrong, not the check.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/4] selftests/mm: remove flaky header check

[Kevin Brodsky]

On 18/12/2025 15:25, Mark Brown wrote:
> On Thu, Dec 18, 2025 at 02:24:10PM +0100, Kevin Brodsky wrote:
>> On 17/12/2025 11:04, Mark Brown wrote:
>>> More generally building selftests with random older kernel versions
>>> isn't really something that's expected to be robust:
>> I suppose that Documentation/dev-tools/kselftest.rst talks about
>> *running* against older kernels, not *building* against them. That said,
> Yeah, running is fairly normal but huge swathes of the selftests won't
> build without current kernel headers and it's not an especially useful
> use of time to support that.
>
>> we are dealing with an out-of-tree kernel module here, so the two are
>> essentially the same... Yunsheng suggested an updated check that I think
>> is reasonable, maybe it is a reasonable compromise?
> Well, there's also the selection of KDIR which for some reason defaults
> to the installed kernel so we get:

Overall the kselftests tend to assume that we're building on the same
machine we'll run them, so at least that feels consistent. The same
default is used for most other out-of-tree kselftests modules
(livepatch, net/bench).

>   $ make -C tools/testing/selftests LLVM=1 ARCH=arm64 TARGETS=mm
>
>   Warning: missing page_frag_cache.h, please use a newer kernel. page_frag test will be skipped.

But yes if cross-compiling the default makes no sense and KDIR has to be
set explicitly.

> Your changelog says it'll work for an in tree build but I can't figure
> out how to do that (using the top level Makefile to recurse doesn't seem
> to DTRT either).  Having looked at this more I think the problem here is
> that the selection of KDIR is wrong, not the check.

I use KBUILD_OUTPUT=out and KDIR needs to be absolute, so:
KDIR=$PWD/out. I suppose for an in-tree build KDIR=$PWD would do the
right thing. But yes it's all very wonky.

Maybe the documentation should be updated to recommend setting KDIR
explicitly? Or maybe it could default to KDIR=$PWD or $(abspath
$(KBUILD_OUTPUT)) when cross-compiling?

- Kevin

[PATCH 2/4] selftests/mm: pass down full CC and CFLAGS to check_config.sh

[Kevin Brodsky]

check_config.sh checks that liburing is available by running the
compiler provided as its first argument. This makes two assumptions:

1. CC consists of only one word
2. No extra flag is required

Unfortunately, there are many situations where these assumptions
don't hold. For instance:

- When using Clang, CC consists of multiple words
- When cross-compiling, extra flags may be required to allow the
  compiler to find headers

Remove these assumptions by passing down CC and CFLAGS as-is from
the Makefile, so that the same command line is used as when actually
building the tests.

Cc: Jason Gunthorpe <jgg@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
 tools/testing/selftests/mm/Makefile        | 2 +-
 tools/testing/selftests/mm/check_config.sh | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index aba51fcac752..b1c949cd7c3d 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -230,7 +230,7 @@ $(OUTPUT)/migration: LDLIBS += -lnuma
 $(OUTPUT)/rmap: LDLIBS += -lnuma
 
 local_config.mk local_config.h: check_config.sh
-	/bin/sh ./check_config.sh $(CC)
+	CC="$(CC)" CFLAGS="$(CFLAGS)" ./check_config.sh
 
 EXTRA_CLEAN += local_config.mk local_config.h
 
diff --git a/tools/testing/selftests/mm/check_config.sh b/tools/testing/selftests/mm/check_config.sh
index 3954f4746161..b84c82bbf875 100755
--- a/tools/testing/selftests/mm/check_config.sh
+++ b/tools/testing/selftests/mm/check_config.sh
@@ -16,8 +16,7 @@ echo "#include <sys/types.h>"        > $tmpfile_c
 echo "#include <liburing.h>"        >> $tmpfile_c
 echo "int func(void) { return 0; }" >> $tmpfile_c
 
-CC=${1:?"Usage: $0 <compiler> # example compiler: gcc"}
-$CC -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1
+$CC $CFLAGS -c $tmpfile_c -o $tmpfile_o
 
 if [ -f $tmpfile_o ]; then
     echo "#define LOCAL_CONFIG_HAVE_LIBURING 1"  > $OUTPUT_H_FILE
-- 
2.51.2

Re: [PATCH 2/4] selftests/mm: pass down full CC and CFLAGS to check_config.sh

[David Hildenbrand (Red Hat)]

On 12/16/25 15:26, Kevin Brodsky wrote:
> check_config.sh checks that liburing is available by running the
> compiler provided as its first argument. This makes two assumptions:
> 
> 1. CC consists of only one word
> 2. No extra flag is required
> 
> Unfortunately, there are many situations where these assumptions
> don't hold. For instance:
> 
> - When using Clang, CC consists of multiple words
> - When cross-compiling, extra flags may be required to allow the
>    compiler to find headers
> 
> Remove these assumptions by passing down CC and CFLAGS as-is from
> the Makefile, so that the same command line is used as when actually
> building the tests.
> 
> Cc: Jason Gunthorpe <jgg@nvidia.com>
> Cc: John Hubbard <jhubbard@nvidia.com>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---

Looks reasonable to me and I hope we find no surpirses :)

Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

-- 
Cheers

David

[PATCH 3/4] selftests/mm: fix faulting-in code in pagemap_ioctl test

[Kevin Brodsky]

One of the pagemap_ioctl tests attempts to fault in pages by
memcpy()'ing them to an unused buffer. This probably worked
originally, but since commit 46036188ea1f ("selftests/mm: build with
-O2") the compiler is free to optimise away that unused buffer and
the memcpy() with it. As a result there might not be any resident
page in the mapping and the test may fail.

We don't need to copy all that memory anyway. Just fault in every
page by forcing the compiler to read the first byte.

Cc: Usama Anjum <Usama.Anjum@arm.com>
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
 tools/testing/selftests/mm/pagemap_ioctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
index 2cb5441f29c7..67a7a3705604 100644
--- a/tools/testing/selftests/mm/pagemap_ioctl.c
+++ b/tools/testing/selftests/mm/pagemap_ioctl.c
@@ -1056,7 +1056,6 @@ int sanity_tests(void)
 	struct page_region *vec;
 	char *mem, *fmem;
 	struct stat sbuf;
-	char *tmp_buf;
 
 	/* 1. wrong operation */
 	mem_size = 10 * page_size;
@@ -1167,8 +1166,9 @@ int sanity_tests(void)
 	if (fmem == MAP_FAILED)
 		ksft_exit_fail_msg("error nomem %d %s\n", errno, strerror(errno));
 
-	tmp_buf = malloc(sbuf.st_size);
-	memcpy(tmp_buf, fmem, sbuf.st_size);
+	/* Fault in every page by reading the first byte */
+	for (i = 0; i < sbuf.st_size; i += page_size)
+		(void)*(volatile char *)(fmem + i);
 
 	ret = pagemap_ioctl(fmem, sbuf.st_size, vec, vec_size, 0, 0,
 			    0, PAGEMAP_NON_WRITTEN_BITS, 0, PAGEMAP_NON_WRITTEN_BITS);
-- 
2.51.2

Re: [PATCH 3/4] selftests/mm: fix faulting-in code in pagemap_ioctl test

[Ryan Roberts]

On 16/12/2025 14:26, Kevin Brodsky wrote:
> One of the pagemap_ioctl tests attempts to fault in pages by
> memcpy()'ing them to an unused buffer. This probably worked
> originally, but since commit 46036188ea1f ("selftests/mm: build with
> -O2") the compiler is free to optimise away that unused buffer and
> the memcpy() with it. As a result there might not be any resident
> page in the mapping and the test may fail.
> 
> We don't need to copy all that memory anyway. Just fault in every
> page by forcing the compiler to read the first byte.
> 
> Cc: Usama Anjum <Usama.Anjum@arm.com>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>  tools/testing/selftests/mm/pagemap_ioctl.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
> index 2cb5441f29c7..67a7a3705604 100644
> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
> @@ -1056,7 +1056,6 @@ int sanity_tests(void)
>  	struct page_region *vec;
>  	char *mem, *fmem;
>  	struct stat sbuf;
> -	char *tmp_buf;
>  
>  	/* 1. wrong operation */
>  	mem_size = 10 * page_size;
> @@ -1167,8 +1166,9 @@ int sanity_tests(void)
>  	if (fmem == MAP_FAILED)
>  		ksft_exit_fail_msg("error nomem %d %s\n", errno, strerror(errno));
>  
> -	tmp_buf = malloc(sbuf.st_size);
> -	memcpy(tmp_buf, fmem, sbuf.st_size);
> +	/* Fault in every page by reading the first byte */
> +	for (i = 0; i < sbuf.st_size; i += page_size)
> +		(void)*(volatile char *)(fmem + i);

We have FORCE_READ() in vm_util.h for this. Perhaps that would be better?

>  
>  	ret = pagemap_ioctl(fmem, sbuf.st_size, vec, vec_size, 0, 0,
>  			    0, PAGEMAP_NON_WRITTEN_BITS, 0, PAGEMAP_NON_WRITTEN_BITS);

Re: [PATCH 3/4] selftests/mm: fix faulting-in code in pagemap_ioctl test

[Kevin Brodsky]

On 16/12/2025 15:56, Ryan Roberts wrote:
> On 16/12/2025 14:26, Kevin Brodsky wrote:
>> One of the pagemap_ioctl tests attempts to fault in pages by
>> memcpy()'ing them to an unused buffer. This probably worked
>> originally, but since commit 46036188ea1f ("selftests/mm: build with
>> -O2") the compiler is free to optimise away that unused buffer and
>> the memcpy() with it. As a result there might not be any resident
>> page in the mapping and the test may fail.
>>
>> We don't need to copy all that memory anyway. Just fault in every
>> page by forcing the compiler to read the first byte.
>>
>> Cc: Usama Anjum <Usama.Anjum@arm.com>
>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> ---
>>  tools/testing/selftests/mm/pagemap_ioctl.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
>> index 2cb5441f29c7..67a7a3705604 100644
>> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
>> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
>> @@ -1056,7 +1056,6 @@ int sanity_tests(void)
>>  	struct page_region *vec;
>>  	char *mem, *fmem;
>>  	struct stat sbuf;
>> -	char *tmp_buf;
>>  
>>  	/* 1. wrong operation */
>>  	mem_size = 10 * page_size;
>> @@ -1167,8 +1166,9 @@ int sanity_tests(void)
>>  	if (fmem == MAP_FAILED)
>>  		ksft_exit_fail_msg("error nomem %d %s\n", errno, strerror(errno));
>>  
>> -	tmp_buf = malloc(sbuf.st_size);
>> -	memcpy(tmp_buf, fmem, sbuf.st_size);
>> +	/* Fault in every page by reading the first byte */
>> +	for (i = 0; i < sbuf.st_size; i += page_size)
>> +		(void)*(volatile char *)(fmem + i);
> We have FORCE_READ() in vm_util.h for this. Perhaps that would be better?

It would, thanks! I wanted to use READ_ONCE() from
tools/include/linux/compiler.h but for some reason we don't add
tools/include to the include path in the mm kselftests Makefile and I
didn't want to dive into that rabbit hole.

- Kevin

Re: [PATCH 3/4] selftests/mm: fix faulting-in code in pagemap_ioctl test

[David Hildenbrand (Red Hat)]

On 12/16/25 15:56, Ryan Roberts wrote:
> On 16/12/2025 14:26, Kevin Brodsky wrote:
>> One of the pagemap_ioctl tests attempts to fault in pages by
>> memcpy()'ing them to an unused buffer. This probably worked
>> originally, but since commit 46036188ea1f ("selftests/mm: build with
>> -O2") the compiler is free to optimise away that unused buffer and
>> the memcpy() with it. As a result there might not be any resident
>> page in the mapping and the test may fail.
>>
>> We don't need to copy all that memory anyway. Just fault in every
>> page by forcing the compiler to read the first byte.
>>
>> Cc: Usama Anjum <Usama.Anjum@arm.com>
>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> ---
>>   tools/testing/selftests/mm/pagemap_ioctl.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
>> index 2cb5441f29c7..67a7a3705604 100644
>> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
>> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
>> @@ -1056,7 +1056,6 @@ int sanity_tests(void)
>>   	struct page_region *vec;
>>   	char *mem, *fmem;
>>   	struct stat sbuf;
>> -	char *tmp_buf;
>>   
>>   	/* 1. wrong operation */
>>   	mem_size = 10 * page_size;
>> @@ -1167,8 +1166,9 @@ int sanity_tests(void)
>>   	if (fmem == MAP_FAILED)
>>   		ksft_exit_fail_msg("error nomem %d %s\n", errno, strerror(errno));
>>   
>> -	tmp_buf = malloc(sbuf.st_size);
>> -	memcpy(tmp_buf, fmem, sbuf.st_size);
>> +	/* Fault in every page by reading the first byte */
>> +	for (i = 0; i < sbuf.st_size; i += page_size)
>> +		(void)*(volatile char *)(fmem + i);
> 
> We have FORCE_READ() in vm_util.h for this. Perhaps that would be better?

Agreed, and if we have multiple patterns where we want to force_read a 
bigger area, maybe we should provide a helper for that?

-- 
Cheers

David

Re: [PATCH 3/4] selftests/mm: fix faulting-in code in pagemap_ioctl test

[Kevin Brodsky]

On 18/12/2025 09:05, David Hildenbrand (Red Hat) wrote:
> On 12/16/25 15:56, Ryan Roberts wrote:
>> On 16/12/2025 14:26, Kevin Brodsky wrote:
>>> One of the pagemap_ioctl tests attempts to fault in pages by
>>> memcpy()'ing them to an unused buffer. This probably worked
>>> originally, but since commit 46036188ea1f ("selftests/mm: build with
>>> -O2") the compiler is free to optimise away that unused buffer and
>>> the memcpy() with it. As a result there might not be any resident
>>> page in the mapping and the test may fail.
>>>
>>> We don't need to copy all that memory anyway. Just fault in every
>>> page by forcing the compiler to read the first byte.
>>>
>>> Cc: Usama Anjum <Usama.Anjum@arm.com>
>>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>>> ---
>>>   tools/testing/selftests/mm/pagemap_ioctl.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c
>>> b/tools/testing/selftests/mm/pagemap_ioctl.c
>>> index 2cb5441f29c7..67a7a3705604 100644
>>> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
>>> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
>>> @@ -1056,7 +1056,6 @@ int sanity_tests(void)
>>>       struct page_region *vec;
>>>       char *mem, *fmem;
>>>       struct stat sbuf;
>>> -    char *tmp_buf;
>>>         /* 1. wrong operation */
>>>       mem_size = 10 * page_size;
>>> @@ -1167,8 +1166,9 @@ int sanity_tests(void)
>>>       if (fmem == MAP_FAILED)
>>>           ksft_exit_fail_msg("error nomem %d %s\n", errno,
>>> strerror(errno));
>>>   -    tmp_buf = malloc(sbuf.st_size);
>>> -    memcpy(tmp_buf, fmem, sbuf.st_size);
>>> +    /* Fault in every page by reading the first byte */
>>> +    for (i = 0; i < sbuf.st_size; i += page_size)
>>> +        (void)*(volatile char *)(fmem + i);
>>
>> We have FORCE_READ() in vm_util.h for this. Perhaps that would be
>> better?
>
> Agreed, and if we have multiple patterns where we want to force_read a
> bigger area, maybe we should provide a helper for that?

I've found just a couple of cases where FORCE_READ() is used for a
larger area (in hugetlb-madvise.c and split_huge_page_test.c). The step
size isn't the same in any of these cases though. We could have
something like fault_area(addr, size, step) but maybe the loops are
clear enough already?

- Kevin

Re: [PATCH 3/4] selftests/mm: fix faulting-in code in pagemap_ioctl test

[David Hildenbrand (Red Hat)]

On 12/18/25 14:18, Kevin Brodsky wrote:
> On 18/12/2025 09:05, David Hildenbrand (Red Hat) wrote:
>> On 12/16/25 15:56, Ryan Roberts wrote:
>>> On 16/12/2025 14:26, Kevin Brodsky wrote:
>>>> One of the pagemap_ioctl tests attempts to fault in pages by
>>>> memcpy()'ing them to an unused buffer. This probably worked
>>>> originally, but since commit 46036188ea1f ("selftests/mm: build with
>>>> -O2") the compiler is free to optimise away that unused buffer and
>>>> the memcpy() with it. As a result there might not be any resident
>>>> page in the mapping and the test may fail.
>>>>
>>>> We don't need to copy all that memory anyway. Just fault in every
>>>> page by forcing the compiler to read the first byte.
>>>>
>>>> Cc: Usama Anjum <Usama.Anjum@arm.com>
>>>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>>>> ---
>>>>    tools/testing/selftests/mm/pagemap_ioctl.c | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c
>>>> b/tools/testing/selftests/mm/pagemap_ioctl.c
>>>> index 2cb5441f29c7..67a7a3705604 100644
>>>> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
>>>> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
>>>> @@ -1056,7 +1056,6 @@ int sanity_tests(void)
>>>>        struct page_region *vec;
>>>>        char *mem, *fmem;
>>>>        struct stat sbuf;
>>>> -    char *tmp_buf;
>>>>          /* 1. wrong operation */
>>>>        mem_size = 10 * page_size;
>>>> @@ -1167,8 +1166,9 @@ int sanity_tests(void)
>>>>        if (fmem == MAP_FAILED)
>>>>            ksft_exit_fail_msg("error nomem %d %s\n", errno,
>>>> strerror(errno));
>>>>    -    tmp_buf = malloc(sbuf.st_size);
>>>> -    memcpy(tmp_buf, fmem, sbuf.st_size);
>>>> +    /* Fault in every page by reading the first byte */
>>>> +    for (i = 0; i < sbuf.st_size; i += page_size)
>>>> +        (void)*(volatile char *)(fmem + i);
>>>
>>> We have FORCE_READ() in vm_util.h for this. Perhaps that would be
>>> better?
>>
>> Agreed, and if we have multiple patterns where we want to force_read a
>> bigger area, maybe we should provide a helper for that?
> 
> I've found just a couple of cases where FORCE_READ() is used for a
> larger area (in hugetlb-madvise.c and split_huge_page_test.c). The step
> size isn't the same in any of these cases though. We could have
> something like fault_area(addr, size, step) but maybe the loops are
> clear enough already?

Note that even for hugtlb we can read page-per-page, no need to 
hugetlb-page-per-hugetlb-page. Not sure if the performance change would 
make any real performance difference in this testing code.

-- 
Cheers

David

Re: [PATCH 3/4] selftests/mm: fix faulting-in code in pagemap_ioctl test

[Kevin Brodsky]

On 19/12/2025 09:29, David Hildenbrand (Red Hat) wrote:
> On 12/18/25 14:18, Kevin Brodsky wrote:
>> On 18/12/2025 09:05, David Hildenbrand (Red Hat) wrote:
>>> On 12/16/25 15:56, Ryan Roberts wrote:
>>>> On 16/12/2025 14:26, Kevin Brodsky wrote:
>>>>> One of the pagemap_ioctl tests attempts to fault in pages by
>>>>> memcpy()'ing them to an unused buffer. This probably worked
>>>>> originally, but since commit 46036188ea1f ("selftests/mm: build with
>>>>> -O2") the compiler is free to optimise away that unused buffer and
>>>>> the memcpy() with it. As a result there might not be any resident
>>>>> page in the mapping and the test may fail.
>>>>>
>>>>> We don't need to copy all that memory anyway. Just fault in every
>>>>> page by forcing the compiler to read the first byte.
>>>>>
>>>>> Cc: Usama Anjum <Usama.Anjum@arm.com>
>>>>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>>>>> ---
>>>>>    tools/testing/selftests/mm/pagemap_ioctl.c | 6 +++---
>>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c
>>>>> b/tools/testing/selftests/mm/pagemap_ioctl.c
>>>>> index 2cb5441f29c7..67a7a3705604 100644
>>>>> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
>>>>> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
>>>>> @@ -1056,7 +1056,6 @@ int sanity_tests(void)
>>>>>        struct page_region *vec;
>>>>>        char *mem, *fmem;
>>>>>        struct stat sbuf;
>>>>> -    char *tmp_buf;
>>>>>          /* 1. wrong operation */
>>>>>        mem_size = 10 * page_size;
>>>>> @@ -1167,8 +1166,9 @@ int sanity_tests(void)
>>>>>        if (fmem == MAP_FAILED)
>>>>>            ksft_exit_fail_msg("error nomem %d %s\n", errno,
>>>>> strerror(errno));
>>>>>    -    tmp_buf = malloc(sbuf.st_size);
>>>>> -    memcpy(tmp_buf, fmem, sbuf.st_size);
>>>>> +    /* Fault in every page by reading the first byte */
>>>>> +    for (i = 0; i < sbuf.st_size; i += page_size)
>>>>> +        (void)*(volatile char *)(fmem + i);
>>>>
>>>> We have FORCE_READ() in vm_util.h for this. Perhaps that would be
>>>> better?
>>>
>>> Agreed, and if we have multiple patterns where we want to force_read a
>>> bigger area, maybe we should provide a helper for that?
>>
>> I've found just a couple of cases where FORCE_READ() is used for a
>> larger area (in hugetlb-madvise.c and split_huge_page_test.c). The step
>> size isn't the same in any of these cases though. We could have
>> something like fault_area(addr, size, step) but maybe the loops are
>> clear enough already?
>
> Note that even for hugtlb we can read page-per-page, no need to
> hugetlb-page-per-hugetlb-page. Not sure if the performance change
> would make any real performance difference in this testing code.

Fair point. In fact in split_huge_page_test.c we're reading every byte
but that's unnecessary. I'll add a helper that reads page-by-page and
use that in all 3 cases.

- Kevin

[PATCH 4/4] selftests/mm: fix exit code in pagemap_ioctl

[Kevin Brodsky]

pagemap_ioctl always reports success, whether the tests succeeded or
not. Call ksft_finished() to report the right status.

While at it also fix the exit code in unexpected situations:

- Report SKIP if userfaultfd isn't available (in line with other
  tests)

- Report FAIL if we failed to open /proc/self/pagemap (returning
  -EINVAL from main() is meaningless)

Cc: Usama Anjum <Usama.Anjum@arm.com>
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
 tools/testing/selftests/mm/pagemap_ioctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
index 67a7a3705604..aeedb96dfffb 100644
--- a/tools/testing/selftests/mm/pagemap_ioctl.c
+++ b/tools/testing/selftests/mm/pagemap_ioctl.c
@@ -1553,7 +1553,7 @@ int main(int __attribute__((unused)) argc, char *argv[])
 	ksft_print_header();
 
 	if (init_uffd())
-		ksft_exit_pass();
+		ksft_exit_skip("Failed to initialize userfaultfd\n");
 
 	ksft_set_plan(117);
 
@@ -1562,7 +1562,7 @@ int main(int __attribute__((unused)) argc, char *argv[])
 
 	pagemap_fd = open(PAGEMAP, O_RDONLY);
 	if (pagemap_fd < 0)
-		return -EINVAL;
+		ksft_exit_fail_msg("Failed to open " PAGEMAP "\n");
 
 	/* 1. Sanity testing */
 	sanity_tests_sd();
@@ -1734,5 +1734,5 @@ int main(int __attribute__((unused)) argc, char *argv[])
 	zeropfn_tests();
 
 	close(pagemap_fd);
-	ksft_exit_pass();
+	ksft_finished();
 }
-- 
2.51.2

Re: [PATCH 4/4] selftests/mm: fix exit code in pagemap_ioctl

[Ryan Roberts]

On 16/12/2025 14:26, Kevin Brodsky wrote:
> pagemap_ioctl always reports success, whether the tests succeeded or
> not. Call ksft_finished() to report the right status.
> 
> While at it also fix the exit code in unexpected situations:
> 
> - Report SKIP if userfaultfd isn't available (in line with other
>   tests)
> 
> - Report FAIL if we failed to open /proc/self/pagemap (returning
>   -EINVAL from main() is meaningless)
> 
> Cc: Usama Anjum <Usama.Anjum@arm.com>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>

Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>

> ---
>  tools/testing/selftests/mm/pagemap_ioctl.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
> index 67a7a3705604..aeedb96dfffb 100644
> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
> @@ -1553,7 +1553,7 @@ int main(int __attribute__((unused)) argc, char *argv[])
>  	ksft_print_header();
>  
>  	if (init_uffd())
> -		ksft_exit_pass();
> +		ksft_exit_skip("Failed to initialize userfaultfd\n");
>  
>  	ksft_set_plan(117);
>  
> @@ -1562,7 +1562,7 @@ int main(int __attribute__((unused)) argc, char *argv[])
>  
>  	pagemap_fd = open(PAGEMAP, O_RDONLY);
>  	if (pagemap_fd < 0)
> -		return -EINVAL;
> +		ksft_exit_fail_msg("Failed to open " PAGEMAP "\n");
>  
>  	/* 1. Sanity testing */
>  	sanity_tests_sd();
> @@ -1734,5 +1734,5 @@ int main(int __attribute__((unused)) argc, char *argv[])
>  	zeropfn_tests();
>  
>  	close(pagemap_fd);
> -	ksft_exit_pass();
> +	ksft_finished();
>  }

Re: [PATCH 4/4] selftests/mm: fix exit code in pagemap_ioctl

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

On Tue, Dec 16, 2025 at 02:26:33PM +0000, Kevin Brodsky wrote:
> pagemap_ioctl always reports success, whether the tests succeeded or
> not. Call ksft_finished() to report the right status.

> While at it also fix the exit code in unexpected situations:

This is a general sign that you should have muliple patches...

> - Report FAIL if we failed to open /proc/self/pagemap (returning
>   -EINVAL from main() is meaningless)

If you do a new version it's probably worth noting that this is a
non-optional feature introduced a long time ago so the open should never
fail.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 4/4] selftests/mm: fix exit code in pagemap_ioctl

[Kevin Brodsky]

On 17/12/2025 11:08, Mark Brown wrote:
> On Tue, Dec 16, 2025 at 02:26:33PM +0000, Kevin Brodsky wrote:
>> pagemap_ioctl always reports success, whether the tests succeeded or
>> not. Call ksft_finished() to report the right status.
>> While at it also fix the exit code in unexpected situations:
> This is a general sign that you should have muliple patches...

I can reword the commit message so it looks less like it's an unrelated
add-on ;)

>> - Report FAIL if we failed to open /proc/self/pagemap (returning
>>   -EINVAL from main() is meaningless)
> If you do a new version it's probably worth noting that this is a
> non-optional feature introduced a long time ago so the open should never
> fail.

Good point, will do.

- Kevin

Re: [PATCH 4/4] selftests/mm: fix exit code in pagemap_ioctl

[David Hildenbrand (Red Hat)]

On 12/16/25 15:26, Kevin Brodsky wrote:
> pagemap_ioctl always reports success, whether the tests succeeded or
> not. Call ksft_finished() to report the right status.
> 
> While at it also fix the exit code in unexpected situations:
> 
> - Report SKIP if userfaultfd isn't available (in line with other
>    tests)
> 
> - Report FAIL if we failed to open /proc/self/pagemap (returning
>    -EINVAL from main() is meaningless)
> 
> Cc: Usama Anjum <Usama.Anjum@arm.com>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---

Fine for me to fix return handling for the overall test in a single patch

Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

-- 
Cheers

David