Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
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,



  parent reply	other threads:[~2026-08-24 10:03 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