Re: [PATCH 3/4] selftests/mm: Report unique test names for each cow test
On Tue, May 27, 2025 at 12:53:30PM +0100, Lorenzo Stoakes wrote:
> On Tue, May 27, 2025 at 12:49:57PM +0100, Mark Brown wrote:
> > On Tue, May 27, 2025 at 11:08:05AM +0100, Lorenzo Stoakes wrote:
> > > On Thu, May 22, 2025 at 06:38:52PM +0100, Mark Brown wrote:
> >
> > > > ret = setup_comm_pipes(&comm_pipes);
> > > > if (ret) {
> > > > - ksft_test_result_fail("pipe() failed\n");
> > > > + log_test_result(KAFT_FAIL);
> > >
> > > Looks like a typo here :) Should be KSFT not KAFT.
Somewhat impressively clang is managing to build this perfectly happily
locally which also isn't helping, I can't figure out why. Can't see a
system include with it.
signature.asc
Description: PGP signature
Re: [PATCH 3/4] selftests/mm: Report unique test names for each cow test
On Tue, May 27, 2025 at 12:53:30PM +0100, Lorenzo Stoakes wrote: > On Tue, May 27, 2025 at 12:49:57PM +0100, Mark Brown wrote: > > Ugh, this was masked because it's part of a series and among the > > problems with the kselftest build system is the fact that it eats > > errors. > Compile errors too? That's... not great. Yes, you only find out that something failed to build when the binary doesn't show up in the output or if you read the logs which is terrible in a CI system. If you install things to actually run them you get a lot of build log from that which scrolls things off the screen and you might have the binary from last time lying around. signature.asc Description: PGP signature
Re: [PATCH 3/4] selftests/mm: Report unique test names for each cow test
On Tue, May 27, 2025 at 12:49:57PM +0100, Mark Brown wrote:
> On Tue, May 27, 2025 at 11:08:05AM +0100, Lorenzo Stoakes wrote:
> > On Thu, May 22, 2025 at 06:38:52PM +0100, Mark Brown wrote:
>
> > > ret = setup_comm_pipes(&comm_pipes);
> > > if (ret) {
> > > - ksft_test_result_fail("pipe() failed\n");
> > > + log_test_result(KAFT_FAIL);
> >
> > Looks like a typo here :) Should be KSFT not KAFT.
> >
> > This is breaking the mm self test build for mm-new, could you
> > fix-patch/respin?
> > Thanks!
>
> Ugh, this was masked because it's part of a series and among the
> problems with the kselftest build system is the fact that it eats
> errors.
Compile errors too? That's... not great.
>
> Please delete unneeded context from mails when replying. Doing this
> makes it much easier to find your reply in the message, helping ensure
> it won't be missed by people scrolling through the irrelevant quoted
> material.
Yup I normally do, I sent this out quick and dirty because I just happened
to hit it when trying to do other work.
Re: [PATCH 3/4] selftests/mm: Report unique test names for each cow test
On Tue, May 27, 2025 at 11:08:05AM +0100, Lorenzo Stoakes wrote:
> On Thu, May 22, 2025 at 06:38:52PM +0100, Mark Brown wrote:
> > ret = setup_comm_pipes(&comm_pipes);
> > if (ret) {
> > - ksft_test_result_fail("pipe() failed\n");
> > + log_test_result(KAFT_FAIL);
>
> Looks like a typo here :) Should be KSFT not KAFT.
>
> This is breaking the mm self test build for mm-new, could you
> fix-patch/respin?
> Thanks!
Ugh, this was masked because it's part of a series and among the
problems with the kselftest build system is the fact that it eats
errors.
Please delete unneeded context from mails when replying. Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.
signature.asc
Description: PGP signature
Re: [PATCH 3/4] selftests/mm: Report unique test names for each cow test
On Thu, May 22, 2025 at 06:38:52PM +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 cow test completely fails to follow this pattern,
> it runs test functions repeatedly with various parameters with each result
> report from those functions being 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
> ---
> tools/testing/selftests/mm/cow.c | 333
> +--
> 1 file changed, 217 insertions(+), 116 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/cow.c
> b/tools/testing/selftests/mm/cow.c
> index e70cd3d900cc..a97f5ef79f17 100644
> --- a/tools/testing/selftests/mm/cow.c
> +++ b/tools/testing/selftests/mm/cow.c
> @@ -112,9 +112,12 @@ struct comm_pipes {
>
> static int setup_comm_pipes(struct comm_pipes *comm_pipes)
> {
> - if (pipe(comm_pipes->child_ready) < 0)
> + if (pipe(comm_pipes->child_ready) < 0) {
> + ksft_perror("pipe()");
> return -errno;
> + }
> if (pipe(comm_pipes->parent_ready) < 0) {
> + ksft_perror("pipe()");
> close(comm_pipes->child_ready[0]);
> close(comm_pipes->child_ready[1]);
> return -errno;
> @@ -207,13 +210,14 @@ static void do_test_cow_in_parent(char *mem, size_t
> size, bool do_mprotect,
>
> ret = setup_comm_pipes(&comm_pipes);
> if (ret) {
> - ksft_test_result_fail("pipe() failed\n");
> + log_test_result(KSFT_FAIL);
> return;
> }
>
> ret = fork();
> if (ret < 0) {
> - ksft_test_result_fail("fork() failed\n");
> + ksft_perror("fork() failed");
> + log_test_result(KSFT_FAIL);
> goto close_comm_pipes;
> } else if (!ret) {
> exit(fn(mem, size, &comm_pipes));
> @@ -228,9 +232,18 @@ static void do_test_cow_in_parent(char *mem, size_t
> size, bool do_mprotect,
>* write-faults by directly mapping pages writable.
>*/
> ret = mprotect(mem, size, PROT_READ);
> - ret |= mprotect(mem, size, PROT_READ|PROT_WRITE);
> if (ret) {
> - ksft_test_result_fail("mprotect() failed\n");
> + ksft_perror("mprotect() failed");
> + log_test_result(KSFT_FAIL);
> + write(comm_pipes.parent_ready[1], "0", 1);
> + wait(&ret);
> + goto close_comm_pipes;
> + }
> +
> + ret = mprotect(mem, size, PROT_READ|PROT_WRITE);
> + if (ret) {
> + ksft_perror("mprotect() failed");
> + log_test_result(KSFT_FAIL);
> write(comm_pipes.parent_ready[1], "0", 1);
> wait(&ret);
> goto close_comm_pipes;
> @@ -248,16 +261,16 @@ static void do_test_cow_in_parent(char *mem, size_t
> size, bool do_mprotect,
> ret = -EINVAL;
>
> if (!ret) {
> - ksft_test_result_pass("No leak from parent into child\n");
> + log_test_result(KSFT_PASS);
> } else if (xfail) {
> /*
>* With hugetlb, some vmsplice() tests are currently expected to
>* fail because (a) harder to fix and (b) nobody really cares.
>* Flag them as expected failure for now.
>*/
> - ksft_test_result_xfail("Leak from parent into child\n");
> + log_test_result(KSFT_XFAIL);
> } else {
> - ksft_test_result_fail("Leak from parent into child\n");
> + log_test_result(KSFT_FAIL);
> }
> close_comm_pipes:
> close_comm_pipes(&comm_pipes);
> @@ -306,26 +319,29 @@ static void do_test_vmsplice_in_parent(char *mem,
> size_t size,
>
> ret = setup_comm_pipes(&comm_pipes);
> if (ret) {
> - ksft_test_result_fail("pipe() failed\n");
> + log_test_result(KSFT_FAIL);
> goto free;
> }
>
> if (pipe(fds) < 0) {
> - ksft_test_result_fail("pipe() failed\n");
> + ksft_perror("pipe() failed");
> + log_test_result(KSFT_FAIL);
> goto close_comm_pipes;
> }
>
> if (before_fork) {
> transferred = vmsplice(fds[1], &iov, 1, 0);
> if (transferred <= 0) {
> - ksft_test_result_fail("vmsplice() failed\n");
> + ksft_print_msg("vmsplice() failed\n");
> + log_test_result(KSFT_FAIL);
>
[PATCH 3/4] selftests/mm: Report unique test names for each cow test
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 cow test completely fails to follow this pattern,
it runs test functions repeatedly with various parameters with each result
report from those functions being 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
---
tools/testing/selftests/mm/cow.c | 333 +--
1 file changed, 217 insertions(+), 116 deletions(-)
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
index e70cd3d900cc..a97f5ef79f17 100644
--- a/tools/testing/selftests/mm/cow.c
+++ b/tools/testing/selftests/mm/cow.c
@@ -112,9 +112,12 @@ struct comm_pipes {
static int setup_comm_pipes(struct comm_pipes *comm_pipes)
{
- if (pipe(comm_pipes->child_ready) < 0)
+ if (pipe(comm_pipes->child_ready) < 0) {
+ ksft_perror("pipe()");
return -errno;
+ }
if (pipe(comm_pipes->parent_ready) < 0) {
+ ksft_perror("pipe()");
close(comm_pipes->child_ready[0]);
close(comm_pipes->child_ready[1]);
return -errno;
@@ -207,13 +210,14 @@ static void do_test_cow_in_parent(char *mem, size_t size,
bool do_mprotect,
ret = setup_comm_pipes(&comm_pipes);
if (ret) {
- ksft_test_result_fail("pipe() failed\n");
+ log_test_result(KSFT_FAIL);
return;
}
ret = fork();
if (ret < 0) {
- ksft_test_result_fail("fork() failed\n");
+ ksft_perror("fork() failed");
+ log_test_result(KSFT_FAIL);
goto close_comm_pipes;
} else if (!ret) {
exit(fn(mem, size, &comm_pipes));
@@ -228,9 +232,18 @@ static void do_test_cow_in_parent(char *mem, size_t size,
bool do_mprotect,
* write-faults by directly mapping pages writable.
*/
ret = mprotect(mem, size, PROT_READ);
- ret |= mprotect(mem, size, PROT_READ|PROT_WRITE);
if (ret) {
- ksft_test_result_fail("mprotect() failed\n");
+ ksft_perror("mprotect() failed");
+ log_test_result(KSFT_FAIL);
+ write(comm_pipes.parent_ready[1], "0", 1);
+ wait(&ret);
+ goto close_comm_pipes;
+ }
+
+ ret = mprotect(mem, size, PROT_READ|PROT_WRITE);
+ if (ret) {
+ ksft_perror("mprotect() failed");
+ log_test_result(KSFT_FAIL);
write(comm_pipes.parent_ready[1], "0", 1);
wait(&ret);
goto close_comm_pipes;
@@ -248,16 +261,16 @@ static void do_test_cow_in_parent(char *mem, size_t size,
bool do_mprotect,
ret = -EINVAL;
if (!ret) {
- ksft_test_result_pass("No leak from parent into child\n");
+ log_test_result(KSFT_PASS);
} else if (xfail) {
/*
* With hugetlb, some vmsplice() tests are currently expected to
* fail because (a) harder to fix and (b) nobody really cares.
* Flag them as expected failure for now.
*/
- ksft_test_result_xfail("Leak from parent into child\n");
+ log_test_result(KSFT_XFAIL);
} else {
- ksft_test_result_fail("Leak from parent into child\n");
+ log_test_result(KSFT_FAIL);
}
close_comm_pipes:
close_comm_pipes(&comm_pipes);
@@ -306,26 +319,29 @@ static void do_test_vmsplice_in_parent(char *mem, size_t
size,
ret = setup_comm_pipes(&comm_pipes);
if (ret) {
- ksft_test_result_fail("pipe() failed\n");
+ log_test_result(KSFT_FAIL);
goto free;
}
if (pipe(fds) < 0) {
- ksft_test_result_fail("pipe() failed\n");
+ ksft_perror("pipe() failed");
+ log_test_result(KSFT_FAIL);
goto close_comm_pipes;
}
if (before_fork) {
transferred = vmsplice(fds[1], &iov, 1, 0);
if (transferred <= 0) {
- ksft_test_result_fail("vmsplice() failed\n");
+ ksft_print_msg("vmsplice() failed\n");
+ log_test_result(KSFT_FAIL);
goto close_pipe;
}
}
ret = fork();
if (ret < 0) {
- ksft_test_result_
