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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <608077bc-4139-4a97-8dc4-7997177d95c4@linux.intel.com>
Date: Thu, 6 Feb 2025 14:53:17 -0500
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Ian Rogers <irogers@...gle.com>
Cc: Thomas Falcon <thomas.falcon@...el.com>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
 Andreas Färber <afaerber@...e.de>,
 Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
 Weilin Wang <weilin.wang@...el.com>, linux-kernel@...r.kernel.org,
 linux-perf-users@...r.kernel.org, Perry Taylor <perry.taylor@...el.com>,
 Samantha Alt <samantha.alt@...el.com>,
 Caleb Biggers <caleb.biggers@...el.com>,
 Edward Baker <edward.baker@...el.com>, Michael Petlan <mpetlan@...hat.com>
Subject: Re: [PATCH v5 11/24] perf vendor events: Update/add Graniterapids
 events/metrics



On 2025-02-06 2:05 p.m., Ian Rogers wrote:
> On Thu, Feb 6, 2025 at 10:59 AM Liang, Kan <kan.liang@...ux.intel.com> wrote:
>>
>> On 2025-02-06 1:53 p.m., Liang, Kan wrote:
>>>>> However, when I dump the debug information,
>>>>> ./perf stat -M tma_frontend_bound -vvv
>>>>>
>>>>> I got below debug information. I have no idea where the slot is from.
>>>>> It seems the perf code mess up the p-core metrics with the e-core
>>>>> metrics. But why only slot?
>>>>> It seems a bug of perf tool.
>>>>>
>>>>> found event cpu_atom@..._CLK_UNHALTED.CORE@
>>>>> found event cpu_atom@...DOWN_FE_BOUND.ALL@
>>>>> found event slots
>>>>> Parsing metric events
>>>>>
>>>>> '{cpu_atom/CPU_CLK_UNHALTED.CORE,metric-id=cpu_atom!3CPU_CLK_UNHALTED.CORE!3/,cpu_atom/TOPDOWN_FE_BOUND.ALL,metric-id=cpu_atom!3TOPDOWN_FE_BOUND.ALL!3/,slots/metric-id=slots/}:W'
>>> It because the perf adds "slot" as a tool event for the e-core Topdown
>>> metrics.
>>> There is no "slot" event for e-core.
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/
>>> tools/perf/util/metricgroup.c#n1481
>>>
>>> I will check why "slot" event is added as a tool event for e-core?
>>> That doesn't make sense.
>>
>>
>> BTW: I checked the list of the tool events. Except the slots, others are
>> all SW (or non-HW) events which should always be available.
>> Slots is a HW event and only be available for p-core.
>> It doesn't look like a good fit here.
>>
>> static const char *const tool_pmu__event_names[TOOL_PMU__EVENT_MAX] = {
>>         NULL,
>>         "duration_time",
>>         "user_time",
>>         "system_time",
>>         "has_pmem",
>>         "num_cores",
>>         "num_cpus",
>>         "num_cpus_online",
>>         "num_dies",
>>         "num_packages",
>>         "slots",
>>         "smt_on",
>>         "system_tsc_freq",
>> };
> 
> IIRC it was added on metric changes I wasn't a reviewer on:
> https://lore.kernel.org/r/1673940573-90503-2-git-send-email-renyu.zj@linux.alibaba.com
> and then carried forward by me. The event should only be enabled on aarch64:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/tool_pmu.c?h=perf-tools-next#n38
> Where is the slots event added to the metric expr (expression) string?
> The metric should have events pulled out, parsed and
> parse_events__sort_events_and_fix_groups deal with related issues to
> this. I thought it was a requirement that the slots be in the metric -
> or else why is the converter script injecting them? :-)
> 


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,
Kan