| 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=fWS+uSvzHNNL_ezGykVtVf7LtuLG8OohGmL3TDBOy921g@mail.gmail.com>
Date: Thu, 3 Apr 2025 12:27:35 -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 12:25 PM Liang, Kan <kan.liang@...ux.intel.com> wrote:
>
>
>
> On 2025-04-03 11:44 a.m., Ian Rogers wrote:
> > 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",
>
>
> The above change should have been merged.
> 2c744f38da7a ("perf test: Fix leak in "Synthesize attr update" test").
>
> It makes the patch failed to be applied to perf-tools-next.
Thanks Kan, I'll send out a rebase.
Ian
> >>> 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
>
> Yes, the pmu should always be available for hybrid case.
>
> >
> > 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
>
> OK. I will do some tests then.
>
> Thanks,
> Kan
>
> >
> > 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