[PATCH v2 0/8] Various mm kselftests improvements/fixes

[PATCH v2 0/8] Various mm kselftests improvements/fixes

[Kevin Brodsky]

Various improvements/fixes for the mm kselftests:

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

- Patch 4-6 fix issues related to faulting in pages, introducing a new
  helper for that purpose.

- Patch 7 fixes the value returned by pagemap_ioctl (PASS was always
  returned, which explains why the issue fixed in patch 6 went
  unnoticed).

- Patch 8 improves the exit code of pfnmap.

Net results:
- 1 test no longer fails (patch 6)
- 3 tests are no longer skipped (patch 4)
- More accurate return values for whole suites (patch 7, 8)
- Extra tests are more likely to be built (patch 1-3)

---
v1..v2:
- New patches: 1, 4, 5, 8

v1: https://lore.kernel.org/all/20251216142633.2401447-1-kevin.brodsky@arm.com/
---
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 (8):
  selftests/mm: default KDIR to build directory
  selftests/mm: remove flaky header check
  selftests/mm: pass down full CC and CFLAGS to check_config.sh
  selftests/mm: fix usage of FORCE_READ() in cow tests
  selftests/mm: introduce helper to read every page in range
  selftests/mm: fix faulting-in code in pagemap_ioctl test
  selftests/mm: fix exit code in pagemap_ioctl
  selftests/mm: report SKIP in pfnmap if a check fails

 tools/testing/selftests/mm/Makefile           |  8 +-
 tools/testing/selftests/mm/check_config.sh    |  3 +-
 tools/testing/selftests/mm/cow.c              | 16 ++--
 tools/testing/selftests/mm/hugetlb-madvise.c  |  9 +-
 tools/testing/selftests/mm/page_frag/Makefile |  2 +-
 tools/testing/selftests/mm/pagemap_ioctl.c    | 10 +-
 tools/testing/selftests/mm/pfnmap.c           | 95 ++++++++++++-------
 .../selftests/mm/split_huge_page_test.c       |  6 +-
 tools/testing/selftests/mm/vm_util.h          |  6 ++
 9 files changed, 84 insertions(+), 71 deletions(-)


base-commit: 9ace4753a5202b02191d54e9fdf7f9e3d02b85eb
-- 
2.51.2

[PATCH v2 1/8] selftests/mm: default KDIR to build directory

[Kevin Brodsky]

KDIR currently defaults to the running kernel's modules directory
when building the page_frag module. The underlying assumption is
that most users build the kselftests in order to run them against
the system they're built on.

This assumption seems questionable, and there is no guarantee that
the module can actually be built against the running kernel.

Switch the default value of KDIR to the kernel's build directory,
i.e. $(O) if O= or KBUILD_OUTPUT= is used, and the source directory
otherwise. This seems like the least surprising option: the test
module is built against the kernel that has been previously built.

Note: we can't use $(top_srcdir) in mm/Makefile because it is only
defined once lib.mk is included.

Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
 tools/testing/selftests/mm/Makefile           | 2 +-
 tools/testing/selftests/mm/page_frag/Makefile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index eaf9312097f7..bb93101e339e 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -44,7 +44,7 @@ LDLIBS = -lrt -lpthread -lm
 # warnings.
 CFLAGS += -U_FORTIFY_SOURCE
 
-KDIR ?= /lib/modules/$(shell uname -r)/build
+KDIR ?= $(if $(O),$(O),$(realpath ../../../..))
 ifneq (,$(wildcard $(KDIR)/Module.symvers))
 ifneq (,$(wildcard $(KDIR)/include/linux/page_frag_cache.h))
 TEST_GEN_MODS_DIR := page_frag
diff --git a/tools/testing/selftests/mm/page_frag/Makefile b/tools/testing/selftests/mm/page_frag/Makefile
index 8c8bb39ffa28..96e5f646e69b 100644
--- a/tools/testing/selftests/mm/page_frag/Makefile
+++ b/tools/testing/selftests/mm/page_frag/Makefile
@@ -1,5 +1,5 @@
 PAGE_FRAG_TEST_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
-KDIR ?= /lib/modules/$(shell uname -r)/build
+KDIR ?= $(if $(O),$(O),$(realpath ../../../../..))
 
 ifeq ($(V),1)
 Q =
-- 
2.51.2

[PATCH v2 2/8] 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=... make) because only generated headers are present under
$KDIR/include/ in that case.

A recent commit switched KDIR to default to the kernel's build
directory, so that check is no longer justified.

Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Yunsheng Lin <linyunsheng@huawei.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
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 bb93101e339e..4e5c8a330a0c 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -46,12 +46,8 @@ CFLAGS += -U_FORTIFY_SOURCE
 
 KDIR ?= $(if $(O),$(O),$(realpath ../../../..))
 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

[PATCH v2 3/8] 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>
Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
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 4e5c8a330a0c..de4afc34e3b1 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 v2 3/8] selftests/mm: pass down full CC and CFLAGS to check_config.sh

[Mark Brown]

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

On Wed, Jan 07, 2026 at 04:48:37PM +0000, Kevin Brodsky wrote:

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

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

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

[PATCH v2 4/8] selftests/mm: fix usage of FORCE_READ() in cow tests

[Kevin Brodsky]

Commit 5bbc2b785e63 ("selftests/mm: fix FORCE_READ to read input
value correctly") modified FORCE_READ() to take a value instead of a
pointer. It also changed most of the call sites accordingly, but
missed many of them in cow.c. In those cases, we ended up with the
pointer itself being read, not the memory it points to.

No failure occurred as a result, so it looks like the tests work
just fine without faulting in. However, the huge_zeropage tests
explicitly check that pages are populated, so those became skipped.

Convert all the remaining FORCE_READ() to fault in the mapped page,
as was originally intended. This allows the huge_zeropage tests to
run again (3 tests in total).

Fixes: 5bbc2b785e63 ("selftests/mm: fix FORCE_READ to read input value correctly")
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
 tools/testing/selftests/mm/cow.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
index accfd198dbda..83b3563be26b 100644
--- a/tools/testing/selftests/mm/cow.c
+++ b/tools/testing/selftests/mm/cow.c
@@ -1612,8 +1612,8 @@ static void run_with_huge_zeropage(non_anon_test_fn fn, const char *desc)
 	 * the first sub-page and test if we get another sub-page populated
 	 * automatically.
 	 */
-	FORCE_READ(mem);
-	FORCE_READ(smem);
+	FORCE_READ(*mem);
+	FORCE_READ(*smem);
 	if (!pagemap_is_populated(pagemap_fd, mem + pagesize) ||
 	    !pagemap_is_populated(pagemap_fd, smem + pagesize)) {
 		ksft_test_result_skip("Did not get THPs populated\n");
@@ -1663,8 +1663,8 @@ static void run_with_memfd(non_anon_test_fn fn, const char *desc)
 	}
 
 	/* Fault the page in. */
-	FORCE_READ(mem);
-	FORCE_READ(smem);
+	FORCE_READ(*mem);
+	FORCE_READ(*smem);
 
 	fn(mem, smem, pagesize);
 munmap:
@@ -1719,8 +1719,8 @@ static void run_with_tmpfile(non_anon_test_fn fn, const char *desc)
 	}
 
 	/* Fault the page in. */
-	FORCE_READ(mem);
-	FORCE_READ(smem);
+	FORCE_READ(*mem);
+	FORCE_READ(*smem);
 
 	fn(mem, smem, pagesize);
 munmap:
@@ -1773,8 +1773,8 @@ static void run_with_memfd_hugetlb(non_anon_test_fn fn, const char *desc,
 	}
 
 	/* Fault the page in. */
-	FORCE_READ(mem);
-	FORCE_READ(smem);
+	FORCE_READ(*mem);
+	FORCE_READ(*smem);
 
 	fn(mem, smem, hugetlbsize);
 munmap:
-- 
2.51.2

Re: [PATCH v2 4/8] selftests/mm: fix usage of FORCE_READ() in cow tests

[SeongJae Park]

On Wed,  7 Jan 2026 16:48:38 +0000 Kevin Brodsky <kevin.brodsky@arm.com> wrote:

> Commit 5bbc2b785e63 ("selftests/mm: fix FORCE_READ to read input
> value correctly") modified FORCE_READ() to take a value instead of a
> pointer. It also changed most of the call sites accordingly, but
> missed many of them in cow.c. In those cases, we ended up with the
> pointer itself being read, not the memory it points to.
> 
> No failure occurred as a result, so it looks like the tests work
> just fine without faulting in. However, the huge_zeropage tests
> explicitly check that pages are populated, so those became skipped.
> 
> Convert all the remaining FORCE_READ() to fault in the mapped page,
> as was originally intended. This allows the huge_zeropage tests to
> run again (3 tests in total).
> 
> Fixes: 5bbc2b785e63 ("selftests/mm: fix FORCE_READ to read input value correctly")
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>

Acked-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

[...]

Re: [PATCH v2 4/8] selftests/mm: fix usage of FORCE_READ() in cow tests

[wang lian]

> Commit 5bbc2b785e63 ("selftests/mm: fix FORCE_READ to read input
> value correctly") modified FORCE_READ() to take a value instead of a
> pointer. It also changed most of the call sites accordingly, but
> missed many of them in cow.c. In those cases, we ended up with the
> pointer itself being read, not the memory it points to.
> 
> No failure occurred as a result, so it looks like the tests work
> just fine without faulting in. However, the huge_zeropage tests
> explicitly check that pages are populated, so those became skipped.
> 
> Convert all the remaining FORCE_READ() to fault in the mapped page,
> as was originally intended. This allows the huge_zeropage tests to
> run again (3 tests in total).
> 
> Fixes: 5bbc2b785e63 ("selftests/mm: fix FORCE_READ to read input value correctly")
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>

Hi Kevin,

Thanks for the fix.

This was indeed an oversight on my part. When we previously discussed this
refactoring with Ziyan and Lorenzo (and the community) regarding commit
3f6bfd4789a0 ("selftests/mm: reuse FORCE_READ to replace "asm volatile
("" : "+r" (XXX));""), the intention was to switch FORCE_READ to take a
value. I clearly missed updating these specific call sites in cow.c
during that transition.

Sorry for the trouble and the skipped tests. The changes look correct to
me.

Reviewed-by: wang lian <lianux.mm@gmail.com>

--
Best Regards,
wang lian

Re: [PATCH v2 7/8] selftests/mm: fix exit code in pagemap_ioctl

[wang lian]

Reviewed-by: wang lian <lianux.mm@gmail.com>

--
Best Regards,
wang lian

Re: [PATCH v2 4/8] selftests/mm: fix usage of FORCE_READ() in cow tests

[David Hildenbrand (Red Hat)]

On 1/7/26 17:48, Kevin Brodsky wrote:
> Commit 5bbc2b785e63 ("selftests/mm: fix FORCE_READ to read input
> value correctly") modified FORCE_READ() to take a value instead of a
> pointer. It also changed most of the call sites accordingly, but
> missed many of them in cow.c. In those cases, we ended up with the
> pointer itself being read, not the memory it points to.
> 
> No failure occurred as a result, so it looks like the tests work
> just fine without faulting in. However, the huge_zeropage tests
> explicitly check that pages are populated, so those became skipped.

Right, that's nasty! Thanks!

For all cases, we could probably just fail if the memory is not 
populated after FORCE_READ().

Would you have time to prepare a patch for that?

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

-- 
Cheers

David

Re: [PATCH v2 4/8] selftests/mm: fix usage of FORCE_READ() in cow tests

[Kevin Brodsky]

On 19/01/2026 11:50, David Hildenbrand (Red Hat) wrote:
> On 1/7/26 17:48, Kevin Brodsky wrote:
>> Commit 5bbc2b785e63 ("selftests/mm: fix FORCE_READ to read input
>> value correctly") modified FORCE_READ() to take a value instead of a
>> pointer. It also changed most of the call sites accordingly, but
>> missed many of them in cow.c. In those cases, we ended up with the
>> pointer itself being read, not the memory it points to.
>>
>> No failure occurred as a result, so it looks like the tests work
>> just fine without faulting in. However, the huge_zeropage tests
>> explicitly check that pages are populated, so those became skipped.
>
> Right, that's nasty! Thanks!
>
> For all cases, we could probably just fail if the memory is not
> populated after FORCE_READ().
>
> Would you have time to prepare a patch for that?

Sure, I guess we could even have a helper that performs a FORCE_READ()
and then returns the result of pagemap_is_populated().

- Kevin

Re: [PATCH v2 4/8] selftests/mm: fix usage of FORCE_READ() in cow tests

[Dev Jain]

On 07/01/26 10:18 pm, Kevin Brodsky wrote:
> Commit 5bbc2b785e63 ("selftests/mm: fix FORCE_READ to read input
> value correctly") modified FORCE_READ() to take a value instead of a
> pointer. It also changed most of the call sites accordingly, but
> missed many of them in cow.c. In those cases, we ended up with the
> pointer itself being read, not the memory it points to.
>
> No failure occurred as a result, so it looks like the tests work
> just fine without faulting in. However, the huge_zeropage tests
> explicitly check that pages are populated, so those became skipped.
>
> Convert all the remaining FORCE_READ() to fault in the mapped page,
> as was originally intended. This allows the huge_zeropage tests to
> run again (3 tests in total).
>
> Fixes: 5bbc2b785e63 ("selftests/mm: fix FORCE_READ to read input value correctly")
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>

Reviewed-by: Dev Jain <dev.jain@arm.com>

[PATCH v2 5/8] selftests/mm: introduce helper to read every page in range

[Kevin Brodsky]

FORCE_READ(*addr) ensures that the compiler will emit a load from
addr. Several tests need to trigger such a load for every page in
the range [addr, addr + len), ensuring that every page is faulted
in, if it wasn't already.

Introduce a new helper force_read_pages_in_range() that does exactly
that and replace existing loops with a call to it. Some of those
loops have a different step size, but reading from every page is
appropriate in all cases.

Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
 tools/testing/selftests/mm/hugetlb-madvise.c     |  9 +--------
 tools/testing/selftests/mm/pfnmap.c              | 16 ++++++----------
 .../testing/selftests/mm/split_huge_page_test.c  |  6 +-----
 tools/testing/selftests/mm/vm_util.h             |  6 ++++++
 4 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c
index 05d9d2805ae4..1f82568ae262 100644
--- a/tools/testing/selftests/mm/hugetlb-madvise.c
+++ b/tools/testing/selftests/mm/hugetlb-madvise.c
@@ -47,14 +47,7 @@ void write_fault_pages(void *addr, unsigned long nr_pages)
 
 void read_fault_pages(void *addr, unsigned long nr_pages)
 {
-	unsigned long i;
-
-	for (i = 0; i < nr_pages; i++) {
-		unsigned long *addr2 =
-			((unsigned long *)(addr + (i * huge_page_size)));
-		/* Prevent the compiler from optimizing out the entire loop: */
-		FORCE_READ(*addr2);
-	}
+	force_read_pages_in_range(addr, nr_pages * huge_page_size);
 }
 
 int main(int argc, char **argv)
diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
index f546dfb10cae..35b0e3ed54cd 100644
--- a/tools/testing/selftests/mm/pfnmap.c
+++ b/tools/testing/selftests/mm/pfnmap.c
@@ -33,20 +33,17 @@ static void signal_handler(int sig)
 	siglongjmp(sigjmp_buf_env, -EFAULT);
 }
 
-static int test_read_access(char *addr, size_t size, size_t pagesize)
+static int test_read_access(char *addr, size_t size)
 {
-	size_t offs;
 	int ret;
 
 	if (signal(SIGSEGV, signal_handler) == SIG_ERR)
 		return -EINVAL;
 
 	ret = sigsetjmp(sigjmp_buf_env, 1);
-	if (!ret) {
-		for (offs = 0; offs < size; offs += pagesize)
-			/* Force a read that the compiler cannot optimize out. */
-			*((volatile char *)(addr + offs));
-	}
+	if (!ret)
+		force_read_pages_in_range(addr, size);
+
 	if (signal(SIGSEGV, SIG_DFL) == SIG_ERR)
 		return -EINVAL;
 
@@ -138,7 +135,7 @@ FIXTURE_SETUP(pfnmap)
 		SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
 
 	/* ... and want to be able to read from them. */
-	if (test_read_access(self->addr1, self->size1, self->pagesize))
+	if (test_read_access(self->addr1, self->size1))
 		SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
 
 	self->size2 = 0;
@@ -243,8 +240,7 @@ TEST_F(pfnmap, fork)
 	ASSERT_GE(pid, 0);
 
 	if (!pid) {
-		EXPECT_EQ(test_read_access(self->addr1, self->size1,
-					   self->pagesize), 0);
+		EXPECT_EQ(test_read_access(self->addr1, self->size1), 0);
 		exit(0);
 	}
 
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
index 40799f3f0213..65a89ceca4a5 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -652,11 +652,7 @@ static int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size,
 	}
 	madvise(*addr, fd_size, MADV_HUGEPAGE);
 
-	for (size_t i = 0; i < fd_size; i++) {
-		char *addr2 = *addr + i;
-
-		FORCE_READ(*addr2);
-	}
+	force_read_pages_in_range(*addr, fd_size);
 
 	if (!check_huge_file(*addr, fd_size / pmd_pagesize, pmd_pagesize)) {
 		ksft_print_msg("No large pagecache folio generated, please provide a filesystem supporting large folio\n");
diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
index 6ad32b1830f1..74bdf96161d7 100644
--- a/tools/testing/selftests/mm/vm_util.h
+++ b/tools/testing/selftests/mm/vm_util.h
@@ -54,6 +54,12 @@ static inline unsigned int pshift(void)
 	return __page_shift;
 }
 
+static inline void force_read_pages_in_range(char *addr, size_t len)
+{
+	for (size_t i = 0; i < len; i += psize())
+		FORCE_READ(addr[i]);
+}
+
 bool detect_huge_zeropage(void);
 
 /*
-- 
2.51.2

Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range

[SeongJae Park]

On Wed,  7 Jan 2026 16:48:39 +0000 Kevin Brodsky <kevin.brodsky@arm.com> wrote:

> FORCE_READ(*addr) ensures that the compiler will emit a load from
> addr. Several tests need to trigger such a load for every page in
> the range [addr, addr + len), ensuring that every page is faulted
> in, if it wasn't already.
> 
> Introduce a new helper force_read_pages_in_range() that does exactly
> that and replace existing loops with a call to it.

Seems like a good cleanup to me.

> Some of those
> loops have a different step size, but reading from every page is
> appropriate in all cases.

So the test program's behavior is slightly be changed.  I believe that
shouldn't be problem, but I'm not that familiar with the test code, so not very
sure.  I'd like to listen voices from people more familiar with those.

Meanwhile, I'm curious what do you think about making the helper function
receives the step size together, and let the callers just pass their current
step size.


Thanks,
SJ

[...]

Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range

[Kevin Brodsky]

On 09/01/2026 02:30, SeongJae Park wrote:
> On Wed,  7 Jan 2026 16:48:39 +0000 Kevin Brodsky <kevin.brodsky@arm.com> wrote:
>
>> FORCE_READ(*addr) ensures that the compiler will emit a load from
>> addr. Several tests need to trigger such a load for every page in
>> the range [addr, addr + len), ensuring that every page is faulted
>> in, if it wasn't already.
>>
>> Introduce a new helper force_read_pages_in_range() that does exactly
>> that and replace existing loops with a call to it.
> Seems like a good cleanup to me.

Thanks for having a look at this series!

>> Some of those
>> loops have a different step size, but reading from every page is
>> appropriate in all cases.
> So the test program's behavior is slightly be changed.  I believe that
> shouldn't be problem, but I'm not that familiar with the test code, so not very
> sure.  I'd like to listen voices from people more familiar with those.
>
> Meanwhile, I'm curious what do you think about making the helper function
> receives the step size together, and let the callers just pass their current
> step size.

That's what I initially considered, but considering this discussion on
v1 [1] this doesn't seem to be justified. In hugetlb-madvise, reading
every page instead of every hugepage is unnecessary but still correct
and the overhead should be negligible. In split_huge_page_test, I don't
think there's any justification for reading every byte - the intention
is to fault in pages, like all the other cases this patch touches.

- Kevin

[1]
https://lore.kernel.org/all/a3ca6293-8f85-4489-a48e-eb8d0d3792c5@kernel.org/

Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range

[SeongJae Park]

On Mon, 12 Jan 2026 10:37:26 +0100 Kevin Brodsky <kevin.brodsky@arm.com> wrote:

> On 09/01/2026 02:30, SeongJae Park wrote:
> > On Wed,  7 Jan 2026 16:48:39 +0000 Kevin Brodsky <kevin.brodsky@arm.com> wrote:
> >
> >> FORCE_READ(*addr) ensures that the compiler will emit a load from
> >> addr. Several tests need to trigger such a load for every page in
> >> the range [addr, addr + len), ensuring that every page is faulted
> >> in, if it wasn't already.
> >>
> >> Introduce a new helper force_read_pages_in_range() that does exactly
> >> that and replace existing loops with a call to it.
> > Seems like a good cleanup to me.
> 
> Thanks for having a look at this series!

My pleasure!

> 
> >> Some of those
> >> loops have a different step size, but reading from every page is
> >> appropriate in all cases.
> > So the test program's behavior is slightly be changed.  I believe that
> > shouldn't be problem, but I'm not that familiar with the test code, so not very
> > sure.  I'd like to listen voices from people more familiar with those.
> >
> > Meanwhile, I'm curious what do you think about making the helper function
> > receives the step size together, and let the callers just pass their current
> > step size.
> 
> That's what I initially considered, but considering this discussion on
> v1 [1] this doesn't seem to be justified. In hugetlb-madvise, reading
> every page instead of every hugepage is unnecessary but still correct
> and the overhead should be negligible. In split_huge_page_test, I don't
> think there's any justification for reading every byte - the intention
> is to fault in pages, like all the other cases this patch touches.
> 
> - Kevin
> 
> [1]
> https://lore.kernel.org/all/a3ca6293-8f85-4489-a48e-eb8d0d3792c5@kernel.org/

Makes sense, thank you for the link!

Please feel free to add

Reviewed-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

[...]

Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range

[David Hildenbrand (Red Hat)]

On 1/7/26 17:48, Kevin Brodsky wrote:
> FORCE_READ(*addr) ensures that the compiler will emit a load from
> addr. Several tests need to trigger such a load for every page in
> the range [addr, addr + len), ensuring that every page is faulted
> in, if it wasn't already.
> 
> Introduce a new helper force_read_pages_in_range() that does exactly
> that and replace existing loops with a call to it. Some of those
> loops have a different step size, but reading from every page is
> appropriate in all cases.
> 
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>   tools/testing/selftests/mm/hugetlb-madvise.c     |  9 +--------
>   tools/testing/selftests/mm/pfnmap.c              | 16 ++++++----------
>   .../testing/selftests/mm/split_huge_page_test.c  |  6 +-----
>   tools/testing/selftests/mm/vm_util.h             |  6 ++++++
>   4 files changed, 14 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c
> index 05d9d2805ae4..1f82568ae262 100644
> --- a/tools/testing/selftests/mm/hugetlb-madvise.c
> +++ b/tools/testing/selftests/mm/hugetlb-madvise.c
> @@ -47,14 +47,7 @@ void write_fault_pages(void *addr, unsigned long nr_pages)
>   
>   void read_fault_pages(void *addr, unsigned long nr_pages)
>   {
> -	unsigned long i;
> -
> -	for (i = 0; i < nr_pages; i++) {
> -		unsigned long *addr2 =
> -			((unsigned long *)(addr + (i * huge_page_size)));
> -		/* Prevent the compiler from optimizing out the entire loop: */
> -		FORCE_READ(*addr2);
> -	}
> +	force_read_pages_in_range(addr, nr_pages * huge_page_size);
>   }
>   
>   int main(int argc, char **argv)
> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
> index f546dfb10cae..35b0e3ed54cd 100644
> --- a/tools/testing/selftests/mm/pfnmap.c
> +++ b/tools/testing/selftests/mm/pfnmap.c
> @@ -33,20 +33,17 @@ static void signal_handler(int sig)
>   	siglongjmp(sigjmp_buf_env, -EFAULT);
>   }
>   
> -static int test_read_access(char *addr, size_t size, size_t pagesize)
> +static int test_read_access(char *addr, size_t size)
>   {
> -	size_t offs;
>   	int ret;
>   
>   	if (signal(SIGSEGV, signal_handler) == SIG_ERR)
>   		return -EINVAL;
>   
>   	ret = sigsetjmp(sigjmp_buf_env, 1);
> -	if (!ret) {
> -		for (offs = 0; offs < size; offs += pagesize)
> -			/* Force a read that the compiler cannot optimize out. */
> -			*((volatile char *)(addr + offs));
> -	}
> +	if (!ret)
> +		force_read_pages_in_range(addr, size);
> +
>   	if (signal(SIGSEGV, SIG_DFL) == SIG_ERR)
>   		return -EINVAL;
>   
> @@ -138,7 +135,7 @@ FIXTURE_SETUP(pfnmap)
>   		SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
>   
>   	/* ... and want to be able to read from them. */
> -	if (test_read_access(self->addr1, self->size1, self->pagesize))
> +	if (test_read_access(self->addr1, self->size1))
>   		SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
>   
>   	self->size2 = 0;
> @@ -243,8 +240,7 @@ TEST_F(pfnmap, fork)
>   	ASSERT_GE(pid, 0);
>   
>   	if (!pid) {
> -		EXPECT_EQ(test_read_access(self->addr1, self->size1,
> -					   self->pagesize), 0);
> +		EXPECT_EQ(test_read_access(self->addr1, self->size1), 0);
>   		exit(0);
>   	}
>   
> diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
> index 40799f3f0213..65a89ceca4a5 100644
> --- a/tools/testing/selftests/mm/split_huge_page_test.c
> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
> @@ -652,11 +652,7 @@ static int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size,
>   	}
>   	madvise(*addr, fd_size, MADV_HUGEPAGE);
>   
> -	for (size_t i = 0; i < fd_size; i++) {
> -		char *addr2 = *addr + i;
> -
> -		FORCE_READ(*addr2);
> -	}
> +	force_read_pages_in_range(*addr, fd_size);
>   
>   	if (!check_huge_file(*addr, fd_size / pmd_pagesize, pmd_pagesize)) {
>   		ksft_print_msg("No large pagecache folio generated, please provide a filesystem supporting large folio\n");
> diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
> index 6ad32b1830f1..74bdf96161d7 100644
> --- a/tools/testing/selftests/mm/vm_util.h
> +++ b/tools/testing/selftests/mm/vm_util.h
> @@ -54,6 +54,12 @@ static inline unsigned int pshift(void)
>   	return __page_shift;
>   }
>   
> +static inline void force_read_pages_in_range(char *addr, size_t len)
> +{
> +	for (size_t i = 0; i < len; i += psize())
> +		FORCE_READ(addr[i]);
> +}
> +

Of course, we could also just pass the pagesize

static inline void force_read_pages_in_range(char *addr, size_t len,
	size_t pagesize)
{
	for (size_t i = 0; i < len; i += pagesize)
		FORCE_READ(addr[i]);
}


Or alternatively even better:

static inline void force_read_pages(char *addr, unsigned int nr_pages,
	size_t pagesize)
{
	for (size_t i = 0; i < nr_pages; i++)
		FORCE_READ(addr[i * pagesize]);
}

Then there is no change at all and we avoid the repeated psize() naturally.

Thoughts?

-- 
Cheers

David

Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range

[Kevin Brodsky]

On 19/01/2026 11:55, David Hildenbrand (Red Hat) wrote:
> On 1/7/26 17:48, Kevin Brodsky wrote:
>> FORCE_READ(*addr) ensures that the compiler will emit a load from
>> addr. Several tests need to trigger such a load for every page in
>> the range [addr, addr + len), ensuring that every page is faulted
>> in, if it wasn't already.
>>
>> Introduce a new helper force_read_pages_in_range() that does exactly
>> that and replace existing loops with a call to it. Some of those
>> loops have a different step size, but reading from every page is
>> appropriate in all cases.
>>
>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> ---
>>   tools/testing/selftests/mm/hugetlb-madvise.c     |  9 +--------
>>   tools/testing/selftests/mm/pfnmap.c              | 16 ++++++----------
>>   .../testing/selftests/mm/split_huge_page_test.c  |  6 +-----
>>   tools/testing/selftests/mm/vm_util.h             |  6 ++++++
>>   4 files changed, 14 insertions(+), 23 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c
>> b/tools/testing/selftests/mm/hugetlb-madvise.c
>> index 05d9d2805ae4..1f82568ae262 100644
>> --- a/tools/testing/selftests/mm/hugetlb-madvise.c
>> +++ b/tools/testing/selftests/mm/hugetlb-madvise.c
>> @@ -47,14 +47,7 @@ void write_fault_pages(void *addr, unsigned long
>> nr_pages)
>>     void read_fault_pages(void *addr, unsigned long nr_pages)
>>   {
>> -    unsigned long i;
>> -
>> -    for (i = 0; i < nr_pages; i++) {
>> -        unsigned long *addr2 =
>> -            ((unsigned long *)(addr + (i * huge_page_size)));
>> -        /* Prevent the compiler from optimizing out the entire loop: */
>> -        FORCE_READ(*addr2);
>> -    }
>> +    force_read_pages_in_range(addr, nr_pages * huge_page_size);
>>   }
>>     int main(int argc, char **argv)
>> diff --git a/tools/testing/selftests/mm/pfnmap.c
>> b/tools/testing/selftests/mm/pfnmap.c
>> index f546dfb10cae..35b0e3ed54cd 100644
>> --- a/tools/testing/selftests/mm/pfnmap.c
>> +++ b/tools/testing/selftests/mm/pfnmap.c
>> @@ -33,20 +33,17 @@ static void signal_handler(int sig)
>>       siglongjmp(sigjmp_buf_env, -EFAULT);
>>   }
>>   -static int test_read_access(char *addr, size_t size, size_t pagesize)
>> +static int test_read_access(char *addr, size_t size)
>>   {
>> -    size_t offs;
>>       int ret;
>>         if (signal(SIGSEGV, signal_handler) == SIG_ERR)
>>           return -EINVAL;
>>         ret = sigsetjmp(sigjmp_buf_env, 1);
>> -    if (!ret) {
>> -        for (offs = 0; offs < size; offs += pagesize)
>> -            /* Force a read that the compiler cannot optimize out. */
>> -            *((volatile char *)(addr + offs));
>> -    }
>> +    if (!ret)
>> +        force_read_pages_in_range(addr, size);
>> +
>>       if (signal(SIGSEGV, SIG_DFL) == SIG_ERR)
>>           return -EINVAL;
>>   @@ -138,7 +135,7 @@ FIXTURE_SETUP(pfnmap)
>>           SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
>>         /* ... and want to be able to read from them. */
>> -    if (test_read_access(self->addr1, self->size1, self->pagesize))
>> +    if (test_read_access(self->addr1, self->size1))
>>           SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
>>         self->size2 = 0;
>> @@ -243,8 +240,7 @@ TEST_F(pfnmap, fork)
>>       ASSERT_GE(pid, 0);
>>         if (!pid) {
>> -        EXPECT_EQ(test_read_access(self->addr1, self->size1,
>> -                       self->pagesize), 0);
>> +        EXPECT_EQ(test_read_access(self->addr1, self->size1), 0);
>>           exit(0);
>>       }
>>   diff --git a/tools/testing/selftests/mm/split_huge_page_test.c
>> b/tools/testing/selftests/mm/split_huge_page_test.c
>> index 40799f3f0213..65a89ceca4a5 100644
>> --- a/tools/testing/selftests/mm/split_huge_page_test.c
>> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
>> @@ -652,11 +652,7 @@ static int create_pagecache_thp_and_fd(const
>> char *testfile, size_t fd_size,
>>       }
>>       madvise(*addr, fd_size, MADV_HUGEPAGE);
>>   -    for (size_t i = 0; i < fd_size; i++) {
>> -        char *addr2 = *addr + i;
>> -
>> -        FORCE_READ(*addr2);
>> -    }
>> +    force_read_pages_in_range(*addr, fd_size);
>>         if (!check_huge_file(*addr, fd_size / pmd_pagesize,
>> pmd_pagesize)) {
>>           ksft_print_msg("No large pagecache folio generated, please
>> provide a filesystem supporting large folio\n");
>> diff --git a/tools/testing/selftests/mm/vm_util.h
>> b/tools/testing/selftests/mm/vm_util.h
>> index 6ad32b1830f1..74bdf96161d7 100644
>> --- a/tools/testing/selftests/mm/vm_util.h
>> +++ b/tools/testing/selftests/mm/vm_util.h
>> @@ -54,6 +54,12 @@ static inline unsigned int pshift(void)
>>       return __page_shift;
>>   }
>>   +static inline void force_read_pages_in_range(char *addr, size_t len)
>> +{
>> +    for (size_t i = 0; i < len; i += psize())
>> +        FORCE_READ(addr[i]);
>> +}
>> +
>
> Of course, we could also just pass the pagesize
>
> static inline void force_read_pages_in_range(char *addr, size_t len,
>     size_t pagesize)
> {
>     for (size_t i = 0; i < len; i += pagesize)
>         FORCE_READ(addr[i]);
> }
>
>
> Or alternatively even better:
>
> static inline void force_read_pages(char *addr, unsigned int nr_pages,
>     size_t pagesize)
> {
>     for (size_t i = 0; i < nr_pages; i++)
>         FORCE_READ(addr[i * pagesize]);
> }
>
> Then there is no change at all and we avoid the repeated psize()
> naturally.
>
> Thoughts?

Sure we can do that. I concluded that all tests fetch the pagesize in a
different way and some aren't using psize() sparingly either (e.g.
gup_test) but I suppose that avoiding some ugliness doesn't hurt :)

Of course this especially makes sense for the hugepage tests (then we
can still read hugepage by hugepage).

- Kevin

Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range

[Dev Jain]

On 07/01/26 10:18 pm, Kevin Brodsky wrote:
> FORCE_READ(*addr) ensures that the compiler will emit a load from
> addr. Several tests need to trigger such a load for every page in
> the range [addr, addr + len), ensuring that every page is faulted
> in, if it wasn't already.
>
> Introduce a new helper force_read_pages_in_range() that does exactly
> that and replace existing loops with a call to it. Some of those
> loops have a different step size, but reading from every page is
> appropriate in all cases.
>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>  tools/testing/selftests/mm/hugetlb-madvise.c     |  9 +--------
>  tools/testing/selftests/mm/pfnmap.c              | 16 ++++++----------
>  .../testing/selftests/mm/split_huge_page_test.c  |  6 +-----
>  tools/testing/selftests/mm/vm_util.h             |  6 ++++++
>  4 files changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c
> index 05d9d2805ae4..1f82568ae262 100644
> --- a/tools/testing/selftests/mm/hugetlb-madvise.c
> +++ b/tools/testing/selftests/mm/hugetlb-madvise.c
> @@ -47,14 +47,7 @@ void write_fault_pages(void *addr, unsigned long nr_pages)
>  
>  void read_fault_pages(void *addr, unsigned long nr_pages)
>  {
> -	unsigned long i;
> -
> -	for (i = 0; i < nr_pages; i++) {
> -		unsigned long *addr2 =
> -			((unsigned long *)(addr + (i * huge_page_size)));
> -		/* Prevent the compiler from optimizing out the entire loop: */
> -		FORCE_READ(*addr2);
> -	}
> +	force_read_pages_in_range(addr, nr_pages * huge_page_size);
>  }

Yeah this should be fine to do.

>  
>  int main(int argc, char **argv)
> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
> index f546dfb10cae..35b0e3ed54cd 100644
> --- a/tools/testing/selftests/mm/pfnmap.c
> +++ b/tools/testing/selftests/mm/pfnmap.c
> @@ -33,20 +33,17 @@ static void signal_handler(int sig)
>  	siglongjmp(sigjmp_buf_env, -EFAULT);
>  }
>  
> -static int test_read_access(char *addr, size_t size, size_t pagesize)
> +static int test_read_access(char *addr, size_t size)
>  {
> -	size_t offs;
>  	int ret;
>  
>  	if (signal(SIGSEGV, signal_handler) == SIG_ERR)
>  		return -EINVAL;
>  
>  	ret = sigsetjmp(sigjmp_buf_env, 1);
> -	if (!ret) {
> -		for (offs = 0; offs < size; offs += pagesize)
> -			/* Force a read that the compiler cannot optimize out. */
> -			*((volatile char *)(addr + offs));
> -	}
> +	if (!ret)
> +		force_read_pages_in_range(addr, size);
> +

LGTM

>  	if (signal(SIGSEGV, SIG_DFL) == SIG_ERR)
>  		return -EINVAL;
>  
> @@ -138,7 +135,7 @@ FIXTURE_SETUP(pfnmap)
>  		SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
>  
>  	/* ... and want to be able to read from them. */
> -	if (test_read_access(self->addr1, self->size1, self->pagesize))
> +	if (test_read_access(self->addr1, self->size1))
>  		SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
>  
>  	self->size2 = 0;
> @@ -243,8 +240,7 @@ TEST_F(pfnmap, fork)
>  	ASSERT_GE(pid, 0);
>  
>  	if (!pid) {
> -		EXPECT_EQ(test_read_access(self->addr1, self->size1,
> -					   self->pagesize), 0);
> +		EXPECT_EQ(test_read_access(self->addr1, self->size1), 0);
>  		exit(0);
>  	}
>  
> diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
> index 40799f3f0213..65a89ceca4a5 100644
> --- a/tools/testing/selftests/mm/split_huge_page_test.c
> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
> @@ -652,11 +652,7 @@ static int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size,
>  	}
>  	madvise(*addr, fd_size, MADV_HUGEPAGE);
>  
> -	for (size_t i = 0; i < fd_size; i++) {
> -		char *addr2 = *addr + i;
> -
> -		FORCE_READ(*addr2);
> -	}
> +	force_read_pages_in_range(*addr, fd_size);

LGTM

>  
>  	if (!check_huge_file(*addr, fd_size / pmd_pagesize, pmd_pagesize)) {
>  		ksft_print_msg("No large pagecache folio generated, please provide a filesystem supporting large folio\n");
> diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
> index 6ad32b1830f1..74bdf96161d7 100644
> --- a/tools/testing/selftests/mm/vm_util.h
> +++ b/tools/testing/selftests/mm/vm_util.h
> @@ -54,6 +54,12 @@ static inline unsigned int pshift(void)
>  	return __page_shift;
>  }
>  
> +static inline void force_read_pages_in_range(char *addr, size_t len)
> +{
> +	for (size_t i = 0; i < len; i += psize())
> +		FORCE_READ(addr[i]);
> +}
> +

Reviewed-by: Dev Jain <dev.jain@arm.com>

>  bool detect_huge_zeropage(void);
>  
>  /*

[PATCH v2 6/8] 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.

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

diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
index 2cb5441f29c7..80d7c391f8f5 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,7 @@ 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);
+	force_read_pages_in_range(fmem, sbuf.st_size);
 
 	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 v2 6/8] selftests/mm: fix faulting-in code in pagemap_ioctl test

[David Hildenbrand (Red Hat)]

On 1/7/26 17:48, 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.

Yes, I assume so. Using FORCE_READ() etc is the way to go.


Should we add

Fixes: 46036188ea1f ("selftests/mm: build with -O2")

?

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

-- 
Cheers

David

Re: [PATCH v2 6/8] selftests/mm: fix faulting-in code in pagemap_ioctl test

[Kevin Brodsky]

On 19/01/2026 12:09, David Hildenbrand (Red Hat) wrote:
> On 1/7/26 17:48, 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.
>
> Yes, I assume so. Using FORCE_READ() etc is the way to go.
>
>
> Should we add
>
> Fixes: 46036188ea1f ("selftests/mm: build with -O2") 

Yes that seems reasonable, the test was arguably broken to start with
but it is this commit that revealed it ;)

- Kevin

Re: [PATCH v2 6/8] selftests/mm: fix faulting-in code in pagemap_ioctl test

[Dev Jain]

On 07/01/26 10:18 pm, 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.
>
> Cc: Usama Anjum <Usama.Anjum@arm.com>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>

Reviewed-by: Dev Jain <dev.jain@arm.com>

> ---
>  tools/testing/selftests/mm/pagemap_ioctl.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
> index 2cb5441f29c7..80d7c391f8f5 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,7 @@ 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);
> +	force_read_pages_in_range(fmem, sbuf.st_size);
>  
>  	ret = pagemap_ioctl(fmem, sbuf.st_size, vec, vec_size, 0, 0,
>  			    0, PAGEMAP_NON_WRITTEN_BITS, 0, PAGEMAP_NON_WRITTEN_BITS);

[PATCH v2 7/8] selftests/mm: fix exit code in pagemap_ioctl

[Kevin Brodsky]

Make sure pagemap_ioctl exits with an appropriate value:

* If the tests are run, call ksft_finished() to report the right
  status instead of reporting PASS unconditionally.

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

* Report FAIL if we failed to open /proc/self/pagemap, as this file
  has been added a long time ago and doesn't depend on any CONFIG
  option (returning -EINVAL from main() is meaningless)

Cc: Usama Anjum <Usama.Anjum@arm.com>
Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
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 80d7c391f8f5..7b214e8755f7 100644
--- a/tools/testing/selftests/mm/pagemap_ioctl.c
+++ b/tools/testing/selftests/mm/pagemap_ioctl.c
@@ -1551,7 +1551,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);
 
@@ -1560,7 +1560,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();
@@ -1732,5 +1732,5 @@ int main(int __attribute__((unused)) argc, char *argv[])
 	zeropfn_tests();
 
 	close(pagemap_fd);
-	ksft_exit_pass();
+	ksft_finished();
 }
-- 
2.51.2

Re: [PATCH v2 7/8] selftests/mm: fix exit code in pagemap_ioctl

[SeongJae Park]

On Wed,  7 Jan 2026 16:48:41 +0000 Kevin Brodsky <kevin.brodsky@arm.com> wrote:

> Make sure pagemap_ioctl exits with an appropriate value:
> 
> * If the tests are run, call ksft_finished() to report the right
>   status instead of reporting PASS unconditionally.
> 
> * Report SKIP if userfaultfd isn't available (in line with other
>   tests)
> 
> * Report FAIL if we failed to open /proc/self/pagemap, as this file
>   has been added a long time ago and doesn't depend on any CONFIG
>   option (returning -EINVAL from main() is meaningless)
> 
> Cc: Usama Anjum <Usama.Anjum@arm.com>
> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
> Reviewed-by: Mark Brown <broonie@kernel.org>
> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>

Acked-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

[...]

Re: [PATCH v2 7/8] selftests/mm: fix exit code in pagemap_ioctl

[wang lian]

> Make sure pagemap_ioctl exits with an appropriate value:
> 
> * If the tests are run, call ksft_finished() to report the right
>   status instead of reporting PASS unconditionally.
> 
> * Report SKIP if userfaultfd isn't available (in line with other
>   tests)
> 
> * Report FAIL if we failed to open /proc/self/pagemap, as this file
>   has been added a long time ago and doesn't depend on any CONFIG
>   option (returning -EINVAL from main() is meaningless)
> 
> Cc: Usama Anjum <Usama.Anjum@arm.com>
> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
> Reviewed-by: Mark Brown <broonie@kernel.org>
> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>

Reviewed-by: wang lian <lianux.mm@gmail.com>

--
Best Regards,
wang lian

Re: [PATCH v2 7/8] selftests/mm: fix exit code in pagemap_ioctl

[Dev Jain]

On 07/01/26 10:18 pm, Kevin Brodsky wrote:
> Make sure pagemap_ioctl exits with an appropriate value:
>
> * If the tests are run, call ksft_finished() to report the right
>   status instead of reporting PASS unconditionally.
>
> * Report SKIP if userfaultfd isn't available (in line with other
>   tests)
>
> * Report FAIL if we failed to open /proc/self/pagemap, as this file
>   has been added a long time ago and doesn't depend on any CONFIG
>   option (returning -EINVAL from main() is meaningless)
>
> Cc: Usama Anjum <Usama.Anjum@arm.com>
> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
> Reviewed-by: Mark Brown <broonie@kernel.org>
> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>

Reviewed-by: Dev Jain <dev.jain@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 80d7c391f8f5..7b214e8755f7 100644
> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
> @@ -1551,7 +1551,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);
>  
> @@ -1560,7 +1560,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();
> @@ -1732,5 +1732,5 @@ int main(int __attribute__((unused)) argc, char *argv[])
>  	zeropfn_tests();
>  
>  	close(pagemap_fd);
> -	ksft_exit_pass();
> +	ksft_finished();
>  }

[PATCH v2 8/8] selftests/mm: report SKIP in pfnmap if a check fails

[Kevin Brodsky]

pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
meaning once for every test, and skips the test if any check fails.

The target file is the same for every test so this is a little
overkill. More importantly, this approach means that the whole suite
will report PASS even if all the tests are skipped because kernel
configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
being mapped, for instance.

Let's ensure that KSFT_SKIP is returned as exit code if any check
fails by performing the checks in pfnmap_init(), run once. That
function also takes care of finding the offset of the pages to be
mapped and saves it in a global. The file is still mapped/unmapped
for every test, as some of them modify the mapping.

Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
 tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 26 deletions(-)

diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
index 35b0e3ed54cd..e41d5464130b 100644
--- a/tools/testing/selftests/mm/pfnmap.c
+++ b/tools/testing/selftests/mm/pfnmap.c
@@ -25,8 +25,11 @@
 #include "kselftest_harness.h"
 #include "vm_util.h"
 
+#define DEV_MEM_NPAGES	2
+
 static sigjmp_buf sigjmp_buf_env;
 static char *file = "/dev/mem";
+static off_t file_offset;
 
 static void signal_handler(int sig)
 {
@@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
 			break;
 
 		/* We need two pages. */
-		if (end > start + 2 * pagesize) {
+		if (end > start + DEV_MEM_NPAGES * pagesize) {
 			fclose(file);
 			*offset = start;
 			return 0;
@@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
 	return -ENOENT;
 }
 
+static void pfnmap_init(void)
+{
+	size_t pagesize = getpagesize();
+	size_t size = DEV_MEM_NPAGES * pagesize;
+	int fd;
+	void *addr;
+
+	if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
+		int err = find_ram_target(&file_offset, pagesize);
+
+		if (err)
+			ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
+				       strerror(-err));
+	} else {
+		file_offset = 0;
+	}
+
+	/*
+	 * Make sure we can open and map the file, and perform some basic
+	 * checks; skip the whole suite if anything goes wrong.
+	 * A fresh mapping is then created for every test case by
+	 * FIXTURE_SETUP(pfnmap).
+	 */
+	fd = open(file, O_RDONLY);
+	if (fd < 0)
+		ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
+
+	addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
+	if (addr == MAP_FAILED)
+		ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
+
+	if (!check_vmflag_pfnmap(addr))
+		ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
+
+	if (test_read_access(addr, size))
+		ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
+
+	munmap(addr, size);
+	close(fd);
+}
+
 FIXTURE(pfnmap)
 {
-	off_t offset;
 	size_t pagesize;
 	int dev_mem_fd;
 	char *addr1;
@@ -112,31 +155,13 @@ FIXTURE_SETUP(pfnmap)
 {
 	self->pagesize = getpagesize();
 
-	if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
-		/* We'll require two physical pages throughout our tests ... */
-		if (find_ram_target(&self->offset, self->pagesize))
-			SKIP(return,
-				   "Cannot find ram target in '/proc/iomem'\n");
-	} else {
-		self->offset = 0;
-	}
-
 	self->dev_mem_fd = open(file, O_RDONLY);
-	if (self->dev_mem_fd < 0)
-		SKIP(return, "Cannot open '%s'\n", file);
+	ASSERT_GE(self->dev_mem_fd, 0);
 
-	self->size1 = self->pagesize * 2;
+	self->size1 = DEV_MEM_NPAGES * self->pagesize;
 	self->addr1 = mmap(NULL, self->size1, PROT_READ, MAP_SHARED,
-			   self->dev_mem_fd, self->offset);
-	if (self->addr1 == MAP_FAILED)
-		SKIP(return, "Cannot mmap '%s'\n", file);
-
-	if (!check_vmflag_pfnmap(self->addr1))
-		SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
-
-	/* ... and want to be able to read from them. */
-	if (test_read_access(self->addr1, self->size1))
-		SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
+			   self->dev_mem_fd, file_offset);
+	ASSERT_NE(self->addr1, MAP_FAILED);
 
 	self->size2 = 0;
 	self->addr2 = MAP_FAILED;
@@ -189,7 +214,7 @@ TEST_F(pfnmap, munmap_split)
 	 */
 	self->size2 = self->pagesize;
 	self->addr2 = mmap(NULL, self->pagesize, PROT_READ, MAP_SHARED,
-			   self->dev_mem_fd, self->offset);
+			   self->dev_mem_fd, file_offset);
 	ASSERT_NE(self->addr2, MAP_FAILED);
 }
 
@@ -258,8 +283,12 @@ int main(int argc, char **argv)
 		if (strcmp(argv[i], "--") == 0) {
 			if (i + 1 < argc && strlen(argv[i + 1]) > 0)
 				file = argv[i + 1];
-			return test_harness_run(i, argv);
+			argc = i;
+			break;
 		}
 	}
+
+	pfnmap_init();
+
 	return test_harness_run(argc, argv);
 }
-- 
2.51.2

Re: [PATCH v2 8/8] selftests/mm: report SKIP in pfnmap if a check fails

[Ryan Roberts]

On 07/01/2026 16:48, Kevin Brodsky wrote:
> pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
> meaning once for every test, and skips the test if any check fails.
> 
> The target file is the same for every test so this is a little
> overkill. More importantly, this approach means that the whole suite
> will report PASS even if all the tests are skipped because kernel
> configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
> being mapped, for instance.
> 
> Let's ensure that KSFT_SKIP is returned as exit code if any check
> fails by performing the checks in pfnmap_init(), run once. That
> function also takes care of finding the offset of the pages to be
> mapped and saves it in a global. The file is still mapped/unmapped
> for every test, as some of them modify the mapping.
> 
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>  tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
>  1 file changed, 55 insertions(+), 26 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
> index 35b0e3ed54cd..e41d5464130b 100644
> --- a/tools/testing/selftests/mm/pfnmap.c
> +++ b/tools/testing/selftests/mm/pfnmap.c
> @@ -25,8 +25,11 @@
>  #include "kselftest_harness.h"
>  #include "vm_util.h"
>  
> +#define DEV_MEM_NPAGES	2
> +
>  static sigjmp_buf sigjmp_buf_env;
>  static char *file = "/dev/mem";
> +static off_t file_offset;
>  
>  static void signal_handler(int sig)
>  {
> @@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
>  			break;
>  
>  		/* We need two pages. */
> -		if (end > start + 2 * pagesize) {
> +		if (end > start + DEV_MEM_NPAGES * pagesize) {
>  			fclose(file);
>  			*offset = start;
>  			return 0;
> @@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
>  	return -ENOENT;
>  }
>  
> +static void pfnmap_init(void)
> +{
> +	size_t pagesize = getpagesize();
> +	size_t size = DEV_MEM_NPAGES * pagesize;
> +	int fd;
> +	void *addr;
> +
> +	if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
> +		int err = find_ram_target(&file_offset, pagesize);
> +
> +		if (err)
> +			ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
> +				       strerror(-err));
> +	} else {
> +		file_offset = 0;
> +	}
> +
> +	/*
> +	 * Make sure we can open and map the file, and perform some basic
> +	 * checks; skip the whole suite if anything goes wrong.
> +	 * A fresh mapping is then created for every test case by
> +	 * FIXTURE_SETUP(pfnmap).
> +	 */
> +	fd = open(file, O_RDONLY);
> +	if (fd < 0)
> +		ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
> +
> +	addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
> +
> +	if (!check_vmflag_pfnmap(addr))
> +		ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
> +
> +	if (test_read_access(addr, size))
> +		ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
> +
> +	munmap(addr, size);
> +	close(fd);
> +}
> +
>  FIXTURE(pfnmap)
>  {
> -	off_t offset;
>  	size_t pagesize;
>  	int dev_mem_fd;
>  	char *addr1;
> @@ -112,31 +155,13 @@ FIXTURE_SETUP(pfnmap)
>  {
>  	self->pagesize = getpagesize();
>  
> -	if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
> -		/* We'll require two physical pages throughout our tests ... */
> -		if (find_ram_target(&self->offset, self->pagesize))
> -			SKIP(return,
> -				   "Cannot find ram target in '/proc/iomem'\n");
> -	} else {
> -		self->offset = 0;
> -	}
> -
>  	self->dev_mem_fd = open(file, O_RDONLY);
> -	if (self->dev_mem_fd < 0)
> -		SKIP(return, "Cannot open '%s'\n", file);
> +	ASSERT_GE(self->dev_mem_fd, 0);
>  
> -	self->size1 = self->pagesize * 2;
> +	self->size1 = DEV_MEM_NPAGES * self->pagesize;
>  	self->addr1 = mmap(NULL, self->size1, PROT_READ, MAP_SHARED,
> -			   self->dev_mem_fd, self->offset);
> -	if (self->addr1 == MAP_FAILED)
> -		SKIP(return, "Cannot mmap '%s'\n", file);
> -
> -	if (!check_vmflag_pfnmap(self->addr1))
> -		SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);

I wonder if we still want this check per-fd, but upgraded to a fail?

Either way:

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

> -
> -	/* ... and want to be able to read from them. */
> -	if (test_read_access(self->addr1, self->size1))
> -		SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
> +			   self->dev_mem_fd, file_offset);
> +	ASSERT_NE(self->addr1, MAP_FAILED);
>  
>  	self->size2 = 0;
>  	self->addr2 = MAP_FAILED;
> @@ -189,7 +214,7 @@ TEST_F(pfnmap, munmap_split)
>  	 */
>  	self->size2 = self->pagesize;
>  	self->addr2 = mmap(NULL, self->pagesize, PROT_READ, MAP_SHARED,
> -			   self->dev_mem_fd, self->offset);
> +			   self->dev_mem_fd, file_offset);
>  	ASSERT_NE(self->addr2, MAP_FAILED);
>  }
>  
> @@ -258,8 +283,12 @@ int main(int argc, char **argv)
>  		if (strcmp(argv[i], "--") == 0) {
>  			if (i + 1 < argc && strlen(argv[i + 1]) > 0)
>  				file = argv[i + 1];
> -			return test_harness_run(i, argv);
> +			argc = i;
> +			break;
>  		}
>  	}
> +
> +	pfnmap_init();
> +
>  	return test_harness_run(argc, argv);
>  }

Re: [PATCH v2 8/8] selftests/mm: report SKIP in pfnmap if a check fails

[Kevin Brodsky]

On 12/01/2026 10:34, Ryan Roberts wrote:
>> -	if (!check_vmflag_pfnmap(self->addr1))
>> -		SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
> I wonder if we still want this check per-fd, but upgraded to a fail?

It seems very unlikely to fail though, I can only think of the
underlying file having changed in between but I really wouldn't expect
that for such special files. I was trying not to duplicate the checks
too much.

- Kevin

Re: [PATCH v2 8/8] selftests/mm: report SKIP in pfnmap if a check fails

[Ryan Roberts]

On 12/01/2026 10:03, Kevin Brodsky wrote:
> On 12/01/2026 10:34, Ryan Roberts wrote:
>>> -	if (!check_vmflag_pfnmap(self->addr1))
>>> -		SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
>> I wonder if we still want this check per-fd, but upgraded to a fail?
> 
> It seems very unlikely to fail though, I can only think of the
> underlying file having changed in between but I really wouldn't expect
> that for such special files. I was trying not to duplicate the checks
> too much.

Fair enough. No strong opinion from me. I just thought that given this is a test
that claims to be checking the behaviour of pfnmaps, then we ought to be pretty
sure we have a pfnmap.

> 
> - Kevin

Re: [PATCH v2 8/8] selftests/mm: report SKIP in pfnmap if a check fails

[David Hildenbrand (Red Hat)]

On 1/7/26 17:48, Kevin Brodsky wrote:
> pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
> meaning once for every test, and skips the test if any check fails.
> 
> The target file is the same for every test so this is a little
> overkill. More importantly, this approach means that the whole suite
> will report PASS even if all the tests are skipped because kernel
> configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
> being mapped, for instance.
> 
> Let's ensure that KSFT_SKIP is returned as exit code if any check
> fails by performing the checks in pfnmap_init(), run once. That
> function also takes care of finding the offset of the pages to be
> mapped and saves it in a global. The file is still mapped/unmapped
> for every test, as some of them modify the mapping.
> 
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>   tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
>   1 file changed, 55 insertions(+), 26 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
> index 35b0e3ed54cd..e41d5464130b 100644
> --- a/tools/testing/selftests/mm/pfnmap.c
> +++ b/tools/testing/selftests/mm/pfnmap.c
> @@ -25,8 +25,11 @@
>   #include "kselftest_harness.h"
>   #include "vm_util.h"
>   
> +#define DEV_MEM_NPAGES	2
> +
>   static sigjmp_buf sigjmp_buf_env;
>   static char *file = "/dev/mem";
> +static off_t file_offset;
>   
>   static void signal_handler(int sig)
>   {
> @@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
>   			break;
>   
>   		/* We need two pages. */
> -		if (end > start + 2 * pagesize) {
> +		if (end > start + DEV_MEM_NPAGES * pagesize) {
>   			fclose(file);
>   			*offset = start;
>   			return 0;
> @@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
>   	return -ENOENT;
>   }
>   
> +static void pfnmap_init(void)
> +{
> +	size_t pagesize = getpagesize();
> +	size_t size = DEV_MEM_NPAGES * pagesize;
> +	int fd;
> +	void *addr;
> +
> +	if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
> +		int err = find_ram_target(&file_offset, pagesize);
> +
> +		if (err)
> +			ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
> +				       strerror(-err));
> +	} else {
> +		file_offset = 0;
> +	}
> +
> +	/*
> +	 * Make sure we can open and map the file, and perform some basic
> +	 * checks; skip the whole suite if anything goes wrong.
> +	 * A fresh mapping is then created for every test case by
> +	 * FIXTURE_SETUP(pfnmap).
> +	 */
> +	fd = open(file, O_RDONLY);
> +	if (fd < 0)
> +		ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
> +
> +	addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
> +
> +	if (!check_vmflag_pfnmap(addr))
> +		ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
> +
> +	if (test_read_access(addr, size))
> +		ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
> +
> +	munmap(addr, size);

Why not keep the fd open then and supply that to all tests without the 
need for them to open/close?

Then, also the file cannot change etc.

-- 
Cheers

David

Re: [PATCH v2 8/8] selftests/mm: report SKIP in pfnmap if a check fails

[Ryan Roberts]

On 19/01/2026 11:16, David Hildenbrand (Red Hat) wrote:
> On 1/7/26 17:48, Kevin Brodsky wrote:
>> pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
>> meaning once for every test, and skips the test if any check fails.
>>
>> The target file is the same for every test so this is a little
>> overkill. More importantly, this approach means that the whole suite
>> will report PASS even if all the tests are skipped because kernel
>> configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
>> being mapped, for instance.
>>
>> Let's ensure that KSFT_SKIP is returned as exit code if any check
>> fails by performing the checks in pfnmap_init(), run once. That
>> function also takes care of finding the offset of the pages to be
>> mapped and saves it in a global. The file is still mapped/unmapped
>> for every test, as some of them modify the mapping.
>>
>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> ---
>>   tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
>>   1 file changed, 55 insertions(+), 26 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/
>> pfnmap.c
>> index 35b0e3ed54cd..e41d5464130b 100644
>> --- a/tools/testing/selftests/mm/pfnmap.c
>> +++ b/tools/testing/selftests/mm/pfnmap.c
>> @@ -25,8 +25,11 @@
>>   #include "kselftest_harness.h"
>>   #include "vm_util.h"
>>   +#define DEV_MEM_NPAGES    2
>> +
>>   static sigjmp_buf sigjmp_buf_env;
>>   static char *file = "/dev/mem";
>> +static off_t file_offset;
>>     static void signal_handler(int sig)
>>   {
>> @@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
>>               break;
>>             /* We need two pages. */
>> -        if (end > start + 2 * pagesize) {
>> +        if (end > start + DEV_MEM_NPAGES * pagesize) {
>>               fclose(file);
>>               *offset = start;
>>               return 0;
>> @@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
>>       return -ENOENT;
>>   }
>>   +static void pfnmap_init(void)
>> +{
>> +    size_t pagesize = getpagesize();
>> +    size_t size = DEV_MEM_NPAGES * pagesize;
>> +    int fd;
>> +    void *addr;
>> +
>> +    if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
>> +        int err = find_ram_target(&file_offset, pagesize);
>> +
>> +        if (err)
>> +            ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
>> +                       strerror(-err));
>> +    } else {
>> +        file_offset = 0;
>> +    }
>> +
>> +    /*
>> +     * Make sure we can open and map the file, and perform some basic
>> +     * checks; skip the whole suite if anything goes wrong.
>> +     * A fresh mapping is then created for every test case by
>> +     * FIXTURE_SETUP(pfnmap).
>> +     */
>> +    fd = open(file, O_RDONLY);
>> +    if (fd < 0)
>> +        ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
>> +
>> +    addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
>> +    if (addr == MAP_FAILED)
>> +        ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
>> +
>> +    if (!check_vmflag_pfnmap(addr))
>> +        ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
>> +
>> +    if (test_read_access(addr, size))
>> +        ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
>> +
>> +    munmap(addr, size);
> 
> Why not keep the fd open then and supply that to all tests without the need for
> them to open/close?
> 
> Then, also the file cannot change etc.

I had a private conversation with Kevin about this before he posted; my very
minor, theorectical concern about that was that it's possible to pass in a
custom file to be pfnmapped and I wondered if such a file could map a device
region that has read side effects? In that case I think you'd want to open it
fresh for each test to ensure consistent starting state?

But if you think that concern is unfounded, certainly just opening it once and
reusing will simplify.

Thanks,
Ryan

Re: [PATCH v2 8/8] selftests/mm: report SKIP in pfnmap if a check fails

[David Hildenbrand (Red Hat)]

On 1/19/26 15:26, Ryan Roberts wrote:
> On 19/01/2026 11:16, David Hildenbrand (Red Hat) wrote:
>> On 1/7/26 17:48, Kevin Brodsky wrote:
>>> pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
>>> meaning once for every test, and skips the test if any check fails.
>>>
>>> The target file is the same for every test so this is a little
>>> overkill. More importantly, this approach means that the whole suite
>>> will report PASS even if all the tests are skipped because kernel
>>> configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
>>> being mapped, for instance.
>>>
>>> Let's ensure that KSFT_SKIP is returned as exit code if any check
>>> fails by performing the checks in pfnmap_init(), run once. That
>>> function also takes care of finding the offset of the pages to be
>>> mapped and saves it in a global. The file is still mapped/unmapped
>>> for every test, as some of them modify the mapping.
>>>
>>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>>> ---
>>>    tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
>>>    1 file changed, 55 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/
>>> pfnmap.c
>>> index 35b0e3ed54cd..e41d5464130b 100644
>>> --- a/tools/testing/selftests/mm/pfnmap.c
>>> +++ b/tools/testing/selftests/mm/pfnmap.c
>>> @@ -25,8 +25,11 @@
>>>    #include "kselftest_harness.h"
>>>    #include "vm_util.h"
>>>    +#define DEV_MEM_NPAGES    2
>>> +
>>>    static sigjmp_buf sigjmp_buf_env;
>>>    static char *file = "/dev/mem";
>>> +static off_t file_offset;
>>>      static void signal_handler(int sig)
>>>    {
>>> @@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
>>>                break;
>>>              /* We need two pages. */
>>> -        if (end > start + 2 * pagesize) {
>>> +        if (end > start + DEV_MEM_NPAGES * pagesize) {
>>>                fclose(file);
>>>                *offset = start;
>>>                return 0;
>>> @@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
>>>        return -ENOENT;
>>>    }
>>>    +static void pfnmap_init(void)
>>> +{
>>> +    size_t pagesize = getpagesize();
>>> +    size_t size = DEV_MEM_NPAGES * pagesize;
>>> +    int fd;
>>> +    void *addr;
>>> +
>>> +    if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
>>> +        int err = find_ram_target(&file_offset, pagesize);
>>> +
>>> +        if (err)
>>> +            ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
>>> +                       strerror(-err));
>>> +    } else {
>>> +        file_offset = 0;
>>> +    }
>>> +
>>> +    /*
>>> +     * Make sure we can open and map the file, and perform some basic
>>> +     * checks; skip the whole suite if anything goes wrong.
>>> +     * A fresh mapping is then created for every test case by
>>> +     * FIXTURE_SETUP(pfnmap).
>>> +     */
>>> +    fd = open(file, O_RDONLY);
>>> +    if (fd < 0)
>>> +        ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
>>> +
>>> +    addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
>>> +    if (addr == MAP_FAILED)
>>> +        ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
>>> +
>>> +    if (!check_vmflag_pfnmap(addr))
>>> +        ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
>>> +
>>> +    if (test_read_access(addr, size))
>>> +        ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
>>> +
>>> +    munmap(addr, size);
>>
>> Why not keep the fd open then and supply that to all tests without the need for
>> them to open/close?
>>
>> Then, also the file cannot change etc.
> 
> I had a private conversation with Kevin about this before he posted; my very
> minor, theorectical concern about that was that it's possible to pass in a
> custom file to be pfnmapped and I wondered if such a file could map a device
> region that has read side effects? In that case I think you'd want to open it
> fresh for each test to ensure consistent starting state?

Are we aware of devices where we would actually require a new open, and 
not just a new mmap()?

The reason we added support for other files was "other pfnmap'ed memory 
like NVIDIA's EGM". I'd assume that people rather should not pass in 
something that has any side-effects.

> 
> But if you think that concern is unfounded, certainly just opening it once and
> reusing will simplify.

I would just keep it simple here, yes. If this ever becomes a real 
problem, my intuition would tell me that probably the caller is doing 
something unsupported that we just cannot easily identify+reject.

-- 
Cheers

David

Re: [PATCH v2 8/8] selftests/mm: report SKIP in pfnmap if a check fails

[Ryan Roberts]

On 19/01/2026 14:32, David Hildenbrand (Red Hat) wrote:
> On 1/19/26 15:26, Ryan Roberts wrote:
>> On 19/01/2026 11:16, David Hildenbrand (Red Hat) wrote:
>>> On 1/7/26 17:48, Kevin Brodsky wrote:
>>>> pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
>>>> meaning once for every test, and skips the test if any check fails.
>>>>
>>>> The target file is the same for every test so this is a little
>>>> overkill. More importantly, this approach means that the whole suite
>>>> will report PASS even if all the tests are skipped because kernel
>>>> configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
>>>> being mapped, for instance.
>>>>
>>>> Let's ensure that KSFT_SKIP is returned as exit code if any check
>>>> fails by performing the checks in pfnmap_init(), run once. That
>>>> function also takes care of finding the offset of the pages to be
>>>> mapped and saves it in a global. The file is still mapped/unmapped
>>>> for every test, as some of them modify the mapping.
>>>>
>>>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>>>> ---
>>>>    tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
>>>>    1 file changed, 55 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/
>>>> pfnmap.c
>>>> index 35b0e3ed54cd..e41d5464130b 100644
>>>> --- a/tools/testing/selftests/mm/pfnmap.c
>>>> +++ b/tools/testing/selftests/mm/pfnmap.c
>>>> @@ -25,8 +25,11 @@
>>>>    #include "kselftest_harness.h"
>>>>    #include "vm_util.h"
>>>>    +#define DEV_MEM_NPAGES    2
>>>> +
>>>>    static sigjmp_buf sigjmp_buf_env;
>>>>    static char *file = "/dev/mem";
>>>> +static off_t file_offset;
>>>>      static void signal_handler(int sig)
>>>>    {
>>>> @@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
>>>>                break;
>>>>              /* We need two pages. */
>>>> -        if (end > start + 2 * pagesize) {
>>>> +        if (end > start + DEV_MEM_NPAGES * pagesize) {
>>>>                fclose(file);
>>>>                *offset = start;
>>>>                return 0;
>>>> @@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
>>>>        return -ENOENT;
>>>>    }
>>>>    +static void pfnmap_init(void)
>>>> +{
>>>> +    size_t pagesize = getpagesize();
>>>> +    size_t size = DEV_MEM_NPAGES * pagesize;
>>>> +    int fd;
>>>> +    void *addr;
>>>> +
>>>> +    if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
>>>> +        int err = find_ram_target(&file_offset, pagesize);
>>>> +
>>>> +        if (err)
>>>> +            ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
>>>> +                       strerror(-err));
>>>> +    } else {
>>>> +        file_offset = 0;
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * Make sure we can open and map the file, and perform some basic
>>>> +     * checks; skip the whole suite if anything goes wrong.
>>>> +     * A fresh mapping is then created for every test case by
>>>> +     * FIXTURE_SETUP(pfnmap).
>>>> +     */
>>>> +    fd = open(file, O_RDONLY);
>>>> +    if (fd < 0)
>>>> +        ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
>>>> +
>>>> +    addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
>>>> +    if (addr == MAP_FAILED)
>>>> +        ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
>>>> +
>>>> +    if (!check_vmflag_pfnmap(addr))
>>>> +        ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
>>>> +
>>>> +    if (test_read_access(addr, size))
>>>> +        ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
>>>> +
>>>> +    munmap(addr, size);
>>>
>>> Why not keep the fd open then and supply that to all tests without the need for
>>> them to open/close?
>>>
>>> Then, also the file cannot change etc.
>>
>> I had a private conversation with Kevin about this before he posted; my very
>> minor, theorectical concern about that was that it's possible to pass in a
>> custom file to be pfnmapped and I wondered if such a file could map a device
>> region that has read side effects? In that case I think you'd want to open it
>> fresh for each test to ensure consistent starting state?
> 
> Are we aware of devices where we would actually require a new open, and not just
> a new mmap()?

Nope; as I said all hypothetical. I was just being cautious.

> 
> The reason we added support for other files was "other pfnmap'ed memory like
> NVIDIA's EGM". I'd assume that people rather should not pass in something that
> has any side-effects.
> 
>>
>> But if you think that concern is unfounded, certainly just opening it once and
>> reusing will simplify.
> 
> I would just keep it simple here, yes. If this ever becomes a real problem, my
> intuition would tell me that probably the caller is doing something unsupported
> that we just cannot easily identify+reject.

Yeah fair enough.


Thanks,
Ryan

Re: [PATCH v2 8/8] selftests/mm: report SKIP in pfnmap if a check fails

[Kevin Brodsky]

On 20/01/2026 17:27, Ryan Roberts wrote:
>>> But if you think that concern is unfounded, certainly just opening it once and
>>> reusing will simplify.
>> I would just keep it simple here, yes. If this ever becomes a real problem, my
>> intuition would tell me that probably the caller is doing something unsupported
>> that we just cannot easily identify+reject.
> Yeah fair enough.

Cool, will make the fd a global as well then.

- Kevin