From: Adrian Hunter <adrian.hunter@intel.com>
To: Amir Ayupov <aaupov@fb.com>, <linux-perf-users@vger.kernel.org>,
<coresight@lists.linaro.org>,
<linux-arm-kernel@lists.infradead.org>,
"Suzuki K Poulose" <suzuki.poulose@arm.com>,
James Clark <james.clark@linaro.org>, "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>,
John Garry <john.g.garry@oracle.com>,
"Will Deacon" <will@kernel.org>
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>
Subject: Re: [PATCH v2 3/5] perf cs-etm: Add branch history to existing samples
Date: Mon, 24 Aug 2026 13:02:38 +0300 [thread overview]
Message-ID: <00f023a5-37da-4884-a92a-f4b35b4be1cb@intel.com> (raw)
In-Reply-To: <454bc49f51eeef9f758fc2d2c7af75889d8aff46.1787005265.git.aaupov@fb.com>
On 18/08/2026 01:22, Amir Ayupov wrote:
> Implement --itrace=L for CoreSight ETM: decode timestamped trace up to
> each existing PMU sample and attach the branch history that led to it.
> The sample keeps its own ip, callchain and event identity, and a sample
> that already carries a branch stack is left alone.
>
> Samples are correlated with the trace by time, so this requires virtual
> ETM timestamps that are correlated to perf time; timeless decoding is
> rejected. The decode loop, which the previous patch left on its own in
> cs_etm__process_timestamped_queues(), grows a timestamp argument and
> stops once the decode frontier reaches it, so on return the
> thread stack holds the branches that executed before the sample and none
> that executed after. Attaching then reduces to the same
> thread_stack__br_sample_late() call intel-pt uses.
>
> No explicit sample-to-queue matching is needed:
> thread_stack__br_sample_late() keys on the thread, and the thread stack
> is already emptied whenever the decoder reports a discontinuity. The one
> case that was not covered is a queue whose trace runs out: flush the
> thread stack there too, otherwise samples recorded after the last trace
> would pick up stale history.
>
> Take the branch history when attaching it rather than leaving it in the
> thread stack. With AUX pause and resume, a pause sample ends a completed
> trace window and that window belongs to the sample. Execution while AUX
> is paused is not traced, so retaining the window would let a later sample
> reuse branches from before the untraced gap. Consuming it ensures that a
> sample with no newly decoded trace gets an empty branch stack instead.
>
> As with intel-pt, the internal reconstruction ring is kept deeper than
> the requested output depth to cover branches decoded between the sampled
> ip and the point at which the sample time was recorded, so --itrace=L<n>
> can actually return n entries. Kernel-inclusive trace gets the same
> conservative 1024-entry headroom that intel-pt uses.
>
> Assisted-by: Devmate:GPT-5.6
> Signed-off-by: Amir Ayupov <aaupov@fb.com>
> ---
> tools/perf/util/cs-etm.c | 185 +++++++++++++++++++++++++++++++--
> tools/perf/util/thread-stack.c | 17 +++
> tools/perf/util/thread-stack.h | 1 +
> 3 files changed, 195 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 4d895f11deb7f..00407a80933e1 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -72,6 +72,11 @@ struct cs_etm_auxtrace {
> bool use_callchain;
>
> int num_cpu;
> + /* Output depth requested with --itrace=L<n> */
> + unsigned int br_stack_sz;
> + /* Internal reconstruction depth, see cs_etm__br_stack_init() */
> + unsigned int br_stack_sz_plus;
> + struct branch_stack *br_stack;
> u64 latest_kernel_timestamp;
> u32 auxtrace_type;
> u32 branches_filter;
> @@ -91,6 +96,7 @@ struct cs_etm_traceid_queue {
> u64 kernel_start;
> union perf_event *event_buf;
> unsigned int br_stack_sz;
> + unsigned int br_stack_sz_plus;
> struct branch_stack *last_branch;
> struct ip_callchain *callchain;
> struct cs_etm_packet *prev_packet;
> @@ -141,7 +147,8 @@ struct cs_etm_queue {
> };
>
> static int cs_etm__update_queues(struct cs_etm_auxtrace *etm);
> -static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm);
> +static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm,
> + u64 timestamp);
> static int cs_etm__flush_timestamped_queues(struct cs_etm_auxtrace *etm);
> static int cs_etm__process_timeless_queues(struct cs_etm_auxtrace *etm,
> pid_t tid);
> @@ -165,6 +172,7 @@ static int cs_etm__metadata_set_trace_id(u8 trace_chan_id, u64 *cpu_metadata);
> #define TO_QUEUE_NR(cs_queue_nr) (cs_queue_nr >> 16)
> #define TO_TRACE_CHAN_ID(cs_queue_nr) (cs_queue_nr & 0x0000ffff)
> #define SINK_UNSET ((u32) -1)
> +#define MAX_TIMESTAMP (~0ULL)
>
> static u32 cs_etm__get_v7_protocol_version(u32 etmidr)
> {
> @@ -674,7 +682,8 @@ static int cs_etm__init_traceid_queue(struct cs_etm_queue *etmq,
> if (!tidq->last_branch)
> goto out_free;
>
> - tidq->br_stack_sz = etm->synth_opts.last_branch_sz;
> + tidq->br_stack_sz = etm->br_stack_sz;
> + tidq->br_stack_sz_plus = etm->br_stack_sz_plus;
> }
>
> if (etm->synth_opts.callchain) {
> @@ -794,7 +803,7 @@ static void cs_etm__packet_swap(struct cs_etm_auxtrace *etm,
> struct cs_etm_packet *tmp;
>
> if (etm->synth_opts.branches || etm->synth_opts.last_branch ||
> - etm->synth_opts.instructions) {
> + etm->synth_opts.add_last_branch || etm->synth_opts.instructions) {
> /*
> * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
> * the next incoming packet.
> @@ -963,7 +972,7 @@ static int cs_etm__flush_events(struct perf_session *session,
> if (ret)
> return ret;
>
> - ret = cs_etm__process_timestamped_queues(etm);
> + ret = cs_etm__process_timestamped_queues(etm, MAX_TIMESTAMP);
> if (ret)
> return ret;
>
> @@ -1060,6 +1069,7 @@ static void cs_etm__free(struct perf_session *session)
> zfree(&aux->metadata[i]);
>
> zfree(&aux->metadata);
> + zfree(&aux->br_stack);
> zfree(&aux);
> }
>
> @@ -1597,7 +1607,8 @@ static void cs_etm__add_stack_event(struct cs_etm_queue *etmq,
> u64 from, to;
> int size;
>
> - if (!etm->synth_opts.branches && !etm->synth_opts.instructions)
> + if (!etm->synth_opts.branches && !etm->synth_opts.instructions &&
> + !etm->synth_opts.add_last_branch)
> return;
>
> if (!cs_etm__packet_has_taken_branch(tidq->prev_packet))
> @@ -1614,7 +1625,7 @@ static void cs_etm__add_stack_event(struct cs_etm_queue *etmq,
> tidq->prev_packet->flags, from, to, size,
> etmq->buffer->buffer_nr + 1,
> etmq->etm->use_callchain,
> - tidq->br_stack_sz, 0);
> + tidq->br_stack_sz_plus, 0);
> } else {
> thread_stack__set_trace_nr(tidq->frontend_thread,
> tidq->prev_packet->cpu,
> @@ -2817,7 +2828,8 @@ static int cs_etm__update_queues(struct cs_etm_auxtrace *etm)
> return ret;
> }
>
> -static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm)
> +static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm,
> + u64 timestamp)
> {
> int ret = 0;
> unsigned int cs_queue_nr, queue_nr;
> @@ -2831,6 +2843,9 @@ static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm)
> if (!etm->heap.heap_cnt)
> break;
>
> + if (etm->heap.heap_array[0].ordinal >= timestamp)
> + break;
> +
> /* Take the entry at the top of the min heap */
> cs_queue_nr = etm->heap.heap_array[0].queue_nr;
> queue_nr = TO_QUEUE_NR(cs_queue_nr);
> @@ -2878,8 +2893,25 @@ static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm)
> * No more auxtrace_buffers to process in this etmq, simply
> * move on to another entry in the auxtrace_heap.
> */
> - if (!ret)
> + if (!ret) {
> + /*
> + * The trace for this physical queue is exhausted. Drop
> + * branch history for every trace ID it carried so that
> + * samples arriving later cannot pick up entries decoded
> + * before the gap.
> + */
> + if (etm->synth_opts.add_last_branch) {
> + struct int_node *inode;
> +
> + intlist__for_each_entry(inode, etmq->traceid_queues_list) {
> + int idx = (int)(intptr_t)inode->priv;
> +
> + tidq = etmq->traceid_queues[idx];
> + thread_stack__flush(tidq->frontend_thread);
> + }
> + }
> continue;
> + }
>
> ret = cs_etm__decode_data_block(etmq);
> if (ret)
> @@ -3011,6 +3043,116 @@ static int cs_etm__process_switch_cpu_wide(struct cs_etm_auxtrace *etm,
> return 0;
> }
>
> +static bool cs_etm__tracing_kernel(struct cs_etm_auxtrace *etm,
> + struct perf_session *session)
> +{
> + struct evsel *evsel;
> +
> + evlist__for_each_entry(session->evlist, evsel) {
> + if (evsel->core.attr.type == etm->pmu_type &&
> + !evsel->core.attr.exclude_kernel)
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static int cs_etm__br_stack_init(struct cs_etm_auxtrace *etm,
> + struct perf_session *session)
> +{
> + struct evsel *evsel;
> +
> + evlist__for_each_entry(session->evlist, evsel) {
> + /*
> + * Only timestamped events can be matched against the decoded
> + * trace, so do not advertise a branch stack on any other.
> + */
> + if (!(evsel->core.attr.sample_type & PERF_SAMPLE_TIME))
> + continue;
> + if (!(evsel->core.attr.sample_type & PERF_SAMPLE_BRANCH_STACK))
> + evsel->synth_sample_type |= PERF_SAMPLE_BRANCH_STACK;
> + }
> +
> + /*
> + * Additional branch stack depth to cater for the branches decoded
> + * between the sampled ip and the point at which the sample time was
> + * recorded. Those are trimmed by thread_stack__br_sample_late(), so
> + * the extra depth keeps the requested output depth achievable. If
> + * kernel space is not traced, only the branch into the kernel needs
> + * to be accounted for.
> + */
> + if (cs_etm__tracing_kernel(etm, session))
> + etm->br_stack_sz_plus += 1024;
> + else
> + etm->br_stack_sz_plus += 1;
> +
> + etm->br_stack = zalloc(sizeof(struct branch_stack) +
> + etm->br_stack_sz * sizeof(struct branch_entry));
> + if (!etm->br_stack)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +/*
> + * Add decoded branch history to an existing sample. The sample keeps its own
> + * ip, callchain and event identity; only an absent branch stack is filled in.
> + */
> +static int cs_etm__process_sample(struct cs_etm_auxtrace *etm,
> + struct perf_session *session,
> + struct perf_sample *sample)
> +{
> + struct machine *machine = &session->machines.host;
> + struct thread *thread;
> + int err;
> +
> + if (!etm->synth_opts.add_last_branch || sample->branch_stack ||
> + !sample->ip || !sample->time || sample->time == (u64)-1)
> + return 0;
> +
> + /* Adding branch history to existing samples supports the host only */
> + if (sample->cpumode == PERF_RECORD_MISC_GUEST_KERNEL ||
> + sample->cpumode == PERF_RECORD_MISC_GUEST_USER)
> + return 0;
> +
> + err = cs_etm__update_queues(etm);
> + if (err)
> + return err;
> +
> + /*
> + * Decode every queue up to this sample's time. Afterwards the thread
> + * stack holds the branches that executed before the sample, and
> + * nothing that executed after it.
> + */
> + err = cs_etm__process_timestamped_queues(etm, sample->time);
> + if (err)
> + return err;
> +
> + thread = machine__findnew_thread(machine, sample->pid, sample->tid);
> + if (!thread)
> + return -ENOMEM;
> +
> + /*
> + * Take the branch history rather than copying it. The trace window
> + * belongs to the sample that ends it, so once it has been attached a
> + * later sample with nothing newly decoded finds an empty stack rather
> + * than being given an earlier window's branches. That is the common
> + * case whenever the trace is duty cycled, by AUX pause/resume or by
> + * ETM strobing.
> + */
> + thread_stack__br_sample_late(thread, sample->cpu, etm->br_stack,
> + etm->br_stack_sz, sample->ip,
> + machine__kernel_start(machine));
> + thread_stack__br_stack_consume(thread, sample->cpu);
Did you consider using the existing thread_stack__set_trace_nr()?
> +
> + if (etm->br_stack->nr)
> + sample->branch_stack = etm->br_stack;
> +
> + thread__put(thread);
> +
> + return 0;
> +}
> +
> static int cs_etm__process_event(struct perf_session *session,
> union perf_event *event,
> struct perf_sample *sample,
> @@ -3049,6 +3191,9 @@ static int cs_etm__process_event(struct perf_session *session,
> case PERF_RECORD_SWITCH_CPU_WIDE:
> return cs_etm__process_switch_cpu_wide(etm, event);
>
> + case PERF_RECORD_SAMPLE:
> + return cs_etm__process_sample(etm, session, sample);
> +
> case PERF_RECORD_AUX:
> /*
> * Record the latest kernel timestamp available in the header
> @@ -3752,11 +3897,34 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
>
> etm->use_thread_stack = etm->synth_opts.thread_stack ||
> etm->synth_opts.last_branch ||
> + etm->synth_opts.add_last_branch ||
> etm->synth_opts.callchain;
>
> etm->use_callchain = etm->synth_opts.thread_stack ||
> etm->synth_opts.callchain;
>
> + if (etm->synth_opts.last_branch || etm->synth_opts.add_last_branch) {
> + etm->br_stack_sz = etm->synth_opts.last_branch_sz;
> + etm->br_stack_sz_plus = etm->br_stack_sz;
> + }
> +
> + if (etm->synth_opts.add_last_branch) {
> + /*
> + * Existing samples are matched to decoded trace by time, so
> + * the trace must carry timestamps that are correlated to perf
> + * time and the queues must be decoded in time order.
> + */
> + if (etm->timeless_decoding || !etm->has_virtual_ts) {
> + pr_err("CS ETM Trace: --itrace=L requires virtual timestamped trace\n");
> + err = -EINVAL;
> + goto err_free_queues;
> + }
> +
> + err = cs_etm__br_stack_init(etm, session);
> + if (err)
> + goto err_free_queues;
> + }
> +
> err = cs_etm__synth_events(etm, session);
> if (err)
> goto err_free_queues;
> @@ -3812,6 +3980,7 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
> auxtrace_queues__free(&etm->queues);
> session->auxtrace = NULL;
> err_free_etm:
> + zfree(&etm->br_stack);
> zfree(&etm);
> err_free_metadata:
> /* No need to check @metadata[j], free(NULL) is supported */
> diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
> index 1360f44421ef8..2713a2ad70b69 100644
> --- a/tools/perf/util/thread-stack.c
> +++ b/tools/perf/util/thread-stack.c
> @@ -614,6 +614,23 @@ void thread_stack__sample_late(struct thread *thread, int cpu,
> }
> }
>
> +/*
> + * Branch history belongs to the sample that ends the trace window, so a
> + * decoder that attaches it to an existing sample should take it rather than
> + * copy it. A later sample with no newly decoded trace then finds an empty
> + * branch stack instead of the previous window's branches.
> + */
> +void thread_stack__br_stack_consume(struct thread *thread, int cpu)
> +{
> + struct thread_stack *ts = thread__stack(thread, cpu);
> +
> + if (!ts || !ts->br_stack_rb)
> + return;
> +
> + ts->br_stack_pos = 0;
> + ts->br_stack_rb->nr = 0;
> +}
> +
> void thread_stack__br_sample(struct thread *thread, int cpu,
> struct branch_stack *dst, unsigned int sz)
> {
> diff --git a/tools/perf/util/thread-stack.h b/tools/perf/util/thread-stack.h
> index b3cd09beb62f0..2aec292bd1bcb 100644
> --- a/tools/perf/util/thread-stack.h
> +++ b/tools/perf/util/thread-stack.h
> @@ -88,6 +88,7 @@ void thread_stack__sample(struct thread *thread, int cpu, struct ip_callchain *c
> void thread_stack__sample_late(struct thread *thread, int cpu,
> struct ip_callchain *chain, size_t sz, u64 ip,
> u64 kernel_start);
> +void thread_stack__br_stack_consume(struct thread *thread, int cpu);
> void thread_stack__br_sample(struct thread *thread, int cpu,
> struct branch_stack *dst, unsigned int sz);
> void thread_stack__br_sample_late(struct thread *thread, int cpu,
next prev parent reply other threads:[~2026-08-24 10:02 UTC|newest]
Thread overview: 13+ 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-24 10:10 ` Adrian Hunter
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-24 10:02 ` Adrian Hunter [this message]
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
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=00f023a5-37da-4884-a92a-f4b35b4be1cb@intel.com \
--to=adrian.hunter@intel.com \
--cc=aaupov@fb.com \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=corbet@lwn.net \
--cc=coresight@lists.linaro.org \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--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