Re: [PATCH v7 00/27] Legacy hardware/cache events as json

[James Clark <james.clark@linaro.org>]

On 16/10/2025 11:29 am, James Clark wrote:
> 
> 
> On 15/10/2025 10:24 pm, Ian Rogers wrote:
>> On Wed, Oct 15, 2025 at 10:39 AM James Clark <james.clark@linaro.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?
> 

Turned out to be an extra json file I had in the Perf tree for my 
compilation database, so that explains why I was the only person seeing 
it. Sent a fix.