| 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
| ||
|
Message-ID: <CAP-5=fUqLXdu2=TPSASFBbZ+B1oTFbuFra38z5YwYHWpX-V=hw@mail.gmail.com>
Date: Fri, 25 Aug 2023 15:56:54 -0700
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Thomas Richter <tmricht@...ux.ibm.com>,
Sumanth Korikkar <sumanthk@...ux.ibm.com>,
Heiko Carstens <hca@...ux.ibm.com>,
Sven Schnelle <svens@...ux.ibm.com>,
Jiri Olsa <jolsa@...nel.org>,
James Clark <james.clark@....com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 06/18] perf s390 s390_cpumcfdg_dump: Don't scan all PMUs
On Fri, Aug 25, 2023 at 1:56 PM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> Em Fri, Aug 25, 2023 at 04:39:22PM +0200, Thomas Richter escreveu:
> > On 8/25/23 15:14, Ian Rogers wrote:
> > > On Fri, Aug 25, 2023 at 1:20 AM Thomas Richter <tmricht@...ux.ibm.com> wrote:
>
> > >> On 8/24/23 15:59, Arnaldo Carvalho de Melo wrote:
> > >>> Em Wed, Aug 23, 2023 at 09:13:18PM -0700, Ian Rogers escreveu:
> > >>>> Rather than scanning all PMUs for a counter name, scan the PMU
> > >>>> associated with the evsel of the sample. This is done to remove a
> > >>>> dependence on pmu-events.h.
>
> > >>> I'm applying this one, and CCing the S/390 developers so that they can
> > >>> try this and maybe provide an Acked-by/Tested-by,
>
> > >> I have downloaded this patch set of 18 patches (using b4), but they do not
> > >> apply on my git tree.
>
> > >> Which git branch do I have to use to test this. Thanks a lot.
>
> > > the changes are in the perf-tools-next tree:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/
>
> > Unfurtunately this patch set fails again on s390.
> > Here is the test output from the current 6.5.0rc7 kernel:
> >
> > # ./perf test 6 10 'perf all metricgroups test' 'perf all metrics test'
> > 6: Parse event definition strings :
> > 6.1: Test event parsing : Ok
> > 6.2: Parsing of all PMU events from sysfs : Ok
> > 6.3: Parsing of given PMU events from sysfs : Ok
> > 6.4: Parsing of aliased events from sysfs : Skip (no aliases in sysfs)
> > 6.5: Parsing of aliased events : Ok
> > 6.6: Parsing of terms (event modifiers) : Ok
> > 10: PMU events :
> > 10.1: PMU event table sanity : Ok
> > 10.2: PMU event map aliases : Ok
> > 10.3: Parsing of PMU event table metrics : Ok
> > 10.4: Parsing of PMU event table metrics with fake PMUs : Ok
> > 10.5: Parsing of metric thresholds with fake PMUs : Ok
> > 95: perf all metricgroups test : Ok
> > 96: perf all metrics test : Ok
> > #
> >
> > This looks good.
>
> Reproduced:
>
> # grep -E vendor_id\|^processor -m2 /proc/cpuinfo
> vendor_id : IBM/S390
> processor 0: version = 00, identification = 1A33E8, machine = 2964
> #
> # perf test 6 10 'perf all metricgroups test' 'perf all metrics test'
> 6: Parse event definition strings :
> 6.1: Test event parsing : Ok
> 6.2: Parsing of all PMU events from sysfs : Ok
> 6.3: Parsing of given PMU events from sysfs : Ok
> 6.4: Parsing of aliased events from sysfs : Skip (no aliases in sysfs)
> 6.5: Parsing of aliased events : Ok
> 6.6: Parsing of terms (event modifiers) : Ok
> 10: PMU events :
> 10.1: PMU event table sanity : Ok
> 10.2: PMU event map aliases : Ok
> 10.3: Parsing of PMU event table metrics : Ok
> 10.4: Parsing of PMU event table metrics with fake PMUs : Ok
> 10.5: Parsing of metric thresholds with fake PMUs : Ok
> 95: perf all metricgroups test : Ok
> 96: perf all metrics test : Ok
> # perf -v
> perf version 6.5.rc7.g6f0edbb833ec
> #
>
> > However when I use the check-out from perf-tools-next, I get this output:
> > # ./perf test 6 10 'perf all metricgroups test' 'perf all metrics test'
> > 6: Parse event definition strings :
> > 6.1: Test event parsing : Ok
> > 6.2: Parsing of all PMU events from sysfs : FAILED!
> > 6.3: Parsing of given PMU events from sysfs : Ok
> > 6.4: Parsing of aliased events from sysfs : Skip (no aliases in sysfs)
> > 6.5: Parsing of aliased events : FAILED!
> > 6.6: Parsing of terms (event modifiers) : Ok
> > 10: PMU events :
> > 10.1: PMU event table sanity : Ok
> > 10.2: PMU event map aliases : FAILED!
> > 10.3: Parsing of PMU event table metrics : Ok
> > 10.4: Parsing of PMU event table metrics with fake PMUs : FAILED!
> > 10.5: Parsing of metric thresholds with fake PMUs : Ok
> > 93: perf all metricgroups test : FAILED!
> > 94: perf all metrics test : FAILED!
> > #
> >
> > So some tests are failing again.
> >
> > I am out for the next two weeks, Sumanth Korikkar (on to list) might be able to help.
> > Thanks a lot.
>
> [root@...nelqe3 linux]# git checkout perf-tools-next
> git Switched to branch 'perf-tools-next'
> Your branch is up to date with 'perf-tools-next/perf-tools-next'.
> [root@...nelqe3 linux]# git log --oneline -5
> eeb6b12992c4 (HEAD -> perf-tools-next, perf-tools-next/perf-tools-next) perf jevents: Don't append Unit to desc
> f208b2c6f984 (perf-tools-next/tmp.perf-tools-next) perf scripts python gecko: Launch the profiler UI on the default browser with the appropriate URL
> 43803cb16f99 perf scripts python: Add support for input args in gecko script
> f85d120c46e7 perf jevents: Sort strings in the big C string to reduce faults
> 8d4b6d37ea78 perf pmu: Lazily load sysfs aliases
> [root@...nelqe3 linux]# make BUILD_BPF_SKEL=1 -C tools/perf O=/tmp/build/perf install-bin
>
> [root@...nelqe3 linux]# perf -v
> perf version 6.5.rc5.geeb6b12992c4
> [root@...nelqe3 linux]# git log --oneline -1
> eeb6b12992c4 (HEAD -> perf-tools-next, perf-tools-next/perf-tools-next) perf jevents: Don't append Unit to desc
> [root@...nelqe3 linux]# perf test 6 10 'perf all metricgroups test' 'perf all metrics test'
> 6: Parse event definition strings :
> 6.1: Test event parsing : Ok
> 6.2: Parsing of all PMU events from sysfs : FAILED!
> 6.3: Parsing of given PMU events from sysfs : Ok
> 6.4: Parsing of aliased events from sysfs : Skip (no aliases in sysfs)
> 6.5: Parsing of aliased events : FAILED!
> 6.6: Parsing of terms (event modifiers) : Ok
> 10: PMU events :
> 10.1: PMU event table sanity : Ok
> 10.2: PMU event map aliases : FAILED!
> 10.3: Parsing of PMU event table metrics : Ok
> 10.4: Parsing of PMU event table metrics with fake PMUs : FAILED!
> 10.5: Parsing of metric thresholds with fake PMUs : Ok
> 93: perf all metricgroups test : FAILED!
> 94: perf all metrics test : FAILED!
> [root@...nelqe3 linux]#
>
> Bisecting the first problem:
>
> 10.2: PMU event map aliases : FAILED!
>
> make: Leaving directory '/root/git/linux/tools/perf'
> 6: Parse event definition strings :
> 6.1: Test event parsing : Ok
> 6.2: Parsing of all PMU events from sysfs : Ok
> 6.3: Parsing of given PMU events from sysfs : Ok
> 6.4: Parsing of aliased events from sysfs : Skip (no aliases in sysfs)
> 6.5: Parsing of aliased events : Ok
> 6.6: Parsing of terms (event modifiers) : Ok
> 10: PMU events :
> 10.1: PMU event table sanity : Ok
> 10.2: PMU event map aliases : FAILED!
> 10.3: Parsing of PMU event table metrics : Ok
> 10.4: Parsing of PMU event table metrics with fake PMUs : Ok
> 10.5: Parsing of metric thresholds with fake PMUs : Ok
> 93: perf all metricgroups test : Ok
> 94: perf all metrics test : Ok
> [root@...nelqe3 linux]# git bisect bad
> 2e255b4f9f41f137d9e3dc4fae3603a9c2c3dd28 is the first bad commit
> commit 2e255b4f9f41f137d9e3dc4fae3603a9c2c3dd28
> Author: Ian Rogers <irogers@...gle.com>
> Date: Wed Aug 23 21:13:16 2023 -0700
>
> perf jevents: Group events by PMU
>
> Prior to this change a cpuid would map to a list of events where the PMU
> would be encoded alongside the event information. This change breaks
> apart each group of events so that there is a group per PMU. A new table
> is added with the PMU's name and the list of events, the original table
> now holding an array of these per PMU tables.
>
> These changes are to make it easier to get per PMU information about
> events, rather than the current approach of scanning all events. The
> perf binary size with BPF skeletons on x86 is reduced by about 1%. The
> unidentified PMU is now always expanded to "cpu".
>
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Cc: Gaosheng Cui <cuigaosheng1@...wei.com>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: James Clark <james.clark@....com>
> Cc: Jing Zhang <renyu.zj@...ux.alibaba.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: John Garry <john.g.garry@...cle.com>
> Cc: Kajol Jain <kjain@...ux.ibm.com>
> Cc: Kan Liang <kan.liang@...ux.intel.com>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Ravi Bangoria <ravi.bangoria@....com>
> Cc: Rob Herring <robh@...nel.org>
> Link: https://lore.kernel.org/r/20230824041330.266337-5-irogers@google.com
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
>
> tools/perf/pmu-events/jevents.py | 181 +++++++++++++++++++++++++++++----------
> tools/perf/tests/pmu-events.c | 30 ++++---
> 2 files changed, 154 insertions(+), 57 deletions(-)
> [root@...nelqe3 linux]#
>
This change defaulted events without a specified PMU to being for the
PMU 'cpu', so that events in pmu-events.c were associated with a PMU
and we could find per-PMU information easily. The test events have no
PMU and so this has broken s390 where the the PMU should be "cpum_cf".
It has probably also broken x86 hybrid and arm where their default PMU
isn't cpu. I'll work on a fix, but the problem will be limited to the
test.
Thanks,
Ian
Powered by blists - more mailing lists