Re: [PATCH v4 09/23] perf annotate: Deduplicate overlapping ARM SPE events for data type profiling
Adrian Hunter <[email protected]>
| Newsgroups | dev.linux.lists.llvm,org.kernel.vger.linux-kernel,org.kernel.vger.linux-perf-users |
|---|---|
| Organization | Intel Finland Oy, Registered Address: c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo, Business Identity Code: 0357606 - 4, Domiciled in Helsinki |
| Message-ID | <[email protected]> |
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 <[email protected]> > --- > 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()? > * @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;