[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aHqQQwI0MckHdBlY@google.com>
Date: Fri, 18 Jul 2025 11:19:47 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Thomas Falcon <thomas.falcon@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...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>,
Kan Liang <kan.liang@...ux.intel.com>,
Ben Gainey <ben.gainey@....com>,
James Clark <james.clark@...aro.org>,
Howard Chu <howardchu95@...il.com>,
Weilin Wang <weilin.wang@...el.com>, Levi Yun <yeoreum.yun@....com>,
"Dr. David Alan Gilbert" <linux@...blig.org>,
Zhongqiu Han <quic_zhonhan@...cinc.com>,
Blake Jones <blakejones@...gle.com>,
Yicong Yang <yangyicong@...ilicon.com>,
Anubhav Shelat <ashelat@...hat.com>,
Thomas Richter <tmricht@...ux.ibm.com>,
Jean-Philippe Romain <jean-philippe.romain@...s.st.com>,
Song Liu <song@...nel.org>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 00/12] CPU mask improvements/fixes particularly for
hybrid
On Thu, Jul 17, 2025 at 02:02:21PM -0700, Ian Rogers wrote:
> On hybrid systems some PMUs apply to all core types, particularly for
> metrics the msr PMU and the tsc event. The metrics often only want the
> values of the counter for their specific core type. These patches
> allow the cpu term in an event to give a PMU name to take the cpumask
> from. For example:
>
> $ perf stat -e msr/tsc,cpu=cpu_atom/ ...
>
> will aggregate the msr/tsc/ value but only for atom cores. In doing
> this problems were identified in how cpumasks are handled by parsing
> and event setup when cpumasks are specified along with a task to
> profile. The event parsing, cpumask evlist propagation code and perf
> stat code are updated accordingly.
>
> The final result of the patch series is to be able to run:
> ```
> $ perf stat --no-scale -e 'msr/tsc/,msr/tsc,cpu=cpu_core/,msr/tsc,cpu=cpu_atom/' perf test -F 10
> 10.1: Basic parsing test : Ok
> 10.2: Parsing without PMU name : Ok
> 10.3: Parsing with PMU name : Ok
>
> Performance counter stats for 'perf test -F 10':
>
> 63,704,975 msr/tsc/
> 47,060,704 msr/tsc,cpu=cpu_core/ (4.62%)
> 16,640,591 msr/tsc,cpu=cpu_atom/ (2.18%)
> ```
>
> This has (further) identified a kernel bug for task events around the
> enabled time being too large leading to invalid scaling (hence the
> --no-scale in the command line above).
>
> v2: Add additional documentation of the cpu term to `perf list`
> (Namhyung), extend the term to also allow CPU ranges. Add Thomas
> Falcon's reviewed-by. Still open for discussion whether the term
> cpu should have >1 variant for PMUs, etc. or whether the single
> term is okay. We could refactor later and add a term, but that
> would break existing users, but they are most likely to be metrics
> so probably not a huge issue.
I slightly prefer a new term to minimize the possible confusion. But
it's not strong and I'm open to other opinion.
On the other hand, the 'cpu' term is new and I don't think it has many
users outside. So probably we can change it later.
Thanks,
Namhyung
>
> Ian Rogers (12):
> perf parse-events: Warn if a cpu term is unsupported by a CPU
> perf stat: Avoid buffer overflow to the aggregation map
> perf stat: Don't size aggregation ids from user_requested_cpus
> perf parse-events: Allow the cpu term to be a PMU or CPU range
> perf tool_pmu: Allow num_cpus(_online) to be specific to a cpumask
> libperf evsel: Rename own_cpus to pmu_cpus
> libperf evsel: Factor perf_evsel__exit out of perf_evsel__delete
> perf evsel: Use libperf perf_evsel__exit
> perf pmus: Factor perf_pmus__find_by_attr out of evsel__find_pmu
> perf parse-events: Minor __add_event refactoring
> perf evsel: Add evsel__open_per_cpu_and_thread
> perf parse-events: Support user CPUs mixed with threads/processes
>
> tools/lib/perf/evlist.c | 118 ++++++++++++++++--------
> tools/lib/perf/evsel.c | 9 +-
> tools/lib/perf/include/internal/evsel.h | 3 +-
> tools/perf/Documentation/perf-list.txt | 25 +++--
> tools/perf/builtin-stat.c | 9 +-
> tools/perf/tests/event_update.c | 4 +-
> tools/perf/util/evlist.c | 15 +--
> tools/perf/util/evsel.c | 55 +++++++++--
> tools/perf/util/evsel.h | 5 +
> tools/perf/util/expr.c | 2 +-
> tools/perf/util/header.c | 4 +-
> tools/perf/util/parse-events.c | 112 ++++++++++++++++------
> tools/perf/util/pmus.c | 29 +++---
> tools/perf/util/pmus.h | 2 +
> tools/perf/util/stat.c | 6 +-
> tools/perf/util/synthetic-events.c | 4 +-
> tools/perf/util/tool_pmu.c | 56 +++++++++--
> tools/perf/util/tool_pmu.h | 2 +-
> 18 files changed, 323 insertions(+), 137 deletions(-)
>
> --
> 2.50.0.727.gbf7dc18ff4-goog
>
Powered by blists - more mailing lists