| 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: Fri, 12 May 2023 23:39:08 -0700
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: 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>,
Ravi Bangoria <ravi.bangoria@....com>,
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 v4 00/44] Fix perf on Intel hybrid CPUs
On Fri, May 12, 2023 at 11:28 AM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> Em Tue, May 02, 2023 at 03:38:07PM -0700, Ian Rogers escreveu:
> > TL;DR: hybrid doesn't crash, json metrics work on hybrid on both PMUs
> > or individually, event parsing doesn't always scan all PMUs, more and
> > new tests that also run without hybrid, less code.
> >
> > The first 4 patches are aimed at Linux 6.4 to address issues raised,
> > in particular by Kan, on the existing perf stat behavior with json
> > metrics. They avoid duplicated events by removing groups. They don't
> > hide events and metrics to make event multiplexing obvious. They avoid
> > terminating perf when paranoia is higher due to certain events that
> > always fail. They avoid rearranging events by PMUs when the events
> > aren't in a group.
>
> Took the first 4, skipped patch 19 that Adrian asked to be in 6.4 and
> then went all the way to:
>
> Applying: perf test: Add cputype testing to perf stat
> error: patch failed: tools/perf/tests/shell/stat.sh:91
> error: tools/perf/tests/shell/stat.sh: patch does not apply
> Patch failed at 0012 perf test: Add cputype testing to perf stat
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> ⬢[acme@...lbox perf-tools-next]$ git am --abort
> ⬢[acme@...lbox perf-tools-next]$
>
> Will test with it and put what I have in tmp.perf-tools-next, testing
> now.
>
> - Arnaldo
Thanks Arnaldo, I checked that all the patches are there
(tmp.perf-tools-next) and tested on a non-hybrid system. Everything
looks good. I resent the missing test patch:
https://lore.kernel.org/lkml/20230513063447.464691-1-irogers@google.com/
It applied cleanly for me, so I'm not sure what the issue was. Getting
the other cherry-picked was no small thing, so I'll not complain :-)
Thanks,
Ian
> > The next 5 patches avoid grouping events for metrics where they could
> > never succeed and were previously posted as:
> > "perf vendor events intel: Add xxx metric constraints"
> > https://lore.kernel.org/all/20230419005423.343862-1-irogers@google.com/
> > In general the generated json is coming from:
> > https://github.com/intel/perfmon/pull/73
> >
> > Next are some general and test improvements.
> >
> > Next event parsing is rewritten to not scan all PMUs for the benefit
> > of raw and legacy cache parsing, instead these are handled by the
> > lexer and a new term type. This ultimately removes the need for the
> > event parser for hybrid to be recursive as legacy cache can be just a
> > term. Tests are re-enabled for events with hyphens, so AMD's
> > branch-brs event is now parsable.
> >
> > The cputype option is made a generic pmu filter flag and is tested
> > even on non-hybrid systems.
> >
> > The final patches address specific json metric issues on hybrid, in
> > both the json metrics and the metric code.
> >
> > The patches add slightly more code than they remove, in areas like
> > better json metric constraints and tests, but in the core util code,
> > the removal of hybrid is a net reduction:
> > 22 files changed, 711 insertions(+), 1016 deletions(-)
> >
> > Sample output is contained in the v1 patch set:
> > https://lore.kernel.org/lkml/bff481ba-e60a-763f-0aa0-3ee53302c480@linux.intel.com/
> >
> > Tested on Tigerlake, Skylake and Alderlake CPUs.
> >
> > The v4 patch set:
> > - rebase, 1 of the Linux 6.4 recommended patches are merged leaving:
> > 1) perf metric: Change divide by zero and !support events behavior
> > 2) perf stat: Introduce skippable evsels
> > 3) perf metric: Json flag to not group events if gathering a metric group
> > 4) perf parse-events: Don't reorder ungrouped events by pmu
> > whose diffstat is:
> > 30 files changed, 326 insertions(+), 33 deletions(-)
> > but without the vendor event updates (the tend to be large as they
> > repeat something per architecture per metric) is just:
> > 10 files changed, 90 insertions(+), 32 deletions(-)
> > - Address review comments from Ravi Bangoria <ravi.bangoria@....com>
> > that are only in the patches targeting Linux 6.5.
> >
> > The v3 patch set:
> > - for Linux 6.4 the first 5 patches are recommended:
> > - changes skippable evsels to always print in case short running
> > benchmarks meet the 0 enable and 0 count condition (suggested by
> > Stephane).
> > - changes metrics to show "nan" so that it is more obvious they
> > couldn't be computed (suggested by Stephane).
> > - fixes a reordering issue that reordered events that lacked a
> > group, especially when the core PMU isn't "cpu".
> > - for Linux 6.5 added extra hybrid type tests in the x86 hybrid test
> > as suggested by Kan.
> > - note, the patches aren't rebased against the tmp.perf-tools branch
> > meaning patches 1 and 11 should be dropped there.
> >
> > The v2 patch set:
> > - restructures the 3 Linux 6.4 patches first;
> > - makes it so that is_event_supported isn't called during core event parsing;
> > - displays skippable 0 count events that were enabled;
> > - addresses output formatting review comments;
> > - addresses some test issues and an uninitialized variable use in strchr;
> > - addresses checkpatch.pl reported issues;
> > - adds reviewed-by to some patches.
> >
> > Ian Rogers (44):
> > perf metric: Change divide by zero and !support events behavior
> > perf stat: Introduce skippable evsels
> > perf metric: Json flag to not group events if gathering a metric group
> > perf parse-events: Don't reorder ungrouped events by pmu
> > perf vendor events intel: Add alderlake metric constraints
> > perf vendor events intel: Add icelake metric constraints
> > perf vendor events intel: Add icelakex metric constraints
> > perf vendor events intel: Add sapphirerapids metric constraints
> > perf vendor events intel: Add tigerlake metric constraints
> > perf test: Test more sysfs events
> > perf test: Use valid for PMU tests
> > perf test: Mask configs with extended types then test
> > perf test: Test more with config_cache
> > perf test: Roundtrip name, don't assume 1 event per name
> > perf parse-events: Set attr.type to PMU type early
> > perf parse-events: Set pmu_name whenever a pmu is given
> > perf print-events: Avoid unnecessary strlist
> > perf parse-events: Avoid scanning PMUs before parsing
> > perf evsel: Modify group pmu name for software events
> > perf test: Move x86 hybrid tests to arch/x86
> > perf test x86 hybrid: Update test expectations
> > perf test x86 hybrid: Add hybrid extended type checks
> > perf parse-events: Support PMUs for legacy cache events
> > perf parse-events: Wildcard legacy cache events
> > perf print-events: Print legacy cache events for each PMU
> > perf parse-events: Support wildcards on raw events
> > perf parse-events: Remove now unused hybrid logic
> > perf parse-events: Minor type safety cleanup
> > perf parse-events: Add pmu filter
> > perf stat: Make cputype filter generic
> > perf test: Add cputype testing to perf stat
> > perf test: Fix parse-events tests for >1 core PMU
> > perf parse-events: Support hardware events as terms
> > perf parse-events: Avoid error when assigning a term
> > perf parse-events: Avoid error when assigning a legacy cache term
> > perf parse-events: Don't auto merge hybrid wildcard events
> > perf parse-events: Don't reorder atom cpu events
> > perf metrics: Be PMU specific for referenced metrics.
> > perf stat: Command line PMU metric filtering
> > perf vendor events intel: Correct alderlake metrics
> > perf jevents: Don't rewrite metrics across PMUs
> > perf metrics: Be PMU specific in event match
> > perf stat: Don't disable TopdownL1 metric on hybrid
> > perf parse-events: Reduce scope of is_event_supported
> >
> > tools/perf/arch/x86/include/arch-tests.h | 1 +
> > tools/perf/arch/x86/tests/Build | 1 +
> > tools/perf/arch/x86/tests/arch-tests.c | 10 +
> > tools/perf/arch/x86/tests/hybrid.c | 287 ++++++
> > tools/perf/arch/x86/util/evlist.c | 4 +-
> > tools/perf/builtin-list.c | 21 +-
> > tools/perf/builtin-record.c | 13 +-
> > tools/perf/builtin-stat.c | 77 +-
> > tools/perf/builtin-top.c | 5 +-
> > tools/perf/builtin-trace.c | 5 +-
> > .../arch/x86/alderlake/adl-metrics.json | 275 +++---
> > .../arch/x86/alderlaken/adln-metrics.json | 20 +-
> > .../arch/x86/broadwell/bdw-metrics.json | 12 +
> > .../arch/x86/broadwellde/bdwde-metrics.json | 12 +
> > .../arch/x86/broadwellx/bdx-metrics.json | 12 +
> > .../arch/x86/cascadelakex/clx-metrics.json | 12 +
> > .../arch/x86/haswell/hsw-metrics.json | 12 +
> > .../arch/x86/haswellx/hsx-metrics.json | 12 +
> > .../arch/x86/icelake/icl-metrics.json | 23 +
> > .../arch/x86/icelakex/icx-metrics.json | 23 +
> > .../arch/x86/ivybridge/ivb-metrics.json | 12 +
> > .../arch/x86/ivytown/ivt-metrics.json | 12 +
> > .../arch/x86/jaketown/jkt-metrics.json | 12 +
> > .../arch/x86/sandybridge/snb-metrics.json | 12 +
> > .../arch/x86/sapphirerapids/spr-metrics.json | 23 +
> > .../arch/x86/skylake/skl-metrics.json | 12 +
> > .../arch/x86/skylakex/skx-metrics.json | 12 +
> > .../arch/x86/tigerlake/tgl-metrics.json | 23 +
> > tools/perf/pmu-events/jevents.py | 10 +-
> > tools/perf/pmu-events/metric.py | 28 +-
> > tools/perf/pmu-events/metric_test.py | 6 +-
> > tools/perf/pmu-events/pmu-events.h | 2 +
> > tools/perf/tests/evsel-roundtrip-name.c | 119 +--
> > tools/perf/tests/expr.c | 3 +-
> > tools/perf/tests/parse-events.c | 858 +++++++++---------
> > tools/perf/tests/parse-metric.c | 1 +
> > tools/perf/tests/pmu-events.c | 12 +-
> > tools/perf/tests/shell/stat.sh | 44 +
> > tools/perf/util/Build | 1 -
> > tools/perf/util/evlist.h | 1 -
> > tools/perf/util/evsel.c | 30 +-
> > tools/perf/util/evsel.h | 1 +
> > tools/perf/util/expr.y | 6 +-
> > tools/perf/util/metricgroup.c | 111 ++-
> > tools/perf/util/metricgroup.h | 3 +-
> > tools/perf/util/parse-events-hybrid.c | 214 -----
> > tools/perf/util/parse-events-hybrid.h | 25 -
> > tools/perf/util/parse-events.c | 720 ++++++---------
> > tools/perf/util/parse-events.h | 63 +-
> > tools/perf/util/parse-events.l | 108 +--
> > tools/perf/util/parse-events.y | 222 ++---
> > tools/perf/util/pmu-hybrid.c | 20 -
> > tools/perf/util/pmu-hybrid.h | 1 -
> > tools/perf/util/pmu.c | 16 +-
> > tools/perf/util/pmu.h | 3 +
> > tools/perf/util/pmus.c | 25 +-
> > tools/perf/util/pmus.h | 3 +
> > tools/perf/util/print-events.c | 127 ++-
> > tools/perf/util/stat-display.c | 2 +-
> > tools/perf/util/stat-shadow.c | 25 +-
> > 60 files changed, 2066 insertions(+), 1699 deletions(-)
> > create mode 100644 tools/perf/arch/x86/tests/hybrid.c
> > delete mode 100644 tools/perf/util/parse-events-hybrid.c
> > delete mode 100644 tools/perf/util/parse-events-hybrid.h
> >
> > --
> > 2.40.1.495.gc816e09b53d-goog
> >
>
> --
>
> - Arnaldo
Powered by blists - more mailing lists