| 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
| ||
|
Message-ID: <CAP-5=fVU1A+faRRCJjNsZuO5-0ifpT1C=846qpL_fjfUgVO1bw@mail.gmail.com>
Date: Thu, 3 Apr 2025 08:44:15 -0700
From: Ian Rogers <irogers@...gle.com>
To: "Liang, Kan" <kan.liang@...ux.intel.com>
Cc: 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>, Leo Yan <leo.yan@....com>,
Yoshihiro Furudera <fj5100bi@...itsu.com>, Weilin Wang <weilin.wang@...el.com>,
Andi Kleen <ak@...ux.intel.com>, James Clark <james.clark@...aro.org>,
Dominique Martinet <asmadeus@...ewreck.org>, Yicong Yang <yangyicong@...ilicon.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 3/4] perf parse-events: Set is_pmu_core for legacy
hardware events
On Thu, Apr 3, 2025 at 8:17 AM Liang, Kan <kan.liang@...ux.intel.com> wrote:
>
>
>
> On 2025-02-10 1:38 p.m., Ian Rogers wrote:
> > Also set the CPU map to all online CPU maps. This is done so the
> > behavior of legacy hardware and hardware cache events better matches
> > that of sysfs and json events during
> > __perf_evlist__propagate_maps. Fix missing cpumap put in "Synthesize
> > attr update" test.
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> > tools/perf/tests/event_update.c | 1 +
> > tools/perf/util/parse-events.c | 37 ++++++++++++++++++++-------------
> > 2 files changed, 24 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
> > index d6b4ce3ef4ee..9301fde11366 100644
> > --- a/tools/perf/tests/event_update.c
> > +++ b/tools/perf/tests/event_update.c
> > @@ -109,6 +109,7 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes
> > TEST_ASSERT_VAL("failed to synthesize attr update name",
> > !perf_event__synthesize_event_update_name(&tmp.tool, evsel, process_event_name));
> >
> > + perf_cpu_map__put(evsel->core.own_cpus);
> > evsel->core.own_cpus = perf_cpu_map__new("1,2,3");
> >
> > TEST_ASSERT_VAL("failed to synthesize attr update cpus",
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index 6c36b98875bc..8cccf1e22cdf 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -231,21 +231,30 @@ __add_event(struct list_head *list, int *idx,
> > struct perf_cpu_map *cpu_list, u64 alternate_hw_config)
> > {
> > struct evsel *evsel;
> > - struct perf_cpu_map *cpus = perf_cpu_map__is_empty(cpu_list) && pmu ? pmu->cpus : cpu_list;
> > + bool is_pmu_core;
> > + struct perf_cpu_map *cpus;
> >
> > - cpus = perf_cpu_map__get(cpus);
> > - if (pmu)
> > + if (pmu) {
> > + is_pmu_core = pmu->is_core;
> > + cpus = perf_cpu_map__get(perf_cpu_map__is_empty(cpu_list) ? pmu->cpus : cpu_list);
> > perf_pmu__warn_invalid_formats(pmu);
> > -
> > - if (pmu && (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX)) {
> > - perf_pmu__warn_invalid_config(pmu, attr->config, name,
> > - PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> > - perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> > - PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> > - perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> > - PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> > - perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> > - PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> > + if (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX) {
> > + perf_pmu__warn_invalid_config(pmu, attr->config, name,
> > + PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> > + perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> > + PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> > + perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> > + PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> > + perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> > + PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> > + }
> > + } else {
> > + is_pmu_core = (attr->type == PERF_TYPE_HARDWARE ||
> > + attr->type == PERF_TYPE_HW_CACHE);
> > + if (perf_cpu_map__is_empty(cpu_list))
> > + cpus = is_pmu_core ? perf_cpu_map__new_online_cpus() : NULL;
>
> All online CPUs? Is there a problem for hybrid?
For hybrid if you do cpu_atom/cycles/ then the PMU is found by
perf_pmus__find here:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.c?h=perf-tools-next#n1651
The wildcard case (e.g. perf stat -e cycles ...) scans all the PMUs
passing the respective pmu into __add_event:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.c?h=perf-tools-next#n1382
For non-hybrid the PMU is given by perf_pmus__find_by_type which only
returns a PMU if there is an exact match on the type, ie it won't
return perf_pmus__find_core_pmu when asked to lookup the PMU for a
legacy type, evsel__find_pmu will do this though.
Anyway, the assumption in this legacy configuration no PMU case is
that the event is heading to all CPUs, which is true for software and
tracepoint. For hybrid we could get into this kind of problem is sysfs
isn't mounted, but I think we're pretty generally broken in that case
anyway.
Fwiw, I'm carrying these changes in google's tree and test it on my
hybrid desktop:
https://github.com/googleprodkernel/linux-perf
Thanks,
Ian
> Thanks,
> Kan> + else
> > + cpus = perf_cpu_map__get(cpu_list);
> > }
> > if (init_attr)
> > event_attr_init(attr);
> > @@ -260,7 +269,7 @@ __add_event(struct list_head *list, int *idx,
> > evsel->core.cpus = cpus;
> > evsel->core.own_cpus = perf_cpu_map__get(cpus);
> > evsel->core.requires_cpu = pmu ? pmu->is_uncore : false;
> > - evsel->core.is_pmu_core = pmu ? pmu->is_core : false;
> > + evsel->core.is_pmu_core = is_pmu_core;
> > evsel->auto_merge_stats = auto_merge_stats;
> > evsel->pmu = pmu;
> > evsel->alternate_hw_config = alternate_hw_config;
>
Powered by blists - more mailing lists