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
| ||
|
Date: Tue, 17 May 2022 15:58:09 -0700 From: Namhyung Kim <namhyung@...nel.org> To: Xing Zhengjun <zhengjun.xing@...ux.intel.com> Cc: "Liang, Kan" <kan.liang@...ux.intel.com>, Arnaldo Carvalho de Melo <acme@...nel.org>, Ian Rogers <irogers@...gle.com>, Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, Mark Rutland <mark.rutland@....com>, Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, Riccardo Mancini <rickyman7@...il.com>, Kim Phillips <kim.phillips@....com>, Madhavan Srinivasan <maddy@...ux.ibm.com>, Shunsuke Nakamura <nakamura.shun@...itsu.com>, Florian Fischer <florian.fischer@...q.space>, Andi Kleen <ak@...ux.intel.com>, John Garry <john.garry@...wei.com>, Adrian Hunter <adrian.hunter@...el.com>, James Clark <james.clark@....com>, linux-perf-users <linux-perf-users@...r.kernel.org>, linux-kernel <linux-kernel@...r.kernel.org>, Stephane Eranian <eranian@...gle.com> Subject: Re: [PATCH 1/5] Revert "perf stat: Support metrics with hybrid events" Hello, On Tue, May 10, 2022 at 2:31 AM Xing Zhengjun <zhengjun.xing@...ux.intel.com> wrote: > > > > On 5/10/2022 5:55 AM, Liang, Kan wrote: > > > > > > On 5/9/2022 9:12 AM, Arnaldo Carvalho de Melo wrote: > >> Em Fri, May 06, 2022 at 10:34:06PM -0700, Ian Rogers escreveu: > >>> This reverts commit 60344f1a9a597f2e0efcd57df5dad0b42da15e21. > >> > >> I picked this from the cover letter and added to this revert, to justify > >> it: > >> > >> "Hybrid metrics place a PMU at the end of the parse string. This is > >> also where tool events are placed. The behavior of the parse string > >> isn't clear and so revert the change for now." > >> > > > > I think the original patch used a "#" to indicate the PMU name, which > > can be used to distinguish between the tool events and the PMU name. > > To be honest, I'm not sure what's unclear here. Could you please clarify? > > > > With this revert, the issue mentioned in the original patch must be > > broken on ADL. I don't see a replacement fix in this patch series. > > Could you please propose a solution for the issue to replace the #PMU > > name solution? > > > > Thanks, > > Kan > > I am surprised the origin patch is reverted during discussion and though > the discussion still has no conclusion. > Let me introduce the purpose of the origin patch. > For a hybrid system such as ADL, if both the metrics and the formula are > different for the different PMUs, without this patch, the metric and > event parser should work ok, nothing should be special for the hybrid. > In fact, both "cpu_core" and "cpu_atom" may have the same metrics--the > same metric name, even the same formula for the metrics. For example, > both "cpu_core" and "cpu_atom" have metrics "IpBranch" and "IPC", For > "IpBranch", both "cpu_core" and "cpu_atom" has the same metric name and > their formula also is the same, the event name is the same though they > belong to different PMUs. The old metric and event parser can not handle > this kind of metric, that's why we need this patch. > > "MetricExpr": "INST_RETIRED.ANY / BR_INST_RETIRED.ALL_BRANCHES", > "MetricName": "IpBranch", > "Unit": "cpu_core" > > "MetricExpr": "INST_RETIRED.ANY / BR_INST_RETIRED.ALL_BRANCHES", > "MetricName": "IpBranch", > "Unit": "cpu_atom" > > > "MetricExpr": "INST_RETIRED.ANY / CPU_CLK_UNHALTED.THREAD", > "MetricName": "IPC", > "Unit": "cpu_core" > > "MetricExpr": "INST_RETIRED.ANY / CPU_CLK_UNHALTED.CORE", > "MetricName": "IPC", > "Unit": "cpu_atom" > > Except for the above two metrics, there are still a lot of similar > metrics, "CPU_Utilization"... > > The original patch expanded the metric group string by adding > "#PMU_name" to indicate the PMU name, which can be used to distinguish > between the tool events and the PMU name, then the metric and event > parser can parse the right PMU for the events. > > With the patch all the ADL metrics can pass, without the patch, a lot of > metrics will fail. I don't think it's a good idea to revert it before > the new solution is proposed. Just an idea. Can we add a pmu prefix when it resolves the event for a metric if it has the "Unit"? It seems we can support something like "cpu_core@...T_RETIRED.ANY@" already.. Or could it be done when creating JSON files? Thanks, Namhyung
Powered by blists - more mailing lists