Re: [PATCH 13/15] perf test: Add -j/--junit option for JUnit XML test reports

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] When start_command() fails to spawn a child process, its failure propagates to __cmd_test's err_out path, which unconditionally invokes finish_test() on the failed child_test object. This causes wait_or_whine() to execute waitpid(-1), inadvertently reaping unrelated parallel test child processes and causing their exit statuses to be lost.
--

commit 9f0a55afca8eec136a897a87513faf7cbd01d8ce
Author: Ian Rogers <irogers@google.com>

perf test: Add -j/--junit option for JUnit XML test reports

This commit adds a -j/--junit option to perf test to generate standard
JUnit XML format test reports. The XML report captures test execution
latency alongside escaped failure logs and skip reasons, while preserving
the concurrency of parallel test execution.

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 4c0caa8d0b27e..1c1e77e19a0a2 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c

[ ... ]

> @@ -963,6 +1043,7 @@ static int start_test(struct test_suite *test, int curr_suite, int curr_test_cas
>  		(*child)->process.err = -1;
>  	}
>  	(*child)->process.no_exec_cmd = run_test_child;
> +	clock_gettime(CLOCK_MONOTONIC, &(*child)->start_time);
>  	if (sequential || pass == 2) {
>  		err = start_command(&(*child)->process);
>  		if (err)

[Severity: Medium]
This is a pre-existing issue, but what happens when start_command() fails to
spawn a child process?

If start_command() fails (e.g., due to fork or pipe failure under parallel
load), it returns an error and child->process.pid is left as -1. The caller
__cmd_test() then catches this error and jumps to its err_out path:

tools/perf/tests/builtin-test.c:__cmd_test() {
    ...
err_out:
    signal(SIGINT, SIG_DFL);
    signal(SIGTERM, SIG_DFL);
    if (err) {
        pr_err("Internal test harness failure. Completing any started tests:\n:");
        for (size_t x = 0; x < num_tests; x++)
            finish_test(child_tests, x, num_tests, width);
    }
    ...
}

This unconditionally invokes finish_test() on the failed child_test object.
When finish_test() subsequently calls finish_command(), it reaches
wait_or_whine() where cmd->pid is still -1:

tools/lib/subcmd/run-command.c:wait_or_whine() {
    ...
    pid_t waiting = waitpid(cmd->pid, &status, block ? 0 : WNOHANG);
    ...
}

Because cmd->pid is -1, waitpid(-1) will inadvertently block and reap any
running parallel test child process, rather than the specific test that failed.

Could this corrupt the test harness state and cause unrelated parallel tests
to lose their exit statuses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601000548.1195604-1-irogers@google.com?part=13