Message-Id: <20230331202949.810326-1-namhyung@kernel.org>
Date:   Fri, 31 Mar 2023 13:29:40 -0700
From:   Namhyung Kim <namhyung@...nel.org>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jiri Olsa <jolsa@...nel.org>
Cc:     Ian Rogers <irogers@...gle.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-perf-users@...r.kernel.org,
        Kan Liang <kan.liang@...ux.intel.com>,
        Leo Yan <leo.yan@...aro.org>
Subject: [PATCHSET 0/9] perf tools: Update pmu scan using openat() (v1)

Hello,

This patchset changes PMU info scanning on sysfs using openat()
basically.  I got reports of occasional contention on the
opening files in sysfs.  While the root cause was a separate
issue, I discovered some inefficiencies in the perf code.

To scan PMUs, it roughly does something like below:

  dir = opendir("/sys/bus/event_source/devices");
  while (dentry = readdir(dir)) {
    char buf[PATH_MAX];

    snprintf(buf, sizeof(buf), "%s/%s",
             "/sys/bus/event_source/devices", dentry->d_name);
    fd = open(buf, O_RDONLY);
    ...
  }

But this is not good since it needs to copy the string to build the
absolute pathname, and it makes redundant pathname walk (from the /sys)
in the kernel unnecessarily.  We can use openat(2) to open the file in
the given directory.

Add a couple of new helper to return the file descriptor of PMU
directory so that it can use it with relative paths.

 * perf_pmu__event_source_devices_fd()
   - returns a fd for the PMU root ("/sys/bus/event_source/devices")

 * perf_pmu__pathname_fd()
   - returns a fd for "<pmu>/<file>" under the PMU root

Now the above code can be converted something like below:

  dirfd = perf_pmu__event_source_devices_fd();
  dir = fdopendir(dirfd);
  while (dentry = readdir(dir)) {
    fd = openat(dirfd, dentry->d_name, O_RDONLY);
    ...
  }

I added a benchmark for pmu-scan and it showed a slight speedup
in the normal case too.

  $ ./perf.old bench internals pmu-scan
  # Running 'internals/pmu-scan' benchmark:
  Computing performance of sysfs PMU event scan for 100 times
    Average PMU scanning took: 6670.970 usec (+- 13.022 usec)

  $ ./perf.new bench internals pmu-scan
  # Running 'internals/pmu-scan' benchmark:
  Computing performance of sysfs PMU event scan for 100 times
    Average PMU scanning took: 6296.980 usec (+- 14.891 usec)

The 5~6% of improvement might be small but it may have bigger impact
when the system is contended.

You can get the code from 'perf/pmu-scan-v1' branch in

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks,
Namhyung

Namhyung Kim (9):
  perf list: Use relative path for tracepoint scan
  perf tools: Fix a asan issue in parse_events_multi_pmu_add()
  perf pmu: Add perf_pmu__destroy() function
  perf bench: Add pmu-scan benchmark
  perf pmu: Use relative path for sysfs scan
  perf pmu: Use relative path in perf_pmu__caps_parse()
  perf pmu: Use relative path in setup_pmu_alias_list()
  perf pmu: Add perf_pmu__{open,scan}_file_at()
  perf intel-pt: Use perf_pmu__scan_file_at() if possible

 tools/perf/arch/x86/util/intel-pt.c |  52 ++++--
 tools/perf/arch/x86/util/pmu.c      |  13 +-
 tools/perf/bench/Build              |   1 +
 tools/perf/bench/bench.h            |   1 +
 tools/perf/bench/pmu-scan.c         | 184 ++++++++++++++++++
 tools/perf/builtin-bench.c          |   1 +
 tools/perf/tests/pmu.c              |   9 +-
 tools/perf/util/parse-events.c      |   2 +-
 tools/perf/util/pmu.c               | 278 ++++++++++++++++++++--------
 tools/perf/util/pmu.h               |  12 +-
 tools/perf/util/print-events.c      |  26 ++-
 11 files changed, 466 insertions(+), 113 deletions(-)
 create mode 100644 tools/perf/bench/pmu-scan.c


base-commit: 417c6adfb155f906f0441cc1034827f6e2b3c372
-- 
2.40.0.348.gf938b09366-goog

Powered by blists - more mailing lists