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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a regression where multiple `--itrace` options silently overwrite a previously user-configured instruction period.
--

commit 37214515750cb3145373998c5f8b034630d80bf7
Author: Tengda Wu <wutengda@huaweicloud.com>

perf auxtrace: Set default period to 1 for PERF_ITRACE_PERIOD_INSTRUCTIONS type

This commit sets the default instruction period to 1 when a period is not 
explicitly specified for the PERF_ITRACE_PERIOD_INSTRUCTIONS type. This fixes 
an issue where a zero period resulted in zero percent values in perf annotate 
output.

> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> --- 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;

[Severity: Medium]
Will this silently overwrite a user-configured instruction period if
multiple --itrace options are used?

Since period_set is a local variable initialized to false on every call to
itrace_do_parse_synth_opts(), if a user provides an explicit period in one
flag and later provides another flag without a period:

First call (e.g., --itrace=i1000i):
    period_set = true;
    synth_opts->period = 1000;

Second call (e.g., --itrace=M):
    period_set = false;
    // synth_opts->period is overwritten to 1 here

Could this lead to extreme performance overhead during decoding by
unexpectedly dropping the sampling period to 1?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701035355.752944-1-wutengda@huaweicloud.com?part=11