linux-kernel - Re: [PATCH v3 0/3] Fix incorrect counts when count the same uncore event multiple times

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: <aCOl5ep67uTeGVmm@x1>
Date: Tue, 13 May 2025 17:04:53 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Chun-Tse Shao <ctshao@...gle.com>
Cc: linux-kernel@...r.kernel.org, peterz@...radead.org, mingo@...hat.com,
	namhyung@...nel.org, mark.rutland@....com,
	alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
	irogers@...gle.com, adrian.hunter@...el.com,
	kan.liang@...ux.intel.com, james.clark@...aro.org,
	howardchu95@...il.com, weilin.wang@...el.com, yeoreum.yun@....com,
	linux@...blig.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v3 0/3] Fix incorrect counts when count the same uncore
 event multiple times

On Tue, May 13, 2025 at 05:02:33PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, May 12, 2025 at 02:50:29PM -0700, Chun-Tse Shao wrote:
> > Let's take a look an example, the machine is SKX with 6 IMC devices.
> > 
> >   perf stat -e clockticks,clockticks -I 1000
> >   #           time             counts unit events
> >        1.001127430      6,901,503,174      uncore_imc_0/clockticks/
> >        1.001127430      3,940,896,301      uncore_imc_0/clockticks/
> >        2.002649722        988,376,876      uncore_imc_0/clockticks/
> >        2.002649722        988,376,141      uncore_imc_0/clockticks/
> >        3.004071319      1,000,292,675      uncore_imc_0/clockticks/
> >        3.004071319      1,000,294,160      uncore_imc_0/clockticks/
> > 
> > 1) The events name should not be uniquified.
> > 2) The initial count for the first `clockticks` is doubled.
> > 3) Subsequent count only report for the first IMC device.
> > 
> > The first patch fixes 1) and 3), and the second patch fixes 2).
> 
> So, after having just the first patch applied I'm getting:
> 
>   CC      /tmp/build/perf-tools-next/util/bpf-filter-flex.o
> util/parse-events.c: In function ‘__parse_events’:
> util/parse-events.c:2270:25: error: implicit declaration of function ‘evlist__uniquify_name’; did you mean ‘evlist__uniquify_evsel_names’? [-Wimplicit-function-declaration]
>  2270 |                         evlist__uniquify_name(evlist);
>       |                         ^~~~~~~~~~~~~~~~~~~~~
>       |                         evlist__uniquify_evsel_names
> make[4]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:85: /tmp/build/perf-tools-next/util/parse-events.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
> make[3]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:142: util] Error 2
> make[2]: *** [Makefile.perf:798: /tmp/build/perf-tools-next/perf-util-in.o] Error 2
> make[2]: *** Waiting for unfinished jobs....
> 
> 
>   CC      /tmp/build/perf-tools-next/pmu-events/pmu-events.o
>   LD      /tmp/build/perf-tools-next/pmu-events/pmu-events-in.o
> make[1]: *** [Makefile.perf:290: sub-make] Error 2
> make: *** [Makefile:119: install-bin] Error 2
> make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
> ⬢ [acme@...lbx perf-tools-next]$ 
> ⬢ [acme@...lbx perf-tools-next]$ 
> ⬢ [acme@...lbx perf-tools-next]$ 
> ⬢ [acme@...lbx perf-tools-next]$ 
> ⬢ [acme@...lbx perf-tools-next]$ 
> ⬢ [acme@...lbx perf-tools-next]$ 
> ⬢ [acme@...lbx perf-tools-next]$ 
> ⬢ [acme@...lbx perf-tools-next]$ git log --oneline -3
> 6ffcaec3ac0d055a (HEAD) perf evlist: Make uniquifying counter names consistent
> 4102ff8b1fdaa588 perf metricgroup: Binary search when resolving referred to metrics
> 754baf426e099fbf perf pmu: Change aliases from list to hashmap
> ⬢ [acme@...lbx perf-tools-next]$
> 
> When test building the second patch, it builds, so I'm now looking if
> you used things from the future or if the second patch removes the
> problem.

At that point:

util/parse-events.c: In function ‘__parse_events’:
util/parse-events.c:2270:25: error: implicit declaration of function ‘evlist__uniquify_name’; did you mean ‘evlist__uniquify_evsel_names’? [-Wimplicit-function-declaration]
 2270 |                         evlist__uniquify_name(evlist);
      |                         ^~~~~~~~~~~~~~~~~~~~~
      |                         evlist__uniquify_evsel_names
make[4]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:85: /tmp/build/perf-tools-next/util/parse-events.o] Error 1
make[4]: *** Waiting for unfinished jobs....
  LD      /tmp/build/perf-tools-next/util/scripting-engines/perf-util-in.o
make[3]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:142: util] Error 2
make[2]: *** [Makefile.perf:798: /tmp/build/perf-tools-next/perf-util-in.o] Error 2
make[1]: *** [Makefile.perf:290: sub-make] Error 2
make: *** [Makefile:119: install-bin] Error 2
make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
⬢ [acme@...lbx perf-tools-next]$ 
⬢ [acme@...lbx perf-tools-next]$ git grep evlist__uniquify_name
tools/perf/util/parse-events.c:                 evlist__uniquify_name(evlist);
⬢ [acme@...lbx perf-tools-next]$

So its the later, the second patch builds because:

⬢ [acme@...lbx perf-tools-next]$ git rebase --continue
Stopped at ed3b26e31f42d1e4...  perf parse-events: Use wildcard processing to set an event to merge into
You can amend the commit now, with

  git commit --amend 

Once you are satisfied with your changes, run

  git rebase --continue
⬢ [acme@...lbx perf-tools-next]$ git grep evlist__uniquify_name
⬢ [acme@...lbx perf-tools-next]$ git show | grep evlist__uniquify_name
-			evlist__uniquify_name(evlist);
⬢ [acme@...lbx perf-tools-next]$

That function isn't there anymore.

Please try to fix this and build it patch by patch so that we don't
introduce bisection breakage patches.

Thanks,

- Arnaldo