[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fWzD0+jPEZ1gZp7Zj+gzb8NB-BDZvtstcx-dPNQr0n8Tw@mail.gmail.com>
Date: Tue, 2 May 2023 10:40:37 -0700
From: Ian Rogers <irogers@...gle.com>
To: Ravi Bangoria <ravi.bangoria@....com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Kan Liang <kan.liang@...ux.intel.com>,
Ahmad Yasin <ahmad.yasin@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Stephane Eranian <eranian@...gle.com>,
Andi Kleen <ak@...ux.intel.com>,
Perry Taylor <perry.taylor@...el.com>,
Samantha Alt <samantha.alt@...el.com>,
Caleb Biggers <caleb.biggers@...el.com>,
Weilin Wang <weilin.wang@...el.com>,
Edward Baker <edward.baker@...el.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Florian Fischer <florian.fischer@...q.space>,
Rob Herring <robh@...nel.org>,
Zhengjun Xing <zhengjun.xing@...ux.intel.com>,
John Garry <john.g.garry@...cle.com>,
Kajol Jain <kjain@...ux.ibm.com>,
Sumanth Korikkar <sumanthk@...ux.ibm.com>,
Thomas Richter <tmricht@...ux.ibm.com>,
Tiezhu Yang <yangtiezhu@...ngson.cn>,
Leo Yan <leo.yan@...aro.org>,
Yang Jihong <yangjihong1@...wei.com>,
James Clark <james.clark@....com>,
Suzuki Poulouse <suzuki.poulose@....com>,
Kang Minchul <tegongkang@...il.com>,
Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 27/46] perf print-events: Print legacy cache events for
each PMU
On Tue, May 2, 2023 at 3:48 AM Ravi Bangoria <ravi.bangoria@....com> wrote:
>
> On 29-Apr-23 11:04 AM, Ian Rogers wrote:
> > Mirroring parse_events_add_cache, list the legacy name alongside its
> > alias with the PMU. Remove the now unnecessary hybrid logic.
>
> Before patch:
>
> ```
> $ sudo ./perf list
> ...
> duration_time [Tool event]
> user_time [Tool event]
> system_time [Tool event]
> L1-dcache-loads [Hardware cache event]
> L1-dcache-load-misses [Hardware cache event]
> L1-dcache-prefetches [Hardware cache event]
> L1-icache-loads [Hardware cache event]
> L1-icache-load-misses [Hardware cache event]
> dTLB-loads [Hardware cache event]
> dTLB-load-misses [Hardware cache event]
> iTLB-loads [Hardware cache event]
> iTLB-load-misses [Hardware cache event]
> branch-loads [Hardware cache event]
> branch-load-misses [Hardware cache event]
> branch-brs OR cpu/branch-brs/ [Kernel PMU event]
> branch-instructions OR cpu/branch-instructions/ [Kernel PMU event]
> branch-misses OR cpu/branch-misses/ [Kernel PMU event]
> ...
> ```
>
> After patch:
>
> ```
> $ sudo ./perf list
> ...
> duration_time [Tool event]
> user_time [Tool event]
> system_time [Tool event]
>
> cpu:
> L1-dcache-loads OR cpu/L1-dcache-loads/
> L1-dcache-load-misses OR cpu/L1-dcache-load-misses/
> L1-dcache-prefetches OR cpu/L1-dcache-prefetches/
> L1-icache-loads OR cpu/L1-icache-loads/
> L1-icache-load-misses OR cpu/L1-icache-load-misses/
> dTLB-loads OR cpu/dTLB-loads/
> dTLB-load-misses OR cpu/dTLB-load-misses/
> iTLB-loads OR cpu/iTLB-loads/
> iTLB-load-misses OR cpu/iTLB-load-misses/
> branch-loads OR cpu/branch-loads/
> branch-load-misses OR cpu/branch-load-misses/
> branch-brs OR cpu/branch-brs/ [Kernel PMU event]
> branch-instructions OR cpu/branch-instructions/ [Kernel PMU event]
> branch-misses OR cpu/branch-misses/ [Kernel PMU event]
> ...
> ```
>\
> Is this intentional change?
Yep, but I think the commit message should call it out, so I'll change
it in v4. When we have an alias the event type descriptor isn't shown,
this is pre-existing perf list behavior but I think we may want to
tweak it as I like the event type descriptor.
> > - for (int type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) {
> > - for (int op = 0; op < PERF_COUNT_HW_CACHE_OP_MAX; op++) {
> > - /* skip invalid cache type */
> > - if (!evsel__is_cache_op_valid(type, op))
> > - continue;
> > + while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> > + /*
> > + * Skip uncore PMUs for performance. Software PMUs can open
> > + * PERF_TYPE_HW_CACHE, so skip.
>
> This statement is bit confusing. Can you please explain how SW pmus can
> open cache events.
If the type is PERF_TYPE_HW_CACHE (3) and the extended type is
PERF_TYPE_SOFTWARE (1) then this yields the encoding '3:0x100000000'
which will succeed perf_event_open:
```
$ perf stat -vv -e '3:0x100000000' true
Using CPUID GenuineIntel-6-8D-1
intel_pt default config: tsc,mtc,mtc_period=3,psb_period=3,pt,branch
Control descriptor is not initialized
------------------------------------------------------------
perf_event_attr:
type 3
size 128
config 0x100000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
enable_on_exec 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid 2128955 cpu -1 group_fd -1 flags 0x8 = 3
3:0x100000000: -1: 265630 261923 261923
3:0x100000000: 265630 261923 261923
Performance counter stats for 'true':
265,630 3:0x100000000
0.000844251 seconds time elapsed
0.000911000 seconds user
0.000000000 seconds sys
```
I agree this isn't expected, but if I don't exclude the PMU type the
print events will list it as an alias. I'll try to improve the comment
in v4.
Thanks,
Ian
Powered by blists - more mailing lists