Re: [PATCH 7/9] perf test cs-etm: Test branch history on existing samples

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `check_callchains` does not verify callchains for each process as claimed in the commit message.
--

--- Patch [7]: [PATCH 7/9] perf test cs-etm: Test branch history on existing samples ---
commit e252ec70028dd10b925f2e4ea78b626688f184d4
Author: Amir Ayupov <aaupov@fb.com>

perf test cs-etm: Test branch history on existing samples

This commit adds a CoreSight shell test to verify branch history on existing
samples using explicitly timestamped ETM traces. It decodes traces with L4 and
L64 depth settings and checks that pause samples contain valid multi-frame
callchains and bounded branch stacks.

> diff --git a/tools/perf/tests/shell/coresight/add_last_branch.sh b/tools/perf/tests/shell/coresight/add_last_branch.sh
> new file mode 100755
> --- /dev/null
> +++ b/tools/perf/tests/shell/coresight/add_last_branch.sh

[ ... ]

> +check_callchains()
> +{
> +	local output="$tmpdir/script-callchain"
> +
> +	perf script -i "$tmpdir/data" -F comm,event,ip >"$output" 2>/dev/null
> +
> +	awk '
> +		/cycles\/aux-action=pause/ {
> +			in_sample = 1
> +			frames = 0
> +			next
> +		}
> +		!NF {
> +			if (in_sample && frames >= 2)
> +				found = 1
> +			in_sample = 0
> +			next
> +		}

[Severity: Medium]
The commit message states the test will "check that the pause samples carry
both a multi-frame callchain and a non-empty branch stack for each of the
workload's two processes."

Does this awk script in check_callchains() verify the callchains for each
process?

Unlike check_process_samples(), which checks for proc1 and proc2, this
script appears to set found=1 as soon as any sample contains a multi-frame
callchain, without validating the comm field.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803090640.2412336-1-aaupov@fb.com?part=7