linux-kernel - Re: [RFC PATCH 1/1] perf arm64: Implement --topdown with 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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 20 Dec 2021 17:21:30 +0000
From:   Andrew Kilroy <andrew.kilroy@....com>
To:     John Garry <john.garry@...wei.com>,
        Ian Rogers <irogers@...gle.com>,
        Andi Kleen <ak@...ux.intel.com>
Cc:     linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        acme@...nel.org, Will Deacon <will@...nel.org>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Leo Yan <leo.yan@...aro.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [RFC PATCH 1/1] perf arm64: Implement --topdown with metrics


On 15/12/2021 10:52, John Garry wrote:
> Hi Andrew,
> 
>>>   const struct pmu_event *metricgroup__find_metric(const char *metric,
>>>                                                   const struct 
>>> pmu_events_map *map);
>>>   int metricgroup__parse_groups_test(struct evlist *evlist,
>>> diff --git a/tools/perf/util/topdown.c b/tools/perf/util/topdown.c
>>> index 1081b20f9891..57c0c5f2c6bd 100644
>>> --- a/tools/perf/util/topdown.c
>>> +++ b/tools/perf/util/topdown.c
>>> @@ -56,3 +56,9 @@ __weak bool arch_topdown_sample_read(struct evsel 
>>> *leader __maybe_unused)
>>>   {
>>>          return false;
>>>   }
>>> +
>>> +__weak bool arch_topdown_use_json_metrics(void)
>>> +{
> 
> AFAICS, only x86 supports topdown today and that is because they have 
> special kernel topdown events exposed for the kernel CPU PMU driver. So 
> other architectures - not only arm - would need rely on metricgroups for 
> topdown support. So let's make this generic for all archs.
> 
>> I like this extension! I've ranted in the past about weak symbols
>> breaking with archives due to lazy loading [1]. In this case
>> tools/perf/arch/arm64/util/topdown.c has no other symbols within it
>> and so the weak symbol has an extra chance of being linked
>> incorrectly. We could add a new command line of --topdown-json to
>> avoid this, but there seems little difference in doing this over just
>> doing '-M TopDownL1'.
> 
> 
>> Is it possible to use the json metric approach
>> for when the CPU version fails?
> 
> I think that's a good idea.
> 


While looking into using the json metrics approach as a fallback to the 
original, I noticed  there are two json metricgroups 'TopdownL1' and 
'TopDownL1' (note the case difference) on x86.  Not sure if the case 
difference is intentional.

On skylake, 'TopdownL1' contains the four json metrics Retiring, 
Bad_Speculation, Frontend_Bound, and Backend_Bound.  'TopDownL1' has 
'SLOTS', 'CoreIPC', 'CoreIPC_SMT', 'Instructions'.  I think its a 
similar situation on other x86 chips.

The search for those metrics by metricgroup name is case insensitive, so 
it's picking up all 8 metrics when using the lookup string 'TopDownL1'. 
  So the extra 'SLOTS', 'CoreIPC', 'CoreIPC_SMT', 'Instructions' metrics 
would be printed as well.

Not sure what the significance of the case difference might be.

Should we use a different string than 'TopDownL1' as the metric group 
name to search for?


Andrew