* [PATCH 0/6] selftests/mm: Some cleanups from trying to run them
@ 2025-02-20 15:03 Brendan Jackman
2025-02-20 15:03 ` [PATCH 1/6] selftests/mm: Report errno when things fail Brendan Jackman
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Brendan Jackman @ 2025-02-20 15:03 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, Shuah Khan
Cc: Mateusz Guzik, linux-mm, linux-kselftest, linux-kernel, Brendan Jackman
I never had much luck running mm selftests so I spent a couple of hours
digging into why.
Looks like most of the reason is missing SKIP checks, so this series is
just adding a bunch of those that I found. I did not do anything like
all of them, just the ones I spotted in gup_test, mmap, userfaultfd and
memfd_secret.
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
Brendan Jackman (6):
selftests/mm: Report errno when things fail
selftests/mm: Fix assumption that sudo is present
selftests/mm: Skip uffd-stress if userfaultfd not available
selftests/mm: Skip uffd-wp-mremap if userfaultfd not available
selftests/mm: Print some details when uffd-stress gets bad params
selftests/mm: Don't fail uffd-stress if too many CPUs
tools/testing/selftests/mm/gup_longterm.c | 32 ++++++++++++++---------------
tools/testing/selftests/mm/run_vmtests.sh | 22 ++++++++++++++++----
tools/testing/selftests/mm/uffd-stress.c | 11 +++++++---
tools/testing/selftests/mm/uffd-wp-mremap.c | 5 ++++-
4 files changed, 46 insertions(+), 24 deletions(-)
---
base-commit: 87a132e73910e8689902aed7f2fc229d6908383b
change-id: 20250220-mm-selftests-2d7d0542face
Best regards,
--
Brendan Jackman <jackmanb@google.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/6] selftests/mm: Report errno when things fail
2025-02-20 15:03 [PATCH 0/6] selftests/mm: Some cleanups from trying to run them Brendan Jackman
@ 2025-02-20 15:03 ` Brendan Jackman
2025-02-20 15:32 ` Dev Jain
2025-02-20 15:03 ` [PATCH 2/6] selftests/mm: Fix assumption that sudo is present Brendan Jackman
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Brendan Jackman @ 2025-02-20 15:03 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, Shuah Khan
Cc: Mateusz Guzik, linux-mm, linux-kselftest, linux-kernel, Brendan Jackman
Just reporting failure doesn't tell you what went wrong. This can fail
in different ways so report errno to help the reader get started
debugging.
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
tools/testing/selftests/mm/gup_longterm.c | 32 +++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/mm/gup_longterm.c b/tools/testing/selftests/mm/gup_longterm.c
index 9423ad439a6140163bdef2974615bb86406a8c14..46a2139b3a646f6c050eb031a770f615be76c433 100644
--- a/tools/testing/selftests/mm/gup_longterm.c
+++ b/tools/testing/selftests/mm/gup_longterm.c
@@ -96,13 +96,13 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
int ret;
if (ftruncate(fd, size)) {
- ksft_test_result_fail("ftruncate() failed\n");
+ ksft_test_result_fail("ftruncate() failed (%d)\n", errno);
return;
}
if (fallocate(fd, 0, 0, size)) {
if (size == pagesize)
- ksft_test_result_fail("fallocate() failed\n");
+ ksft_test_result_fail("fallocate() failed (%d)\n", errno);
else
ksft_test_result_skip("need more free huge pages\n");
return;
@@ -112,7 +112,7 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
shared ? MAP_SHARED : MAP_PRIVATE, fd, 0);
if (mem == MAP_FAILED) {
if (size == pagesize || shared)
- ksft_test_result_fail("mmap() failed\n");
+ ksft_test_result_fail("mmap() failed (%d)\n", errno);
else
ksft_test_result_skip("need more free huge pages\n");
return;
@@ -130,7 +130,7 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
*/
ret = mprotect(mem, size, PROT_READ);
if (ret) {
- ksft_test_result_fail("mprotect() failed\n");
+ ksft_test_result_fail("mprotect() failed (%d)\n", errno);
goto munmap;
}
/* FALLTHROUGH */
@@ -165,18 +165,18 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
args.flags |= rw ? PIN_LONGTERM_TEST_FLAG_USE_WRITE : 0;
ret = ioctl(gup_fd, PIN_LONGTERM_TEST_START, &args);
if (ret && errno == EINVAL) {
- ksft_test_result_skip("PIN_LONGTERM_TEST_START failed\n");
+ ksft_test_result_skip("PIN_LONGTERM_TEST_START failed (EINVAL)n");
break;
} else if (ret && errno == EFAULT) {
ksft_test_result(!should_work, "Should have failed\n");
break;
} else if (ret) {
- ksft_test_result_fail("PIN_LONGTERM_TEST_START failed\n");
+ ksft_test_result_fail("PIN_LONGTERM_TEST_START failed (%d)\n", errno);
break;
}
if (ioctl(gup_fd, PIN_LONGTERM_TEST_STOP))
- ksft_print_msg("[INFO] PIN_LONGTERM_TEST_STOP failed\n");
+ ksft_print_msg("[INFO] PIN_LONGTERM_TEST_STOP failed (%d)\n", errno);
/*
* TODO: if the kernel ever supports long-term R/W pinning on
@@ -202,7 +202,7 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
/* Skip on errors, as we might just lack kernel support. */
ret = io_uring_queue_init(1, &ring, 0);
if (ret < 0) {
- ksft_test_result_skip("io_uring_queue_init() failed\n");
+ ksft_test_result_skip("io_uring_queue_init() failed (%d)\n", errno);
break;
}
/*
@@ -215,13 +215,13 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
/* Only new kernels return EFAULT. */
if (ret && (errno == ENOSPC || errno == EOPNOTSUPP ||
errno == EFAULT)) {
- ksft_test_result(!should_work, "Should have failed\n");
+ ksft_test_result(!should_work, "Should have failed (%d)\n", errno);
} else if (ret) {
/*
* We might just lack support or have insufficient
* MEMLOCK limits.
*/
- ksft_test_result_skip("io_uring_register_buffers() failed\n");
+ ksft_test_result_skip("io_uring_register_buffers() failed (%d)\n", errno);
} else {
ksft_test_result(should_work, "Should have worked\n");
io_uring_unregister_buffers(&ring);
@@ -249,7 +249,7 @@ static void run_with_memfd(test_fn fn, const char *desc)
fd = memfd_create("test", 0);
if (fd < 0) {
- ksft_test_result_fail("memfd_create() failed\n");
+ ksft_test_result_fail("memfd_create() failed (%d)\n", errno);
return;
}
@@ -266,13 +266,13 @@ static void run_with_tmpfile(test_fn fn, const char *desc)
file = tmpfile();
if (!file) {
- ksft_test_result_fail("tmpfile() failed\n");
+ ksft_test_result_fail("tmpfile() failed (%d)\n", errno);
return;
}
fd = fileno(file);
if (fd < 0) {
- ksft_test_result_fail("fileno() failed\n");
+ ksft_test_result_fail("fileno() failed (%d)\n", errno);
goto close;
}
@@ -290,12 +290,12 @@ static void run_with_local_tmpfile(test_fn fn, const char *desc)
fd = mkstemp(filename);
if (fd < 0) {
- ksft_test_result_fail("mkstemp() failed\n");
+ ksft_test_result_fail("mkstemp() failed (%d)\n", errno);
return;
}
if (unlink(filename)) {
- ksft_test_result_fail("unlink() failed\n");
+ ksft_test_result_fail("unlink() failed (%d)\n", errno);
goto close;
}
@@ -317,7 +317,7 @@ static void run_with_memfd_hugetlb(test_fn fn, const char *desc,
fd = memfd_create("test", flags);
if (fd < 0) {
- ksft_test_result_skip("memfd_create() failed\n");
+ ksft_test_result_skip("memfd_create() failed (%d)\n", errno);
return;
}
--
2.48.1.601.g30ceb7b040-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/6] selftests/mm: Fix assumption that sudo is present
2025-02-20 15:03 [PATCH 0/6] selftests/mm: Some cleanups from trying to run them Brendan Jackman
2025-02-20 15:03 ` [PATCH 1/6] selftests/mm: Report errno when things fail Brendan Jackman
@ 2025-02-20 15:03 ` Brendan Jackman
2025-02-20 15:03 ` [PATCH 3/6] selftests/mm: Skip uffd-stress if userfaultfd not available Brendan Jackman
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Brendan Jackman @ 2025-02-20 15:03 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, Shuah Khan
Cc: Mateusz Guzik, linux-mm, linux-kselftest, linux-kernel, Brendan Jackman
If we are root, sudo isn't needed. If we are not root, we need sudo, so
skip the test if it isn't present.
We already do this for on-fault-limit, but this uses separate
infrastructure since that is specifically for sudo-ing to the nobody
user.
Note this ptrace_skip configuration still fails if that file doesn't
exist, but in that case the test is still fine, so this just prints an
error but doesn't break anything. I suspect that's probably deliberate.
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
tools/testing/selftests/mm/run_vmtests.sh | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index da7e266681031d2772fb0c4139648904a18e0bf9..9c963f50927ab2b10c3f942cedd087087d4d0def 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -89,6 +89,17 @@ RUN_ALL=false
RUN_DESTRUCTIVE=false
TAP_PREFIX="# "
+# We can do stuff as root either if we are already root, or if sudo exists.
+if [ "$(id -u)" == 0 ]; then
+ HAVE_SUDO_ROOT=true
+ SUDO_ROOT=
+elif command -v sudo >/dev/null 2>&1; then
+ HAVE_SUDO_ROOT=true
+ SUDO_ROOT=sudo
+else
+ HAVE_SUDO_ROOT=false
+fi
+
while getopts "aht:n" OPT; do
case ${OPT} in
"a") RUN_ALL=true ;;
@@ -384,10 +395,13 @@ CATEGORY="madv_guard" run_test ./guard-pages
# MADV_POPULATE_READ and MADV_POPULATE_WRITE tests
CATEGORY="madv_populate" run_test ./madv_populate
-if [ -x ./memfd_secret ]
-then
-(echo 0 | sudo tee /proc/sys/kernel/yama/ptrace_scope 2>&1) | tap_prefix
-CATEGORY="memfd_secret" run_test ./memfd_secret
+if [ -x ./memfd_secret ]; then
+ if $HAVE_SUDO_ROOT; then
+ (echo 0 | $SUDO_ROOT tee /proc/sys/kernel/yama/ptrace_scope 2>&1) | tap_prefix
+ CATEGORY="memfd_secret" run_test ./memfd_secret
+ else
+ echo "# SKIP ./memfd_secret"
+ fi
fi
# KSM KSM_MERGE_TIME_HUGE_PAGES test with size of 100
--
2.48.1.601.g30ceb7b040-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/6] selftests/mm: Skip uffd-stress if userfaultfd not available
2025-02-20 15:03 [PATCH 0/6] selftests/mm: Some cleanups from trying to run them Brendan Jackman
2025-02-20 15:03 ` [PATCH 1/6] selftests/mm: Report errno when things fail Brendan Jackman
2025-02-20 15:03 ` [PATCH 2/6] selftests/mm: Fix assumption that sudo is present Brendan Jackman
@ 2025-02-20 15:03 ` Brendan Jackman
2025-02-20 18:06 ` Dev Jain
2025-02-20 15:03 ` [PATCH 4/6] selftests/mm: Skip uffd-wp-mremap " Brendan Jackman
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Brendan Jackman @ 2025-02-20 15:03 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, Shuah Khan
Cc: Mateusz Guzik, linux-mm, linux-kselftest, linux-kernel, Brendan Jackman
It's pretty obvious that the test wouldn't work if you don't have the
feature enabled. But, it's still useful to SKIP instead of failing so
the reader can immediately tell that this is the reason why.
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
tools/testing/selftests/mm/uffd-stress.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c
index a4b83280998ab7ce8d31e91d8f9fbb47ef11d742..db5366b4766e5bfa2d1150d2f3c2d32469a6e28b 100644
--- a/tools/testing/selftests/mm/uffd-stress.c
+++ b/tools/testing/selftests/mm/uffd-stress.c
@@ -411,8 +411,8 @@ static void parse_test_type_arg(const char *raw_type)
* feature.
*/
- if (uffd_get_features(&features))
- err("failed to get available features");
+ if (uffd_get_features(&features) && errno == ENOENT)
+ ksft_exit_skip("failed to get avialable features (%d)\n", errno);
test_uffdio_wp = test_uffdio_wp &&
(features & UFFD_FEATURE_PAGEFAULT_FLAG_WP);
--
2.48.1.601.g30ceb7b040-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/6] selftests/mm: Skip uffd-wp-mremap if userfaultfd not available
2025-02-20 15:03 [PATCH 0/6] selftests/mm: Some cleanups from trying to run them Brendan Jackman
` (2 preceding siblings ...)
2025-02-20 15:03 ` [PATCH 3/6] selftests/mm: Skip uffd-stress if userfaultfd not available Brendan Jackman
@ 2025-02-20 15:03 ` Brendan Jackman
2025-02-20 15:03 ` [PATCH 5/6] selftests/mm: Print some details when uffd-stress gets bad params Brendan Jackman
2025-02-20 15:03 ` [PATCH 6/6] selftests/mm: Don't fail uffd-stress if too many CPUs Brendan Jackman
5 siblings, 0 replies; 14+ messages in thread
From: Brendan Jackman @ 2025-02-20 15:03 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, Shuah Khan
Cc: Mateusz Guzik, linux-mm, linux-kselftest, linux-kernel, Brendan Jackman
It's obvious that this should fail in that case, but still, save the
reader the effort of figuring out that they've run into this by just
SKIPping
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
tools/testing/selftests/mm/uffd-wp-mremap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/uffd-wp-mremap.c b/tools/testing/selftests/mm/uffd-wp-mremap.c
index 2c4f984bd73caa17e12b9f4a5bb71e7fdf5d8554..c2ba7d46c7b4581a3c32a6b6acd148e3e89c2172 100644
--- a/tools/testing/selftests/mm/uffd-wp-mremap.c
+++ b/tools/testing/selftests/mm/uffd-wp-mremap.c
@@ -182,7 +182,10 @@ static void test_one_folio(size_t size, bool private, bool swapout, bool hugetlb
/* Register range for uffd-wp. */
if (userfaultfd_open(&features)) {
- ksft_test_result_fail("userfaultfd_open() failed\n");
+ if (errno == ENOENT)
+ ksft_test_result_skip("userfaultfd not available\n");
+ else
+ ksft_test_result_fail("userfaultfd_open() failed\n");
goto out;
}
if (uffd_register(uffd, mem, size, false, true, false)) {
--
2.48.1.601.g30ceb7b040-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/6] selftests/mm: Print some details when uffd-stress gets bad params
2025-02-20 15:03 [PATCH 0/6] selftests/mm: Some cleanups from trying to run them Brendan Jackman
` (3 preceding siblings ...)
2025-02-20 15:03 ` [PATCH 4/6] selftests/mm: Skip uffd-wp-mremap " Brendan Jackman
@ 2025-02-20 15:03 ` Brendan Jackman
2025-02-20 15:17 ` Brendan Jackman
2025-02-20 15:03 ` [PATCH 6/6] selftests/mm: Don't fail uffd-stress if too many CPUs Brendan Jackman
5 siblings, 1 reply; 14+ messages in thread
From: Brendan Jackman @ 2025-02-20 15:03 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, Shuah Khan
Cc: Mateusz Guzik, linux-mm, linux-kselftest, linux-kernel, Brendan Jackman
So this can be debugged more easily.
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
tools/testing/selftests/mm/uffd-stress.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c
index db5366b4766e5bfa2d1150d2f3c2d32469a6e28b..1facfb79e09aa4113e344d7d90dec06a37264058 100644
--- a/tools/testing/selftests/mm/uffd-stress.c
+++ b/tools/testing/selftests/mm/uffd-stress.c
@@ -456,7 +456,8 @@ int main(int argc, char **argv)
nr_pages_per_cpu = bytes / page_size / nr_cpus;
if (!nr_pages_per_cpu) {
- _err("invalid MiB");
+ _err("invalid MiB %lu (%lu / %lu / %lu)",
+ nr_pages_per_cpu, bytes, page_size, nr_cpus);
usage();
}
--
2.48.1.601.g30ceb7b040-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 6/6] selftests/mm: Don't fail uffd-stress if too many CPUs
2025-02-20 15:03 [PATCH 0/6] selftests/mm: Some cleanups from trying to run them Brendan Jackman
` (4 preceding siblings ...)
2025-02-20 15:03 ` [PATCH 5/6] selftests/mm: Print some details when uffd-stress gets bad params Brendan Jackman
@ 2025-02-20 15:03 ` Brendan Jackman
2025-02-20 15:48 ` Dev Jain
5 siblings, 1 reply; 14+ messages in thread
From: Brendan Jackman @ 2025-02-20 15:03 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, Shuah Khan
Cc: Mateusz Guzik, linux-mm, linux-kselftest, linux-kernel, Brendan Jackman
This calculation divides a fixed parameter by an environment-dependent
parameter i.e. the number of CPUs.
The simple way to avoid machine-specific failures here is to just put a
cap on the max value of the latter.
Suggested-by: Mateusz Guzik <mjguzik@gmail.com>
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
tools/testing/selftests/mm/uffd-stress.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c
index 1facfb79e09aa4113e344d7d90dec06a37264058..f306accbef255c79bc3eeba8b9e42161a88fc10e 100644
--- a/tools/testing/selftests/mm/uffd-stress.c
+++ b/tools/testing/selftests/mm/uffd-stress.c
@@ -453,6 +453,10 @@ int main(int argc, char **argv)
}
nr_cpus = sysconf(_SC_NPROCESSORS_ONLN);
+ if (nr_cpus > 32) {
+ /* Don't let calculation below go to zero. */
+ nr_cpus = 32;
+ }
nr_pages_per_cpu = bytes / page_size / nr_cpus;
if (!nr_pages_per_cpu) {
--
2.48.1.601.g30ceb7b040-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/6] selftests/mm: Print some details when uffd-stress gets bad params
2025-02-20 15:03 ` [PATCH 5/6] selftests/mm: Print some details when uffd-stress gets bad params Brendan Jackman
@ 2025-02-20 15:17 ` Brendan Jackman
0 siblings, 0 replies; 14+ messages in thread
From: Brendan Jackman @ 2025-02-20 15:17 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, Shuah Khan
Cc: Mateusz Guzik, linux-mm, linux-kselftest, linux-kernel
On Thu, 20 Feb 2025 at 16:03, Brendan Jackman <jackmanb@google.com> wrote:
> nr_pages_per_cpu = bytes / page_size / nr_cpus;
> if (!nr_pages_per_cpu) {
> - _err("invalid MiB");
> + _err("invalid MiB %lu (%lu / %lu / %lu)",
> + nr_pages_per_cpu, bytes, page_size, nr_cpus);
Oh this is actually wrong - the number it's printing isn't MiB.
Assuming there's a v2 I'll fix that up. Otherwise honestly this patch
could just be dropped, it's not important.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] selftests/mm: Report errno when things fail
2025-02-20 15:03 ` [PATCH 1/6] selftests/mm: Report errno when things fail Brendan Jackman
@ 2025-02-20 15:32 ` Dev Jain
0 siblings, 0 replies; 14+ messages in thread
From: Dev Jain @ 2025-02-20 15:32 UTC (permalink / raw)
To: Brendan Jackman, Lorenzo Stoakes, Andrew Morton, Shuah Khan
Cc: Mateusz Guzik, linux-mm, linux-kselftest, linux-kernel
On 20/02/25 8:33 pm, Brendan Jackman wrote:
> Just reporting failure doesn't tell you what went wrong. This can fail
> in different ways so report errno to help the reader get started
> debugging.
IMHO it would be even better if we reported strerror(errno).
>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
> tools/testing/selftests/mm/gup_longterm.c | 32 +++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/gup_longterm.c b/tools/testing/selftests/mm/gup_longterm.c
> index 9423ad439a6140163bdef2974615bb86406a8c14..46a2139b3a646f6c050eb031a770f615be76c433 100644
> --- a/tools/testing/selftests/mm/gup_longterm.c
> +++ b/tools/testing/selftests/mm/gup_longterm.c
> @@ -96,13 +96,13 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> int ret;
>
> if (ftruncate(fd, size)) {
> - ksft_test_result_fail("ftruncate() failed\n");
> + ksft_test_result_fail("ftruncate() failed (%d)\n", errno);
> return;
> }
>
> if (fallocate(fd, 0, 0, size)) {
> if (size == pagesize)
> - ksft_test_result_fail("fallocate() failed\n");
> + ksft_test_result_fail("fallocate() failed (%d)\n", errno);
> else
> ksft_test_result_skip("need more free huge pages\n");
> return;
> @@ -112,7 +112,7 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> shared ? MAP_SHARED : MAP_PRIVATE, fd, 0);
> if (mem == MAP_FAILED) {
> if (size == pagesize || shared)
> - ksft_test_result_fail("mmap() failed\n");
> + ksft_test_result_fail("mmap() failed (%d)\n", errno);
> else
> ksft_test_result_skip("need more free huge pages\n");
> return;
> @@ -130,7 +130,7 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> */
> ret = mprotect(mem, size, PROT_READ);
> if (ret) {
> - ksft_test_result_fail("mprotect() failed\n");
> + ksft_test_result_fail("mprotect() failed (%d)\n", errno);
> goto munmap;
> }
> /* FALLTHROUGH */
> @@ -165,18 +165,18 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> args.flags |= rw ? PIN_LONGTERM_TEST_FLAG_USE_WRITE : 0;
> ret = ioctl(gup_fd, PIN_LONGTERM_TEST_START, &args);
> if (ret && errno == EINVAL) {
> - ksft_test_result_skip("PIN_LONGTERM_TEST_START failed\n");
> + ksft_test_result_skip("PIN_LONGTERM_TEST_START failed (EINVAL)n");
> break;
> } else if (ret && errno == EFAULT) {
> ksft_test_result(!should_work, "Should have failed\n");
> break;
> } else if (ret) {
> - ksft_test_result_fail("PIN_LONGTERM_TEST_START failed\n");
> + ksft_test_result_fail("PIN_LONGTERM_TEST_START failed (%d)\n", errno);
> break;
> }
>
> if (ioctl(gup_fd, PIN_LONGTERM_TEST_STOP))
> - ksft_print_msg("[INFO] PIN_LONGTERM_TEST_STOP failed\n");
> + ksft_print_msg("[INFO] PIN_LONGTERM_TEST_STOP failed (%d)\n", errno);
>
> /*
> * TODO: if the kernel ever supports long-term R/W pinning on
> @@ -202,7 +202,7 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> /* Skip on errors, as we might just lack kernel support. */
> ret = io_uring_queue_init(1, &ring, 0);
> if (ret < 0) {
> - ksft_test_result_skip("io_uring_queue_init() failed\n");
> + ksft_test_result_skip("io_uring_queue_init() failed (%d)\n", errno);
> break;
> }
> /*
> @@ -215,13 +215,13 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> /* Only new kernels return EFAULT. */
> if (ret && (errno == ENOSPC || errno == EOPNOTSUPP ||
> errno == EFAULT)) {
> - ksft_test_result(!should_work, "Should have failed\n");
> + ksft_test_result(!should_work, "Should have failed (%d)\n", errno);
> } else if (ret) {
> /*
> * We might just lack support or have insufficient
> * MEMLOCK limits.
> */
> - ksft_test_result_skip("io_uring_register_buffers() failed\n");
> + ksft_test_result_skip("io_uring_register_buffers() failed (%d)\n", errno);
> } else {
> ksft_test_result(should_work, "Should have worked\n");
> io_uring_unregister_buffers(&ring);
> @@ -249,7 +249,7 @@ static void run_with_memfd(test_fn fn, const char *desc)
>
> fd = memfd_create("test", 0);
> if (fd < 0) {
> - ksft_test_result_fail("memfd_create() failed\n");
> + ksft_test_result_fail("memfd_create() failed (%d)\n", errno);
> return;
> }
>
> @@ -266,13 +266,13 @@ static void run_with_tmpfile(test_fn fn, const char *desc)
>
> file = tmpfile();
> if (!file) {
> - ksft_test_result_fail("tmpfile() failed\n");
> + ksft_test_result_fail("tmpfile() failed (%d)\n", errno);
> return;
> }
>
> fd = fileno(file);
> if (fd < 0) {
> - ksft_test_result_fail("fileno() failed\n");
> + ksft_test_result_fail("fileno() failed (%d)\n", errno);
> goto close;
> }
>
> @@ -290,12 +290,12 @@ static void run_with_local_tmpfile(test_fn fn, const char *desc)
>
> fd = mkstemp(filename);
> if (fd < 0) {
> - ksft_test_result_fail("mkstemp() failed\n");
> + ksft_test_result_fail("mkstemp() failed (%d)\n", errno);
> return;
> }
>
> if (unlink(filename)) {
> - ksft_test_result_fail("unlink() failed\n");
> + ksft_test_result_fail("unlink() failed (%d)\n", errno);
> goto close;
> }
>
> @@ -317,7 +317,7 @@ static void run_with_memfd_hugetlb(test_fn fn, const char *desc,
>
> fd = memfd_create("test", flags);
> if (fd < 0) {
> - ksft_test_result_skip("memfd_create() failed\n");
> + ksft_test_result_skip("memfd_create() failed (%d)\n", errno);
> return;
> }
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 6/6] selftests/mm: Don't fail uffd-stress if too many CPUs
2025-02-20 15:03 ` [PATCH 6/6] selftests/mm: Don't fail uffd-stress if too many CPUs Brendan Jackman
@ 2025-02-20 15:48 ` Dev Jain
2025-02-20 15:55 ` Brendan Jackman
0 siblings, 1 reply; 14+ messages in thread
From: Dev Jain @ 2025-02-20 15:48 UTC (permalink / raw)
To: Brendan Jackman, Lorenzo Stoakes, Andrew Morton, Shuah Khan
Cc: Mateusz Guzik, linux-mm, linux-kselftest, linux-kernel
On 20/02/25 8:33 pm, Brendan Jackman wrote:
> This calculation divides a fixed parameter by an environment-dependent
> parameter i.e. the number of CPUs.
>
> The simple way to avoid machine-specific failures here is to just put a
> cap on the max value of the latter.
I haven't read the test, but if nr_cpus is being computed, then this
value must be important to the test somehow? Would it potentially be
wrong to let the test run for nr_cpus != actual number of cpus?
Also, if the patch is correct then will it be better to also print a
diagnostic telling the user that the number of cpus is going to be
capped for the test to run?
>
> Suggested-by: Mateusz Guzik <mjguzik@gmail.com>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
> tools/testing/selftests/mm/uffd-stress.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c
> index 1facfb79e09aa4113e344d7d90dec06a37264058..f306accbef255c79bc3eeba8b9e42161a88fc10e 100644
> --- a/tools/testing/selftests/mm/uffd-stress.c
> +++ b/tools/testing/selftests/mm/uffd-stress.c
> @@ -453,6 +453,10 @@ int main(int argc, char **argv)
> }
>
> nr_cpus = sysconf(_SC_NPROCESSORS_ONLN);
> + if (nr_cpus > 32) {
> + /* Don't let calculation below go to zero. */
> + nr_cpus = 32;
> + }
>
> nr_pages_per_cpu = bytes / page_size / nr_cpus;
> if (!nr_pages_per_cpu) {
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 6/6] selftests/mm: Don't fail uffd-stress if too many CPUs
2025-02-20 15:48 ` Dev Jain
@ 2025-02-20 15:55 ` Brendan Jackman
2025-02-20 16:01 ` Brendan Jackman
0 siblings, 1 reply; 14+ messages in thread
From: Brendan Jackman @ 2025-02-20 15:55 UTC (permalink / raw)
To: Dev Jain
Cc: Lorenzo Stoakes, Andrew Morton, Shuah Khan, Mateusz Guzik,
linux-mm, linux-kselftest, linux-kernel
On Thu, 20 Feb 2025 at 16:48, Dev Jain <dev.jain@arm.com> wrote:
>
>
>
> On 20/02/25 8:33 pm, Brendan Jackman wrote:
> > This calculation divides a fixed parameter by an environment-dependent
> > parameter i.e. the number of CPUs.
> >
> > The simple way to avoid machine-specific failures here is to just put a
> > cap on the max value of the latter.
>
> I haven't read the test, but if nr_cpus is being computed, then this
> value must be important to the test somehow? Would it potentially be
> wrong to let the test run for nr_cpus != actual number of cpus?
Based on my _extremely hasty_ reading, the variable is misnamed and
it's actually a thread count not a CPU count. I can double check
that's the case and rename it.
> Also, if the patch is correct then will it be better to also print a
> diagnostic telling the user that the number of cpus is going to be
> capped for the test to run?
Sure. The level of detail in the logging and error messages is
extremely low here so I didn't feel like being too anomalous, but why
not.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 6/6] selftests/mm: Don't fail uffd-stress if too many CPUs
2025-02-20 15:55 ` Brendan Jackman
@ 2025-02-20 16:01 ` Brendan Jackman
0 siblings, 0 replies; 14+ messages in thread
From: Brendan Jackman @ 2025-02-20 16:01 UTC (permalink / raw)
To: Dev Jain
Cc: Lorenzo Stoakes, Andrew Morton, Shuah Khan, Mateusz Guzik,
linux-mm, linux-kselftest, linux-kernel
On Thu, 20 Feb 2025 at 16:55, Brendan Jackman <jackmanb@google.com> wrote:
>
> On Thu, 20 Feb 2025 at 16:48, Dev Jain <dev.jain@arm.com> wrote:
> >
> >
> >
> > On 20/02/25 8:33 pm, Brendan Jackman wrote:
> > > This calculation divides a fixed parameter by an environment-dependent
> > > parameter i.e. the number of CPUs.
> > >
> > > The simple way to avoid machine-specific failures here is to just put a
> > > cap on the max value of the latter.
> >
> > I haven't read the test, but if nr_cpus is being computed, then this
> > value must be important to the test somehow? Would it potentially be
> > wrong to let the test run for nr_cpus != actual number of cpus?
>
> Based on my _extremely hasty_ reading, the variable is misnamed and
> it's actually a thread count not a CPU count. I can double check
> that's the case and rename it.
Oh yeah actually, it's only misnamed because I made it misnamed. So
this patch needs to rename it for sure, thanks for pointing it out.
(But yeah I upgraded my extremely hasty reading to an only hasty
reading and I still don't think this test cares about the actual CPU
topology).
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/6] selftests/mm: Skip uffd-stress if userfaultfd not available
2025-02-20 15:03 ` [PATCH 3/6] selftests/mm: Skip uffd-stress if userfaultfd not available Brendan Jackman
@ 2025-02-20 18:06 ` Dev Jain
2025-02-20 18:17 ` Brendan Jackman
0 siblings, 1 reply; 14+ messages in thread
From: Dev Jain @ 2025-02-20 18:06 UTC (permalink / raw)
To: Brendan Jackman, Lorenzo Stoakes, Andrew Morton, Shuah Khan
Cc: Mateusz Guzik, linux-mm, linux-kselftest, linux-kernel
On 20/02/25 8:33 pm, Brendan Jackman wrote:
> It's pretty obvious that the test wouldn't work if you don't have the
> feature enabled. But, it's still useful to SKIP instead of failing so
> the reader can immediately tell that this is the reason why.
>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
> tools/testing/selftests/mm/uffd-stress.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c
> index a4b83280998ab7ce8d31e91d8f9fbb47ef11d742..db5366b4766e5bfa2d1150d2f3c2d32469a6e28b 100644
> --- a/tools/testing/selftests/mm/uffd-stress.c
> +++ b/tools/testing/selftests/mm/uffd-stress.c
> @@ -411,8 +411,8 @@ static void parse_test_type_arg(const char *raw_type)
> * feature.
> */
>
> - if (uffd_get_features(&features))
> - err("failed to get available features");
> + if (uffd_get_features(&features) && errno == ENOENT)
> + ksft_exit_skip("failed to get avialable features (%d)\n", errno);
>
s/avialable/available
> test_uffdio_wp = test_uffdio_wp &&
> (features & UFFD_FEATURE_PAGEFAULT_FLAG_WP);
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/6] selftests/mm: Skip uffd-stress if userfaultfd not available
2025-02-20 18:06 ` Dev Jain
@ 2025-02-20 18:17 ` Brendan Jackman
0 siblings, 0 replies; 14+ messages in thread
From: Brendan Jackman @ 2025-02-20 18:17 UTC (permalink / raw)
To: Dev Jain
Cc: Lorenzo Stoakes, Andrew Morton, Shuah Khan, Mateusz Guzik,
linux-mm, linux-kselftest, linux-kernel
On Thu, 20 Feb 2025 at 19:06, Dev Jain <dev.jain@arm.com> wrote:
> > - if (uffd_get_features(&features))
> > - err("failed to get available features");
> > + if (uffd_get_features(&features) && errno == ENOENT)
> > + ksft_exit_skip("failed to get avialable features (%d)\n", errno);
> >
>
> s/avialable/available
Oh thanks. I thought I had codespell running automatically on my
diffs, I'll have to look at why that wasn't caught.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-02-20 18:18 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-20 15:03 [PATCH 0/6] selftests/mm: Some cleanups from trying to run them Brendan Jackman
2025-02-20 15:03 ` [PATCH 1/6] selftests/mm: Report errno when things fail Brendan Jackman
2025-02-20 15:32 ` Dev Jain
2025-02-20 15:03 ` [PATCH 2/6] selftests/mm: Fix assumption that sudo is present Brendan Jackman
2025-02-20 15:03 ` [PATCH 3/6] selftests/mm: Skip uffd-stress if userfaultfd not available Brendan Jackman
2025-02-20 18:06 ` Dev Jain
2025-02-20 18:17 ` Brendan Jackman
2025-02-20 15:03 ` [PATCH 4/6] selftests/mm: Skip uffd-wp-mremap " Brendan Jackman
2025-02-20 15:03 ` [PATCH 5/6] selftests/mm: Print some details when uffd-stress gets bad params Brendan Jackman
2025-02-20 15:17 ` Brendan Jackman
2025-02-20 15:03 ` [PATCH 6/6] selftests/mm: Don't fail uffd-stress if too many CPUs Brendan Jackman
2025-02-20 15:48 ` Dev Jain
2025-02-20 15:55 ` Brendan Jackman
2025-02-20 16:01 ` Brendan Jackman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox