Re: [PATCH v2 04/14] perf cs-etm: Use per-CPU queues for per-thread mode

[Leo Yan]

On Fri, Aug 21, 2026 at 10:49:02AM +0100, James Clark wrote:
> This fixes two problems, firstly per-thread mode didn't support multiple
> threads well or at all because we always use queue 0 and assumed it was
> for a single thread. However, Perf attaches to all child threads of one
> process or allows multiple threads to be specified on the commandline.
> This meant that trace before a context packet could use the wrong MMAP
> for decoding, and threads running concurrently wouldn't be interleaved
> properly.

If I connect this with patch 03, I understand "a context packet" here as
the trace synchronization with a timestamp, which is needed to correlate
with MMAP records.

What I don't quite understand is how switching to multiple queues fixes
the "trace before a context packet" case. Wouldn't decoding still need
to wait until the first timestamp/context information is available?

> The second problem is that we'd put all HW_IDs into queue 0, so if a
> thread runs on multiple CPUs then it would receive multiple HW_IDs and
> trigger the assert that unformatted trace should only have 1 decoder
> per-CPU:
> 
>   $ perf record --per-thread -e cs_etm//u -- taskset --cpu-list 0 \
>      taskset --cpu-list 1 true
>   $ perf script
> 
>   perf: util/cs-etm.c:3381: cs_etm__create_queue_decoders: Assertion `decoders == 1' failed.

Seems the commit log misses to explain how to fix issue.

> Fixes: 9182f04a85b2 ("perf cs-etm: Pass unformatted flag to decoder")
> Signed-off-by: James Clark <james.clark@xxxxxxxxxx>
> ---
>  tools/perf/util/cs-etm.c | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index d423f1cb408f..ccf886efff95 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -295,9 +295,6 @@ static int cs_etm__insert_trace_id_node(struct cs_etm_queue *etmq,
>  
>  static struct cs_etm_queue *cs_etm__get_queue(struct cs_etm_auxtrace *etm, int cpu)
>  {
> -	if (etm->per_thread_decoding)
> -		return etm->queues.queue_array[0].priv;
> -
>  	if (cpu < 0 || cpu >= (int)etm->queues.nr_queues)
>  		return NULL;
>  
> @@ -1268,7 +1265,7 @@ static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm,
>  	queue->priv = etmq;
>  	etmq->etm = etm;
>  	etmq->queue_nr = queue_nr;
> -	queue->cpu = queue_nr; /* Placeholder, may be reset to -1 in per-thread mode */
> +	queue->cpu = queue_nr;
>  	etmq->offset = 0;
>  	etmq->sink_id = SINK_UNSET;
>  
> @@ -2160,6 +2157,7 @@ static void cs_etm__flush_all_stack(struct cs_etm_queue *etmq)
>   */
>  static int cs_etm__get_data_block(struct cs_etm_queue *etmq)
>  {
> +	struct cs_etm_auxtrace *etm = etmq->etm;
>  	int ret;
>  
>  	/* The current block is not finished */
> @@ -2188,6 +2186,27 @@ static int cs_etm__get_data_block(struct cs_etm_queue *etmq)
>  	 */
>  	cs_etm__flush_all_stack(etmq);
>  
> +	/*
> +	 * Per-thread mode still uses a queue for each CPU, but that CPU can run
> +	 * different threads. When the TID from the AUX record on a CPU changes,
> +	 * re-initialize the thread using the AUX record/buffer fragment TID so
> +	 * we can start decoding even if the context ID packet was cropped or
> +	 * they're disabled.
> +	 */
> +	if (etm->per_thread_decoding) {
> +		struct cs_etm_traceid_queue *tidq = cs_etm__etmq_get_traceid_queue(etmq,
> +							CS_ETM_PER_THREAD_TRACEID);
> +
> +		if (thread__tid(tidq->decode_thread) != etmq->buffer->tid) {
> +			thread__zput(tidq->frontend_thread);
> +			thread__zput(tidq->decode_thread);
> +			tidq->frontend_thread = machine__findnew_thread(&etm->session->machines.host,
> +									-1, etmq->buffer->tid);
> +			tidq->decode_thread = machine__findnew_thread(&etm->session->machines.host,
> +								      -1, etmq->buffer->tid);
> +		}
> +	}

This works if the auxtrace fragment initially represents trace from that
TID. However, doesn't this introduce an assumption that an AUX fragment
contains trace from only one thread?

It seems this assumption impacts the kernel patch [1], the tool's
implementatoin imposes the limitation that driver to cannot support
multiple threads run concurrently with shared sink.

Would it be better to enable contextidr for per-thread mode and rely on
Context ID packets to update the thread context? We could then reuse
cs_etm__etmq_update_thread() / cs_etm__etmq_update_decode_context()
rather than relying on the AUX fragment TID for this purpose.

[1] https://lore.kernel.org/linux-arm-kernel/d833aeed-7376-48af-b772-4fe4ac66471d@xxxxxxxxxx/