Re: [PATCH v2 4/4] selftests/mm: Fix test result reporting in gup_longterm

This seems to be causing tests to fail rather than be skipped if hugetlb isn't configured. I bisected the problem to this patch so it's definitely changed how things are handled (though of course it might just be _revealing_ some previously existing bug in this test...).

Using a couple of tests as an example:

Before this patch:

# [RUN] R/O longterm GUP-fast pin in MAP_PRIVATE file mapping ... with memfd hugetlb (2048 kB) ok 39 # SKIP memfd_create() failed (Cannot allocate memory) # [RUN] R/O longterm GUP-fast pin in MAP_PRIVATE file mapping ... with memfd hugetlb (1048576 kB) ok 40 # SKIP memfd_create() failed (Cannot allocate memory)

After:

# [RUN] R/O longterm GUP-fast pin in MAP_PRIVATE file mapping ... with memfd hugetlb (2048 kB) # memfd_create() failed (Cannot allocate memory) not ok 39 R/O longterm GUP-fast pin in MAP_PRIVATE file mapping ... with memfd hugetlb (2048 kB) # [RUN] R/O longterm GUP-fast pin in MAP_PRIVATE file mapping ... with memfd hugetlb (1048576 kB) # memfd_create() failed (Cannot allocate memory) not ok 40 R/O longterm GUP-fast pin in MAP_PRIVATE file mapping ... with memfd hugetlb (1048576 kB)

See below, I think I've found where it happens...

On Tue, May 27, 2025 at 05:04:48PM +0100, Mark Brown wrote:

The kselftest framework uses the string logged when a test result is reported as the unique identifier for a test, using it to track test results between runs. The gup_longterm test fails to follow this pattern, it runs a single test function repeatedly with various parameters but each result report is a string logging an error message which is fixed between runs.

Since the code already logs each test uniquely before it starts refactor to also print this to a buffer, then use that name as the test result. This isn't especially pretty but is relatively straightforward and is a great help to tooling.

Signed-off-by: Mark Brown broonie@kernel.org

tools/testing/selftests/mm/gup_longterm.c | 150 +++++++++++++++++++----------- 1 file changed, 94 insertions(+), 56 deletions(-)

diff --git a/tools/testing/selftests/mm/gup_longterm.c b/tools/testing/selftests/mm/gup_longterm.c index e60e62809186..f84ea97c2543 100644 --- a/tools/testing/selftests/mm/gup_longterm.c +++ b/tools/testing/selftests/mm/gup_longterm.c @@ -93,33 +93,48 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared) __fsword_t fs_type = get_fs_type(fd); bool should_work; char *mem;

• int result = KSFT_PASS; int ret;

• if (fd < 0) {

• result = KSFT_FAIL;
  

• goto report;
  

• }

• if (ftruncate(fd, size)) { if (errno == ENOENT) { skip_test_dodgy_fs("ftruncate()"); } else {

• 	ksft_test_result_fail("ftruncate() failed (%s)\n", strerror(errno));
  

• 	ksft_print_msg("ftruncate() failed (%s)\n",
  

• 		       strerror(errno));
  

• 	result = KSFT_FAIL;
  

• 	goto report;
  

  } return; }

  if (fallocate(fd, 0, 0, size)) {

• if (size == pagesize)
  

• 	ksft_test_result_fail("fallocate() failed (%s)\n", strerror(errno));
  

• else
  

• 	ksft_test_result_skip("need more free huge pages\n");
  

• return;
  

• if (size == pagesize) {
  

• 	ksft_print_msg("fallocate() failed (%s)\n", strerror(errno));
  

• 	result = KSFT_FAIL;
  

• } else {
  

• 	ksft_print_msg("need more free huge pages\n");
  

• 	result = KSFT_SKIP;
  

• }
  

• goto report;
  

  }

  mem = mmap(NULL, size, PROT_READ | PROT_WRITE, shared ? MAP_SHARED : MAP_PRIVATE, fd, 0); if (mem == MAP_FAILED) {

• if (size == pagesize || shared)
  

• 	ksft_test_result_fail("mmap() failed (%s)\n", strerror(errno));
  

• else
  

• 	ksft_test_result_skip("need more free huge pages\n");
  

• return;
  

• if (size == pagesize || shared) {
  

• 	ksft_print_msg("mmap() failed (%s)\n", strerror(errno));
  

• 	result = KSFT_FAIL;
  

• } else {
  

• 	ksft_print_msg("need more free huge pages\n");
  

• 	result = KSFT_SKIP;
  

• }
  

• goto report;
  

  }

  /* Fault in the page such that GUP-fast can pin it directly. */

@@ -134,7 +149,8 @@ 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 (%s)\n", strerror(errno));
  

• 	ksft_print_msg("mprotect() failed (%s)\n", strerror(errno));
  

• 	result = KSFT_FAIL;
  goto munmap;
  

  } /* FALLTHROUGH */

@@ -147,12 +163,14 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared) type == TEST_TYPE_RW_FAST;

if (gup_fd < 0) {

• 	ksft_test_result_skip("gup_test not available\n");
  

• 	ksft_print_msg("gup_test not available\n");
  

• 	result = KSFT_SKIP;
  break;
  

  }

  if (rw && shared && fs_is_unknown(fs_type)) {

• 	ksft_test_result_skip("Unknown filesystem\n");
  

• 	ksft_print_msg("Unknown filesystem\n");
  

• 	result = KSFT_SKIP;
  return;
  

  } /*

@@ -169,14 +187,19 @@ 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 (EINVAL)n");
  

• 	ksft_print_msg("PIN_LONGTERM_TEST_START failed (EINVAL)n");
  

• 	result = KSFT_SKIP;
  break;
  

  } else if (ret && errno == EFAULT) {

• 	ksft_test_result(!should_work, "Should have failed\n");
  

• 	if (should_work)
  

• 		result = KSFT_FAIL;
  

• 	else
  

• 		result = KSFT_PASS;
  break;
  

  } else if (ret) {

• 	ksft_test_result_fail("PIN_LONGTERM_TEST_START failed (%s)\n",
  

• 			      strerror(errno));
  

• 	ksft_print_msg("PIN_LONGTERM_TEST_START failed (%s)\n",
  

• 		       strerror(errno));
  

• 	result = KSFT_FAIL;
  break;
  

  }

@@ -189,7 +212,10 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared) * some previously unsupported filesystems, we might want to * perform some additional tests for possible data corruptions. */

• ksft_test_result(should_work, "Should have worked\n");
  

• if (should_work)
  

• 	result = KSFT_PASS;
  

• else
  

• 	result = KSFT_FAIL;
  

  break; }

#ifdef LOCAL_CONFIG_HAVE_LIBURING @@ -199,8 +225,9 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)

/* io_uring always pins pages writable. */
if (shared && fs_is_unknown(fs_type)) {

• 	ksft_test_result_skip("Unknown filesystem\n");
  

• 	return;
  

• 	ksft_print_msg("Unknown filesystem\n");
  

• 	result = KSFT_SKIP;
  

• 	goto report;
  

  } should_work = !shared || fs_supports_writable_longterm_pinning(fs_type);

@@ -208,8 +235,9 @@ 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 (%s)\n",
  

• 			      strerror(-ret));
  

• 	ksft_print_msg("io_uring_queue_init() failed (%s)\n",
  

• 		       strerror(-ret));
  

• 	result = KSFT_SKIP;
  break;
  

  } /*

@@ -222,17 +250,28 @@ 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 (%s)\n",
  

• 			 strerror(errno));
  

• 	if (should_work) {
  

• 		ksft_print_msg("Should have failed (%s)\n",
  

• 			       strerror(errno));
  

• 		result = KSFT_FAIL;
  

• 	} else {
  

• 		result = KSFT_PASS;
  

• 	}
  

  } else if (ret) { /* * We might just lack support or have insufficient * MEMLOCK limits. */

• 	ksft_test_result_skip("io_uring_register_buffers() failed (%s)\n",
  

• 			      strerror(-ret));
  

• 	ksft_print_msg("io_uring_register_buffers() failed (%s)\n",
  

• 		       strerror(-ret));
  

• 	result = KSFT_SKIP;
  

  } else {

• 	ksft_test_result(should_work, "Should have worked\n");
  

• 	if (should_work) {
  

• 		result = KSFT_PASS;
  

• 	} else {
  

• 		ksft_print_msg("Should have worked\n");
  

• 		result = KSFT_FAIL;
  

• 	}
  io_uring_unregister_buffers(&ring);
  

  }

@@ -246,6 +285,8 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)

munmap: munmap(mem, size); +report:

• log_test_result(result);

}

typedef void (*test_fn)(int fd, size_t size); @@ -254,13 +295,11 @@ static void run_with_memfd(test_fn fn, const char *desc) { int fd;

• ksft_print_msg("[RUN] %s ... with memfd\n", desc);

• log_test_start("%s ... with memfd", desc);

  fd = memfd_create("test", 0);

• if (fd < 0) {

• ksft_test_result_fail("memfd_create() failed (%s)\n", strerror(errno));
  

• return;
  

• }

• if (fd < 0)

• ksft_print_msg("memfd_create() failed (%s)\n", strerror(errno));
  
  

  fn(fd, pagesize); close(fd);

@@ -271,23 +310,23 @@ static void run_with_tmpfile(test_fn fn, const char *desc) FILE *file; int fd;

• ksft_print_msg("[RUN] %s ... with tmpfile\n", desc);

• log_test_start("%s ... with tmpfile", desc);

  file = tmpfile(); if (!file) {

• ksft_test_result_fail("tmpfile() failed (%s)\n", strerror(errno));
  

• return;
  

• }
• fd = fileno(file);
• if (fd < 0) {

• ksft_test_result_fail("fileno() failed (%s)\n", strerror(errno));
  

• goto close;
  

• ksft_print_msg("tmpfile() failed (%s)\n", strerror(errno));
  

• fd = -1;
  

• } else {

• fd = fileno(file);
  

• if (fd < 0) {
  

• 	ksft_print_msg("fileno() failed (%s)\n", strerror(errno));
  

• }
  

  }

  fn(fd, pagesize);

-close:

• fclose(file);

• if (file)

• fclose(file);
  

}

static void run_with_local_tmpfile(test_fn fn, const char *desc) @@ -295,22 +334,22 @@ static void run_with_local_tmpfile(test_fn fn, const char *desc) char filename[] = __FILE__"_tmpfile_XXXXXX"; int fd;

• ksft_print_msg("[RUN] %s ... with local tmpfile\n", desc);

• log_test_start("%s ... with local tmpfile", desc);

  fd = mkstemp(filename);

• if (fd < 0) {

• ksft_test_result_fail("mkstemp() failed (%s)\n", strerror(errno));
  

• return;
  

• }

• if (fd < 0)

• ksft_print_msg("mkstemp() failed (%s)\n", strerror(errno));
  
  

  if (unlink(filename)) {

• ksft_test_result_fail("unlink() failed (%s)\n", strerror(errno));
  

• goto close;
  

• ksft_print_msg("unlink() failed (%s)\n", strerror(errno));
  

• close(fd);
  

• fd = -1;
  

  }

  fn(fd, pagesize);

-close:

• close(fd);

• if (fd >= 0)

• close(fd);
  

}

static void run_with_memfd_hugetlb(test_fn fn, const char *desc, @@ -319,15 +358,14 @@ static void run_with_memfd_hugetlb(test_fn fn, const char *desc, int flags = MFD_HUGETLB; int fd;

• ksft_print_msg("[RUN] %s ... with memfd hugetlb (%zu kB)\n", desc,

• log_test_start("%s ... with memfd hugetlb (%zu kB)", desc, hugetlbsize / 1024);

  flags |= __builtin_ctzll(hugetlbsize) << MFD_HUGE_SHIFT;

  fd = memfd_create("test", flags); if (fd < 0) {

• ksft_test_result_skip("memfd_create() failed (%s)\n", strerror(errno));
  

• return;
  

• ksft_print_msg("memfd_create() failed (%s)\n", strerror(errno));
  

I think it's this change that does it (probably).

I wonder if it's worth checking for errno == ENOMEM and skipping in this case? Or is that too general?

}

fn(fd, hugetlbsize);

-- 2.39.5