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
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
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
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();
}
linux-kselftest-mirror@lists.linaro.org
-
Kevin Brodsky -
Ryan Roberts