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=fXnMgwoa=VF2oK6D_rXvEOLCyq2PHkXoyN42KOgaWBTgQ@mail.gmail.com>
Date: Wed, 21 Jan 2026 06:33:22 -0800
From: Ian Rogers <irogers@...gle.com>
To: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
Cc: Zide Chen <zide.chen@...el.com>, Peter Zijlstra <peterz@...radead.org>, 
	Ingo Molnar <mingo@...hat.com>, Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Andi Kleen <ak@...ux.intel.com>, 
	Eranian Stephane <eranian@...gle.com>, linux-kernel@...r.kernel.org, 
	linux-perf-users@...r.kernel.org, Xudong Hao <xudong.hao@...el.com>, 
	Falcon Thomas <thomas.falcon@...el.com>
Subject: Re: [PATCH V2 11/13] perf pmu: Relax uncore wildcard matching to
 allow numeric suffix

On Wed, Jan 21, 2026 at 12:02 AM Mi, Dapeng <dapeng1.mi@...ux.intel.com> wrote:
>
>
> On 1/21/2026 3:18 PM, Ian Rogers wrote:
> > On Wed, Dec 31, 2025 at 2:49 PM Zide Chen <zide.chen@...el.com> wrote:
> >> Diamond Rapids introduces two types of PCIe related uncore PMUs:
> >> "uncore_pcie4_*" and "uncore_pcie6_*".
> >>
> >> To ensure that generic PCIe events (e.g., UNC_PCIE_CLOCKTICKS) can match
> >> and collect events from both PMU types, slightly relax the wildcard
> >> matching logic in perf_pmu__match_wildcard().
> >>
> >> This change allows a wildcard such as "pcie" to match PMU names that
> >> include a numeric suffix, such as "pcie4_*" and "pcie6_*".
> >>
> >> Co-developed-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
> >> Signed-off-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
> >> Reviewed-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
> >> Signed-off-by: Zide Chen <zide.chen@...el.com>
> > Can we not merge this. I'd missed a perf tool patch as it was hiding
> > in a bunch of kernel uncore updates. At the very least if wildcard
> > conventions are updated then the corresponding documentation needs
> > updating:
> > Documentation/ABI/testing/sysfs-bus-event_source-devices
>
> Ian, thanks for the information. We didn't notice there is such
> documentation to describe the name. :(
>
> Besides the documentation, are there other comments? We can update it
> together. Thanks.

The suffix handling is notoriously brittle. For example, ARM added hex
suffixes which are generally 12 characters of physical address rather
than the typical _0, _1, etc. What could go wrong? Well in some
situations ARM make their core PMU's follow the model name, so rather
than armv8_pmuv3_0 the core pmu is a name that ends with a model
number something like _a76, however, that is also a valid hex suffix.
So we have bumped the hex suffix to be at least 2 characters:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmus.c?h=tmp.perf-tools-next#n81
This also works around the naming of s390 PMUs (comments in the code).
ARM now have models like a720ae which would appear as a 5 character
hex suffix, and so this whole hex suffix thing is hanging together for
some part because we treat core and uncore PMUs differently. From my
pov, ideally the ARM uncore PMUs would have just used the _0, _1, etc.
naming convention and placed the physical address information into a
caps file, rather than trying to shoehorn it into the PMU name.
s390 pmu names have discrepancies that mean lots of their core PMUs
can match suffixes and Intel's i915 PMU name ("i915") will happily
match as just "i" as the underscore before the number is optional. A
change like this needs a range of testing on a variety of
architectures because the code has broken things in a lot of different
architecture types.

Besides a lack of testing, going in through the wrong tree, the change
is changing suffix handling in one place but not all - at least
pmu_name_len_no_suffix wasn't updated. Let's get this out of the tree
and start again.

Thanks,
Ian

> >
> > Thanks,
> > Ian
> >
> >> ---
> >>  tools/perf/util/pmu.c | 14 ++++++++------
> >>  1 file changed, 8 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> >> index 956ea273c2c7..01a21b6aa031 100644
> >> --- a/tools/perf/util/pmu.c
> >> +++ b/tools/perf/util/pmu.c
> >> @@ -939,6 +939,7 @@ static bool perf_pmu__match_wildcard(const char *pmu_name, const char *tok)
> >>  {
> >>         const char *p, *suffix;
> >>         bool has_hex = false;
> >> +       bool has_underscore = false;
> >>         size_t tok_len = strlen(tok);
> >>
> >>         /* Check start of pmu_name for equality. */
> >> @@ -949,13 +950,14 @@ static bool perf_pmu__match_wildcard(const char *pmu_name, const char *tok)
> >>         if (*p == 0)
> >>                 return true;
> >>
> >> -       if (*p == '_') {
> >> -               ++p;
> >> -               ++suffix;
> >> -       }
> >> -
> >> -       /* Ensure we end in a number */
> >> +       /* Ensure we end in a number or a mix of number and "_". */
> >>         while (1) {
> >> +               if (!has_underscore && (*p == '_')) {
> >> +                       has_underscore = true;
> >> +                       ++p;
> >> +                       ++suffix;
> >> +               }
> >> +
> >>                 if (!isxdigit(*p))
> >>                         return false;
> >>                 if (!has_hex)
> >> --
> >> 2.52.0
> >>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ