Re: [PATCH v2 14/14] perf cs-etm: Test multiple per-thread threads

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The script will abort prematurely on non-TRBE systems due to `set -e` triggering on expected errors from `wait $perf` or `perf script`.
--

--- Patch [14]: [PATCH v2 14/14] perf cs-etm: Test multiple per-thread threads ---
commit 94423fa9c68e780595c834e59e6281ca36e48a32
Author: James Clark <james.clark@linaro.org>

perf cs-etm: Test multiple per-thread threads

This commit adds a test to verify that attaching to a process with multiple
child threads correctly records samples for all child threads using CoreSight.

> diff --git a/tools/perf/tests/shell/coresight/per-thread-multi.sh b/tools/perf/tests/shell/coresight/per-thread-multi.sh
> new file mode 100755
> index 0000000000000..ea525ed5e18bc
> --- /dev/null
> +++ b/tools/perf/tests/shell/coresight/per-thread-multi.sh
> @@ -0,0 +1,78 @@
> +#!/bin/bash -e

[ ... ]

> +kill $parent
> +wait $parent || true
> +unset parent
> +wait $perf

[Severity: Medium]
Will this script abort prematurely on non-TRBE systems?

As noted in the script's own comments, `perf record` is expected to fail
with `-EBUSY` on systems using shared sinks (non-TRBE) because two threads
try to use them simultaneously.

When this expected failure occurs, `perf record` exits with a non-zero
status. Because the script is running with `set -e` on the first line, the
unshielded `wait $perf` command will propagate this non-zero exit status,
causing the script to immediately abort.

This would prevent the script from reaching the intended TRBE skip logic
below. Could this be updated to `wait $perf || true` similar to the
`wait $parent || true` above?

> +unset perf
> +
> +echo "Decoding..."
> +perf script -i "$tmpdir/data" > "$tmpdir/script" 2>/dev/null

[Severity: Medium]
Similarly, will this unshielded `perf script` command also abort the script
under `set -e` if the `perf.data` file is missing or invalid due to the
preceding recording error?

> +
> +# Exit early unless there is a dedicated sink per core which only TRBE
> +# guarantees. This is because shared sinks will report BUSY if two threads try
> +# to use them at the same time
> +if ! ls /sys/bus/coresight/devices/trbe* > /dev/null 2>&1; then
> +	echo "No TRBE sinks, skipping output validation"
> +	exit 0
> +fi

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-james-cs-unformatted-per-thread-fix-v2-0-00c4fd0701b4@linaro.org?part=14