Re: [PATCH v5 11/24] perf vendor events: Update/add Graniterapids events/metrics

[Ian Rogers]

On Thu, Feb 6, 2025 at 11:53 AM Liang, Kan <kan.liang@xxxxxxxxxxxxxxx> wrote:
[snip]
> No, it's the perf tool which inject the "slots" event in parse_ids().
>
> In parse_groups(), the tool_events[] is constructed here.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/metricgroup.c#n1557
>
> In the find_tool_events(), the tool_pmu__event_to_str() is used to
> compare the tool_events. It only check the event name, no PMU or arch.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/metricgroup.c#n1389
>
> So the tool_events[TOOL_PMU__EVENT_SLOTS] is set to true, because the
> p-core Topdown metrics has "slots" event.
>
> The tool_events is shared. So when parsing the e-core metrics, the
> "slots" is automatically added.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/metricgroup.c#n1476
>
> I think we may need a similar fix in the tool_pmu__event_to_str() to
> skip the "slots" for x86.

Thanks! Just thinking out loud, I think the reason we'd starting to
see this problem on hybrid and not on seen it on non-hybrid ICL and
later models (where I run the code every day) is that the
parse_events__sort_events_and_fix_groups shouldn't fix the topdown
grouping for the atom case. Fwiw, if I had a test machine ;-)

Ian