Message-ID: <b1a461b4-4358-4556-80ef-e7abd686f2e5@linaro.org>
Date: Thu, 16 Oct 2025 11:29:49 +0100
From: James Clark <james.clark@...aro.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Namhyung Kim <namhyung@...nel.org>, Peter Zijlstra
 <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
 Xu Yang <xu.yang_2@....com>, Thomas Falcon <thomas.falcon@...el.com>,
 Andi Kleen <ak@...ux.intel.com>, linux-kernel@...r.kernel.org,
 linux-perf-users@...r.kernel.org, Atish Patra <atishp@...osinc.com>,
 Beeman Strong <beeman@...osinc.com>, Leo Yan <leo.yan@....com>,
 Vince Weaver <vincent.weaver@...ne.edu>
Subject: Re: [PATCH v7 00/27] Legacy hardware/cache events as json



On 15/10/2025 10:24 pm, Ian Rogers wrote:
> On Wed, Oct 15, 2025 at 10:39 AM James Clark <james.clark@...aro.org> wrote:
>>
>>
>>
>> On 15/10/2025 4:53 pm, Namhyung Kim wrote:
>>> On Sun, 05 Oct 2025 11:24:03 -0700, Ian Rogers wrote:
>>>
>>>> Mirroring similar work for software events in commit 6e9fa4131abb
>>>> ("perf parse-events: Remove non-json software events"). These changes
>>>> migrate the legacy hardware and cache events to json.  With no hard
>>>> coded legacy hardware or cache events the wild card, case
>>>> insensitivity, etc. is consistent for events. This does, however, mean
>>>> events like cycles will wild card against all PMUs. A change doing the
>>>> same was originally posted and merged from:
>>>> https://lore.kernel.org/r/20240416061533.921723-10-irogers@google.com
>>>> and reverted by Linus in commit 4f1b067359ac ("Revert "perf
>>>> parse-events: Prefer sysfs/JSON hardware events over legacy"") due to
>>>> his dislike for the cycles behavior on ARM with perf record. Earlier
>>>> patches in this series make perf record event opening failures
>>>> non-fatal and hide the cycles event's failure to open on ARM in perf
>>>> record, so it is expected the behavior will now be transparent in perf
>>>> record on ARM. perf stat with a cycles event will wildcard open the
>>>> event on all PMUs, however, with default events the cycles event will
>>>> only be opened on core PMUs.
>>>>
>>>> [...]
>>>
>>> Applied to perf-tools-next, thanks!
>>>
>>> Best regards,
>>> Namhyung
>>>
>>
>> Hi Namhyung,
>>
>> I'm still getting the build failure that I mentioned on patch 5. This
>> only seems to happen with out of source builds:
>>
>>     $ make -C tools/perf O=../build/local/ V=1
>>
>>
>>     static const struct pmu_sys_events pmu_sys_event_tables[] = {
>>           {
>> -               .event_table = {
>> -                       .pmus = pmu_events__test_soc_sys,
>> -                       .num_pmus = ARRAY_SIZE(pmu_events__test_soc_sys)
>> -               },
>> -               .name = "pmu_events__test_soc_sys",
>> -       },
>> -       {
>>                   .event_table = { 0, 0 },
>>                   .metric_table = { 0, 0 },
>>           },
>> make[3]: *** [pmu-events/Build:54:
>> /home/james/workspace/linux/build/local/pmu-events/empty-pmu-events.log]
>> Error 1
> 
> Sorry for the issue. Is this happening when you don't do a clean
> first? I tried recreating your output path, but I can't reproduce the
> issue on a clean build. The diff above indicates some issue with the

The clean issue was separate to the build failure. I meant that the 
build failure was sticky when I was bisecting. So after commit 5 the 
build breaks on a clean build but then it stayed broken even on builds 
before commit 5 unless I cleaned. I think we can ignore this for now.

> Makefile processing tools/perf/pmu-events/arch/test/test_soc/. This
> directory should be copied to
> ../build/local/pmu-events/arch/test/test_soc/ and so I wonder if the
> copy failed for some reason.
> 
> The copy rule is:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/Build?h=perf-tools-next#n33
> ```
> # Copy checked-in json for generation.
> $(OUTPUT)pmu-events/arch/%: pmu-events/arch/%
> $(call rule_mkdir)
> $(Q)$(call echo-cmd,gen)cp $< $@
> ```
> 
> The mapping of file names happens in the patsubst in:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/Build?h=perf-tools-next#n42
> ```
> GEN_JSON = $(patsubst %,$(OUTPUT)%,$(JSON)) $(LEGACY_CACHE_JSON)
> ```
> 
> Those files are dependencies for the empty-pmu-events.c test so I'm
> not sure how this can be failing for you.
> 
> Thanks,
> Ian

If I apply this patchset to commit 2a67955de136 ("perf bpf_counter: Fix 
opening of "any"(-1) CPU events"), then:

   $ rm -r ../build/local ; mkdir ../build/local
   $ git clean -xfd
   $ make -C tools/perf O=../build/local/ -j1 V=1

I get no copy of the test jsons:

   $ ls ../build/local/pmu-events/arch/test/test_soc/

   ls: cannot access '../build/local/pmu-events/arch/test/test_soc/': No
   such file or directory

Looking at the dependencies of the rule for $(PMU_EVENTS_C), it's 
$(JSON_TEST), but JSON_TEST is the in-source version without the 
$(OUTPUT) prefix. That's already satisfied so it skips the copy.

If I modify the generator for JSON_TEST to include the OUTPUT prefix:

JSON_TEST	=  $(shell [ -d $(JDIR_TEST) ] &   \
	find $(JDIR_TEST) -name '*.json' | sed -e 's|^|$(OUTPUT)|g')

Now I get the copy:

   $ ls ../build/local/pmu-events/arch/test/test_soc/

   total 0
   drwxrwxr-x 1 james  56 Oct 16 11:13 ..
   drwxrwxr-x 1 james 108 Oct 16 11:13 cpu
   drwxrwxr-x 1 james  12 Oct 16 11:13 .
   drwxrwxr-x 1 james  22 Oct 16 11:13 sys

Now the diff check is slightly different/better, but the build still 
fails. Weirdly I don't see this failure on my Arm machine which is where 
I tested the whole set, I only see this failure on x86. Maybe some 
difference in the version of make?

Powered by blists - more mailing lists