Re: [PATCH v10 7/9] 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] Unnecessary allocation and tracking of branch stacks when only thread stack is requested.
--

commit 169dda630ecfdc4196986b24719c6d9bd8a72971
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.
This commit enables thread-stack when either itrace thread-stack support or
last branch entries are requested, and feeds taken branches to
thread_stack__event().

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index e43f0c1dd0078..30536b919af73 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
[ ... ]
> @@ -638,7 +640,7 @@ static int cs_etm__init_traceid_queue(struct cs_etm_queue *etmq,
>  	if (!tidq->prev_packet)
>  		goto out_free;
>  
> -	if (etm->synth_opts.last_branch) {
> +	if (etm->use_thread_stack) {
>  		size_t sz = sizeof(struct branch_stack);
>  
>  		sz += etm->synth_opts.last_branch_sz *

[Severity: Medium]
Does this code unnecessarily allocate and track branch stacks when only the
thread stack is requested?

By changing this condition from etm->synth_opts.last_branch to
etm->use_thread_stack, it appears tidq->last_branch will be allocated and
tidq->br_stack_sz set to a non-zero value even when the user only
requests thread stacks.

[ ... ]
> @@ -1563,7 +1565,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 {

[Severity: Medium]
Because tidq->br_stack_sz is now non-zero, this feeds into
thread_stack__event() and forces the thread stack subsystem to allocate an
internal branch ring buffer and update it on every taken branch.

Since etm->synth_opts.last_branch is false, isn't this tracked branch stack
completely ignored later, resulting in unnecessary memory allocation and CPU
overhead during trace decoding?

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