Re: REGRESSION linux-next since Nov 16th, perf tool broken (at least on s390)

[Thomas Richter <tmricht@linux.ibm.com>]

On 11/22/23 10:31, Greg KH wrote:
> On Wed, Nov 22, 2023 at 08:38:28AM +0100, Thomas Richter wrote:
>> perf tool fails on linux-next since Nov 16th on s390 when invoking perf on
>> any PMU event, as in
>>
>>   # perf stat -e cycles -- true
>>
>>    Performance counter stats for 'true':
>>
>>      <not supported>      cycles                                                               
>>
>>        0.000598399 seconds time elapsed
>>
>>        0.000055000 seconds user
>>        0.000567000 seconds sys
>>
>>   # perf stat -e pai_ext/NNPA_ALL/ -C0 -- true
>>   event syntax error: 'pai_ext/NNPA_ALL/'
>>                        \___ Bad event or PMU
>>
>>   Unable to find PMU or event on a PMU of 'pai_ext'
>>
>>   Initial error:
>>   event syntax error: 'pai_ext/NNPA_ALL/'
>>                        \___ Cannot find PMU `pai_ext'. Missing kernel support?
>>   #
>>
>> This error is caused by following commit, which got into linux-next around November 15th:
>>
>> commit 652ffc2104ec1f69dd4a46313888c33527145ccf
>> Author: Greg KH <gregkh@linuxfoundation.org>
>> Date:   Mon Jun 12 15:09:09 2023 +0200
>>
>>     perf/core: Fix narrow startup race when creating the perf nr_addr_filters sysfs file
>>
>>     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>     Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>>     Link: https://lkml.kernel.org/r/2023061204-decal-flyable-6090@gregkh
>>
>> This commit adds function pmu_dev_is_visible() as the PMU bus function
>> to make sysfs attribute files visible.
>> On a failing system these sysfs attribute files are exported:
>>
>> #  ll /sys/devices/pai_ext/
>> drwxr-xr-x. 2 root root    0 Nov 21 14:59 events
>> drwxr-xr-x. 2 root root    0 Nov 21 14:59 format
>> lrwxrwxrwx. 1 root root    0 Nov 21 14:43 subsystem -> ../../bus/event_source
>> -rw-r--r--. 1 root root 4096 Nov 21 14:43 uevent
>> #
> 
> /sys/devices/pai_ext/ is a real device?  Why there?  That's a very odd
> location, what type of device is this?

S390 has 5 different PMU devices, all show up at /sys/devices/ directory:
drwxr-xr-x   4 root root 0 Nov 22 14:30 cpum_cf
drwxr-xr-x   4 root root 0 Nov 22 14:30 cpum_cf_diag
drwxr-xr-x   4 root root 0 Nov 22 14:30 cpum_sf
drwxr-xr-x   4 root root 0 Nov 22 14:30 pai_crypto
drwxr-xr-x   4 root root 0 Nov 22 14:30 pai_ext

The cpum_xxx driver hardware for counting and sampling, with its
own memory mapping, device control registers and assembly instructions.
cpumf stands for CPU Measurement Facility.

The directory entry are created when the device is registered
via perf_pmu_register(). This call creates all complete directory
entry for this device. Nothing else done to it.

The pai_xxx are pseudo devices, again with its own memory mapping
and control registers, are  sort of a successor to cpumf.
pai stands for processor assist instrumentation

I have inherited this code many years ago and never encountered
any issue with this approach. I have little knowledge about the
sys device tree and its setup for individual devices.

> 
>> These is no file named 'type'. Other files are also missing.
>> Therefore the perf tool can not access any PMU event defined by this PMU.
>>
>> On a system with proper export of all PMU sysfs attributes (same linux-next kernel):
>> #  ll /sys/devices/pai_crypto/
>> total 0
>> drwxr-xr-x. 2 root root    0 Nov 21 14:44 events
>> drwxr-xr-x. 2 root root    0 Nov 21 14:44 format
>> -r--r--r--. 1 root root 4096 Nov 21 14:44 nr_addr_filters
>> -rw-r--r--. 1 root root 4096 Nov 21 14:44 perf_event_mux_interval_ms
>> lrwxrwxrwx. 1 root root    0 Nov 21 14:43 subsystem -> ../../bus/event_source
>> -r--r--r--. 1 root root 4096 Nov 21 14:44 type
>> -rw-r--r--. 1 root root 4096 Nov 21 14:44 uevent
> 
> Ick, again, a "root" device?  That's not right and needs to be fixed
> first.
> 
>> # cat /sys/devices/pai_crypto/type
>> 12
>> #
>>
>> Also on such a system the perf stat works again:
>> # perf stat -e pai_crypto/CRYPTO_ALL/ -C0 -- true
>>
>>  Performance counter stats for 'CPU(s) 0':
>>
>>                  0      pai_crypto/CRYPTO_ALL/
>>
>>        0.000741258 seconds time elapsed
>>
>> #
>>
>> On s390 there is no need to export sysfs file 'nr_addr_filters', but setting
>> this member pmu::nr_addr_filters to a non-zero value before
>> calling perf_pmu_register() exhibits the old behavior again.
>>
>> In my opinion this breaks previous behavior.
>>
>> Is this intended?
>> How to fix this?
> 
> Looks like no attributes are exported unless nr_addr_filters is present
> in the device, right?  That's the way the patch was written, maybe the
> change is not correct?  I wrote it a while ago, sorry, I can't
> remember...
> 
> thanks,
> 
> greg k-h

I am fine when the indented change was to hide 'nr_addr_filters'
as a single sysfs file. But right now the complete device attribute group
is hidden and the perf tool seems to have trouble with this.

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294