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

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

[Thomas Richter]

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
#

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
# 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?

Thanks for your help.



-- 
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

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

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 3378 bytes --]

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
> #
> 
> 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
> # 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.
> 

Thanks for the regression report. I'm adding it to regzbot:

#regzbot ^introduced: 652ffc2104ec1f
#regzbot title: event syntax error due to narrow startup race fix

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Greg KH]

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?

> 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

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

[Peter Zijlstra]

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:

Does this help? I'm not entirely sure on the is_visible thing, but the
way I understand it, it was only supposed to hide the nr_filters thing,
not the entire group and @n helps select the attribute.

Greg, did I get that right?

Also, spurious return... clearly I should've woken up before merging
that patch... oh well.

---

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 4f0c45ab8d7d..59b332cce9e7 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11417,12 +11417,10 @@ static umode_t pmu_dev_is_visible(struct kobject *kobj, struct attribute *a, int
 	struct device *dev = kobj_to_dev(kobj);
 	struct pmu *pmu = dev_get_drvdata(dev);
 
-	if (!pmu->nr_addr_filters)
+	if (n == 2 && !pmu->nr_addr_filters)
 		return 0;
 
 	return a->mode;
-
-	return 0;
 }
 
 static struct attribute_group pmu_dev_attr_group = {

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

[Thomas Richter]

On 11/22/23 11:07, Peter Zijlstra 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:
> 
> Does this help? I'm not entirely sure on the is_visible thing, but the
> way I understand it, it was only supposed to hide the nr_filters thing,
> not the entire group and @n helps select the attribute.
> 
> Greg, did I get that right?

Yes, the is_visible call back function hides whole group, not just this
one attribute.

static struct attribute *pmu_dev_attrs[] = {    <--- This group controlled by is_visibly                              
        &dev_attr_type.attr,                                                  
        &dev_attr_perf_event_mux_interval_ms.attr,                            
        &dev_attr_nr_addr_filters.attr,                                       
        NULL,                                                                 
};    
static struct attribute_group pmu_dev_attr_group = {                          
        .is_visible = pmu_dev_is_visible,                                     
        .attrs = pmu_dev_attrs,                                               
};

So is_visible call back hides whole pmu_dev_attrs group with all 3 files.

> 
> Also, spurious return... clearly I should've woken up before merging
> that patch... oh well.
> 
> 

-- 
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

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

[Peter Zijlstra]

On Wed, Nov 22, 2023 at 03:38:36PM +0100, Thomas Richter wrote:
> On 11/22/23 11:07, Peter Zijlstra 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:
> > 
> > Does this help? I'm not entirely sure on the is_visible thing, but the
> > way I understand it, it was only supposed to hide the nr_filters thing,
> > not the entire group and @n helps select the attribute.
> > 
> > Greg, did I get that right?
> 
> Yes, the is_visible call back function hides whole group, not just this
> one attribute.
> 
> static struct attribute *pmu_dev_attrs[] = {    <--- This group controlled by is_visibly                              
>         &dev_attr_type.attr,                                                  
>         &dev_attr_perf_event_mux_interval_ms.attr,                            
>         &dev_attr_nr_addr_filters.attr,                                       
>         NULL,                                                                 
> };    
> static struct attribute_group pmu_dev_attr_group = {                          
>         .is_visible = pmu_dev_is_visible,                                     
>         .attrs = pmu_dev_attrs,                                               
> };
> 
> So is_visible call back hides whole pmu_dev_attrs group with all 3 files.

Right, so patchlet I send should only result in hiding n==2, which is
that nr_addr_filters thing IIUC.

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

[Thomas Richter]

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

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

[Greg KH]

On Thu, Nov 23, 2023 at 08:54:19AM +0100, Thomas Richter wrote:
> 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

And that's messed up, we need to put them in the "proper" place, in
either a bus-specific directory, or somewhere lower, they shouldn't be
in the "root" of the device hierarchy which is what I think started this
whole conversation a while ago.

I'll work on that as a different patch...

thanks,

greg k-h

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

[Thomas Richter]

On 11/22/23 11:07, Peter Zijlstra 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:
> 
> Does this help? I'm not entirely sure on the is_visible thing, but the
> way I understand it, it was only supposed to hide the nr_filters thing,
> not the entire group and @n helps select the attribute.
> 
> Greg, did I get that right?
> 
> Also, spurious return... clearly I should've woken up before merging
> that patch... oh well.
> 
> ---
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 4f0c45ab8d7d..59b332cce9e7 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -11417,12 +11417,10 @@ static umode_t pmu_dev_is_visible(struct kobject *kobj, struct attribute *a, int
>  	struct device *dev = kobj_to_dev(kobj);
>  	struct pmu *pmu = dev_get_drvdata(dev);
>  
> -	if (!pmu->nr_addr_filters)
> +	if (n == 2 && !pmu->nr_addr_filters)
>  		return 0;
>  
>  	return a->mode;
> -
> -	return 0;
>  }
>  
>  static struct attribute_group pmu_dev_attr_group = {

Yep, this fixes the issue. As you can see, the type file etc is exported again:


# ll /sys/devices/pai_crypto/
total 0
drwxr-xr-x. 2 root root    0 Nov 23 12:19 events
drwxr-xr-x. 2 root root    0 Nov 23 12:19 format
-rw-r--r--. 1 root root 4096 Nov 23 12:19 perf_event_mux_interval_ms
lrwxrwxrwx. 1 root root    0 Nov 23 12:18 subsystem -> ../../bus/event_source
-r--r--r--. 1 root root 4096 Nov 23 12:19 type
-rw-r--r--. 1 root root 4096 Nov 23 12:19 uevent
# perf stat -a -e pai_crypto/KM_DEA/ -- true

 Performance counter stats for 'system wide':

                 0      pai_crypto/KM_DEA/                                                    

       0.000983239 seconds time elapsed
#

This looks very and and you have my 
Tested-by: Thomas Richter <tmricht@linux.ibm.com

--
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