On 26/08/2026 20:06, Leo Yan wrote:
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?
Context IDs aren't enabled in per-thread mode, we still use the TID initially assigned to the queue for mmap lookup. That TID comes from "etmq->buffer->tid" which is always correct in per-thread mode. Even if we turned on context IDs they might either be cropped in snapshot mode, or not exist when we drop the PID_IN_CONTEXTIDR hard requirement.
The commit message is a bit misleading, it should say "trace before a trace synchronization *timestamp*".
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.
This issue is gone now, V3 will be based on "perf: cs-etm: HW_ID improvements" which doesn't read HW_IDs at all for unformatted mode. I'll remove this and the fixes tag.
For information, the fix explanation was:
"The second problem is that we'd put all HW_IDs into queue 0" ... making that queue have multiple decoders ... Fix it by not special casing queue 0 in per-thread mode in cs_etm__get_queue(), so each HW_ID goes into it's correct per-CPU queue and leaves only 1 decoder"
Fixes: 9182f04a85b2 ("perf cs-etm: Pass unformatted flag to decoder") Signed-off-by: James Clark james.clark@linaro.org
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.
Seems like we've decided to prohibit multiple per-thread events with shared sinks, so we can keep the assumption that each fragment only contains trace from buffer->tid. This is similar to the assumption in the aux sampling patches where we initialise the thread with the TID of the sample.
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-4fe4ac66471...
This comment is probably a bit stale now given the latest version of [1] where a per-thread event can only contain trace from one thread. So the context packets aren't useful. I thought the plan was to eventually drop the requirement for PID_IN_CONTEXTIDR and rely on timestamps anyway, so it doesn't make sense to add it in more places.