[PATCH v2 0/6] Assume sysfs event names are always lowercase

Ian Rogers posted 6 patches 1 year, 9 months ago
.../sysfs-bus-event_source-devices-events     |   6 +
tools/perf/tests/pmu-events.c                 |   2 +-
tools/perf/tests/pmu.c                        | 444 +++++++++++-------
tools/perf/util/parse-events.c                |   2 +-
tools/perf/util/parse-events.h                |   2 +-
tools/perf/util/pmu.c                         |  97 ++--
tools/perf/util/pmu.h                         |   4 +-
tools/perf/util/pmus.c                        |  16 +-
tools/perf/util/pmus.h                        |   2 +
9 files changed, 377 insertions(+), 198 deletions(-)
[PATCH v2 0/6] Assume sysfs event names are always lowercase
Posted by Ian Rogers 1 year, 9 months ago
By assuming sysfs events are lower case, the case insensitive event
parsing can probe for the existence of a file rather then loading all
events in a directory. When the event is a json event like
inst_retired.any on Intel, this reduces the number of openat calls on
a Tigerlake laptop from 325 down to 255.

v1 sent as an RFC:
    https://lore.kernel.org/lkml/20240413040812.4042051-1-irogers@google.com/

v2: addresses review feedback from Kan Liang, by updating
    documentation and adding tests.

Ian Rogers (6):
  perf test pmu-events: Make it clearer that pmu-events tests json
    events
  perf Document: Capture that sysfs event names must be lower case
  perf test pmu: Refactor format test and exposed test APIs
  perf test pmu: Add an eagerly loaded event test
  perf test pmu: Test all sysfs PMU event names are lowercase
  perf pmu: Assume sysfs events are always lowercase

 .../sysfs-bus-event_source-devices-events     |   6 +
 tools/perf/tests/pmu-events.c                 |   2 +-
 tools/perf/tests/pmu.c                        | 444 +++++++++++-------
 tools/perf/util/parse-events.c                |   2 +-
 tools/perf/util/parse-events.h                |   2 +-
 tools/perf/util/pmu.c                         |  97 ++--
 tools/perf/util/pmu.h                         |   4 +-
 tools/perf/util/pmus.c                        |  16 +-
 tools/perf/util/pmus.h                        |   2 +
 9 files changed, 377 insertions(+), 198 deletions(-)

-- 
2.44.0.769.g3c40516874-goog
Re: [PATCH v2 0/6] Assume sysfs event names are always lowercase
Posted by Thomas Richter 1 year, 9 months ago
On 4/23/24 05:17, Ian Rogers wrote:
> By assuming sysfs events are lower case, the case insensitive event
> parsing can probe for the existence of a file rather then loading all
> events in a directory. When the event is a json event like
> inst_retired.any on Intel, this reduces the number of openat calls on
> a Tigerlake laptop from 325 down to 255.
> 

Ian, sorry for the late reply.
On s390 the events in the sysfs tree are all upper case:

[root@a35lp67 ~]# ls -l /sys/devices/cpum_cf/events/ | head -10
total 0
-r--r--r-- 1 root root 4096 Apr 17 14:47 AES_BLOCKED_CYCLES
-r--r--r-- 1 root root 4096 Apr 17 14:47 AES_BLOCKED_FUNCTIONS
-r--r--r-- 1 root root 4096 Apr 17 14:47 AES_CYCLES
-r--r--r-- 1 root root 4096 Apr 17 14:47 AES_FUNCTIONS
-r--r--r-- 1 root root 4096 Apr 17 14:47 BCD_DFP_EXECUTION_SLOTS
-r--r--r-- 1 root root 4096 Apr 17 14:47 CPU_CYCLES
-r--r--r-- 1 root root 4096 Apr 17 14:47 CRSTE_1MB_WRITES
-r--r--r-- 1 root root 4096 Apr 17 14:47 DCW_OFF_DRAWER
-r--r--r-- 1 root root 4096 Apr 17 14:47 DCW_OFF_DRAWER_MEMORY
[root@a35lp67 ~]# 

Same is true for all other PMUs (currently 5 and growing).

Is there a branch to pull to try out the effect of your patch on s390?

Thanks Thomas
-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH v2 0/6] Assume sysfs event names are always lowercase
Posted by Ian Rogers 1 year, 9 months ago
On Mon, Apr 22, 2024 at 11:53 PM Thomas Richter <tmricht@linux.ibm.com> wrote:
>
> On 4/23/24 05:17, Ian Rogers wrote:
> > By assuming sysfs events are lower case, the case insensitive event
> > parsing can probe for the existence of a file rather then loading all
> > events in a directory. When the event is a json event like
> > inst_retired.any on Intel, this reduces the number of openat calls on
> > a Tigerlake laptop from 325 down to 255.
> >
>
> Ian, sorry for the late reply.
> On s390 the events in the sysfs tree are all upper case:

Yikes, sigh.. thanks for catching this :-)

> [root@a35lp67 ~]# ls -l /sys/devices/cpum_cf/events/ | head -10
> total 0
> -r--r--r-- 1 root root 4096 Apr 17 14:47 AES_BLOCKED_CYCLES
> -r--r--r-- 1 root root 4096 Apr 17 14:47 AES_BLOCKED_FUNCTIONS
> -r--r--r-- 1 root root 4096 Apr 17 14:47 AES_CYCLES
> -r--r--r-- 1 root root 4096 Apr 17 14:47 AES_FUNCTIONS
> -r--r--r-- 1 root root 4096 Apr 17 14:47 BCD_DFP_EXECUTION_SLOTS
> -r--r--r-- 1 root root 4096 Apr 17 14:47 CPU_CYCLES
> -r--r--r-- 1 root root 4096 Apr 17 14:47 CRSTE_1MB_WRITES
> -r--r--r-- 1 root root 4096 Apr 17 14:47 DCW_OFF_DRAWER
> -r--r--r-- 1 root root 4096 Apr 17 14:47 DCW_OFF_DRAWER_MEMORY
> [root@a35lp67 ~]#
>
> Same is true for all other PMUs (currently 5 and growing).
>
> Is there a branch to pull to try out the effect of your patch on s390?

Sorry not, I think everything would break anyway. I'll extend what
I've done so that event names can be lowercase or in shouty UPPERCASE.

Thanks,
Ian

> Thanks Thomas
> --
> Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
> --
> IBM Deutschland Research & Development GmbH
>
> Vorsitzender des Aufsichtsrats: Wolfgang Wendt
>
> Geschäftsführung: David Faller
>
> Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
>