Re: [PATCH 6/9] perf cs-etm: Add branch history to existing samples

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Stale branch history is left unflushed for multiplexed trace IDs when a queue is exhausted.
- [Medium] cs_etm__tracing_kernel only evaluates the exclude_kernel attribute of the first matching cs_etm event.
--

--- Patch [6]: [PATCH 6/9] perf cs-etm: Add branch history to existing samples ---
commit fed4660720fbc50c143c4ee374a10328c53686f9
Author: Amir Ayupov <aaupov@fb.com>

perf cs-etm: Add branch history to existing samples

This commit implements --itrace=L for CoreSight ETM to decode timestamped
trace up to each existing PMU sample and attach the branch history. It adds
virtual ETM timestamp correlation and keeps an internal reconstruction ring
deeper than the requested output depth to cover branches decoded between the
sampled IP and the recorded sample time.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 4d895f11deb7f..048ff97caa936 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
[ ... ]
> @@ -2878,8 +2893,16 @@ 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 queue is exhausted. Drop any
> +			 * branch history so that samples arriving later
> +			 * cannot pick up entries decoded before the gap.
> +			 */
> +			if (etm->synth_opts.add_last_branch)
> +				thread_stack__flush(tidq->frontend_thread);

[Severity: Medium]
When an ETM queue (etmq) multiplexing multiple trace channels runs out of
auxtrace data blocks, is it sufficient to flush only the active trace ID?

Since this only flushes the specific tidq (from trace_chan_id) that was
popped from the min-heap, could other trace IDs sharing the exhausted queue
be skipped? This might leave stale branch history that could be incorrectly
attributed to PMU samples in a later trace window.

>  			continue;
> +		}
>  
>  		ret = cs_etm__decode_data_block(etmq);
[ ... ]
> @@ -3011,6 +3034,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)
> +			return !evsel->core.attr.exclude_kernel;

[Severity: Medium]
If the evlist contains multiple cs_etm events (such as one excluding kernel
and a later one including it), does returning on the first match prevent
subsequent events from being evaluated?

If a subsequent event includes kernel tracing, missing it here could lead to
an undersized internal branch reconstruction stack (br_stack_sz_plus), which
might cause branches leading up to the sample to be overwritten.

> +	}
> +
> +	return false;
> +}

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