Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: James Clark <james.clark@linaro.org>
To: Amir Ayupov <aaupov@fb.com>
Cc: linux-doc@vger.kernel.org, Mike Leach <mike.leach@arm.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Swapnil Sapkal <swapnil.sapkal@amd.com>,
	linux-perf-users@vger.kernel.org, coresight@lists.linaro.org,
	linux-arm-kernel@lists.infradead.org,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Leo Yan <leo.yan@arm.com>, Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	John Garry <john.g.garry@oracle.com>,
	Will Deacon <will@kernel.org>
Subject: Re: [PATCH v2 4/5] perf test cs-etm: Test branch history on existing samples
Date: Tue, 18 Aug 2026 15:24:02 +0100	[thread overview]
Message-ID: <a96a5b96-707f-4a98-bd84-1aa919b9bccb@linaro.org> (raw)
In-Reply-To: <70f7ccb31e81282212161177124dfc3beff24ad9.1787005265.git.aaupov@fb.com>



On 17/08/2026 23:22, Amir Ayupov wrote:
> Add a CoreSight shell test for --itrace=L. Record timestamped ETM trace
> with explicit -T sample timestamps and AUX pause/resume events, then
> 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.
> 
> Decode the same recording with L4 and L64 and reject any branch stack
> deeper than the requested depth.
> 
> The test skips when cs_etm is absent, when not run as root, or when the
> recording turns out to lack virtual timestamps. It exercises the
> timestamp-gated path and the requested-depth bound; it does not attempt
> to verify that the attached history is correlated to the sample.
> 
> Assisted-by: Devmate:GPT-5.6
> Signed-off-by: Amir Ayupov <aaupov@fb.com>
> ---
>   .../tests/shell/coresight/add_last_branch.sh  | 203 ++++++++++++++++++
>   1 file changed, 203 insertions(+)
>   create mode 100755 tools/perf/tests/shell/coresight/add_last_branch.sh
> 
> 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
> index 0000000000000..6f09e720abe80
> --- /dev/null
> +++ b/tools/perf/tests/shell/coresight/add_last_branch.sh
> @@ -0,0 +1,203 @@
> +#!/bin/bash -e
> +# SPDX-License-Identifier: GPL-2.0
> +# CoreSight branch history on existing samples (exclusive)
> +
> +perf list pmu | grep -q 'cs_etm//' || exit 2
> +
> +if [ "$(id -u)" != 0 ]; then
> +	echo "[Skip] No root permission"
> +	exit 2
> +fi
> +
> +tmpdir=$(mktemp -d /tmp/perf-cs-add-last-branch.XXXXX)
> +
> +cleanup()
> +{
> +	rm -rf "$tmpdir"
> +	trap - EXIT TERM INT
> +}
> +
> +# shellcheck disable=SC2317 # Called through trap.
> +trap_cleanup()
> +{
> +	cleanup
> +	exit 1
> +}
> +trap trap_cleanup EXIT TERM INT
> +
> +record_data()
> +{
> +	local cf="$tmpdir/ctl"
> +	local af="$tmpdir/ack"
> +
> +	mkfifo "$cf" "$af"
> +
> +	# Pin to one CPU so proc1 and proc2 alternate in one per-CPU trace
> +	# buffer. Start disabled and use the control FIFO to record only the
> +	# workload, not perf test setup and teardown.

This is more of a what comment than a why. I got that we were doing 
that, but the reason I left the comment on V1 was because I couldn't see 
why context switching is related to branch history. Doesn't the test 
still test the same thing if you record a single process without -C?

It's a bit hard to see what behavior the test is exercising. I see it 
also adds call-graph=fp, but it doesn't look for symbol names. How is 
testing for any non zero callchain related to coresight unless we also 
check the coresight branch stack matches it exactly via symbol names? 
Seems like it's just testing some other part of Perf here.

I would expect some references to the named symbols 
"context_switch_loop_proc2" from the workload, but I don't see them. It 
also seems to be very interested in timestamps as per the commit 
message. But how do you know it's not attaching the branch stack from 
proc1 to proc2 because it gets the timestamps wrong?

If it's easier to add a new workload with a deterministic branch history 
written in asm after a deterministic call chain we can do that. Then the 
test can test if a fragment of the branch history appears after an end 
fragment of the callchain.

> +	if perf record -T -o "$tmpdir/data" -C 0 -D -1 \
> +		--control fifo:"$cf","$af" \
> +		-e cs_etm/aux-action=start-paused/u \
> +		-e cycles/aux-action=resume,period=550019/u \
> +		-e cycles/aux-action=pause,period=100003,call-graph=fp/u -- \
> +		taskset --cpu-list 0 perf test --record-ctl fifo:"$cf","$af" \
> +		-w context_switch_loop 10000 \
> +		>/dev/null 2>"$tmpdir/stderr"; then
> +		return 0
> +	fi
> +
> +	echo "Failed to record ETM trace with AUX pause/resume" >&2
> +	cat "$tmpdir/stderr" >&2
> +	return 1
> +}
> +
> +decode()
> +{
> +	local size=$1
> +	local output=$2
> +
> +	if perf script -i "$tmpdir/data" --itrace="L$size" \
> +		-F comm,pid,tid,event,ip,brstack >"$output" \
> +		2>"$tmpdir/stderr"; then
> +		return 0
> +	fi
> +
> +	if grep -q "itrace=L requires virtual timestamped trace" \
> +		"$tmpdir/stderr"; then
> +		echo "[Skip] Virtual CoreSight timestamps are not available"
> +		cleanup
> +		exit 2
> +	fi
> +
> +	cat "$tmpdir/stderr" >&2
> +	return 1
> +}
> +
> +check_process_samples()
> +{
> +	local output=$1
> +	local comm
> +
> +	# Expect each process to have a pause-event sample followed by at least
> +	# one branch entry in 0xFROM/0xTO/... form.

Can we have an exact copy paste of the output instead of the text 
description? I still can't tell if what I'm seeing is expected based on 
this.

For example I get this, which you could paste verbatim into the test:

   proc2 
armv8_pmuv3_0/cycles,aux-action=pause,period=100003,call-graph=fp/u:
	ffff800080021440
	ffff8000813a6a04

(This is output from V1, I couldn't run V2 because of the invalid group 
desc issue)

> +	for comm in proc1 proc2; do
> +		awk -v comm="$comm" '
> +			$1 == comm && /cycles\/aux-action=pause/ {
> +				in_sample = 1
> +				next
> +			}
> +			!NF {
> +				in_sample = 0
> +				next
> +			}
> +			in_sample && /0x[[:xdigit:]]+\/0x[[:xdigit:]]+\// {
> +				found = 1
> +			}
> +			END { exit !found }
> +		' "$output" || {
> +			echo "No pause-event branch stack found for $comm" >&2
> +			grep -A 4 "^$comm .*cycles/aux-action=pause" "$output" \
> +				| head -n 20 >&2 || true
> +			return 1
> +		}
> +	done
> +}
> +
> +check_callchains()
> +{
> +	local output="$tmpdir/script-callchain"
> +	local comm
> +
> +	if ! perf script -i "$tmpdir/data" -F comm,event,ip >"$output" \
> +		2>"$tmpdir/stderr"; then
> +		echo "Failed to dump pause-event callchains" >&2
> +		cat "$tmpdir/stderr" >&2
> +		return 1
> +	fi
> +
> +	# Expect a pause-event header for each process followed by at least two
> +	# indented instruction-pointer frames.

Ditto

> +	for comm in proc1 proc2; do
> +		awk -v comm="$comm" '
> +			$1 == comm && /cycles\/aux-action=pause/ {
> +				in_sample = 1
> +				frames = 0
> +				next
> +			}
> +			!NF {
> +				if (in_sample && frames >= 2)
> +					found = 1
> +				in_sample = 0
> +				next
> +			}
> +			in_sample && /^[[:space:]]+[[:xdigit:]]+([[:space:]]|$)/ {
> +				frames++
> +			}
> +			END {
> +				if (in_sample && frames >= 2)
> +					found = 1
> +				exit !found
> +			}
> +		' "$output" || {
> +			echo "No multi-frame pause-event callchain found for $comm" >&2
> +			grep -A 8 "^$comm .*cycles/aux-action=pause" "$output" \
> +				| head -n 40 >&2 || true
> +			return 1
> +		}
> +	done
> +}
> +
> +check_branch_stacks()
> +{
> +	local output=$1
> +	local max_entries=$2
> +
> +	local ret
> +
> +	if awk -v max="$max_entries" '
> +		/0x[[:xdigit:]]+\/0x[[:xdigit:]]+\// {
> +			entries = 0
> +			for (i = 1; i <= NF; i++)
> +				if ($i ~ /^0x[[:xdigit:]]+\/0x[[:xdigit:]]+\//)
> +					entries++
> +			if (entries)
> +				found = 1
> +			if (entries > max) {
> +				status = 2
> +				exit
> +			}
> +		}
> +		END {
> +			if (status)
> +				exit status
> +			if (!found)
> +				exit 1
> +		}
> +	' "$output"; then
> +		return 0
> +	else
> +		ret=$?
> +	fi
> +
> +	case $ret in
> +	1) echo "No ETM branch stacks found" >&2 ;;
> +	2) echo "Branch stack exceeds requested L$max_entries depth" >&2 ;;
> +	esac
> +	# Expected decoded pause-event lines contain at most L<n> branch entries.
> +	grep 'cycles/aux-action=pause' "$output" | head -n 5 >&2 || true
> +	return 1
> +}
> +
> +record_data
> +check_callchains
> +
> +decode 4 "$tmpdir/script-L4"
> +check_process_samples "$tmpdir/script-L4"
> +check_branch_stacks "$tmpdir/script-L4" 4
> +
> +decode 64 "$tmpdir/script-L64"
> +check_process_samples "$tmpdir/script-L64"
> +check_branch_stacks "$tmpdir/script-L64" 64
> +
> +cleanup
> +exit 0



  reply	other threads:[~2026-08-18 14:24 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17 22:22 [PATCH v2 0/5] perf: Add CoreSight branch history to existing samples Amir Ayupov
2026-08-17 22:22 ` [PATCH v2 1/5] perf dlfilter: Add non-empty branch stack filter Amir Ayupov
2026-08-17 22:22 ` [PATCH v2 2/5] perf cs-etm: Split up cs_etm__process_timestamped_queues() Amir Ayupov
2026-08-18 14:38   ` James Clark
2026-08-17 22:22 ` [PATCH v2 3/5] perf cs-etm: Add branch history to existing samples Amir Ayupov
2026-08-18 15:09   ` James Clark
2026-08-18 15:13     ` James Clark
2026-08-17 22:22 ` [PATCH v2 4/5] perf test cs-etm: Test branch history on " Amir Ayupov
2026-08-18 14:24   ` James Clark [this message]
2026-08-17 22:22 ` [PATCH v2 5/5] Documentation: coresight: Document context-sensitive PGO workflow Amir Ayupov
2026-08-18 13:56 ` [PATCH v2 0/5] perf: Add CoreSight branch history to existing samples James Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a96a5b96-707f-4a98-bd84-1aa919b9bccb@linaro.org \
    --to=james.clark@linaro.org \
    --cc=aaupov@fb.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=corbet@lwn.net \
    --cc=coresight@lists.linaro.org \
    --cc=irogers@google.com \
    --cc=john.g.garry@oracle.com \
    --cc=jolsa@kernel.org \
    --cc=leo.yan@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mike.leach@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=skhan@linuxfoundation.org \
    --cc=suzuki.poulose@arm.com \
    --cc=swapnil.sapkal@amd.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox