linux-kernel - Re: [PATCH 0/7] perf stat: Make default perf stat command work on Arm big.LITTLE

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]

Message-ID: <CAP-5=fUVchraZxYg9LY-CtqYZ5DN05-T3vhJmaUG+24Ka6Bsyg@mail.gmail.com>
Date: Tue, 13 Aug 2024 07:35:27 -0700
From: Ian Rogers <irogers@...gle.com>
To: James Clark <james.clark@...aro.org>
Cc: linux-perf-users@...r.kernel.org, John Garry <john.g.garry@...cle.com>, 
	Will Deacon <will@...nel.org>, Mike Leach <mike.leach@...aro.org>, Leo Yan <leo.yan@...ux.dev>, 
	Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, "Liang, Kan" <kan.liang@...ux.intel.com>, 
	Dominique Martinet <asmadeus@...ewreck.org>, Yang Jihong <yangjihong1@...wei.com>, 
	Ze Gao <zegao2021@...il.com>, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 0/7] perf stat: Make default perf stat command work on Arm big.LITTLE

On Tue, Aug 13, 2024 at 6:24 AM James Clark <james.clark@...aro.org> wrote:
>
> The important patches are 3 and 5, the rest are tidyups and tests.
>
> I don't think there is any interaction with the other open issues
> about the uncore DSU cycles event or JSON/legacy hw event priorities
> because only hw events on core PMUs are used for the default
> stat command. And also just sharing the existing x86 code works so
> no big changes are required.
>
> For patch 3 the weak arch specific symbol has to continue to be used
> rather than picking the implementation based on
> perf_pmus__supports_extended_type() like in patch 5. This is because
> that function ends up calling evsel__hw_name() itself which results
> in recursion. But at least one weak arch_* construct has been removed,
> so it's better than nothing.

Let's not do things this way. The use of strings is architecture
neutral, means we don't need to create new arch functions on things
like RISC-V, it encapsulates the complexity of things like topdown
events, Apple ARM M CPUs not supporting legacy events, etc.
Duplicating the existing x86 logic, when that was something trying to
be removed, is not the way to go. That logic was a holdover from the
hybrid tech debt we've been working to remove with a generic approach.

Thanks,
Ian

> James Clark (7):
>   perf stat: Initialize instead of overwriting clock event
>   perf stat: Remove unused default_null_attrs
>   perf evsel: Use the same arch_evsel__hw_name() on arm64 as x86
>   perf evsel: Remove duplicated __evsel__hw_name() code
>   perf evlist: Use hybrid default attrs whenever extended type is
>     supported
>   perf test: Make stat test work on DT devices
>   perf test: Add a test for default perf stat command
>
>  tools/perf/arch/arm64/util/Build   |  1 +
>  tools/perf/arch/arm64/util/evsel.c |  7 ++++
>  tools/perf/arch/x86/util/evlist.c  | 65 ------------------------------
>  tools/perf/arch/x86/util/evsel.c   | 17 +-------
>  tools/perf/builtin-stat.c          | 12 ++----
>  tools/perf/tests/shell/stat.sh     | 33 ++++++++++++---
>  tools/perf/util/evlist.c           | 65 ++++++++++++++++++++++++++----
>  tools/perf/util/evlist.h           |  6 +--
>  tools/perf/util/evsel.c            | 19 +++++++++
>  tools/perf/util/evsel.h            |  2 +-
>  10 files changed, 119 insertions(+), 108 deletions(-)
>  create mode 100644 tools/perf/arch/arm64/util/evsel.c
>
> --
> 2.34.1
>