Re: [PATCH v4 09/23] perf annotate: Deduplicate overlapping ARM SPE events for data type profiling

[Adrian Hunter]

On 12/08/2026 05:28, Tengda Wu wrote:
> 
> 
> On 2026/8/10 14:57, Adrian Hunter wrote:
>> On 08/08/2026 15:23, Tengda Wu wrote:
>>> When data type profiling is enabled on ARM SPE, multiple overlapping
>>> events (e.g., l1d-miss, tlb-access) are synthesized for a single sampled
>>> instruction, as shown below:
>>>
>>>   Available samples
>>>   0 arm_spe_0/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=30/
>>>   0 dummy:u
>>>   84K l1d-miss
>>>   95K l1d-access
>>>   77K llc-miss
>>>   58K llc-access
>>>   9K tlb-miss
>>>   108K tlb-access
>>>   0 branch
>>>   13K remote-access
>>>   108K memory
>>>   108K instructions
>>>
>>> While 'perf report' provides an interactive menu for users to select a
>>> specific event to prevent duplicate counting, 'perf annotate' lacks such
>>> a mechanism. Consequently, it counts all instructions across these
>>> overlapping events, which inflates the profile and distorts the data
>>> type statistics.
>>>
>>> Although using the '--itrace' option can work around this issue (e.g.:
>>> perf annotate --data-type --stdio --itrace=i1i), it is inconvenient for
>>> users to specify this explicitly every time.
>>>
>>> To address this, introduce itrace_synth_opts.dont_overlap. Set this to true
>>> when data type profiling is enabled and the user has not explicitly
>>> specified an itrace option. Then, during arm_spe_process_auxtrace_info(),
>>> adjust the synthesized event options based on the dont_overlap value to
>>> only enable instruction event synthesis, thereby achieving automatic
>>> deduplication.
>>>
>>> Signed-off-by: Tengda Wu <wutengda@xxxxxxxxxxxxxxx>
>>> ---
>>>  tools/perf/builtin-annotate.c |  8 ++++++++
>>>  tools/perf/util/arm-spe.c     | 17 +++++++++++++++++
>>>  tools/perf/util/auxtrace.h    |  2 ++
>>>  3 files changed, 27 insertions(+)
>>>
>>> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
>>> index 69cb72b2082a..616f54bf4868 100644
>>> --- a/tools/perf/builtin-annotate.c
>>> +++ b/tools/perf/builtin-annotate.c
>>> @@ -873,6 +873,14 @@ int cmd_annotate(int argc, const char **argv)
>>>  	annotate.session = perf_session__new(&data, &annotate.tool);
>>>  	if (IS_ERR(annotate.session))
>>>  		return PTR_ERR(annotate.session);
>>> +	/*
>>> +	 * Hardware tracing (e.g.: ARM SPE) may generate overlapping events
>>> +	 * per instruction. When data type profiling is enabled, enable
>>> +	 * dont_overlap to deduplicate them to avoid skewed stats, but only
>>> +	 * if user hasn't specified itrace options (respect user override).
>>> +	 */
>>> +	if (annotate.data_type && !itrace_synth_opts.set)
>>> +		itrace_synth_opts.dont_overlap = true;
>>>  
>>>  	annotate.session->itrace_synth_opts = &itrace_synth_opts;
>>>  
>>> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
>>> index 401aab529309..1721882423f6 100644
>>> --- a/tools/perf/util/arm-spe.c
>>> +++ b/tools/perf/util/arm-spe.c
>>> @@ -2033,6 +2033,9 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
>>>  		/* Default nanoseconds period not supported */
>>>  		spe->synth_opts.period_type = PERF_ITRACE_PERIOD_INSTRUCTIONS;
>>>  		spe->synth_opts.period = 1;
>>> +
>>> +		if (session->itrace_synth_opts)
>>> +			spe->synth_opts.dont_overlap = session->itrace_synth_opts->dont_overlap;
>>>  	}
>>>  
>>>  	if (spe->synth_opts.period_type != PERF_ITRACE_PERIOD_INSTRUCTIONS) {
>>> @@ -2044,6 +2047,20 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
>>>  		ui__warning("Arm SPE has a hardware-based sampling period.\n\n"
>>>  			    "--itrace periods > 1i downsample by an interval of n SPE samples rather than n instructions.\n");
>>>  
>>> +	if (spe->synth_opts.dont_overlap) {
>>> +		/*
>>> +		 * The 'instructions' event is the most comprehensive,
>>> +		 * synthesize it exclusively.
>>> +		 */
>>> +		spe->synth_opts.flc = false;
>>> +		spe->synth_opts.llc = false;
>>> +		spe->synth_opts.tlb = false;
>>> +		spe->synth_opts.branches = false;
>>> +		spe->synth_opts.remote_access = false;
>>> +		spe->synth_opts.mem = false;
>>> +		spe->synth_opts.instructions = true;
>>> +	}
>>> +
>>>  	err = arm_spe_synth_events(spe, session);
>>>  	if (err)
>>>  		goto err_free_queues;
>>> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
>>> index 6947f3f284c0..ebb4e9e8e574 100644
>>> --- a/tools/perf/util/auxtrace.h
>>> +++ b/tools/perf/util/auxtrace.h
>>> @@ -80,6 +80,7 @@ enum itrace_period_type {
>>>   * @intr_events: whether to synthesize interrupt events
>>>   * @errors: whether to synthesize decoder error events
>>>   * @dont_decode: whether to skip decoding entirely
>>> + * @dont_overlap: whether to deduplicate overlapping events
>>
>> This doesn't seem to match what you are actually doing, which
>> seems to be to choose particular default itrace options for
>> 'perf annotate --data-type'
>>
>> I wonder if this should really be handled by
>> itrace_synth_opts__set_default()?
>>
> 
> Placing this in itrace_synth_opts__set_default() may not be appropriate.

Your current approach introduces the concepts of
"dont_overlap" and "deduplicate" but doesn't actually
map them to specific functionality.  They are a mismatch
with what you are actually doing.

> 
> The current approach is designed to constrain the following conditions:
> 
> 1. Using perf annotate
> 2. With --data-type specified
> 3. Based on ARM SPE
> 4. Without --itrace explicitly specified
> 
> When these conditions are all met, we want to enable --itrace=i1i by default,
> effectively making this option built-in.
> 
> If we were to place the --itrace=i1i configuration inside
> itrace_synth_opts__set_default(), conditions 1, 2, and 3 would not be
> reachable from that context, as there's no way to propagate that information
> into the function.

itrace_synth_opts__set_default() is only called when 4 is true.

WRT 1 and 2, you are planning to pass information via synth_opts,
anyway, so it is the same in that regard.

>                    Configuring it directly there would take effect globally,
> impacting other commands such as perf report and perf script.

Not if they did not pass the new information, same as your current
approach.

> 
> Therefore, the current approach passes the dont_overlap information through
> itrace (satisfying conditions 1, 2, and 4), and then configures --itrace=i1i
> inside the ARM SPE driver (satisfying condition 3).

Why wouldn't that default be ok for other auxtrace implementations?

In any case, the problem is with the mismatch between the naming,
definition, and framing of "dont_overlap" and how it is actually used.

Perhaps instead:

	default_single_event_per_ip

and explain the annotate data-type use-case in its kernel-doc
description.

> 
> Thanks,
> Tengda
> 
>>
>>>   * @log: write a decoding log
>>>   * @calls: limit branch samples to calls (can be combined with @returns)
>>>   * @returns: limit branch samples to returns (can be combined with @calls)
>>> @@ -128,6 +129,7 @@ struct itrace_synth_opts {
>>>  	bool			intr_events;
>>>  	bool			errors;
>>>  	bool			dont_decode;
>>> +	bool			dont_overlap;
>>>  	bool			log;
>>>  	bool			calls;
>>>  	bool			returns;
>