2024-05-02 21:35:50

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v5 2/6] perf Document: Sysfs event names must be lower or upper case

To avoid directory scans in perf it is going to be assumed that sysfs
event names are either lower or upper case.

Signed-off-by: Ian Rogers <[email protected]>
Reviewed-by: Kan Liang <[email protected]>
---
.../ABI/testing/sysfs-bus-event_source-devices-events | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
index 77de58d03822..e7efeab2ee83 100644
--- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
+++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
@@ -37,6 +37,12 @@ Description: Per-pmu performance monitoring events specific to the running syste
performance monitoring event supported by the <pmu>. The name
of the file is the name of the event.

+ As performance monitoring event names are case
+ insensitive in the perf tool, the perf tool only looks
+ for lower or upper case event names in sysfs to avoid
+ scanning the directory. It is therefore required the
+ name of the event here is either lower or upper case.
+
File contents:

<term>[=<value>][,<term>[=<value>]]...
--
2.45.0.rc1.225.g2a3ae87e7f-goog



2024-05-12 22:08:48

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v5 2/6] perf Document: Sysfs event names must be lower or upper case

Hi,

On 5/2/24 2:35 PM, Ian Rogers wrote:
> To avoid directory scans in perf it is going to be assumed that sysfs
> event names are either lower or upper case.
>
> Signed-off-by: Ian Rogers <[email protected]>
> Reviewed-by: Kan Liang <[email protected]>
> ---
> .../ABI/testing/sysfs-bus-event_source-devices-events | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> index 77de58d03822..e7efeab2ee83 100644
> --- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> +++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> @@ -37,6 +37,12 @@ Description: Per-pmu performance monitoring events specific to the running syste
> performance monitoring event supported by the <pmu>. The name
> of the file is the name of the event.
>
> + As performance monitoring event names are case
> + insensitive in the perf tool, the perf tool only looks
> + for lower or upper case event names in sysfs to avoid
> + scanning the directory. It is therefore required the
> + name of the event here is either lower or upper case.
> +

This is ambiguous to me. Is it clear to everyone else?

"for lower or upper case event names":

Is that a logical OR or an exclusive OR?
"AbC" contains lower case or upper case characters. :)

I think the code [static bool permitted_event_name()]
implements an exclusive OR.
The code also allows event names to contain numbers AFAICT.
The documentation doesn't mention this.

HTH.

> File contents:
>
> <term>[=<value>][,<term>[=<value>]]...

--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html

2024-05-13 16:24:04

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH v5 2/6] perf Document: Sysfs event names must be lower or upper case

On Sun, May 12, 2024 at 3:08 PM Randy Dunlap <[email protected]> wrote:
>
> Hi,
>
> On 5/2/24 2:35 PM, Ian Rogers wrote:
> > To avoid directory scans in perf it is going to be assumed that sysfs
> > event names are either lower or upper case.
> >
> > Signed-off-by: Ian Rogers <[email protected]>
> > Reviewed-by: Kan Liang <[email protected]>
> > ---
> > .../ABI/testing/sysfs-bus-event_source-devices-events | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> > index 77de58d03822..e7efeab2ee83 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> > +++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> > @@ -37,6 +37,12 @@ Description: Per-pmu performance monitoring events specific to the running syste
> > performance monitoring event supported by the <pmu>. The name
> > of the file is the name of the event.
> >
> > + As performance monitoring event names are case
> > + insensitive in the perf tool, the perf tool only looks
> > + for lower or upper case event names in sysfs to avoid
> > + scanning the directory. It is therefore required the
> > + name of the event here is either lower or upper case.
> > +
>
> This is ambiguous to me. Is it clear to everyone else?
>
> "for lower or upper case event names":
>
> Is that a logical OR or an exclusive OR?
> "AbC" contains lower case or upper case characters. :)
>
> I think the code [static bool permitted_event_name()]
> implements an exclusive OR.
> The code also allows event names to contain numbers AFAICT.
> The documentation doesn't mention this.
>
> HTH.
>
> > File contents:
> >
> > <term>[=<value>][,<term>[=<value>]]...

Sorry, this reads like a troll. Assuming good intentions, could you
propose a fix in the form of a patch? When a word is made upper or
lower case I believe it is generally assumed the operation applies to
all characters within the word, but I'm happy to see ambiguity cleared
up.

Thanks,
Ian

> --
> #Randy
> https://people.kernel.org/tglx/notes-about-netiquette
> https://subspace.kernel.org/etiquette.html

2024-05-13 18:54:51

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v5 2/6] perf Document: Sysfs event names must be lower or upper case



On 5/13/24 9:22 AM, Ian Rogers wrote:
> On Sun, May 12, 2024 at 3:08 PM Randy Dunlap <[email protected]> wrote:
>>
>> Hi,
>>
>> On 5/2/24 2:35 PM, Ian Rogers wrote:
>>> To avoid directory scans in perf it is going to be assumed that sysfs
>>> event names are either lower or upper case.
>>>
>>> Signed-off-by: Ian Rogers <[email protected]>
>>> Reviewed-by: Kan Liang <[email protected]>
>>> ---
>>> .../ABI/testing/sysfs-bus-event_source-devices-events | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
>>> index 77de58d03822..e7efeab2ee83 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
>>> +++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
>>> @@ -37,6 +37,12 @@ Description: Per-pmu performance monitoring events specific to the running syste
>>> performance monitoring event supported by the <pmu>. The name
>>> of the file is the name of the event.
>>>
>>> + As performance monitoring event names are case
>>> + insensitive in the perf tool, the perf tool only looks
>>> + for lower or upper case event names in sysfs to avoid
>>> + scanning the directory. It is therefore required the
>>> + name of the event here is either lower or upper case.
>>> +
>>
>> This is ambiguous to me. Is it clear to everyone else?
>>
>> "for lower or upper case event names":
>>
>> Is that a logical OR or an exclusive OR?
>> "AbC" contains lower case or upper case characters. :)
>>
>> I think the code [static bool permitted_event_name()]
>> implements an exclusive OR.
>> The code also allows event names to contain numbers AFAICT.
>> The documentation doesn't mention this.
>>
>> HTH.
>>
>>> File contents:
>>>
>>> <term>[=<value>][,<term>[=<value>]]...
>
> Sorry, this reads like a troll. Assuming good intentions, could you
> propose a fix in the form of a patch? When a word is made upper or

Sure, I'll send a patch.
No trolling here.

> lower case I believe it is generally assumed the operation applies to
> all characters within the word, but I'm happy to see ambiguity cleared
> up.
>
> Thanks,
> Ian


--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html