linux-kernel - Re: [PATCH v1 0/3] perf list: Remove duplicate PMUs

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:   Tue, 11 Jul 2023 08:10:15 -0700
From:   Ian Rogers <irogers@...gle.com>
To:     John Garry <john.g.garry@...cle.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        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>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Ravi Bangoria <ravi.bangoria@....com>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 0/3] perf list: Remove duplicate PMUs

On Tue, Jul 11, 2023 at 1:26 AM John Garry <john.g.garry@...cle.com> wrote:
>
> On 11/07/2023 06:58, Ian Rogers wrote:
> > When there are multiple PMUs differing by ordered suffixes only
> > display one. This avoids repeated listing of events, in particular
> > when there are 10s of uncore PMUs. This also helps speed the all PMU
> > event tests.
> >
> > Before:
> > ```
> > $ perf list
> > ...
> >    uncore_imc_free_running_0/data_read/               [Kernel PMU event]
> >    uncore_imc_free_running_0/data_total/              [Kernel PMU event]
> >    uncore_imc_free_running_0/data_write/              [Kernel PMU event]
> >    uncore_imc_free_running_1/data_read/               [Kernel PMU event]
> >    uncore_imc_free_running_1/data_total/              [Kernel PMU event]
> >    uncore_imc_free_running_1/data_write/              [Kernel PMU event]
> > ```
> >
> > After:
> > ```
> > $ perf list
> > ...
> >    uncore_imc_free_running/data_read/                 [Kernel PMU event]
> >    uncore_imc_free_running/data_total/                [Kernel PMU event]
> >    uncore_imc_free_running/data_write/                [Kernel PMU event]
>
> So with this change can we run something like:
>
> perf stat -e uncore_imc_free_running/data_read/
>
> ?

It is a long standing behavior of the event parser that we match the
numeric suffixes, so:

```
$ sudo perf stat -e uncore_imc_free_running/data_read/ -a sleep 1

Performance counter stats for 'system wide':

         6,969.93 MiB  uncore_imc_free_running/data_read/

      1.001163027 seconds time elapsed
```

The "uncore_" at the beginning is also optional, I kind of wish the
"free_running" was too. The code doing this is:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.y?h=perf-tools-next#n316
adding a * after the PMU name in:
asprintf(&pattern, "%s*", $1)
Then using fnmatch here:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmu.c?h=perf-tools-next#n1707

> If so, does that match all PMUs whose name beings with
> "uncore_imc_free_running" (and give aggregate result for those PMUs)?

Yep. As we're matching with a filename '*' glob then it will actually
potentially grab a bunch more. I think this should likely be made a
lot more precise.

The merging of the counters happens throughout the code, but it is set up here:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/stat.c?h=perf-tools-next#n559

I didn't write this behavior, it has pre-existed my contributions. I'm
hoping to change the perf list behavior as we're seeing large server
systems with getting on toward 100 PMUs, the events are replicated for
each one and the perf list and testing behaviors are somewhat
exploding in size.

Thanks,
Ian

> Thanks,
> John
>
> > ```
> >
> > The PMUs are sorted by name then suffix as a part of this change.
> >
> > Ian Rogers (3):
> >    perf pmus: Sort pmus by name then suffix
> >    perf pmus: Add scan that ignores duplicates, use for perf list
> >    perf pmus: Don't print PMU suffix in list
> >
> >   tools/perf/util/pmus.c | 107 +++++++++++++++++++++++++++++++++++++++--
> >   1 file changed, 102 insertions(+), 5 deletions(-)
> >
>