[PATCH v7 0/4] libperf userspace counter access

[PATCH v7 0/4] libperf userspace counter access

[Rob Herring]

I'm resending just the libperf userspace counter access without the Arm 
bits so hopefully it can be picked up for 5.13. The Arm bits seem to be 
a never ending review filled with long periods of silence. :(

Prior versions are here[1][2][3][4][5][6][7].

Rob

[1] https://lore.kernel.org/r/20190822144220.27860-1-raphael.gault@arm.com/
[2] https://lore.kernel.org/r/20200707205333.624938-1-robh@kernel.org/
[3] https://lore.kernel.org/r/20200828205614.3391252-1-robh@kernel.org/
[4] https://lore.kernel.org/r/20200911215118.2887710-1-robh@kernel.org/  
[5] https://lore.kernel.org/r/20201001140116.651970-1-robh@kernel.org/
[6] https://lore.kernel.org/r/20210114020605.3943992-1-robh@kernel.org/
[7] https://lore.kernel.org/r/20210311000837.3630499-1-robh@kernel.org/

Rob Herring (4):
  tools/include: Add an initial math64.h
  libperf: Add evsel mmap support
  libperf: tests: Add support for verbose printing
  libperf: Add support for user space counter access

 tools/include/linux/math64.h             | 75 ++++++++++++++++++++
 tools/lib/perf/Documentation/libperf.txt |  2 +
 tools/lib/perf/evsel.c                   | 58 ++++++++++++++++
 tools/lib/perf/include/internal/evsel.h  |  1 +
 tools/lib/perf/include/internal/mmap.h   |  3 +
 tools/lib/perf/include/internal/tests.h  | 32 +++++++++
 tools/lib/perf/include/perf/evsel.h      |  2 +
 tools/lib/perf/libperf.map               |  2 +
 tools/lib/perf/mmap.c                    | 88 ++++++++++++++++++++++++
 tools/lib/perf/tests/Makefile            |  6 +-
 tools/lib/perf/tests/test-evsel.c        | 65 +++++++++++++++++
 11 files changed, 332 insertions(+), 2 deletions(-)
 create mode 100644 tools/include/linux/math64.h

-- 
2.27.0

[PATCH v7 1/4] tools/include: Add an initial math64.h

[Rob Herring]

Add an initial math64.h similar to linux/math64.h with functions
mul_u64_u64_div64() and mul_u64_u32_shr(). This isn't a direct copy of
include/linux/math64.h as that doesn't define mul_u64_u64_div64().

Implementation was written by Peter Zilkstra based on linux/math64.h
and div64.h[1]. The original implementation was not optimal on arm64 as
__int128 division is not optimal with a call out to __udivti3, so I
dropped the __int128 variant of mul_u64_u64_div64().

[1] https://lore.kernel.org/lkml/20200322101848.GF2452@worktop.programming.kicks-ass.net/

Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 tools/include/linux/math64.h | 75 ++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 tools/include/linux/math64.h

diff --git a/tools/include/linux/math64.h b/tools/include/linux/math64.h
new file mode 100644
index 000000000000..4ad45d5943dc
--- /dev/null
+++ b/tools/include/linux/math64.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_MATH64_H
+#define _LINUX_MATH64_H
+
+#include <linux/types.h>
+
+#ifdef __x86_64__
+static inline u64 mul_u64_u64_div64(u64 a, u64 b, u64 c)
+{
+	u64 q;
+
+	asm ("mulq %2; divq %3" : "=a" (q)
+				: "a" (a), "rm" (b), "rm" (c)
+				: "rdx");
+
+	return q;
+}
+#define mul_u64_u64_div64 mul_u64_u64_div64
+#endif
+
+#ifdef __SIZEOF_INT128__
+static inline u64 mul_u64_u32_shr(u64 a, u32 b, unsigned int shift)
+{
+	return (u64)(((unsigned __int128)a * b) >> shift);
+}
+
+#else
+
+#ifdef __i386__
+static inline u64 mul_u32_u32(u32 a, u32 b)
+{
+	u32 high, low;
+
+	asm ("mull %[b]" : "=a" (low), "=d" (high)
+			 : [a] "a" (a), [b] "rm" (b) );
+
+	return low | ((u64)high) << 32;
+}
+#else
+static inline u64 mul_u32_u32(u32 a, u32 b)
+{
+	return (u64)a * b;
+}
+#endif
+
+static inline u64 mul_u64_u32_shr(u64 a, u32 b, unsigned int shift)
+{
+	u32 ah, al;
+	u64 ret;
+
+	al = a;
+	ah = a >> 32;
+
+	ret = mul_u32_u32(al, b) >> shift;
+	if (ah)
+		ret += mul_u32_u32(ah, b) << (32 - shift);
+
+	return ret;
+}
+
+#endif	/* __SIZEOF_INT128__ */
+
+#ifndef mul_u64_u64_div64
+static inline u64 mul_u64_u64_div64(u64 a, u64 b, u64 c)
+{
+	u64 quot, rem;
+
+	quot = a / c;
+	rem = a % c;
+
+	return quot * b + (rem * b) / c;
+}
+#endif
+
+#endif /* _LINUX_MATH64_H */
-- 
2.27.0

[PATCH v7 2/4] libperf: Add evsel mmap support

[Rob Herring]

In order to support usersapce access, an event must be mmapped. While
there's already mmap support for evlist, the usecase is a bit different
than the self monitoring with userspace access. So let's add a new
perf_evsel__mmap() function to mmap an evsel. This allows implementing
userspace access as a fastpath for perf_evsel__read().

The mmapped address is returned by perf_evsel__mmap_base() which
primarily for users/tests to check if userspace access is enabled.

Signed-off-by: Rob Herring <robh@kernel.org>
---
v7:
 - Add NULL fd check to perf_evsel__mmap
v6:
 - split mmap struct into it's own xyarray
v5:
 - Create an mmap for every underlying event opened. Due to this, we
   need a different way to get the mmap ptr, so perf_evsel__mmap_base()
   is introduced.
v4:
 - Change perf_evsel__mmap size to pages instead of bytes
v3:
 - New patch split out from user access patch
---
 tools/lib/perf/Documentation/libperf.txt |  2 +
 tools/lib/perf/evsel.c                   | 54 ++++++++++++++++++++++++
 tools/lib/perf/include/internal/evsel.h  |  1 +
 tools/lib/perf/include/perf/evsel.h      |  2 +
 tools/lib/perf/libperf.map               |  2 +
 5 files changed, 61 insertions(+)

diff --git a/tools/lib/perf/Documentation/libperf.txt b/tools/lib/perf/Documentation/libperf.txt
index 0c74c30ed23a..a2c73df191ca 100644
--- a/tools/lib/perf/Documentation/libperf.txt
+++ b/tools/lib/perf/Documentation/libperf.txt
@@ -136,6 +136,8 @@ SYNOPSIS
                        struct perf_thread_map *threads);
   void perf_evsel__close(struct perf_evsel *evsel);
   void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
+  int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
+  void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
   int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
                        struct perf_counts_values *count);
   int perf_evsel__enable(struct perf_evsel *evsel);
diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index 4dc06289f4c7..7e140763552f 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -11,10 +11,12 @@
 #include <stdlib.h>
 #include <internal/xyarray.h>
 #include <internal/cpumap.h>
+#include <internal/mmap.h>
 #include <internal/threadmap.h>
 #include <internal/lib.h>
 #include <linux/string.h>
 #include <sys/ioctl.h>
+#include <sys/mman.h>
 
 void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr)
 {
@@ -38,6 +40,7 @@ void perf_evsel__delete(struct perf_evsel *evsel)
 }
 
 #define FD(e, x, y) (*(int *) xyarray__entry(e->fd, x, y))
+#define MMAP(e, x, y) (e->mmap ? ((struct perf_mmap *) xyarray__entry(e->mmap, x, y)) : NULL)
 
 int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
 {
@@ -55,6 +58,13 @@ int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
 	return evsel->fd != NULL ? 0 : -ENOMEM;
 }
 
+static int perf_evsel__alloc_mmap(struct perf_evsel *evsel, int ncpus, int nthreads)
+{
+	evsel->mmap = xyarray__new(ncpus, nthreads, sizeof(struct perf_mmap));
+
+	return evsel->mmap != NULL ? 0 : -ENOMEM;
+}
+
 static int
 sys_perf_event_open(struct perf_event_attr *attr,
 		    pid_t pid, int cpu, int group_fd,
@@ -137,6 +147,8 @@ void perf_evsel__free_fd(struct perf_evsel *evsel)
 {
 	xyarray__delete(evsel->fd);
 	evsel->fd = NULL;
+	xyarray__delete(evsel->mmap);
+	evsel->mmap = NULL;
 }
 
 void perf_evsel__close(struct perf_evsel *evsel)
@@ -156,6 +168,48 @@ void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu)
 	perf_evsel__close_fd_cpu(evsel, cpu);
 }
 
+int perf_evsel__mmap(struct perf_evsel *evsel, int pages)
+{
+	int ret, cpu, thread;
+	struct perf_mmap_param mp = {
+		.prot = PROT_READ | PROT_WRITE,
+		.mask = (pages * page_size) - 1,
+	};
+
+	if (evsel->fd == NULL)
+		return -EINVAL;
+
+	if (evsel->mmap == NULL &&
+	    perf_evsel__alloc_mmap(evsel, xyarray__max_x(evsel->fd), xyarray__max_y(evsel->fd)) < 0)
+		return -ENOMEM;
+
+	for (cpu = 0; cpu < xyarray__max_x(evsel->fd); cpu++) {
+		for (thread = 0; thread < xyarray__max_y(evsel->fd); thread++) {
+			int fd = FD(evsel, cpu, thread);
+			struct perf_mmap *map = MMAP(evsel, cpu, thread);
+
+			if (fd < 0)
+				continue;
+
+			perf_mmap__init(map, NULL, false, NULL);
+
+			ret = perf_mmap__mmap(map, &mp, fd, cpu);
+			if (ret)
+				return -1;
+		}
+	}
+
+	return 0;
+}
+
+void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
+{
+	if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
+		return NULL;
+
+	return MMAP(evsel, cpu, thread)->base;
+}
+
 int perf_evsel__read_size(struct perf_evsel *evsel)
 {
 	u64 read_format = evsel->attr.read_format;
diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
index 1ffd083b235e..1c067d088bc6 100644
--- a/tools/lib/perf/include/internal/evsel.h
+++ b/tools/lib/perf/include/internal/evsel.h
@@ -41,6 +41,7 @@ struct perf_evsel {
 	struct perf_cpu_map	*own_cpus;
 	struct perf_thread_map	*threads;
 	struct xyarray		*fd;
+	struct xyarray		*mmap;
 	struct xyarray		*sample_id;
 	u64			*id;
 	u32			 ids;
diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
index c82ec39a4ad0..9f5265f2f39f 100644
--- a/tools/lib/perf/include/perf/evsel.h
+++ b/tools/lib/perf/include/perf/evsel.h
@@ -27,6 +27,8 @@ LIBPERF_API int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *
 				 struct perf_thread_map *threads);
 LIBPERF_API void perf_evsel__close(struct perf_evsel *evsel);
 LIBPERF_API void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
+LIBPERF_API int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
+LIBPERF_API void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
 LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
 				 struct perf_counts_values *count);
 LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
index 7be1af8a546c..0b993de15830 100644
--- a/tools/lib/perf/libperf.map
+++ b/tools/lib/perf/libperf.map
@@ -23,6 +23,8 @@ LIBPERF_0.0.1 {
 		perf_evsel__disable;
 		perf_evsel__open;
 		perf_evsel__close;
+		perf_evsel__mmap;
+		perf_evsel__mmap_base;
 		perf_evsel__read;
 		perf_evsel__cpus;
 		perf_evsel__threads;
-- 
2.27.0

[PATCH v7 3/4] libperf: tests: Add support for verbose printing

[Rob Herring]

Add __T_VERBOSE() so tests can add verbose output. The verbose output is
enabled with the '-v' command line option.

Signed-off-by: Rob Herring <robh@kernel.org>
---
v5:
 - Pass verbose flag to static tests
 - Fix getopt loop with unsigned char (arm64)
v3:
 - New patch
---
 tools/lib/perf/include/internal/tests.h | 32 +++++++++++++++++++++++++
 tools/lib/perf/tests/Makefile           |  6 +++--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/tools/lib/perf/include/internal/tests.h b/tools/lib/perf/include/internal/tests.h
index 2093e8868a67..29425c2dabe1 100644
--- a/tools/lib/perf/include/internal/tests.h
+++ b/tools/lib/perf/include/internal/tests.h
@@ -3,11 +3,32 @@
 #define __LIBPERF_INTERNAL_TESTS_H
 
 #include <stdio.h>
+#include <unistd.h>
 
 int tests_failed;
+int tests_verbose;
+
+static inline int get_verbose(char **argv, int argc)
+{
+	int c;
+	int verbose = 0;
+
+	while ((c = getopt(argc, argv, "v")) != -1) {
+		switch (c)
+		{
+		case 'v':
+			verbose = 1;
+			break;
+		default:
+			break;
+		}
+	}
+	return verbose;
+}
 
 #define __T_START					\
 do {							\
+	tests_verbose = get_verbose(argv, argc);	\
 	fprintf(stdout, "- running %s...", __FILE__);	\
 	fflush(NULL);					\
 	tests_failed = 0;				\
@@ -30,4 +51,15 @@ do {
 	}                                                                        \
 } while (0)
 
+#define __T_VERBOSE(...)						\
+do {									\
+	if (tests_verbose) {						\
+		if (tests_verbose == 1) {				\
+			fputc('\n', stderr);				\
+			tests_verbose++;				\
+		}							\
+		fprintf(stderr, ##__VA_ARGS__);				\
+	}								\
+} while (0)
+
 #endif /* __LIBPERF_INTERNAL_TESTS_H */
diff --git a/tools/lib/perf/tests/Makefile b/tools/lib/perf/tests/Makefile
index 96841775feaf..b536cc9a26dd 100644
--- a/tools/lib/perf/tests/Makefile
+++ b/tools/lib/perf/tests/Makefile
@@ -5,6 +5,8 @@ TESTS = test-cpumap test-threadmap test-evlist test-evsel
 TESTS_SO := $(addsuffix -so,$(TESTS))
 TESTS_A  := $(addsuffix -a,$(TESTS))
 
+TEST_ARGS := $(if $(V),-v)
+
 # Set compile option CFLAGS
 ifdef EXTRA_CFLAGS
   CFLAGS := $(EXTRA_CFLAGS)
@@ -28,9 +30,9 @@ all: $(TESTS_A) $(TESTS_SO)
 
 run:
 	@echo "running static:"
-	@for i in $(TESTS_A); do ./$$i; done
+	@for i in $(TESTS_A); do ./$$i $(TEST_ARGS); done
 	@echo "running dynamic:"
-	@for i in $(TESTS_SO); do LD_LIBRARY_PATH=../ ./$$i; done
+	@for i in $(TESTS_SO); do LD_LIBRARY_PATH=../ ./$$i $(TEST_ARGS); done
 
 clean:
 	$(call QUIET_CLEAN, tests)$(RM) $(TESTS_A) $(TESTS_SO)
-- 
2.27.0

[PATCH v7 4/4] libperf: Add support for user space counter access

[Rob Herring]

x86 and arm64 can both support direct access of event counters in
userspace. The access sequence is less than trivial and currently exists
in perf test code (tools/perf/arch/x86/tests/rdpmc.c) with copies in
projects such as PAPI and libpfm4.

In order to support usersapce access, an event must be mmapped first
with perf_evsel__mmap(). Then subsequent calls to perf_evsel__read()
will use the fast path (assuming the arch supports it).

Signed-off-by: Rob Herring <robh@kernel.org>
---
v6:
 - Adapt to mmap changes adding MMAP NULL check
v5:
 - Make raw count s64 instead of u64 so that counter width shifting
   works
 - Adapt to mmap changes
v4:
 - Update perf_evsel__mmap size to pages
v3:
 - Split out perf_evsel__mmap() to separate patch
---
 tools/lib/perf/evsel.c                 |  4 ++
 tools/lib/perf/include/internal/mmap.h |  3 +
 tools/lib/perf/mmap.c                  | 88 ++++++++++++++++++++++++++
 tools/lib/perf/tests/test-evsel.c      | 65 +++++++++++++++++++
 4 files changed, 160 insertions(+)

diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index 7e140763552f..f0543244635c 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -245,6 +245,10 @@ int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
 	if (FD(evsel, cpu, thread) < 0)
 		return -EINVAL;
 
+	if (MMAP(evsel, cpu, thread) &&
+	    !perf_mmap__read_self(MMAP(evsel, cpu, thread), count))
+		return 0;
+
 	if (readn(FD(evsel, cpu, thread), count->values, size) <= 0)
 		return -errno;
 
diff --git a/tools/lib/perf/include/internal/mmap.h b/tools/lib/perf/include/internal/mmap.h
index be7556e0a2b2..5e3422f40ed5 100644
--- a/tools/lib/perf/include/internal/mmap.h
+++ b/tools/lib/perf/include/internal/mmap.h
@@ -11,6 +11,7 @@
 #define PERF_SAMPLE_MAX_SIZE (1 << 16)
 
 struct perf_mmap;
+struct perf_counts_values;
 
 typedef void (*libperf_unmap_cb_t)(struct perf_mmap *map);
 
@@ -52,4 +53,6 @@ void perf_mmap__put(struct perf_mmap *map);
 
 u64 perf_mmap__read_head(struct perf_mmap *map);
 
+int perf_mmap__read_self(struct perf_mmap *map, struct perf_counts_values *count);
+
 #endif /* __LIBPERF_INTERNAL_MMAP_H */
diff --git a/tools/lib/perf/mmap.c b/tools/lib/perf/mmap.c
index 79d5ed6c38cc..915469f00cf4 100644
--- a/tools/lib/perf/mmap.c
+++ b/tools/lib/perf/mmap.c
@@ -8,9 +8,11 @@
 #include <linux/perf_event.h>
 #include <perf/mmap.h>
 #include <perf/event.h>
+#include <perf/evsel.h>
 #include <internal/mmap.h>
 #include <internal/lib.h>
 #include <linux/kernel.h>
+#include <linux/math64.h>
 #include "internal.h"
 
 void perf_mmap__init(struct perf_mmap *map, struct perf_mmap *prev,
@@ -273,3 +275,89 @@ union perf_event *perf_mmap__read_event(struct perf_mmap *map)
 
 	return event;
 }
+
+#if defined(__i386__) || defined(__x86_64__)
+static u64 read_perf_counter(unsigned int counter)
+{
+	unsigned int low, high;
+
+	asm volatile("rdpmc" : "=a" (low), "=d" (high) : "c" (counter));
+
+	return low | ((u64)high) << 32;
+}
+
+static u64 read_timestamp(void)
+{
+	unsigned int low, high;
+
+	asm volatile("rdtsc" : "=a" (low), "=d" (high));
+
+	return low | ((u64)high) << 32;
+}
+#else
+static u64 read_perf_counter(unsigned int counter) { return 0; }
+static u64 read_timestamp(void) { return 0; }
+#endif
+
+int perf_mmap__read_self(struct perf_mmap *map, struct perf_counts_values *count)
+{
+	struct perf_event_mmap_page *pc = map->base;
+	u32 seq, idx, time_mult = 0, time_shift = 0;
+	u64 cnt, cyc = 0, time_offset = 0, time_cycles = 0, time_mask = ~0ULL;
+
+	if (!pc || !pc->cap_user_rdpmc)
+		return -1;
+
+	do {
+		seq = READ_ONCE(pc->lock);
+		barrier();
+
+		count->ena = READ_ONCE(pc->time_enabled);
+		count->run = READ_ONCE(pc->time_running);
+
+		if (pc->cap_user_time && count->ena != count->run) {
+			cyc = read_timestamp();
+			time_mult = READ_ONCE(pc->time_mult);
+			time_shift = READ_ONCE(pc->time_shift);
+			time_offset = READ_ONCE(pc->time_offset);
+
+			if (pc->cap_user_time_short) {
+				time_cycles = READ_ONCE(pc->time_cycles);
+				time_mask = READ_ONCE(pc->time_mask);
+			}
+		}
+
+		idx = READ_ONCE(pc->index);
+		cnt = READ_ONCE(pc->offset);
+		if (pc->cap_user_rdpmc && idx) {
+			s64 evcnt = read_perf_counter(idx - 1);
+			u16 width = READ_ONCE(pc->pmc_width);
+
+			evcnt <<= 64 - width;
+			evcnt >>= 64 - width;
+			cnt += evcnt;
+		} else
+			return -1;
+
+		barrier();
+	} while (READ_ONCE(pc->lock) != seq);
+
+	if (count->ena != count->run) {
+		u64 delta;
+
+		/* Adjust for cap_usr_time_short, a nop if not */
+		cyc = time_cycles + ((cyc - time_cycles) & time_mask);
+
+		delta = time_offset + mul_u64_u32_shr(cyc, time_mult, time_shift);
+
+		count->ena += delta;
+		if (idx)
+			count->run += delta;
+
+		cnt = mul_u64_u64_div64(cnt, count->ena, count->run);
+	}
+
+	count->val = cnt;
+
+	return 0;
+}
diff --git a/tools/lib/perf/tests/test-evsel.c b/tools/lib/perf/tests/test-evsel.c
index 0ad82d7a2a51..54fb4809b9ee 100644
--- a/tools/lib/perf/tests/test-evsel.c
+++ b/tools/lib/perf/tests/test-evsel.c
@@ -120,6 +120,69 @@ static int test_stat_thread_enable(void)
 	return 0;
 }
 
+static int test_stat_user_read(int event)
+{
+	struct perf_counts_values counts = { .val = 0 };
+	struct perf_thread_map *threads;
+	struct perf_evsel *evsel;
+	struct perf_event_mmap_page *pc;
+	struct perf_event_attr attr = {
+		.type	= PERF_TYPE_HARDWARE,
+		.config	= event,
+	};
+	int err, i;
+
+	threads = perf_thread_map__new_dummy();
+	__T("failed to create threads", threads);
+
+	perf_thread_map__set_pid(threads, 0, 0);
+
+	evsel = perf_evsel__new(&attr);
+	__T("failed to create evsel", evsel);
+
+	err = perf_evsel__open(evsel, NULL, threads);
+	__T("failed to open evsel", err == 0);
+
+	err = perf_evsel__mmap(evsel, 0);
+	__T("failed to mmap evsel", err == 0);
+
+	pc = perf_evsel__mmap_base(evsel, 0, 0);
+
+#if defined(__i386__) || defined(__x86_64__) || defined(__aarch64__)
+	__T("userspace counter access not supported", pc->cap_user_rdpmc);
+	__T("userspace counter access not enabled", pc->index);
+	__T("userspace counter width not set", pc->pmc_width >= 32);
+#endif
+
+	perf_evsel__read(evsel, 0, 0, &counts);
+	__T("failed to read value for evsel", counts.val != 0);
+
+	for (i = 0; i < 5; i++) {
+		volatile int count = 0x10000 << i;
+		__u64 start, end, last = 0;
+
+		__T_VERBOSE("\tloop = %u, ", count);
+
+		perf_evsel__read(evsel, 0, 0, &counts);
+		start = counts.val;
+
+		while (count--) ;
+
+		perf_evsel__read(evsel, 0, 0, &counts);
+		end = counts.val;
+
+		__T("invalid counter data", (end - start) > last);
+		last = end - start;
+		__T_VERBOSE("count = %llu\n", end - start);
+	}
+
+	perf_evsel__close(evsel);
+	perf_evsel__delete(evsel);
+
+	perf_thread_map__put(threads);
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	__T_START;
@@ -129,6 +192,8 @@ int main(int argc, char **argv)
 	test_stat_cpu();
 	test_stat_thread();
 	test_stat_thread_enable();
+	test_stat_user_read(PERF_COUNT_HW_INSTRUCTIONS);
+	test_stat_user_read(PERF_COUNT_HW_CPU_CYCLES);
 
 	__T_END;
 	return tests_failed == 0 ? 0 : -1;
-- 
2.27.0

Re: [PATCH v7 2/4] libperf: Add evsel mmap support

[Arnaldo Carvalho de Melo]

Em Tue, Apr 13, 2021 at 12:16:04PM -0500, Rob Herring escreveu:
> In order to support usersapce access, an event must be mmapped. While
> there's already mmap support for evlist, the usecase is a bit different
> than the self monitoring with userspace access. So let's add a new
> perf_evsel__mmap() function to mmap an evsel. This allows implementing
> userspace access as a fastpath for perf_evsel__read().
> 
> The mmapped address is returned by perf_evsel__mmap_base() which
> primarily for users/tests to check if userspace access is enabled.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> v7:
>  - Add NULL fd check to perf_evsel__mmap
> v6:
>  - split mmap struct into it's own xyarray
> v5:
>  - Create an mmap for every underlying event opened. Due to this, we
>    need a different way to get the mmap ptr, so perf_evsel__mmap_base()
>    is introduced.
> v4:
>  - Change perf_evsel__mmap size to pages instead of bytes
> v3:
>  - New patch split out from user access patch
> ---
>  tools/lib/perf/Documentation/libperf.txt |  2 +
>  tools/lib/perf/evsel.c                   | 54 ++++++++++++++++++++++++
>  tools/lib/perf/include/internal/evsel.h  |  1 +
>  tools/lib/perf/include/perf/evsel.h      |  2 +
>  tools/lib/perf/libperf.map               |  2 +
>  5 files changed, 61 insertions(+)
> 
> diff --git a/tools/lib/perf/Documentation/libperf.txt b/tools/lib/perf/Documentation/libperf.txt
> index 0c74c30ed23a..a2c73df191ca 100644
> --- a/tools/lib/perf/Documentation/libperf.txt
> +++ b/tools/lib/perf/Documentation/libperf.txt
> @@ -136,6 +136,8 @@ SYNOPSIS
>                         struct perf_thread_map *threads);
>    void perf_evsel__close(struct perf_evsel *evsel);
>    void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
> +  int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
> +  void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
>    int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
>                         struct perf_counts_values *count);
>    int perf_evsel__enable(struct perf_evsel *evsel);
> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> index 4dc06289f4c7..7e140763552f 100644
> --- a/tools/lib/perf/evsel.c
> +++ b/tools/lib/perf/evsel.c
> @@ -11,10 +11,12 @@
>  #include <stdlib.h>
>  #include <internal/xyarray.h>
>  #include <internal/cpumap.h>
> +#include <internal/mmap.h>
>  #include <internal/threadmap.h>
>  #include <internal/lib.h>
>  #include <linux/string.h>
>  #include <sys/ioctl.h>
> +#include <sys/mman.h>
>  
>  void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr)
>  {
> @@ -38,6 +40,7 @@ void perf_evsel__delete(struct perf_evsel *evsel)
>  }
>  
>  #define FD(e, x, y) (*(int *) xyarray__entry(e->fd, x, y))
> +#define MMAP(e, x, y) (e->mmap ? ((struct perf_mmap *) xyarray__entry(e->mmap, x, y)) : NULL)
>  
>  int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
>  {
> @@ -55,6 +58,13 @@ int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
>  	return evsel->fd != NULL ? 0 : -ENOMEM;
>  }
>  
> +static int perf_evsel__alloc_mmap(struct perf_evsel *evsel, int ncpus, int nthreads)
> +{
> +	evsel->mmap = xyarray__new(ncpus, nthreads, sizeof(struct perf_mmap));
> +
> +	return evsel->mmap != NULL ? 0 : -ENOMEM;
> +}
> +
>  static int
>  sys_perf_event_open(struct perf_event_attr *attr,
>  		    pid_t pid, int cpu, int group_fd,
> @@ -137,6 +147,8 @@ void perf_evsel__free_fd(struct perf_evsel *evsel)
>  {
>  	xyarray__delete(evsel->fd);
>  	evsel->fd = NULL;
> +	xyarray__delete(evsel->mmap);
> +	evsel->mmap = NULL;
>  }
>  
>  void perf_evsel__close(struct perf_evsel *evsel)
> @@ -156,6 +168,48 @@ void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu)
>  	perf_evsel__close_fd_cpu(evsel, cpu);
>  }
>  
> +int perf_evsel__mmap(struct perf_evsel *evsel, int pages)
> +{
> +	int ret, cpu, thread;
> +	struct perf_mmap_param mp = {
> +		.prot = PROT_READ | PROT_WRITE,
> +		.mask = (pages * page_size) - 1,
> +	};
> +
> +	if (evsel->fd == NULL)
> +		return -EINVAL;
> +
> +	if (evsel->mmap == NULL &&
> +	    perf_evsel__alloc_mmap(evsel, xyarray__max_x(evsel->fd), xyarray__max_y(evsel->fd)) < 0)
> +		return -ENOMEM;
> +
> +	for (cpu = 0; cpu < xyarray__max_x(evsel->fd); cpu++) {
> +		for (thread = 0; thread < xyarray__max_y(evsel->fd); thread++) {
> +			int fd = FD(evsel, cpu, thread);
> +			struct perf_mmap *map = MMAP(evsel, cpu, thread);
> +
> +			if (fd < 0)
> +				continue;
> +
> +			perf_mmap__init(map, NULL, false, NULL);
> +
> +			ret = perf_mmap__mmap(map, &mp, fd, cpu);
> +			if (ret)
> +				return -1;
> +		}
> +	}
> +
> +	return 0;
> +}

Where is the counterpart? I.e. perf_evsel__munmap(), and it should be
called if perf_evsel__mmap() fails, right?

- Arnaldo

> +void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
> +{
> +	if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
> +		return NULL;
> +
> +	return MMAP(evsel, cpu, thread)->base;
> +}
> +
>  int perf_evsel__read_size(struct perf_evsel *evsel)
>  {
>  	u64 read_format = evsel->attr.read_format;
> diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
> index 1ffd083b235e..1c067d088bc6 100644
> --- a/tools/lib/perf/include/internal/evsel.h
> +++ b/tools/lib/perf/include/internal/evsel.h
> @@ -41,6 +41,7 @@ struct perf_evsel {
>  	struct perf_cpu_map	*own_cpus;
>  	struct perf_thread_map	*threads;
>  	struct xyarray		*fd;
> +	struct xyarray		*mmap;
>  	struct xyarray		*sample_id;
>  	u64			*id;
>  	u32			 ids;
> diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> index c82ec39a4ad0..9f5265f2f39f 100644
> --- a/tools/lib/perf/include/perf/evsel.h
> +++ b/tools/lib/perf/include/perf/evsel.h
> @@ -27,6 +27,8 @@ LIBPERF_API int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *
>  				 struct perf_thread_map *threads);
>  LIBPERF_API void perf_evsel__close(struct perf_evsel *evsel);
>  LIBPERF_API void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
> +LIBPERF_API int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
> +LIBPERF_API void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
>  LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
>  				 struct perf_counts_values *count);
>  LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
> diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
> index 7be1af8a546c..0b993de15830 100644
> --- a/tools/lib/perf/libperf.map
> +++ b/tools/lib/perf/libperf.map
> @@ -23,6 +23,8 @@ LIBPERF_0.0.1 {
>  		perf_evsel__disable;
>  		perf_evsel__open;
>  		perf_evsel__close;
> +		perf_evsel__mmap;
> +		perf_evsel__mmap_base;
>  		perf_evsel__read;
>  		perf_evsel__cpus;
>  		perf_evsel__threads;
> -- 
> 2.27.0
> 

-- 

- Arnaldo

Re: [PATCH v7 3/4] libperf: tests: Add support for verbose printing

[Arnaldo Carvalho de Melo]

Em Tue, Apr 13, 2021 at 12:16:05PM -0500, Rob Herring escreveu:
> Add __T_VERBOSE() so tests can add verbose output. The verbose output is
> enabled with the '-v' command line option.

You forgot to show how this is used, I'm trying:

  # cd tools/lib/perf
  # sudo make tests

So how from the command line one asks for verbose output from the tests?

  Should be:

  # sudo make tests V=1

?

The default output, non-verbose, is:

[acme@five perf]$ sudo make tests
  LINK     test-cpumap-a
  LINK     test-threadmap-a
  LINK     test-evlist-a
  LINK     test-evsel-a
  LINK     test-cpumap-so
  LINK     test-threadmap-so
  LINK     test-evlist-so
  LINK     test-evsel-so
running static:
- running test-cpumap.c...OK
- running test-threadmap.c...OK
- running test-evlist.c...OK
- running test-evsel.c...OK
running dynamic:
- running test-cpumap.c...OK
- running test-threadmap.c...OK
- running test-evlist.c...OK
- running test-evsel.c...OK
[acme@five perf]$

Trying a verbose mode:

[acme@five perf]$ sudo make tests V=1
make -f /home/acme/git/perf/tools/build/Makefile.build dir=. obj=libperf
make -C /home/acme/git/perf/tools/lib/api/ O= libapi.a
make -f /home/acme/git/perf/tools/build/Makefile.build dir=./fd obj=libapi
make -f /home/acme/git/perf/tools/build/Makefile.build dir=./fs obj=libapi
make -C tests
gcc -I/home/acme/git/perf/tools/lib/perf/include -I/home/acme/git/perf/tools/include -I/home/acme/git/perf/tools/lib -g -Wall -o test-cpumap-a test-cpumap.c ../libperf.a /home/acme/git/perf/tools/lib/api/libapi.a
gcc -I/home/acme/git/perf/tools/lib/perf/include -I/home/acme/git/perf/tools/include -I/home/acme/git/perf/tools/lib -g -Wall -o test-threadmap-a test-threadmap.c ../libperf.a /home/acme/git/perf/tools/lib/api/libapi.a
gcc -I/home/acme/git/perf/tools/lib/perf/include -I/home/acme/git/perf/tools/include -I/home/acme/git/perf/tools/lib -g -Wall -o test-evlist-a test-evlist.c ../libperf.a /home/acme/git/perf/tools/lib/api/libapi.a
gcc -I/home/acme/git/perf/tools/lib/perf/include -I/home/acme/git/perf/tools/include -I/home/acme/git/perf/tools/lib -g -Wall -o test-evsel-a test-evsel.c ../libperf.a /home/acme/git/perf/tools/lib/api/libapi.a
gcc -I/home/acme/git/perf/tools/lib/perf/include -I/home/acme/git/perf/tools/include -I/home/acme/git/perf/tools/lib -g -Wall -L.. -o test-cpumap-so test-cpumap.c /home/acme/git/perf/tools/lib/api/libapi.a -lperf
gcc -I/home/acme/git/perf/tools/lib/perf/include -I/home/acme/git/perf/tools/include -I/home/acme/git/perf/tools/lib -g -Wall -L.. -o test-threadmap-so test-threadmap.c /home/acme/git/perf/tools/lib/api/libapi.a -lperf
gcc -I/home/acme/git/perf/tools/lib/perf/include -I/home/acme/git/perf/tools/include -I/home/acme/git/perf/tools/lib -g -Wall -L.. -o test-evlist-so test-evlist.c /home/acme/git/perf/tools/lib/api/libapi.a -lperf
gcc -I/home/acme/git/perf/tools/lib/perf/include -I/home/acme/git/perf/tools/include -I/home/acme/git/perf/tools/lib -g -Wall -L.. -o test-evsel-so test-evsel.c /home/acme/git/perf/tools/lib/api/libapi.a -lperf
make -C tests run
running static:
- running test-cpumap.c...OK
- running test-threadmap.c...OK
- running test-evlist.c...OK
- running test-evsel.c...OK
running dynamic:
- running test-cpumap.c...OK
- running test-threadmap.c...OK
- running test-evlist.c...OK
- running test-evsel.c...OK
[acme@five perf]$


I'm only getting a move verbose output for the Makefile steps, not from
the actual tests.

Perhaps if I read the last cset... will do that now.

- Arnaldo
 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> v5:
>  - Pass verbose flag to static tests
>  - Fix getopt loop with unsigned char (arm64)
> v3:
>  - New patch
> ---
>  tools/lib/perf/include/internal/tests.h | 32 +++++++++++++++++++++++++
>  tools/lib/perf/tests/Makefile           |  6 +++--
>  2 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/perf/include/internal/tests.h b/tools/lib/perf/include/internal/tests.h
> index 2093e8868a67..29425c2dabe1 100644
> --- a/tools/lib/perf/include/internal/tests.h
> +++ b/tools/lib/perf/include/internal/tests.h
> @@ -3,11 +3,32 @@
>  #define __LIBPERF_INTERNAL_TESTS_H
>  
>  #include <stdio.h>
> +#include <unistd.h>
>  
>  int tests_failed;
> +int tests_verbose;
> +
> +static inline int get_verbose(char **argv, int argc)
> +{
> +	int c;
> +	int verbose = 0;
> +
> +	while ((c = getopt(argc, argv, "v")) != -1) {
> +		switch (c)
> +		{
> +		case 'v':
> +			verbose = 1;
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +	return verbose;
> +}
>  
>  #define __T_START					\
>  do {							\
> +	tests_verbose = get_verbose(argv, argc);	\
>  	fprintf(stdout, "- running %s...", __FILE__);	\
>  	fflush(NULL);					\
>  	tests_failed = 0;				\
> @@ -30,4 +51,15 @@ do {
>  	}                                                                        \
>  } while (0)
>  
> +#define __T_VERBOSE(...)						\
> +do {									\
> +	if (tests_verbose) {						\
> +		if (tests_verbose == 1) {				\
> +			fputc('\n', stderr);				\
> +			tests_verbose++;				\
> +		}							\
> +		fprintf(stderr, ##__VA_ARGS__);				\
> +	}								\
> +} while (0)
> +
>  #endif /* __LIBPERF_INTERNAL_TESTS_H */
> diff --git a/tools/lib/perf/tests/Makefile b/tools/lib/perf/tests/Makefile
> index 96841775feaf..b536cc9a26dd 100644
> --- a/tools/lib/perf/tests/Makefile
> +++ b/tools/lib/perf/tests/Makefile
> @@ -5,6 +5,8 @@ TESTS = test-cpumap test-threadmap test-evlist test-evsel
>  TESTS_SO := $(addsuffix -so,$(TESTS))
>  TESTS_A  := $(addsuffix -a,$(TESTS))
>  
> +TEST_ARGS := $(if $(V),-v)
> +
>  # Set compile option CFLAGS
>  ifdef EXTRA_CFLAGS
>    CFLAGS := $(EXTRA_CFLAGS)
> @@ -28,9 +30,9 @@ all: $(TESTS_A) $(TESTS_SO)
>  
>  run:
>  	@echo "running static:"
> -	@for i in $(TESTS_A); do ./$$i; done
> +	@for i in $(TESTS_A); do ./$$i $(TEST_ARGS); done
>  	@echo "running dynamic:"
> -	@for i in $(TESTS_SO); do LD_LIBRARY_PATH=../ ./$$i; done
> +	@for i in $(TESTS_SO); do LD_LIBRARY_PATH=../ ./$$i $(TEST_ARGS); done
>  
>  clean:
>  	$(call QUIET_CLEAN, tests)$(RM) $(TESTS_A) $(TESTS_SO)
> -- 
> 2.27.0
> 

-- 

- Arnaldo

Re: [PATCH v7 3/4] libperf: tests: Add support for verbose printing

[Arnaldo Carvalho de Melo]

Em Tue, Apr 13, 2021 at 03:49:31PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Apr 13, 2021 at 12:16:05PM -0500, Rob Herring escreveu:
> > Add __T_VERBOSE() so tests can add verbose output. The verbose output is
> > enabled with the '-v' command line option.
> 
> You forgot to show how this is used, I'm trying:
> 
>   # cd tools/lib/perf
>   # sudo make tests
> 
> So how from the command line one asks for verbose output from the tests?
> 
>   Should be:
> 
>   # sudo make tests V=1
> 

> I'm only getting a move verbose output for the Makefile steps, not from
> the actual tests.
> 
> Perhaps if I read the last cset... will do that now.

Ok, I misread, I thought that was adding a way to enable verbose mode
for _pre-existing tests_, so I tried to use it, it is only used in the
following patch...

- Arnaldo

Re: [PATCH v7 2/4] libperf: Add evsel mmap support

[Rob Herring]

On Tue, Apr 13, 2021 at 1:39 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Tue, Apr 13, 2021 at 12:16:04PM -0500, Rob Herring escreveu:
> > In order to support usersapce access, an event must be mmapped. While
> > there's already mmap support for evlist, the usecase is a bit different
> > than the self monitoring with userspace access. So let's add a new
> > perf_evsel__mmap() function to mmap an evsel. This allows implementing
> > userspace access as a fastpath for perf_evsel__read().
> >
> > The mmapped address is returned by perf_evsel__mmap_base() which
> > primarily for users/tests to check if userspace access is enabled.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > v7:
> >  - Add NULL fd check to perf_evsel__mmap
> > v6:
> >  - split mmap struct into it's own xyarray
> > v5:
> >  - Create an mmap for every underlying event opened. Due to this, we
> >    need a different way to get the mmap ptr, so perf_evsel__mmap_base()
> >    is introduced.
> > v4:
> >  - Change perf_evsel__mmap size to pages instead of bytes
> > v3:
> >  - New patch split out from user access patch
> > ---
> >  tools/lib/perf/Documentation/libperf.txt |  2 +
> >  tools/lib/perf/evsel.c                   | 54 ++++++++++++++++++++++++
> >  tools/lib/perf/include/internal/evsel.h  |  1 +
> >  tools/lib/perf/include/perf/evsel.h      |  2 +
> >  tools/lib/perf/libperf.map               |  2 +
> >  5 files changed, 61 insertions(+)
> >
> > diff --git a/tools/lib/perf/Documentation/libperf.txt b/tools/lib/perf/Documentation/libperf.txt
> > index 0c74c30ed23a..a2c73df191ca 100644
> > --- a/tools/lib/perf/Documentation/libperf.txt
> > +++ b/tools/lib/perf/Documentation/libperf.txt
> > @@ -136,6 +136,8 @@ SYNOPSIS
> >                         struct perf_thread_map *threads);
> >    void perf_evsel__close(struct perf_evsel *evsel);
> >    void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
> > +  int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
> > +  void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
> >    int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
> >                         struct perf_counts_values *count);
> >    int perf_evsel__enable(struct perf_evsel *evsel);
> > diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> > index 4dc06289f4c7..7e140763552f 100644
> > --- a/tools/lib/perf/evsel.c
> > +++ b/tools/lib/perf/evsel.c
> > @@ -11,10 +11,12 @@
> >  #include <stdlib.h>
> >  #include <internal/xyarray.h>
> >  #include <internal/cpumap.h>
> > +#include <internal/mmap.h>
> >  #include <internal/threadmap.h>
> >  #include <internal/lib.h>
> >  #include <linux/string.h>
> >  #include <sys/ioctl.h>
> > +#include <sys/mman.h>
> >
> >  void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr)
> >  {
> > @@ -38,6 +40,7 @@ void perf_evsel__delete(struct perf_evsel *evsel)
> >  }
> >
> >  #define FD(e, x, y) (*(int *) xyarray__entry(e->fd, x, y))
> > +#define MMAP(e, x, y) (e->mmap ? ((struct perf_mmap *) xyarray__entry(e->mmap, x, y)) : NULL)
> >
> >  int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
> >  {
> > @@ -55,6 +58,13 @@ int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
> >       return evsel->fd != NULL ? 0 : -ENOMEM;
> >  }
> >
> > +static int perf_evsel__alloc_mmap(struct perf_evsel *evsel, int ncpus, int nthreads)
> > +{
> > +     evsel->mmap = xyarray__new(ncpus, nthreads, sizeof(struct perf_mmap));
> > +
> > +     return evsel->mmap != NULL ? 0 : -ENOMEM;
> > +}
> > +
> >  static int
> >  sys_perf_event_open(struct perf_event_attr *attr,
> >                   pid_t pid, int cpu, int group_fd,
> > @@ -137,6 +147,8 @@ void perf_evsel__free_fd(struct perf_evsel *evsel)
> >  {
> >       xyarray__delete(evsel->fd);
> >       evsel->fd = NULL;
> > +     xyarray__delete(evsel->mmap);
> > +     evsel->mmap = NULL;
> >  }
> >
> >  void perf_evsel__close(struct perf_evsel *evsel)
> > @@ -156,6 +168,48 @@ void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu)
> >       perf_evsel__close_fd_cpu(evsel, cpu);
> >  }
> >
> > +int perf_evsel__mmap(struct perf_evsel *evsel, int pages)
> > +{
> > +     int ret, cpu, thread;
> > +     struct perf_mmap_param mp = {
> > +             .prot = PROT_READ | PROT_WRITE,
> > +             .mask = (pages * page_size) - 1,
> > +     };
> > +
> > +     if (evsel->fd == NULL)
> > +             return -EINVAL;
> > +
> > +     if (evsel->mmap == NULL &&
> > +         perf_evsel__alloc_mmap(evsel, xyarray__max_x(evsel->fd), xyarray__max_y(evsel->fd)) < 0)
> > +             return -ENOMEM;
> > +
> > +     for (cpu = 0; cpu < xyarray__max_x(evsel->fd); cpu++) {
> > +             for (thread = 0; thread < xyarray__max_y(evsel->fd); thread++) {
> > +                     int fd = FD(evsel, cpu, thread);
> > +                     struct perf_mmap *map = MMAP(evsel, cpu, thread);
> > +
> > +                     if (fd < 0)
> > +                             continue;
> > +
> > +                     perf_mmap__init(map, NULL, false, NULL);
> > +
> > +                     ret = perf_mmap__mmap(map, &mp, fd, cpu);
> > +                     if (ret)
> > +                             return -1;
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
>
> Where is the counterpart?

I was assuming implicitly unmapped when closing the fd(s), but looks
like it's when exiting the process only.

I.e. perf_evsel__munmap(), and it should be
> called if perf_evsel__mmap() fails, right?

If perf_evsel__mmap() fails, the caller shouldn't have to do anything
WRT mmap, right? But if the perf_mmap__mmap() call fails, we do need
some internal clean-up. I'll fix both.

Rob

Re: [PATCH v7 2/4] libperf: Add evsel mmap support

[Arnaldo Carvalho de Melo]

Em Tue, Apr 13, 2021 at 02:07:57PM -0500, Rob Herring escreveu:
> On Tue, Apr 13, 2021 at 1:39 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > --- a/tools/lib/perf/evsel.c
> > > +int perf_evsel__mmap(struct perf_evsel *evsel, int pages)
> > > +{
> > > +     int ret, cpu, thread;
> > Where is the counterpart?
> 
> I was assuming implicitly unmapped when closing the fd(s), but looks
> like it's when exiting the process only.
> 
> I.e. perf_evsel__munmap(), and it should be
> > called if perf_evsel__mmap() fails, right?
> 
> If perf_evsel__mmap() fails, the caller shouldn't have to do anything
> WRT mmap, right? But if the perf_mmap__mmap() call fails, we do need
> some internal clean-up. I'll fix both.

You're right, thanks!

- Arnaldo