Messages in this thread | | | From | Ian Rogers <> | Date | Mon, 3 Apr 2023 10:28:28 -0700 | Subject | Re: [PATCHSET 0/9] perf tools: Update pmu scan using openat() (v1) |
| |
On Fri, Mar 31, 2023 at 1:29 PM Namhyung Kim <namhyung@kernel.org> wrote: > > 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
Some nice stack size savings too. The reentrant fix was pre-existing. Series: Acked-by: Ian Rogers <irogers@google.com>
Thanks, Ian
> 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 >
| |