Re: [PATCH v2 03/18] perf test: Add a workload that forces context switches

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Parent process fails to close unused pipe ends, causing a deadlock if the child exits prematurely, and leaks file descriptors on all paths.
--

commit 4b7337e6446674341b89bdd15dec4b286776529f
Author: James Clark <james.clark@linaro.org>

perf test: Add a workload that forces context switches

This commit adds a new test workload that forces context switches by launching
two processes that block when reading and writing to each other.

> diff --git a/tools/perf/tests/workloads/context_switch_loop.c b/tools/perf/tests/workloads/context_switch_loop.c
> new file mode 100644
> index 0000000000000..73bfcaeff5b09
> --- /dev/null
> +++ b/tools/perf/tests/workloads/context_switch_loop.c

[ ... ]

> +static int context_switch_loop(int argc, const char **argv)
> +{
> +	int a_to_b[2], b_to_a[2];
> +	pid_t proc1_pid;
> +	int status;

[ ... ]

> +	if (pipe(a_to_b) || pipe(b_to_a)) {

[Severity: Medium]
If pipe(b_to_a) fails, does this return 1 and leak the file descriptors
successfully allocated by pipe(a_to_b)?

> +		perror("Pipe error");
> +		return 1;
> +	}
> +
> +	proc1_pid = fork();
> +	if (proc1_pid < 0) {
> +		perror("Fork error");
> +		return 1;

[Severity: Medium]
Similarly, does this error path leak all 4 file descriptors allocated by the
pipes?

> +	}
> +
> +	if (!proc1_pid) {
> +		close(a_to_b[0]);
> +		close(b_to_a[1]);
> +		prctl(PR_SET_NAME, "proc1", 0, 0, 0);
> +		context_switch_loop_proc1(b_to_a[0], a_to_b[1]);
> +		exit(0);
> +	}
> +
> +	prctl(PR_SET_NAME, "proc2", 0, 0, 0);
> +	context_switch_loop_proc2(a_to_b[0], b_to_a[1]);

[Severity: Medium]
Does the parent process (proc2) need to close its unused pipe ends (a_to_b[1]
and b_to_a[0]) before executing its workload loop?

If the child process (proc1) terminates unexpectedly or calls exit(1) (e.g.,
due to a read/write error), it closes its file descriptors. However, the parent
waiting in read(a_to_b[0]) will not receive an EOF (0) because it still holds
the write end a_to_b[1] open itself. This could cause the parent to block
indefinitely and hang the test suite.

> +
> +	if (waitpid(proc1_pid, &status, 0) != proc1_pid || !WIFEXITED(status) ||
> +	    WEXITSTATUS(status))
> +		return 1;
> +
> +	return 0;

[Severity: Medium]
Do the waitpid error path and successful return path both exit without closing
the remaining pipe file descriptors?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-james-cs-context-tracking-fix-v2-0-85b5ce6f55c6@linaro.org?part=3