Re: [PATCH v4 00/44] Fix perf on Intel hybrid CPUs

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, May 03, 2023 at 04:56:36PM -0400, Liang, Kan escreveu:
> 
> 
> On 2023-05-02 6:38 p.m., Ian Rogers wrote:
> > 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.
> > 
> > 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(-)
> 
> I have tested the 4 patches on top of the perf-tools-next branch on both
> Cascade Lake and Raptor Lake. The result looks good to me.
> 
> They address the permission error found in the default mode of perf stat
> on the Cascade Lake. Thanks Ian for the fix.
> 
> Arnaldo, could you please consider to back port them for the 6.4?

Yes, its in perf-tools now, will go to Linus next week.

What about the other patches? I saw some you provided your review, what
about the others, are you ok with them?

- Arnaldo