Re: [PATCH v10 6/9] perf cs-etm: Flush thread stacks after decoder reset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing thread stack flush for traces recorded without context IDs (`CS_ETM_PIDFMT_NONE`), leaving the stale callchain bug unfixed.
- [High] Global thread stack flush on a per-queue (per-CPU) trace discontinuity inappropriately wipes valid callchains for unaffected threads on other CPUs.
--

commit 50abc287497c8a3fa0588b51842fb028097af238
Author: Leo Yan <leo.yan@arm.com>

perf cs-etm: Flush thread stacks after decoder reset

Perf resets the CoreSight decoder when moving to a new AUX trace buffer,
which causes a trace discontinuity globally. For callchain synthesis, keeping
thread-stack state after a decoder reset can leave stale call/return history
attached to threads that are decoded later. This patch flushes all host thread
stacks, and guest thread stacks if virtualization is present, after a decoder
reset.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 5ede0f0ff8c6e..e43f0c1dd0078 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c

[ ... ]

> +static void cs_etm__flush_all_stack(struct cs_etm_queue *etmq)
> +{
> +	enum cs_etm_pid_fmt pid_fmt = cs_etm__get_pid_fmt(etmq);
> +
> +	if (!etmq->etm->synth_opts.last_branch)
> +		return;
> +
> +	switch (pid_fmt) {
> +	case CS_ETM_PIDFMT_CTXTID2:
> +		/* Clear the guest stack if virtualization is supported */
> +		cs_etm__flush_machine_stack(etmq, DEFAULT_GUEST_KERNEL_ID);
> +		fallthrough;
> +	case CS_ETM_PIDFMT_CTXTID:
> +		cs_etm__flush_machine_stack(etmq, HOST_KERNEL_ID);
> +		break;
> +	case CS_ETM_PIDFMT_NONE:
> +	default:
> +		break;

[Severity: High]
Does skipping the flush for CS_ETM_PIDFMT_NONE leave the stale callchain
bug unfixed for traces without context IDs?

Even when Context IDs are absent (e.g., in --per-thread mode or unprivileged
system-wide tracing), perf still maps execution to tidq->frontend_thread on
the host machine (HOST_KERNEL_ID), accumulating branch stack history on those
threads.

By bypassing the flush here, a decoder reset discontinuity would leave stale
call/return history on these thread stacks, producing the exact same incorrect
synthesized callchains this patch intends to fix.

> +
> +	}
> +}
> +
>  /*
>   * cs_etm__get_data_block: Fetch a block from the auxtrace_buffer queue
>   *			   if need be.
> @@ -2034,6 +2073,12 @@ static int cs_etm__get_data_block(struct cs_etm_queue *etmq)
>  		ret = cs_etm_decoder__reset(etmq->decoder);
>  		if (ret)
>  			return ret;
> +
> +		/*
> +		 * Since the decoder is reset, this causes a global trace
> +		 * discontinuity. Flush all thread stacks.
> +		 */
> +		cs_etm__flush_all_stack(etmq);

[Severity: High]
Does this code unnecessarily wipe active branch histories on unaffected CPUs?

When a trace buffer hits a discontinuity, cs_etm__get_data_block() resets the
decoder for that specific etmq. However, cs_etm__flush_all_stack() ends up
calling machine__for_each_thread() to indiscriminately flush the thread stacks
of every single traced thread system-wide.

In a per-CPU tracing scenario, a trace buffer wrap on CPU 0 is a local,
queue-specific discontinuity. By flushing the global machine, doesn't this
unnecessarily wipe the contiguous and valid branch histories of threads
currently executing on other CPUs, potentially corrupting their callchain
synthesis?

>  	}
>  
>  	return etmq->buf_len;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-b4-arm_cs_callchain_support_v1-v10-0-e8b6e5d63db5@arm.com?part=6