<div dir="ltr"><div dir="ltr">On Wed, Feb 22, 2023 at 5:47 AM Arnaldo Carvalho de Melo <<a href="mailto:acme@kernel.org">acme@kernel.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Em Tue, Feb 21, 2023 at 09:44:36AM -0800, Ian Rogers escreveu:<br>
> On Sun, Feb 19, 2023 at 7:43 AM Ian Rogers <<a href="mailto:irogers@google.com" target="_blank">irogers@google.com</a>> wrote:<br>
> ><br>
> > On Sun, Feb 19, 2023, 3:17 AM Arnaldo Carvalho de Melo <<a href="mailto:acme@kernel.org" target="_blank">acme@kernel.org</a>> wrote:<br>
> > ><br>
> > > Em Sun, Feb 19, 2023 at 01:27:57AM -0800, Ian Rogers escreveu:<br>
> > > > Recently the shadow stat metrics broke due to repeated aggregation and<br>
> > > > a quick fix was applied:<br>
> > > > <a href="https://lore.kernel.org/lkml/20230209064447.83733-1-irogers@google.com/" rel="noreferrer" target="_blank">https://lore.kernel.org/lkml/20230209064447.83733-1-irogers@google.com/</a><br>
> > > > This is the longer fix but one that comes with some extras. To avoid<br>
> > > > fixing issues for hard coded metrics, the topdown, SMI cost and<br>
> > > > transaction flags are moved into json metrics. A side effect of this<br>
> > > > is that TopdownL1 metrics will now be displayed when supported, if no<br>
> > > > "perf stat" events are specified.<br>
> > > ><br>
> > > > Another fix included here is for event grouping as raised in:<br>
> > > > <a href="https://lore.kernel.org/lkml/CA+icZUU_ew7pzWJJZLbj1xsU6MQTPrj8tkFfDhNdTDRQfGUBMQ@mail.gmail.com/" rel="noreferrer" target="_blank">https://lore.kernel.org/lkml/CA+icZUU_ew7pzWJJZLbj1xsU6MQTPrj8tkFfDhNdTDRQfGUBMQ@mail.gmail.com/</a><br>
> > > > Metrics are now tagged with NMI and SMT flags, meaning that the events<br>
> > > > shouldn't be grouped if the NMI watchdog is enabled or SMT is enabled.<br>
> > > ><br>
> > > > Given the two issues, the metrics are re-generated and the patches<br>
> > > > also include the latest Intel vendor events. The changes to the metric<br>
> > > > generation code can be seen in:<br>
> > > > <a href="https://github.com/intel/perfmon/pull/56" rel="noreferrer" target="_blank">https://github.com/intel/perfmon/pull/56</a><br>
> > > ><br>
> > > > Hard coded metrics support thresholds, the patches add this ability to<br>
> > > > json metrics so that the hard coded metrics can be removed. Migrate<br>
> > > > remaining hard coded metrics to looking up counters from the<br>
> > > > evlist/aggregation count. Finally, get rid of the saved_value logic<br>
> > > > and thereby look to fix the aggregation issues.<br>
> > > ><br>
> > > > Some related fix ups and code clean ups are included in the changes,<br>
> > > > in particular to aid with the code's readability and to keep topdown<br>
> > > > documentation in sync.<br>
> > ><br>
> > > That is great work but won't have a reasonable time sitting on<br>
> > > linux-next to make it into 6.3.<br>
> > ><br>
> > > I have just applied it locally for the usual set of tests, that I'll<br>
> > > report back here.<br>
> ><br>
> ><br>
> > Ugh. I'm guessing it won't be useful if I point out more things broken<br>
> > with the current workaround, like metrics with --repeat :-/<br>
> ><br>
> > Thanks,<br>
> > Ian<br>
> <br>
> So currently the flow of patches is:<br>
> <br>
> 1) initially testing - acme tmp.perf/core<br>
> 2) things staged for next release - acme perf/core (perhaps this<br>
> should be called perf-next)<br>
<br>
Yeah, perf-tools-next probably.<br>
<br>
> 3) linux wide next release testing - linux-next<br>
> 4) release - linus/master<br>
> <br>
> I wonder if there should be a perf-next-next branch to work around the<br>
> "sitting time" problem. Otherwise anybody who touches code in these 51<br>
> patches will create a merge conflict. Given the aggregation issues<br>
> we're likely to see changes in this code and so conflicts are likely<br>
> to happen.<br>
> <br>
> The patch flow with perf-next-next would be:<br>
> <br>
> 1) initial testing - tmp.perf-next-next<br>
> 2) things acquiring sitting time and where developers work - perf-next-next<br>
> 3) things staged for the next release - perf-next<br>
> 4) as 3 above<br>
> 5) as 4 above<br>
> <br>
> With linux-next picking up acme perf/core (aka perf-next) daily it<br>
> isn't clear whether we should work off of perf-core or linux-next as<br>
> they are so in sync. This process means we've lost a sitting place for<br>
> developer patches and we're going to feel the pain in terms of merge<br>
> conflicts on the list, difficulty building off of the latest work<br>
> without cherry-picking from the list, etc.<br>
<br>
I'll send perf/core to Linus today and then your 51 patches will appear<br>
on perf-tools-next, where we'll do what was done before in perf/core<br>
(remained like that for historical reasons).<br>
<br>
I'll make perf/urgent become 'perf-tools', and when the merge window<br>
closes, we turn 'perf-tools-next' into 'perf-tools',<br>
<br>
- Arnaldo<br></blockquote><div><br></div><div>Awesome, thanks! I'm also happy if people review the changes :-)</div><div><br></div><div>Ian</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Thanks,<br>
> Ian<br>
> <br>
> > > - Arnaldo<br>
> > ><br>
> > > > Ian Rogers (51):<br>
> > > > perf tools: Ensure evsel name is initialized<br>
> > > > perf metrics: Improve variable names<br>
> > > > perf pmu-events: Remove aggr_mode from pmu_event<br>
> > > > perf pmu-events: Change aggr_mode to be an enum<br>
> > > > perf pmu-events: Change deprecated to be a bool<br>
> > > > perf pmu-events: Change perpkg to be a bool<br>
> > > > perf expr: Make the online topology accessible globally<br>
> > > > perf pmu-events: Make the metric_constraint an enum<br>
> > > > perf pmu-events: Don't '\0' terminate enum values<br>
> > > > perf vendor events intel: Refresh alderlake events<br>
> > > > perf vendor events intel: Refresh alderlake-n metrics<br>
> > > > perf vendor events intel: Refresh broadwell metrics<br>
> > > > perf vendor events intel: Refresh broadwellde metrics<br>
> > > > perf vendor events intel: Refresh broadwellx metrics<br>
> > > > perf vendor events intel: Refresh cascadelakex events<br>
> > > > perf vendor events intel: Add graniterapids events<br>
> > > > perf vendor events intel: Refresh haswell metrics<br>
> > > > perf vendor events intel: Refresh haswellx metrics<br>
> > > > perf vendor events intel: Refresh icelake events<br>
> > > > perf vendor events intel: Refresh icelakex metrics<br>
> > > > perf vendor events intel: Refresh ivybridge metrics<br>
> > > > perf vendor events intel: Refresh ivytown metrics<br>
> > > > perf vendor events intel: Refresh jaketown events<br>
> > > > perf vendor events intel: Refresh knightslanding events<br>
> > > > perf vendor events intel: Refresh sandybridge events<br>
> > > > perf vendor events intel: Refresh sapphirerapids events<br>
> > > > perf vendor events intel: Refresh silvermont events<br>
> > > > perf vendor events intel: Refresh skylake events<br>
> > > > perf vendor events intel: Refresh skylakex metrics<br>
> > > > perf vendor events intel: Refresh tigerlake events<br>
> > > > perf vendor events intel: Refresh westmereep-dp events<br>
> > > > perf jevents: Add rand support to metrics<br>
> > > > perf jevent: Parse metric thresholds<br>
> > > > perf pmu-events: Test parsing metric thresholds with the fake PMU<br>
> > > > perf list: Support for printing metric thresholds<br>
> > > > perf metric: Compute and print threshold values<br>
> > > > perf expr: More explicit NAN handling<br>
> > > > perf metric: Add --metric-no-threshold option<br>
> > > > perf stat: Add TopdownL1 metric as a default if present<br>
> > > > perf stat: Implement --topdown using json metrics<br>
> > > > perf stat: Remove topdown event special handling<br>
> > > > perf doc: Refresh topdown documentation<br>
> > > > perf stat: Remove hard coded transaction events<br>
> > > > perf stat: Use metrics for --smi-cost<br>
> > > > perf stat: Remove perf_stat_evsel_id<br>
> > > > perf stat: Move enums from header<br>
> > > > perf stat: Hide runtime_stat<br>
> > > > perf stat: Add cpu_aggr_map for loop<br>
> > > > perf metric: Directly use counts rather than saved_value<br>
> > > > perf stat: Use counts rather than saved_value<br>
> > > > perf stat: Remove saved_value/runtime_stat<br>
> > > ><br>
> > > > tools/perf/Documentation/perf-stat.txt | 27 +-<br>
> > > > tools/perf/Documentation/topdown.txt | 70 +-<br>
> > > > tools/perf/arch/powerpc/util/header.c | 2 +-<br>
> > > > tools/perf/arch/x86/util/evlist.c | 6 +-<br>
> > > > tools/perf/arch/x86/util/topdown.c | 78 +-<br>
> > > > tools/perf/arch/x86/util/topdown.h | 1 -<br>
> > > > tools/perf/builtin-list.c | 13 +-<br>
> > > > tools/perf/builtin-script.c | 9 +-<br>
> > > > tools/perf/builtin-stat.c | 233 +-<br>
> > > > .../arch/x86/alderlake/adl-metrics.json | 3190 ++++++++++-------<br>
> > > > .../pmu-events/arch/x86/alderlake/cache.json | 36 +-<br>
> > > > .../arch/x86/alderlake/floating-point.json | 27 +<br>
> > > > .../arch/x86/alderlake/frontend.json | 9 +<br>
> > > > .../pmu-events/arch/x86/alderlake/memory.json | 3 +-<br>
> > > > .../arch/x86/alderlake/pipeline.json | 14 +-<br>
> > > > .../arch/x86/alderlake/uncore-other.json | 28 +-<br>
> > > > .../arch/x86/alderlaken/adln-metrics.json | 811 +++--<br>
> > > > .../arch/x86/broadwell/bdw-metrics.json | 1439 ++++----<br>
> > > > .../arch/x86/broadwellde/bdwde-metrics.json | 1405 ++++----<br>
> > > > .../arch/x86/broadwellx/bdx-metrics.json | 1626 +++++----<br>
> > > > .../arch/x86/broadwellx/uncore-cache.json | 74 +-<br>
> > > > .../x86/broadwellx/uncore-interconnect.json | 64 +-<br>
> > > > .../arch/x86/broadwellx/uncore-other.json | 4 +-<br>
> > > > .../arch/x86/cascadelakex/cache.json | 24 +-<br>
> > > > .../arch/x86/cascadelakex/clx-metrics.json | 2198 ++++++------<br>
> > > > .../arch/x86/cascadelakex/frontend.json | 8 +-<br>
> > > > .../arch/x86/cascadelakex/pipeline.json | 16 +<br>
> > > > .../arch/x86/cascadelakex/uncore-memory.json | 18 +-<br>
> > > > .../arch/x86/cascadelakex/uncore-other.json | 120 +-<br>
> > > > .../arch/x86/cascadelakex/uncore-power.json | 8 +-<br>
> > > > .../arch/x86/graniterapids/cache.json | 54 +<br>
> > > > .../arch/x86/graniterapids/frontend.json | 10 +<br>
> > > > .../arch/x86/graniterapids/memory.json | 174 +<br>
> > > > .../arch/x86/graniterapids/other.json | 29 +<br>
> > > > .../arch/x86/graniterapids/pipeline.json | 102 +<br>
> > > > .../x86/graniterapids/virtual-memory.json | 26 +<br>
> > > > .../arch/x86/haswell/hsw-metrics.json | 1220 ++++---<br>
> > > > .../arch/x86/haswellx/hsx-metrics.json | 1397 ++++----<br>
> > > > .../pmu-events/arch/x86/icelake/cache.json | 16 +<br>
> > > > .../arch/x86/icelake/floating-point.json | 31 +<br>
> > > > .../arch/x86/icelake/icl-metrics.json | 1932 +++++-----<br>
> > > > .../pmu-events/arch/x86/icelake/pipeline.json | 23 +-<br>
> > > > .../arch/x86/icelake/uncore-other.json | 56 +<br>
> > > > .../arch/x86/icelakex/icx-metrics.json | 2153 +++++------<br>
> > > > .../arch/x86/icelakex/uncore-memory.json | 2 +-<br>
> > > > .../arch/x86/icelakex/uncore-other.json | 4 +-<br>
> > > > .../arch/x86/ivybridge/ivb-metrics.json | 1270 ++++---<br>
> > > > .../arch/x86/ivytown/ivt-metrics.json | 1311 ++++---<br>
> > > > .../pmu-events/arch/x86/jaketown/cache.json | 6 +-<br>
> > > > .../arch/x86/jaketown/floating-point.json | 2 +-<br>
> > > > .../arch/x86/jaketown/frontend.json | 12 +-<br>
> > > > .../arch/x86/jaketown/jkt-metrics.json | 602 ++--<br>
> > > > .../arch/x86/jaketown/pipeline.json | 2 +-<br>
> > > > .../arch/x86/jaketown/uncore-cache.json | 22 +-<br>
> > > > .../x86/jaketown/uncore-interconnect.json | 74 +-<br>
> > > > .../arch/x86/jaketown/uncore-memory.json | 4 +-<br>
> > > > .../arch/x86/jaketown/uncore-other.json | 22 +-<br>
> > > > .../arch/x86/jaketown/uncore-power.json | 8 +-<br>
> > > > .../arch/x86/knightslanding/cache.json | 94 +-<br>
> > > > .../arch/x86/knightslanding/pipeline.json | 8 +-<br>
> > > > .../arch/x86/knightslanding/uncore-other.json | 8 +-<br>
> > > > tools/perf/pmu-events/arch/x86/mapfile.csv | 29 +-<br>
> > > > .../arch/x86/sandybridge/cache.json | 8 +-<br>
> > > > .../arch/x86/sandybridge/floating-point.json | 2 +-<br>
> > > > .../arch/x86/sandybridge/frontend.json | 12 +-<br>
> > > > .../arch/x86/sandybridge/pipeline.json | 2 +-<br>
> > > > .../arch/x86/sandybridge/snb-metrics.json | 601 ++--<br>
> > > > .../arch/x86/sapphirerapids/cache.json | 24 +-<br>
> > > > .../x86/sapphirerapids/floating-point.json | 32 +<br>
> > > > .../arch/x86/sapphirerapids/frontend.json | 8 +<br>
> > > > .../arch/x86/sapphirerapids/pipeline.json | 19 +-<br>
> > > > .../arch/x86/sapphirerapids/spr-metrics.json | 2283 ++++++------<br>
> > > > .../arch/x86/sapphirerapids/uncore-other.json | 60 +<br>
> > > > .../arch/x86/silvermont/frontend.json | 2 +-<br>
> > > > .../arch/x86/silvermont/pipeline.json | 2 +-<br>
> > > > .../pmu-events/arch/x86/skylake/cache.json | 25 +-<br>
> > > > .../pmu-events/arch/x86/skylake/frontend.json | 8 +-<br>
> > > > .../pmu-events/arch/x86/skylake/other.json | 1 +<br>
> > > > .../pmu-events/arch/x86/skylake/pipeline.json | 16 +<br>
> > > > .../arch/x86/skylake/skl-metrics.json | 1877 ++++++----<br>
> > > > .../arch/x86/skylake/uncore-other.json | 1 +<br>
> > > > .../pmu-events/arch/x86/skylakex/cache.json | 8 +-<br>
> > > > .../arch/x86/skylakex/frontend.json | 8 +-<br>
> > > > .../arch/x86/skylakex/pipeline.json | 16 +<br>
> > > > .../arch/x86/skylakex/skx-metrics.json | 2097 +++++------<br>
> > > > .../arch/x86/skylakex/uncore-memory.json | 2 +-<br>
> > > > .../arch/x86/skylakex/uncore-other.json | 96 +-<br>
> > > > .../arch/x86/skylakex/uncore-power.json | 6 +-<br>
> > > > .../arch/x86/tigerlake/floating-point.json | 31 +<br>
> > > > .../arch/x86/tigerlake/pipeline.json | 18 +<br>
> > > > .../arch/x86/tigerlake/tgl-metrics.json | 1942 +++++-----<br>
> > > > .../arch/x86/tigerlake/uncore-other.json | 28 +-<br>
> > > > .../arch/x86/westmereep-dp/cache.json | 2 +-<br>
> > > > .../x86/westmereep-dp/virtual-memory.json | 2 +-<br>
> > > > tools/perf/pmu-events/jevents.py | 58 +-<br>
> > > > tools/perf/pmu-events/metric.py | 8 +-<br>
> > > > tools/perf/pmu-events/pmu-events.h | 35 +-<br>
> > > > tools/perf/tests/expand-cgroup.c | 3 +-<br>
> > > > tools/perf/tests/expr.c | 7 +-<br>
> > > > tools/perf/tests/parse-metric.c | 21 +-<br>
> > > > tools/perf/tests/pmu-events.c | 49 +-<br>
> > > > tools/perf/util/cpumap.h | 3 +<br>
> > > > tools/perf/util/cputopo.c | 14 +<br>
> > > > tools/perf/util/cputopo.h | 5 +<br>
> > > > tools/perf/util/evsel.h | 2 +-<br>
> > > > tools/perf/util/expr.c | 16 +-<br>
> > > > tools/perf/util/expr.y | 12 +-<br>
> > > > tools/perf/util/metricgroup.c | 178 +-<br>
> > > > tools/perf/util/metricgroup.h | 5 +-<br>
> > > > tools/perf/util/pmu.c | 17 +-<br>
> > > > tools/perf/util/print-events.h | 1 +<br>
> > > > tools/perf/util/smt.c | 11 +-<br>
> > > > tools/perf/util/smt.h | 12 +-<br>
> > > > tools/perf/util/stat-display.c | 117 +-<br>
> > > > tools/perf/util/stat-shadow.c | 1287 ++-----<br>
> > > > tools/perf/util/stat.c | 74 -<br>
> > > > tools/perf/util/stat.h | 96 +-<br>
> > > > tools/perf/util/synthetic-events.c | 2 +-<br>
> > > > tools/perf/util/topdown.c | 68 +-<br>
> > > > tools/perf/util/topdown.h | 11 +-<br>
> > > > 120 files changed, 18025 insertions(+), 15590 deletions(-)<br>
> > > > create mode 100644 tools/perf/pmu-events/arch/x86/graniterapids/cache.json<br>
> > > > create mode 100644 tools/perf/pmu-events/arch/x86/graniterapids/frontend.json<br>
> > > > create mode 100644 tools/perf/pmu-events/arch/x86/graniterapids/memory.json<br>
> > > > create mode 100644 tools/perf/pmu-events/arch/x86/graniterapids/other.json<br>
> > > > create mode 100644 tools/perf/pmu-events/arch/x86/graniterapids/pipeline.json<br>
> > > > create mode 100644 tools/perf/pmu-events/arch/x86/graniterapids/virtual-memory.json<br>
> > > ><br>
> > > > --<br>
> > > > 2.39.2.637.g21b0678d19-goog<br>
> > > ><br>
> > ><br>
> > > --<br>
> > ><br>
> > > - Arnaldo<br>
<br>
-- <br>
<br>
- Arnaldo<br>
</blockquote></div></div>