linux-kernel - Re: [PATCH] perf jevents: Don't stop at the first matched pmu when searching a events table

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=fX=QR66nQ6VKRMFfiFdueiC1EUGmxSbT9RCkW8dz67e6w@mail.gmail.com>
Date: Thu, 26 Sep 2024 14:34:43 -0700
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: kan.liang@...ux.intel.com, namhyung@...nel.org, adrian.hunter@...el.com, 
	peterz@...radead.org, mingo@...nel.org, linux-kernel@...r.kernel.org, 
	kernel test robot <oliver.sang@...el.com>
Subject: Re: [PATCH] perf jevents: Don't stop at the first matched pmu when
 searching a events table

On Wed, Jul 31, 2024 at 6:31 AM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> On Tue, Jul 16, 2024 at 09:30:01AM -0700, Ian Rogers wrote:
> > On Tue, Jul 16, 2024 at 6:29 AM <kan.liang@...ux.intel.com> wrote:
> > >
> > > From: Kan Liang <kan.liang@...ux.intel.com>
> > >
> > > The "perf all PMU test" fails on a Coffee Lake machine.
> > >
> > > The failure is caused by the below change in the commit e2641db83f18
> > > ("perf vendor events: Add/update skylake events/metrics").
> > >
> > > +    {
> > > +        "BriefDescription": "This 48-bit fixed counter counts the UCLK cycles",
> > > +        "Counter": "FIXED",
> > > +        "EventCode": "0xff",
> > > +        "EventName": "UNC_CLOCK.SOCKET",
> > > +        "PerPkg": "1",
> > > +        "PublicDescription": "This 48-bit fixed counter counts the UCLK cycles.",
> > > +        "Unit": "cbox_0"
> > >      }
> > >
> > > The other cbox events have the unit name "CBOX", while the fixed counter
> > > has a unit name "cbox_0". So the events_table will maintain separate
> > > entries for cbox and cbox_0.
> > >
> > > The perf_pmus__print_pmu_events() calculates the total number of events,
> > > allocate an aliases buffer, store all the events into the buffer, sort,
> > > and print all the aliases one by one.
> > >
> > > The problem is that the calculated total number of events doesn't match
> > > the stored events in the aliases buffer.
> > >
> > > The perf_pmu__num_events() is used to calculate the number of events. It
> > > invokes the pmu_events_table__num_events() to go through the entire
> > > events_table to find all events. Because of the
> > > pmu_uncore_alias_match(), the suffix of uncore PMU will be ignored. So
> > > the events for cbox and cbox_0 are all counted.
> > >
> > > When storing events into the aliases buffer, the
> > > perf_pmu__for_each_event() only process the events for cbox.
> > >
> > > Since a bigger buffer was allocated, the last entry are all 0.
> > > When printing all the aliases, null will be outputted, and trigger the
> > > failure.
> > >
> > > The mismatch was introduced from the commit e3edd6cf6399 ("perf
> > > pmu-events: Reduce processed events by passing PMU"). The
> > > pmu_events_table__for_each_event() stops immediately once a pmu is set.
> > > But for uncore, especially this case, the method is wrong and mismatch
> > > what perf does in the perf_pmu__num_events().
> > >
> > > With the patch,
> > > $ perf list pmu | grep -A 1 clock.socket
> > >    unc_clock.socket
> > >         [This 48-bit fixed counter counts the UCLK cycles. Unit: uncore_cbox_0
> > > $ perf test "perf all PMU test"
> > >   107: perf all PMU test                                               : Ok
> > >
> > > Reported-by: kernel test robot <oliver.sang@...el.com>
> > > Closes: https://lore.kernel.org/all/202407101021.2c8baddb-oliver.sang@intel.com/
> > > Fixes: e3edd6cf6399 ("perf pmu-events: Reduce processed events by passing PMU")
> > > Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
> >
> > Awesome sauce, thanks!
> >
> > Reviewed-by: Ian Rogers <irogers@...gle.com>
>
> Thanks, applied to tmp.perf-tools-next,

Did this get applied? I'm not seeing it in perf-tools-next:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/tools/perf/pmu-events/jevents.py?h=perf-tools-next

Thanks,
Ian