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
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
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
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
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.
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
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
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.
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
On Mon, Dec 29, 2025 at 04:40:26PM +0100, Kevin Brodsky wrote:
On 18/12/2025 15:25, Mark Brown wrote:
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).
That's really not the expected usage pattern, I'd be surprised if a non-trivial propoprtion of kselftest builds were intended to be run on the system they're built on - a lot of people test interactively in VMs, or on some other target hardware, and automated systems are going to be building separately. The two you've identified look like special snowflakes TBH (livepatch in particular has a bunch of other issues due to what it's trying to do).
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?
I think defaulting to something related to the current kernel build is more sensible here.
On 05/01/2026 13:46, Mark Brown wrote:
On Mon, Dec 29, 2025 at 04:40:26PM +0100, Kevin Brodsky wrote:
On 18/12/2025 15:25, Mark Brown wrote:
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).
That's really not the expected usage pattern, I'd be surprised if a non-trivial propoprtion of kselftest builds were intended to be run on the system they're built on - a lot of people test interactively in VMs, or on some other target hardware, and automated systems are going to be building separately. The two you've identified look like special snowflakes TBH (livepatch in particular has a bunch of other issues due to what it's trying to do).
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?
I think defaulting to something related to the current kernel build is more sensible here.
Fair enough, fine by me. Then I'll change the default to the output directory and remove the header check.
- Kevin
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
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:
- CC consists of only one word
- 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
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);
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);
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
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?
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
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.
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
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(); }
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();
}
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
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
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
linux-kselftest-mirror@lists.linaro.org
-
David Hildenbrand (Red Hat) -
Kevin Brodsky -
Mark Brown -
Ryan Roberts -
Yunsheng Lin