Re: [PATCH v8 6/8] perf cs-etm: Support call indentation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cs_etm__packet_swap() is missing the etm->use_thread_stack condition, which breaks callindent when basic tracing options are disabled.
--

commit 0a18549301f4498325c22df50ff20dde5b1f2489
Author: Leo Yan <leo.yan@linux.dev>

perf cs-etm: Support call indentation

The perf script callindent is derived from call stack in thread context,
CS ETM ignores the requirement for callindent without pushing and poping
call stack.

Enable thread-stack when either itrace thread-stack support or last branch
entries are requested, allocate the branch stack storage accordingly, and
feed taken branches to thread_stack__event() whenever thread-stack state
is needed.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 7069b4990e610..830618763d8b1 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
[ ... ]
> @@ -1545,7 +1547,7 @@ static void cs_etm__add_stack_event(struct cs_etm_queue *etmq,
>  	if (!cs_etm__packet_has_taken_branch(tidq->prev_packet))
>  		return;
>  
> -	if (etmq->etm->synth_opts.last_branch) {
> +	if (etmq->etm->use_thread_stack) {

[Severity: Medium]
When callindent is requested but basic synthesis options are disabled
(for example, using --itrace=e), could cs_etm__packet_swap() fail to
swap the packets?

If etm->use_thread_stack is set, but branches, last_branch, and instructions
are disabled, it appears the packet swap is skipped:

tools/perf/util/cs-etm.c:cs_etm__packet_swap() {
	if (etm->synth_opts.branches || etm->synth_opts.last_branch ||
	    etm->synth_opts.instructions) {
		/*
		 * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
		 * the next incoming packet.
		 */
		tmp = tidq->packet;
		tidq->packet = tidq->prev_packet;
		tidq->prev_packet = tmp;
	}
}

Would this leave tidq->prev_packet zero-initialized, causing
cs_etm__packet_has_taken_branch() to always return false and break
callindent functionality?

>  		from = cs_etm__last_executed_instr(tidq->prev_packet);
>  		to = cs_etm__first_executed_instr(tidq->packet);
>  
> @@ -1554,7 +1556,8 @@ static void cs_etm__add_stack_event(struct cs_etm_queue *etmq,
>  		/* Enable callchain so thread stack entry can be allocated */
>  		thread_stack__event(tidq->frontend_thread, tidq->prev_packet->cpu,
>  				    tidq->prev_packet->flags, from, to, size,
> -				    etmq->buffer->buffer_nr + 1, false,
> +				    etmq->buffer->buffer_nr + 1,
> +				    etmq->etm->use_callchain,
>  				    tidq->br_stack_sz, 0);
>  	} else {
>  		thread_stack__set_trace_nr(tidq->frontend_thread,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-b4-arm_cs_callchain_support_v1-v8-0-737948584fea@arm.com?part=6