A collection of non-functional updates from David Hildenbrand's review.
Signed-off-by: Mark Brown broonie@kernel.org --- Mark Brown (4): kselftest/mm: Clarify errors for pipe() selftests/mm: Convert some cow error reports to ksft_perror() selftests/mm: Don't compare return values to in cow selftests/mm: Add messages about test errors to the cow tests
tools/testing/selftests/mm/cow.c | 44 +++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 16 deletions(-) --- base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 change-id: 20250603-selftest-mm-cow-tweaks-0199c5132b3e
Best regards,
Specify that errors reported from pipe() failures are the result of failures.
Suggested-by: David Hildenbrand david@redhat.com Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/mm/cow.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c index dbbcc5eb3dce..0adc0ab142c1 100644 --- a/tools/testing/selftests/mm/cow.c +++ b/tools/testing/selftests/mm/cow.c @@ -113,11 +113,11 @@ struct comm_pipes { static int setup_comm_pipes(struct comm_pipes *comm_pipes) { if (pipe(comm_pipes->child_ready) < 0) { - ksft_perror("pipe()"); + ksft_perror("pipe() failed"); return -errno; } if (pipe(comm_pipes->parent_ready) < 0) { - ksft_perror("pipe()"); + ksft_perror("pipe() failed"); close(comm_pipes->child_ready[0]); close(comm_pipes->child_ready[1]); return -errno;
On 10.06.25 16:13, Mark Brown wrote:
Specify that errors reported from pipe() failures are the result of failures.
Suggested-by: David Hildenbrand david@redhat.com Signed-off-by: Mark Brown broonie@kernel.org
Acked-by: David Hildenbrand david@redhat.com
This prints the errno and a string decode of it.
Reported-by: David Hildenbrand david@redhat.com Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/mm/cow.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c index 0adc0ab142c1..ff80329313e4 100644 --- a/tools/testing/selftests/mm/cow.c +++ b/tools/testing/selftests/mm/cow.c @@ -332,7 +332,7 @@ static void do_test_vmsplice_in_parent(char *mem, size_t size, if (before_fork) { transferred = vmsplice(fds[1], &iov, 1, 0); if (transferred <= 0) { - ksft_print_msg("vmsplice() failed\n"); + ksft_perror("vmsplice() failed\n"); log_test_result(KSFT_FAIL); goto close_pipe; } @@ -562,7 +562,7 @@ static void do_test_iouring(char *mem, size_t size, bool use_fork) while (total < size) { cur = pread(fd, tmp + total, size - total, total); if (cur < 0) { - ksft_print_msg("pread() failed\n"); + ksft_perror("pread() failed\n"); log_test_result(KSFT_FAIL); goto quit_child; } @@ -628,7 +628,7 @@ static void do_test_ro_pin(char *mem, size_t size, enum ro_pin_test test,
tmp = malloc(size); if (!tmp) { - ksft_print_msg("malloc() failed\n"); + ksft_perror("malloc() failed\n"); log_test_result(KSFT_FAIL); return; }
On 10.06.25 16:13, Mark Brown wrote:
This prints the errno and a string decode of it.
Reported-by: David Hildenbrand david@redhat.com
Probably not "Reported-by". Did you mean "Suggested-by" like for the others?
Signed-off-by: Mark Brown broonie@kernel.org
Acked-by: David Hildenbrand david@redhat.com
On Wed, Jun 11, 2025 at 02:10:20PM +0200, David Hildenbrand wrote:
On 10.06.25 16:13, Mark Brown wrote:
This prints the errno and a string decode of it.
Reported-by: David Hildenbrand david@redhat.com
Probably not "Reported-by". Did you mean "Suggested-by" like for the others?
Probably. I'm sure I was thinking something there but not sure what.
Tweak the coding style for checking for non-zero return values. While we're at it also remove a now redundant oring of the madvise() return code.
Suggested-by: David Hildenbrand david@redhat.com Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/mm/cow.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c index ff80329313e4..48fcf03aa4cd 100644 --- a/tools/testing/selftests/mm/cow.c +++ b/tools/testing/selftests/mm/cow.c @@ -1613,13 +1613,13 @@ static void run_with_huge_zeropage(non_anon_test_fn fn, const char *desc) smem = (char *)(((uintptr_t)mmap_smem + pmdsize) & ~(pmdsize - 1));
ret = madvise(mem, pmdsize, MADV_HUGEPAGE); - if (ret != 0) { + if (ret) { ksft_perror("madvise()"); log_test_result(KSFT_FAIL); goto munmap; } - ret |= madvise(smem, pmdsize, MADV_HUGEPAGE); - if (ret != 0) { + ret = madvise(smem, pmdsize, MADV_HUGEPAGE); + if (ret) { ksft_perror("madvise()"); log_test_result(KSFT_FAIL); goto munmap;
On 10.06.25 16:13, Mark Brown wrote:
Tweak the coding style for checking for non-zero return values. While we're at it also remove a now redundant oring of the madvise() return code.
Suggested-by: David Hildenbrand david@redhat.com Signed-off-by: Mark Brown broonie@kernel.org
Acked-by: David Hildenbrand david@redhat.com
It is not sufficiently clear what the individual tests in the cow test program are checking so add messages for the failure cases.
Suggested-by: David Hildenbrand david@redhat.com Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/mm/cow.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c index 48fcf03aa4cd..29767e2afdd0 100644 --- a/tools/testing/selftests/mm/cow.c +++ b/tools/testing/selftests/mm/cow.c @@ -268,8 +268,10 @@ static void do_test_cow_in_parent(char *mem, size_t size, bool do_mprotect, * fail because (a) harder to fix and (b) nobody really cares. * Flag them as expected failure for now. */ + ksft_print_msg("Leak from parent into child\n"); log_test_result(KSFT_XFAIL); } else { + ksft_print_msg("Leak from parent into child\n"); log_test_result(KSFT_FAIL); } close_comm_pipes: @@ -397,8 +399,10 @@ static void do_test_vmsplice_in_parent(char *mem, size_t size, * fail because (a) harder to fix and (b) nobody really cares. * Flag them as expected failure for now. */ + ksft_print_msg("Leak from child into parent\n"); log_test_result(KSFT_XFAIL); } else { + ksft_print_msg("Leak from child into parent\n"); log_test_result(KSFT_FAIL); } close_pipe: @@ -570,10 +574,12 @@ static void do_test_iouring(char *mem, size_t size, bool use_fork) }
/* Finally, check if we read what we expected. */ - if (!memcmp(mem, tmp, size)) + if (!memcmp(mem, tmp, size)) { log_test_result(KSFT_PASS); - else + } else { + ksft_print_msg("Longtom R/W pin is not reliable\n"); log_test_result(KSFT_FAIL); + }
quit_child: if (use_fork) { @@ -725,10 +731,12 @@ static void do_test_ro_pin(char *mem, size_t size, enum ro_pin_test test, ksft_perror("PIN_LONGTERM_TEST_READ failed"); log_test_result(KSFT_FAIL); } else { - if (!memcmp(mem, tmp, size)) + if (!memcmp(mem, tmp, size)) { log_test_result(KSFT_PASS); - else + } else { + ksft_print_msg("Longterm R/O pin is not reliable\n"); log_test_result(KSFT_FAIL); + } }
ret = ioctl(gup_fd, PIN_LONGTERM_TEST_STOP); @@ -1417,10 +1425,12 @@ static void do_test_anon_thp_collapse(char *mem, size_t size, else ret = -EINVAL;
- if (!ret) + if (!ret) { log_test_result(KSFT_PASS); - else + } else { + ksft_print_msg("Leak from parent into child\n"); log_test_result(KSFT_FAIL); + } close_comm_pipes: close_comm_pipes(&comm_pipes); } @@ -1528,10 +1538,12 @@ static void test_cow(char *mem, const char *smem, size_t size) memset(mem, 0xff, size);
/* See if we still read the old values via the other mapping. */ - if (!memcmp(smem, old, size)) + if (!memcmp(smem, old, size)) { log_test_result(KSFT_PASS); - else + } else { + ksft_print_msg("Other mapping modified\n"); log_test_result(KSFT_FAIL); + } free(old); }
On 10.06.25 16:13, Mark Brown wrote:
It is not sufficiently clear what the individual tests in the cow test program are checking so add messages for the failure cases.
Suggested-by: David Hildenbrand david@redhat.com Signed-off-by: Mark Brown broonie@kernel.org
Thanks!
Acked-by: David Hildenbrand david@redhat.com
linux-kselftest-mirror@lists.linaro.org
-
David Hildenbrand -
Mark Brown