Re: [PATCH v3 11/21] perf auxtrace: Set default period to 1 for PERF_ITRACE_PERIOD_INSTRUCTIONS type

On 21/07/2026 14:39, Tengda Wu wrote:
> Hi Adrian,
> 
> On 2026/7/21 16:08, Adrian Hunter wrote:
>> On 01/07/2026 06:53, Tengda Wu wrote:
>>> When using --itrace=M for data type profiling on arm64, the 'Percent'
>>> values in perf annotate output are all zero:
>>>
>>> Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
>>> ============================================================================
>>>  Percent     offset       size  field
>>>     0.00          0       0x10  struct mmu_gather_batch  {
>>>     0.00          0        0x8      struct mmu_gather_batch*    next;
>>>     0.00        0x8        0x4      unsigned int        nr;
>>>     0.00        0xc        0x4      unsigned int        max;
>>>     0.00       0x10          0      struct encoded_page*[]      encoded_pages;
>>>                                 };
>>>
>>> However, adding the -n option reveals non-zero sample counts:
>>>
>>> Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
>>> ============================================================================
>>>  Samples     offset       size  field
>>>       15          0       0x10  struct mmu_gather_batch  {
>>>       13          0        0x8      struct mmu_gather_batch*    next;
>>>        2        0x8        0x4      unsigned int        nr;
>>>        0        0xc        0x4      unsigned int        max;
>>>        0       0x10          0      struct encoded_page*[]      encoded_pages;
>>>                                 };
>>>
>>> The root cause is that when period is not explicitly specified in the
>>> --itrace option, it remains zero after itrace_do_parse_synth_opts().
>>> The zero period then flows through annotated_data_type__update_samples()
>>> where h->period accumulates to zero, and print_annotated_data_type_value()
>>> calculates the 'Percent' as zero.
>>
>> The period is for instructions samples i.e. options 'i' or 'y'.  Why is
>> it being used in the 'M' case?  What samples are being synthesized in
>> that case?
>>
> 
> The 'M' case is just one example. On arm64, 't' and 'f' also have the
> same percent-zero issue.
> 
> How ARM SPE synthesizes events:
> 
> ARM SPE can synthesize multiple event types from a single instruction.
> For example, the 'M' case targets instructions that cause memory
> interaction events, while the 't' case targets instructions that trigger
> TLB access/miss events. Users can specify the desired event types via
> the itrace option to obtain corresponding data type profiling results.
> 
> How data type computes Percent:
> 
> Data type profiling computes Percent based on the period. The flow is as
> follows:
> 
> arm_spe_prep_sample():
>         sample->period = spe->synth_opts.period;

It seems like only arm-spe has the unusual period handling.
Please make the period non-zero when it is set up in
arm_spe_process_auxtrace_info() and add a comment explaining why.

> 
> __hists__add_entry():
>         he.stat.period = sample->period;
> 
> annotated_data_type__update_samples():
>         h->period += period;
>         h->addr[offset].period += period;
> 
> print_annotated_data_value():
>         double percent = h->period ? (100.0 * period / h->period) : 0;
> 
> Therefore, if synth_opts.period is not specified (remains zero), the
> final Percent calculation results in zero.
> 
> As you pointed out, if period is intended to be limited to only the 'i'
> and 'y' options, then setting period to 1 in the current patch may not
> be appropriate.
> 
> In fact, within print_annotated_data_value(), Percent could also be
> computed from the sample count. Moreover, I noticed that 'perf annotate'
> already has a --percent-type parameter that allows users to specify
> whether Percent is based on period or hit (which I understand to be the
> sample count). Perhaps it would be more reasonable to adapt
> print_annotated_data_value() to honor the --percent-type parameter
> instead.
> 
> Thanks,
> Tengda
> 
>>>
>>> In itrace_do_parse_synth_opts(), non-'iy' options have their period
>>> type set to PERF_ITRACE_PERIOD_INSTRUCTIONS, but period remains zero.
>>> Since a zero period is meaningless for this type, default to 1 (one
>>> sample per instruction).
>>>
>>> With this fix applied, the result is as follows:
>>>
>>> Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
>>> ============================================================================
>>>  Percent     offset       size  field
>>>   100.00          0       0x10  struct mmu_gather_batch  {
>>>    86.67          0        0x8      struct mmu_gather_batch*    next;
>>>    13.33        0x8        0x4      unsigned int        nr;
>>>     0.00        0xc        0x4      unsigned int        max;
>>>     0.00       0x10          0      struct encoded_page*[]      encoded_pages;
>>>                                 };
>>>
>>> Signed-off-by: Tengda Wu <[email protected]>
>>> ---
>>>  tools/perf/util/auxtrace.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
>>> index 0b851f32e98c..415b68a2bba9 100644
>>> --- a/tools/perf/util/auxtrace.c
>>> +++ b/tools/perf/util/auxtrace.c
>>> @@ -1759,6 +1759,12 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
>>>  			synth_opts->period = PERF_ITRACE_DEFAULT_PERIOD;
>>>  	}
>>>  
>>> +	if (!period_set &&
>>> +	    synth_opts->period_type == PERF_ITRACE_PERIOD_INSTRUCTIONS) {
>>> +		/* Indicates a sample is taken for every instruction. */
>>> +		synth_opts->period = 1;
>>> +	}
>>> +
>>>  	return 0;
>>>  
>>>  out_err:
>>
>