Re: [PATCH v4 06/19] perf test cs-etm: Replace unroll loop thread with deterministic decode test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incorrect thread attribution for branch samples spanning context switches.
--

commit fd2f91c5aeb50268fe7509b8158aa03f25f00def
Author: James Clark <james.clark@linaro.org>

perf cs-etm: Queue context packets for frontend

PE_CONTEXT elements update the context ID and exception level, but the
decoder may still have prior packets cached for frontend processing.
Updating the context immediately in the decoder backend can make those
cached packets get consumed with the wrong thread or EL state.

Link: https://lore.kernel.org/linux-perf-users/20260515021135.1729028-1-aaupov@meta.com/

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 40c6ddfa8c8d9..5e92359f51a7c 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c

[ ... ]

> @@ -1649,15 +1685,15 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
>  
>  	event->sample.header.type = PERF_RECORD_SAMPLE;
>  	event->sample.header.misc = cs_etm__cpu_mode(etmq, ip,
> -						     tidq->prev_packet_el);
> +						     tidq->prev_packet->el);
>  	event->sample.header.size = sizeof(struct perf_event_header);
>  
>  	/* Set time field based on etm auxtrace config. */
>  	sample.time = cs_etm__resolve_sample_time(etmq, tidq);
>  
>  	sample.ip = ip;
> -	sample.pid = thread__pid(tidq->prev_packet_thread);
> -	sample.tid = thread__tid(tidq->prev_packet_thread);
> +	sample.pid = thread__pid(tidq->frontend_thread);
> +	sample.tid = thread__tid(tidq->frontend_thread);

[Severity: Medium]
Will this correctly attribute branch samples spanning context switches?

When a CS_ETM_CONTEXT packet marks a context switch between two execution
ranges, the branch connecting them is executed by the outgoing thread.

Because the incoming thread replaces the previous thread in
tidq->frontend_thread before branch synthesis, doesn't this inaccurately
attribute the branch execution to the incoming thread instead of the actual
executing outgoing thread?

[ ... ]

> @@ -2469,6 +2501,19 @@ static int cs_etm__process_traceid_queue(struct cs_etm_queue *etmq,
>  			 */
>  			cs_etm__sample(etmq, tidq);
>  			break;
> +		case CS_ETM_CONTEXT:
> +			/*
> +			 * Update context but don't swap packet. Keep the
> +			 * previous one for branch source address info, if
> +			 * tracing the kernel the context packet will be emitted
> +			 * between two ranges.
> +			 */
> +			ret = cs_etm__etmq_update_thread(etmq, tidq->packet->el,
> +							 tidq->packet->tid,
> +							 &tidq->frontend_thread);

[Severity: Medium]
If the CS_ETM_CONTEXT packet updates the frontend_thread here to the incoming
thread while preserving the prev_packet, won't the subsequent branch synthesis
wrongly use this updated thread for the old packet?

> +			if (ret)
> +				goto out;
> +			break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-james-cs-context-tracking-fix-v4-0-44f9fb9e5c42@linaro.org?part=6