1. Patchew
  2. linux
  3. [PATCH v5 0/6] Assume sysfs event names are always the same case
  4. View patch

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

Ian Rogers posted 6 patches 1 year, 7 months ago
[PATCH v5 2/6] perf Document: Sysfs event names must be lower or upper case
Posted by Ian Rogers 1 year, 7 months ago 
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 <irogers@google.com>
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
---
 .../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
Re: [PATCH v5 2/6] perf Document: Sysfs event names must be lower or upper case
Posted by Randy Dunlap 1 year, 7 months ago 
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 <irogers@google.com>
> Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
> ---
>  .../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
Re: [PATCH v5 2/6] perf Document: Sysfs event names must be lower or upper case
Posted by Ian Rogers 1 year, 7 months ago 
On Sun, May 12, 2024 at 3:08 PM Randy Dunlap <rdunlap@infradead.org> 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 <irogers@google.com>
> > Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
> > ---
> >  .../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
Re: [PATCH v5 2/6] perf Document: Sysfs event names must be lower or upper case
Posted by Randy Dunlap 1 year, 7 months ago 

On 5/13/24 9:22 AM, Ian Rogers wrote:
> On Sun, May 12, 2024 at 3:08 PM Randy Dunlap <rdunlap@infradead.org> 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 <irogers@google.com>
>>> Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
>>> ---
>>>  .../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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.