* [PATCH v3 0/3] Tidy user rdpmc documentation and testing
@ 2022-07-19 22:39 Ian Rogers
2022-07-19 22:39 ` [PATCH v3 1/3] perf: Align user space counter reading with code Ian Rogers
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Ian Rogers @ 2022-07-19 22:39 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Kajol Jain, Andi Kleen, Adrian Hunter, Anshuman Khandual,
linux-kernel, linux-perf-users, Rob Herring
Cc: Stephane Eranian, Ian Rogers
libperf's perf_mmap__read_self and the addition of arm64 support mean
that the perf_event.h and the rdpmc perf test have become
stale. Refresh the documentation in perf_event.h, remove the x86 rdpmc
test and port the libperf test as a non-architecture specific test.
A comment is added to perf_event.h to avoid a divide by zero when
scaling counts if the running time is 0. This was previously discussed
in this thread:
https://lore.kernel.org/lkml/CAP-5=fVRdqvswtyQMg5cB+ntTGda+SAYskjTQednEH-AeZo13g@mail.gmail.com/
v3. Incorporates fixes from Rob Herring <robh@kernel.org> to make the
perf_event.h comments better. It drops the already merged sanitizer fix.
v2. Alters the skip in test_stat_user_read for open to always be a
skip as perf_event_open may fail with EACCES (permissions), ENOSYS
(not supported) and ENOENT (hypervisor). Adds Rob Herring's
acked-by on patch 3.
Ian Rogers (3):
perf: Align user space counter reading with code
perf test: Remove x86 rdpmc test
perf test: Add user space counter reading tests
include/uapi/linux/perf_event.h | 35 +++--
tools/include/uapi/linux/perf_event.h | 35 +++--
tools/perf/arch/x86/tests/Build | 1 -
tools/perf/arch/x86/tests/arch-tests.c | 2 -
tools/perf/arch/x86/tests/rdpmc.c | 182 -------------------------
tools/perf/tests/mmap-basic.c | 127 ++++++++++++++++-
6 files changed, 170 insertions(+), 212 deletions(-)
delete mode 100644 tools/perf/arch/x86/tests/rdpmc.c
--
2.37.0.170.g444d1eabd0-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 1/3] perf: Align user space counter reading with code
2022-07-19 22:39 [PATCH v3 0/3] Tidy user rdpmc documentation and testing Ian Rogers
@ 2022-07-19 22:39 ` Ian Rogers
2022-07-19 23:05 ` Rob Herring
` (2 more replies)
2022-07-19 22:39 ` [PATCH v3 2/3] perf test: Remove x86 rdpmc test Ian Rogers
2022-07-19 22:39 ` [PATCH v3 3/3] perf test: Add user space counter reading tests Ian Rogers
2 siblings, 3 replies; 13+ messages in thread
From: Ian Rogers @ 2022-07-19 22:39 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Kajol Jain, Andi Kleen, Adrian Hunter, Anshuman Khandual,
linux-kernel, linux-perf-users, Rob Herring
Cc: Stephane Eranian, Ian Rogers
Align the user space counter reading documentation with the code in
perf_mmap__read_self. Previously the documentation was based on the perf
rdpmc test, but now general purpose code is provided by libperf.
Signed-off-by: Ian Rogers <irogers@google.com>
---
include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
tools/include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
2 files changed, 44 insertions(+), 26 deletions(-)
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index d37629dbad72..6826dabb7e03 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -538,9 +538,13 @@ struct perf_event_mmap_page {
*
* if (pc->cap_usr_time && enabled != running) {
* cyc = rdtsc();
- * time_offset = pc->time_offset;
* time_mult = pc->time_mult;
* time_shift = pc->time_shift;
+ * time_offset = pc->time_offset;
+ * if (pc->cap_user_time_short) {
+ * time_cycles = pc->time_cycles;
+ * time_mask = pc->time_mask;
+ * }
* }
*
* index = pc->index;
@@ -548,6 +552,9 @@ struct perf_event_mmap_page {
* if (pc->cap_user_rdpmc && index) {
* width = pc->pmc_width;
* pmc = rdpmc(index - 1);
+ * pmc <<= 64 - width;
+ * pmc >>= 64 - width;
+ * count += pmc;
* }
*
* barrier();
@@ -590,25 +597,27 @@ struct perf_event_mmap_page {
* If cap_usr_time the below fields can be used to compute the time
* delta since time_enabled (in ns) using rdtsc or similar.
*
- * u64 quot, rem;
- * u64 delta;
- *
- * quot = (cyc >> time_shift);
- * rem = cyc & (((u64)1 << time_shift) - 1);
- * delta = time_offset + quot * time_mult +
- * ((rem * time_mult) >> time_shift);
+ * cyc = time_cycles + ((cyc - time_cycles) & time_mask);
+ * delta = time_offset + mul_u64_u32_shr(cyc, time_mult, time_shift);
*
* Where time_offset,time_mult,time_shift and cyc are read in the
- * seqcount loop described above. This delta can then be added to
- * enabled and possible running (if index), improving the scaling:
+ * seqcount loop described above. mul_u64_u32_shr will compute:
+ *
+ * (u64)(((unsigned __int128)cyc * time_mult) >> time_shift)
+ *
+ * This delta can then be added to enabled and possible running (if
+ * index) to improve the scaling. Due to event multiplexing, running
+ * may be zero and so care is needed to avoid division by zero.
*
* enabled += delta;
* if (index)
* running += delta;
*
- * quot = count / running;
- * rem = count % running;
- * count = quot * enabled + (rem * enabled) / running;
+ * if (running != 0) {
+ * quot = count / running;
+ * rem = count % running;
+ * count = quot * enabled + (rem * enabled) / running;
+ * }
*/
__u16 time_shift;
__u32 time_mult;
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index d37629dbad72..6826dabb7e03 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -538,9 +538,13 @@ struct perf_event_mmap_page {
*
* if (pc->cap_usr_time && enabled != running) {
* cyc = rdtsc();
- * time_offset = pc->time_offset;
* time_mult = pc->time_mult;
* time_shift = pc->time_shift;
+ * time_offset = pc->time_offset;
+ * if (pc->cap_user_time_short) {
+ * time_cycles = pc->time_cycles;
+ * time_mask = pc->time_mask;
+ * }
* }
*
* index = pc->index;
@@ -548,6 +552,9 @@ struct perf_event_mmap_page {
* if (pc->cap_user_rdpmc && index) {
* width = pc->pmc_width;
* pmc = rdpmc(index - 1);
+ * pmc <<= 64 - width;
+ * pmc >>= 64 - width;
+ * count += pmc;
* }
*
* barrier();
@@ -590,25 +597,27 @@ struct perf_event_mmap_page {
* If cap_usr_time the below fields can be used to compute the time
* delta since time_enabled (in ns) using rdtsc or similar.
*
- * u64 quot, rem;
- * u64 delta;
- *
- * quot = (cyc >> time_shift);
- * rem = cyc & (((u64)1 << time_shift) - 1);
- * delta = time_offset + quot * time_mult +
- * ((rem * time_mult) >> time_shift);
+ * cyc = time_cycles + ((cyc - time_cycles) & time_mask);
+ * delta = time_offset + mul_u64_u32_shr(cyc, time_mult, time_shift);
*
* Where time_offset,time_mult,time_shift and cyc are read in the
- * seqcount loop described above. This delta can then be added to
- * enabled and possible running (if index), improving the scaling:
+ * seqcount loop described above. mul_u64_u32_shr will compute:
+ *
+ * (u64)(((unsigned __int128)cyc * time_mult) >> time_shift)
+ *
+ * This delta can then be added to enabled and possible running (if
+ * index) to improve the scaling. Due to event multiplexing, running
+ * may be zero and so care is needed to avoid division by zero.
*
* enabled += delta;
* if (index)
* running += delta;
*
- * quot = count / running;
- * rem = count % running;
- * count = quot * enabled + (rem * enabled) / running;
+ * if (running != 0) {
+ * quot = count / running;
+ * rem = count % running;
+ * count = quot * enabled + (rem * enabled) / running;
+ * }
*/
__u16 time_shift;
__u32 time_mult;
--
2.37.0.170.g444d1eabd0-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 2/3] perf test: Remove x86 rdpmc test
2022-07-19 22:39 [PATCH v3 0/3] Tidy user rdpmc documentation and testing Ian Rogers
2022-07-19 22:39 ` [PATCH v3 1/3] perf: Align user space counter reading with code Ian Rogers
@ 2022-07-19 22:39 ` Ian Rogers
2022-08-01 12:19 ` Arnaldo Carvalho de Melo
2022-07-19 22:39 ` [PATCH v3 3/3] perf test: Add user space counter reading tests Ian Rogers
2 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2022-07-19 22:39 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Kajol Jain, Andi Kleen, Adrian Hunter, Anshuman Khandual,
linux-kernel, linux-perf-users, Rob Herring
Cc: Stephane Eranian, Ian Rogers
This test has been superseded by test_stat_user_read in:
tools/lib/perf/tests/test-evsel.c
The updated test doesn't divide-by-0 when running time of a counter is
0. It also supports ARM64.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/arch/x86/tests/Build | 1 -
tools/perf/arch/x86/tests/arch-tests.c | 2 -
tools/perf/arch/x86/tests/rdpmc.c | 182 -------------------------
3 files changed, 185 deletions(-)
delete mode 100644 tools/perf/arch/x86/tests/rdpmc.c
diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
index 28d793390198..70b5bcbc15df 100644
--- a/tools/perf/arch/x86/tests/Build
+++ b/tools/perf/arch/x86/tests/Build
@@ -2,7 +2,6 @@ perf-$(CONFIG_DWARF_UNWIND) += regs_load.o
perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
perf-y += arch-tests.o
-perf-y += rdpmc.o
perf-y += sample-parsing.o
perf-$(CONFIG_AUXTRACE) += insn-x86.o intel-pt-pkt-decoder-test.o
perf-$(CONFIG_X86_64) += bp-modify.o
diff --git a/tools/perf/arch/x86/tests/arch-tests.c b/tools/perf/arch/x86/tests/arch-tests.c
index 64fb73d14d2f..04018b8aa85b 100644
--- a/tools/perf/arch/x86/tests/arch-tests.c
+++ b/tools/perf/arch/x86/tests/arch-tests.c
@@ -3,7 +3,6 @@
#include "tests/tests.h"
#include "arch-tests.h"
-DEFINE_SUITE("x86 rdpmc", rdpmc);
#ifdef HAVE_AUXTRACE_SUPPORT
DEFINE_SUITE("x86 instruction decoder - new instructions", insn_x86);
DEFINE_SUITE("Intel PT packet decoder", intel_pt_pkt_decoder);
@@ -14,7 +13,6 @@ DEFINE_SUITE("x86 bp modify", bp_modify);
DEFINE_SUITE("x86 Sample parsing", x86_sample_parsing);
struct test_suite *arch_tests[] = {
- &suite__rdpmc,
#ifdef HAVE_DWARF_UNWIND_SUPPORT
&suite__dwarf_unwind,
#endif
diff --git a/tools/perf/arch/x86/tests/rdpmc.c b/tools/perf/arch/x86/tests/rdpmc.c
deleted file mode 100644
index 498413ad9c97..000000000000
--- a/tools/perf/arch/x86/tests/rdpmc.c
+++ /dev/null
@@ -1,182 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <errno.h>
-#include <unistd.h>
-#include <stdlib.h>
-#include <signal.h>
-#include <sys/mman.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <linux/string.h>
-#include <linux/types.h>
-#include "perf-sys.h"
-#include "debug.h"
-#include "tests/tests.h"
-#include "cloexec.h"
-#include "event.h"
-#include <internal/lib.h> // page_size
-#include "arch-tests.h"
-
-static u64 rdpmc(unsigned int counter)
-{
- unsigned int low, high;
-
- asm volatile("rdpmc" : "=a" (low), "=d" (high) : "c" (counter));
-
- return low | ((u64)high) << 32;
-}
-
-static u64 rdtsc(void)
-{
- unsigned int low, high;
-
- asm volatile("rdtsc" : "=a" (low), "=d" (high));
-
- return low | ((u64)high) << 32;
-}
-
-static u64 mmap_read_self(void *addr)
-{
- struct perf_event_mmap_page *pc = addr;
- u32 seq, idx, time_mult = 0, time_shift = 0;
- u64 count, cyc = 0, time_offset = 0, enabled, running, delta;
-
- do {
- seq = pc->lock;
- barrier();
-
- enabled = pc->time_enabled;
- running = pc->time_running;
-
- if (enabled != running) {
- cyc = rdtsc();
- time_mult = pc->time_mult;
- time_shift = pc->time_shift;
- time_offset = pc->time_offset;
- }
-
- idx = pc->index;
- count = pc->offset;
- if (idx)
- count += rdpmc(idx - 1);
-
- barrier();
- } while (pc->lock != seq);
-
- if (enabled != running) {
- u64 quot, rem;
-
- quot = (cyc >> time_shift);
- rem = cyc & (((u64)1 << time_shift) - 1);
- delta = time_offset + quot * time_mult +
- ((rem * time_mult) >> time_shift);
-
- enabled += delta;
- if (idx)
- running += delta;
-
- quot = count / running;
- rem = count % running;
- count = quot * enabled + (rem * enabled) / running;
- }
-
- return count;
-}
-
-/*
- * If the RDPMC instruction faults then signal this back to the test parent task:
- */
-static void segfault_handler(int sig __maybe_unused,
- siginfo_t *info __maybe_unused,
- void *uc __maybe_unused)
-{
- exit(-1);
-}
-
-static int __test__rdpmc(void)
-{
- volatile int tmp = 0;
- u64 i, loops = 1000;
- int n;
- int fd;
- void *addr;
- struct perf_event_attr attr = {
- .type = PERF_TYPE_HARDWARE,
- .config = PERF_COUNT_HW_INSTRUCTIONS,
- .exclude_kernel = 1,
- };
- u64 delta_sum = 0;
- struct sigaction sa;
- char sbuf[STRERR_BUFSIZE];
-
- sigfillset(&sa.sa_mask);
- sa.sa_sigaction = segfault_handler;
- sa.sa_flags = 0;
- sigaction(SIGSEGV, &sa, NULL);
-
- fd = sys_perf_event_open(&attr, 0, -1, -1,
- perf_event_open_cloexec_flag());
- if (fd < 0) {
- pr_err("Error: sys_perf_event_open() syscall returned "
- "with %d (%s)\n", fd,
- str_error_r(errno, sbuf, sizeof(sbuf)));
- return -1;
- }
-
- addr = mmap(NULL, page_size, PROT_READ, MAP_SHARED, fd, 0);
- if (addr == (void *)(-1)) {
- pr_err("Error: mmap() syscall returned with (%s)\n",
- str_error_r(errno, sbuf, sizeof(sbuf)));
- goto out_close;
- }
-
- for (n = 0; n < 6; n++) {
- u64 stamp, now, delta;
-
- stamp = mmap_read_self(addr);
-
- for (i = 0; i < loops; i++)
- tmp++;
-
- now = mmap_read_self(addr);
- loops *= 10;
-
- delta = now - stamp;
- pr_debug("%14d: %14Lu\n", n, (long long)delta);
-
- delta_sum += delta;
- }
-
- munmap(addr, page_size);
- pr_debug(" ");
-out_close:
- close(fd);
-
- if (!delta_sum)
- return -1;
-
- return 0;
-}
-
-int test__rdpmc(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
-{
- int status = 0;
- int wret = 0;
- int ret;
- int pid;
-
- pid = fork();
- if (pid < 0)
- return -1;
-
- if (!pid) {
- ret = __test__rdpmc();
-
- exit(ret);
- }
-
- wret = waitpid(pid, &status, 0);
- if (wret < 0 || status)
- return -1;
-
- return 0;
-}
--
2.37.0.170.g444d1eabd0-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 3/3] perf test: Add user space counter reading tests
2022-07-19 22:39 [PATCH v3 0/3] Tidy user rdpmc documentation and testing Ian Rogers
2022-07-19 22:39 ` [PATCH v3 1/3] perf: Align user space counter reading with code Ian Rogers
2022-07-19 22:39 ` [PATCH v3 2/3] perf test: Remove x86 rdpmc test Ian Rogers
@ 2022-07-19 22:39 ` Ian Rogers
2022-07-20 14:57 ` Vince Weaver
2022-08-01 12:23 ` Arnaldo Carvalho de Melo
2 siblings, 2 replies; 13+ messages in thread
From: Ian Rogers @ 2022-07-19 22:39 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Kajol Jain, Andi Kleen, Adrian Hunter, Anshuman Khandual,
linux-kernel, linux-perf-users, Rob Herring
Cc: Stephane Eranian, Ian Rogers
These tests are based on test_stat_user_read in
tools/lib/perf/tests/test-evsel.c. The tests are modified to skip if
perf_event_open fails or rdpmc isn't supported.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/mmap-basic.c | 127 +++++++++++++++++++++++++++++++++-
1 file changed, 126 insertions(+), 1 deletion(-)
diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
index 30bbe144648a..dfb6173b2a82 100644
--- a/tools/perf/tests/mmap-basic.c
+++ b/tools/perf/tests/mmap-basic.c
@@ -170,14 +170,139 @@ static int test__basic_mmap(struct test_suite *test __maybe_unused, int subtest
return err;
}
+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,
+#ifdef __aarch64__
+ .config1 = 0x2, /* Request user access */
+#endif
+ };
+ int err, i, ret = TEST_FAIL;
+ bool opened = false, mapped = false;
+
+ threads = perf_thread_map__new_dummy();
+ TEST_ASSERT_VAL("failed to create threads", threads);
+
+ perf_thread_map__set_pid(threads, 0, 0);
+
+ evsel = perf_evsel__new(&attr);
+ TEST_ASSERT_VAL("failed to create evsel", evsel);
+
+ err = perf_evsel__open(evsel, NULL, threads);
+ if (err) {
+ pr_err("failed to open evsel: %s\n", strerror(-err));
+ ret = TEST_SKIP;
+ goto out;
+ }
+ opened = true;
+
+ err = perf_evsel__mmap(evsel, 0);
+ if (err) {
+ pr_err("failed to mmap evsel: %s\n", strerror(-err));
+ goto out;
+ }
+ mapped = true;
+
+ pc = perf_evsel__mmap_base(evsel, 0, 0);
+ if (!pc) {
+ pr_err("failed to get mmapped address\n");
+ goto out;
+ }
+
+ if (!pc->cap_user_rdpmc || !pc->index) {
+ pr_err("userspace counter access not %s\n",
+ !pc->cap_user_rdpmc ? "supported" : "enabled");
+ ret = TEST_SKIP;
+ goto out;
+ }
+ if (pc->pmc_width < 32) {
+ pr_err("userspace counter width not set (%d)\n", pc->pmc_width);
+ goto out;
+ }
+
+ perf_evsel__read(evsel, 0, 0, &counts);
+ if (counts.val == 0) {
+ pr_err("failed to read value for evsel\n");
+ goto out;
+ }
+
+ for (i = 0; i < 5; i++) {
+ volatile int count = 0x10000 << i;
+ __u64 start, end, last = 0;
+
+ pr_debug("\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;
+
+ if ((end - start) < last) {
+ pr_err("invalid counter data: end=%llu start=%llu last= %llu\n",
+ end, start, last);
+ goto out;
+ }
+ last = end - start;
+ pr_debug("count = %llu\n", end - start);
+ }
+ ret = TEST_OK;
+
+out:
+ if (mapped)
+ perf_evsel__munmap(evsel);
+ if (opened)
+ perf_evsel__close(evsel);
+ perf_evsel__delete(evsel);
+
+ perf_thread_map__put(threads);
+ return ret;
+}
+
+static int test__mmap_user_read_instr(struct test_suite *test __maybe_unused,
+ int subtest __maybe_unused)
+{
+ return test_stat_user_read(PERF_COUNT_HW_INSTRUCTIONS);
+}
+
+static int test__mmap_user_read_cycles(struct test_suite *test __maybe_unused,
+ int subtest __maybe_unused)
+{
+ return test_stat_user_read(PERF_COUNT_HW_CPU_CYCLES);
+}
+
static struct test_case tests__basic_mmap[] = {
TEST_CASE_REASON("Read samples using the mmap interface",
basic_mmap,
"permissions"),
+ TEST_CASE_REASON("User space counter reading of instructions",
+ mmap_user_read_instr,
+#if defined(__i386__) || defined(__x86_64__) || defined(__aarch64__)
+ "permissions"
+#else
+ "unsupported"
+#endif
+ ),
+ TEST_CASE_REASON("User space counter reading of cycles",
+ mmap_user_read_cycles,
+#if defined(__i386__) || defined(__x86_64__) || defined(__aarch64__)
+ "permissions"
+#else
+ "unsupported"
+#endif
+ ),
{ .name = NULL, }
};
struct test_suite suite__basic_mmap = {
- .desc = "Read samples using the mmap interface",
+ .desc = "mmap interface tests",
.test_cases = tests__basic_mmap,
};
--
2.37.0.170.g444d1eabd0-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] perf: Align user space counter reading with code
2022-07-19 22:39 ` [PATCH v3 1/3] perf: Align user space counter reading with code Ian Rogers
@ 2022-07-19 23:05 ` Rob Herring
2022-07-20 15:06 ` Vince Weaver
2022-08-01 12:17 ` Arnaldo Carvalho de Melo
2 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2022-07-19 23:05 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Kajol Jain, Andi Kleen, Adrian Hunter, Anshuman Khandual,
linux-kernel, linux-perf-users, Stephane Eranian
On Tue, Jul 19, 2022 at 4:39 PM Ian Rogers <irogers@google.com> wrote:
>
> Align the user space counter reading documentation with the code in
> perf_mmap__read_self. Previously the documentation was based on the perf
> rdpmc test, but now general purpose code is provided by libperf.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
> tools/include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
> 2 files changed, 44 insertions(+), 26 deletions(-)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] perf test: Add user space counter reading tests
2022-07-19 22:39 ` [PATCH v3 3/3] perf test: Add user space counter reading tests Ian Rogers
@ 2022-07-20 14:57 ` Vince Weaver
2022-07-20 15:09 ` Ian Rogers
2022-08-01 12:23 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 13+ messages in thread
From: Vince Weaver @ 2022-07-20 14:57 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Kajol Jain, Andi Kleen, Adrian Hunter, Anshuman Khandual,
linux-kernel, linux-perf-users, Rob Herring, Stephane Eranian
On Tue, 19 Jul 2022, Ian Rogers wrote:
> These tests are based on test_stat_user_read in
> tools/lib/perf/tests/test-evsel.c. The tests are modified to skip if
> perf_event_open fails or rdpmc isn't supported.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> + .type = PERF_TYPE_HARDWARE,
> + .config = event,
> +#ifdef __aarch64__
> + .config1 = 0x2, /* Request user access */
> +#endif
should this value have a name rather than being a "magic constant"?
Vince
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] perf: Align user space counter reading with code
2022-07-19 22:39 ` [PATCH v3 1/3] perf: Align user space counter reading with code Ian Rogers
2022-07-19 23:05 ` Rob Herring
@ 2022-07-20 15:06 ` Vince Weaver
2022-07-20 15:18 ` Ian Rogers
2022-08-04 8:49 ` Ingo Molnar
2022-08-01 12:17 ` Arnaldo Carvalho de Melo
2 siblings, 2 replies; 13+ messages in thread
From: Vince Weaver @ 2022-07-20 15:06 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Kajol Jain, Andi Kleen, Adrian Hunter, Anshuman Khandual,
linux-kernel, linux-perf-users, Rob Herring, Stephane Eranian
On Tue, 19 Jul 2022, Ian Rogers wrote:
> Align the user space counter reading documentation with the code in
> perf_mmap__read_self. Previously the documentation was based on the perf
> rdpmc test, but now general purpose code is provided by libperf.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
> tools/include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
> 2 files changed, 44 insertions(+), 26 deletions(-)
>
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index d37629dbad72..6826dabb7e03 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -538,9 +538,13 @@ struct perf_event_mmap_page {
> *
> * if (pc->cap_usr_time && enabled != running) {
> * cyc = rdtsc();
> - * time_offset = pc->time_offset;
> * time_mult = pc->time_mult;
> * time_shift = pc->time_shift;
> + * time_offset = pc->time_offset;
> + * if (pc->cap_user_time_short) {
> + * time_cycles = pc->time_cycles;
> + * time_mask = pc->time_mask;
> + * }
From what I've been told, and from what perf_mmap__read_self() actually
does, many of these MMAP fields need to be accessed by READ_ONCE()
(a GPLv2 only interface) to be correct.
Should we update perf_event.h to reflect this? Otherwise it's confusing
when the actual code and the documentation in the header don't match like
this. As an example, see the actual code snippets from
perf_mmap__read_self()
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);
...
Vince
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] perf test: Add user space counter reading tests
2022-07-20 14:57 ` Vince Weaver
@ 2022-07-20 15:09 ` Ian Rogers
0 siblings, 0 replies; 13+ messages in thread
From: Ian Rogers @ 2022-07-20 15:09 UTC (permalink / raw)
To: Vince Weaver
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Kajol Jain, Andi Kleen, Adrian Hunter, Anshuman Khandual,
linux-kernel, linux-perf-users, Rob Herring, Stephane Eranian
On Wed, Jul 20, 2022 at 7:57 AM Vince Weaver <vincent.weaver@maine.edu> wrote:
>
> On Tue, 19 Jul 2022, Ian Rogers wrote:
>
> > These tests are based on test_stat_user_read in
> > tools/lib/perf/tests/test-evsel.c. The tests are modified to skip if
> > perf_event_open fails or rdpmc isn't supported.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> > + .type = PERF_TYPE_HARDWARE,
> > + .config = event,
> > +#ifdef __aarch64__
> > + .config1 = 0x2, /* Request user access */
> > +#endif
>
> should this value have a name rather than being a "magic constant"?
Thanks! Firstly, this is just moving code around and so not a
regression introduced by this code:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/lib/perf/tests/test-evsel.c?h=perf/core#n134
I don't believe there is an existing constant for this purpose. The
kernel isn't using one either:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/perf_event.c#n309
So I agree with you and hopefully this is something ARM will clean up.
Thanks,
Ian
> Vince
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] perf: Align user space counter reading with code
2022-07-20 15:06 ` Vince Weaver
@ 2022-07-20 15:18 ` Ian Rogers
2022-08-04 8:49 ` Ingo Molnar
1 sibling, 0 replies; 13+ messages in thread
From: Ian Rogers @ 2022-07-20 15:18 UTC (permalink / raw)
To: Vince Weaver
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Kajol Jain, Andi Kleen, Adrian Hunter, Anshuman Khandual,
linux-kernel, linux-perf-users, Rob Herring, Stephane Eranian
On Wed, Jul 20, 2022 at 8:06 AM Vince Weaver <vincent.weaver@maine.edu> wrote:
>
> On Tue, 19 Jul 2022, Ian Rogers wrote:
>
> > Align the user space counter reading documentation with the code in
> > perf_mmap__read_self. Previously the documentation was based on the perf
> > rdpmc test, but now general purpose code is provided by libperf.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
> > tools/include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
> > 2 files changed, 44 insertions(+), 26 deletions(-)
> >
> > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> > index d37629dbad72..6826dabb7e03 100644
> > --- a/include/uapi/linux/perf_event.h
> > +++ b/include/uapi/linux/perf_event.h
> > @@ -538,9 +538,13 @@ struct perf_event_mmap_page {
> > *
> > * if (pc->cap_usr_time && enabled != running) {
> > * cyc = rdtsc();
> > - * time_offset = pc->time_offset;
> > * time_mult = pc->time_mult;
> > * time_shift = pc->time_shift;
> > + * time_offset = pc->time_offset;
> > + * if (pc->cap_user_time_short) {
> > + * time_cycles = pc->time_cycles;
> > + * time_mask = pc->time_mask;
> > + * }
>
> From what I've been told, and from what perf_mmap__read_self() actually
> does, many of these MMAP fields need to be accessed by READ_ONCE()
> (a GPLv2 only interface) to be correct.
>
> Should we update perf_event.h to reflect this? Otherwise it's confusing
> when the actual code and the documentation in the header don't match like
> this. As an example, see the actual code snippets from
> perf_mmap__read_self()
>
> 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);
>
> ...
Thanks! So I think what this patch proposes is an improvement,
specifically it aligns it better with the code and deals with the
divide by zero. I think what you're asking for makes sense but as you
point out READ_ONCE probably isn't the correct API for something
outside the kernel. Given the kernel is now C11:
https://lwn.net/Articles/885941/
This opens the possibility of using stdatomic.h, so perhaps we can
move these variables to more correct atomic types. So, I think we can
land this and worry about the atomic API in a follow up.
Thanks,
Ian
> Vince
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] perf: Align user space counter reading with code
2022-07-19 22:39 ` [PATCH v3 1/3] perf: Align user space counter reading with code Ian Rogers
2022-07-19 23:05 ` Rob Herring
2022-07-20 15:06 ` Vince Weaver
@ 2022-08-01 12:17 ` Arnaldo Carvalho de Melo
2 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-08-01 12:17 UTC (permalink / raw)
To: Peter Zijlstra, Ian Rogers
Cc: Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Kajol Jain, Andi Kleen, Adrian Hunter,
Anshuman Khandual, linux-kernel, linux-perf-users, Rob Herring,
Stephane Eranian
Em Tue, Jul 19, 2022 at 03:39:44PM -0700, Ian Rogers escreveu:
> Align the user space counter reading documentation with the code in
> perf_mmap__read_self. Previously the documentation was based on the perf
> rdpmc test, but now general purpose code is provided by libperf.
Peter, can you merge this so as not to make Linus raise eyebrows with me
processing things outside tools/perf/ when asking him to pull perf
userspace?
- Arnaldo
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
> tools/include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
> 2 files changed, 44 insertions(+), 26 deletions(-)
>
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index d37629dbad72..6826dabb7e03 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -538,9 +538,13 @@ struct perf_event_mmap_page {
> *
> * if (pc->cap_usr_time && enabled != running) {
> * cyc = rdtsc();
> - * time_offset = pc->time_offset;
> * time_mult = pc->time_mult;
> * time_shift = pc->time_shift;
> + * time_offset = pc->time_offset;
> + * if (pc->cap_user_time_short) {
> + * time_cycles = pc->time_cycles;
> + * time_mask = pc->time_mask;
> + * }
> * }
> *
> * index = pc->index;
> @@ -548,6 +552,9 @@ struct perf_event_mmap_page {
> * if (pc->cap_user_rdpmc && index) {
> * width = pc->pmc_width;
> * pmc = rdpmc(index - 1);
> + * pmc <<= 64 - width;
> + * pmc >>= 64 - width;
> + * count += pmc;
> * }
> *
> * barrier();
> @@ -590,25 +597,27 @@ struct perf_event_mmap_page {
> * If cap_usr_time the below fields can be used to compute the time
> * delta since time_enabled (in ns) using rdtsc or similar.
> *
> - * u64 quot, rem;
> - * u64 delta;
> - *
> - * quot = (cyc >> time_shift);
> - * rem = cyc & (((u64)1 << time_shift) - 1);
> - * delta = time_offset + quot * time_mult +
> - * ((rem * time_mult) >> time_shift);
> + * cyc = time_cycles + ((cyc - time_cycles) & time_mask);
> + * delta = time_offset + mul_u64_u32_shr(cyc, time_mult, time_shift);
> *
> * Where time_offset,time_mult,time_shift and cyc are read in the
> - * seqcount loop described above. This delta can then be added to
> - * enabled and possible running (if index), improving the scaling:
> + * seqcount loop described above. mul_u64_u32_shr will compute:
> + *
> + * (u64)(((unsigned __int128)cyc * time_mult) >> time_shift)
> + *
> + * This delta can then be added to enabled and possible running (if
> + * index) to improve the scaling. Due to event multiplexing, running
> + * may be zero and so care is needed to avoid division by zero.
> *
> * enabled += delta;
> * if (index)
> * running += delta;
> *
> - * quot = count / running;
> - * rem = count % running;
> - * count = quot * enabled + (rem * enabled) / running;
> + * if (running != 0) {
> + * quot = count / running;
> + * rem = count % running;
> + * count = quot * enabled + (rem * enabled) / running;
> + * }
> */
> __u16 time_shift;
> __u32 time_mult;
> diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
> index d37629dbad72..6826dabb7e03 100644
> --- a/tools/include/uapi/linux/perf_event.h
> +++ b/tools/include/uapi/linux/perf_event.h
> @@ -538,9 +538,13 @@ struct perf_event_mmap_page {
> *
> * if (pc->cap_usr_time && enabled != running) {
> * cyc = rdtsc();
> - * time_offset = pc->time_offset;
> * time_mult = pc->time_mult;
> * time_shift = pc->time_shift;
> + * time_offset = pc->time_offset;
> + * if (pc->cap_user_time_short) {
> + * time_cycles = pc->time_cycles;
> + * time_mask = pc->time_mask;
> + * }
> * }
> *
> * index = pc->index;
> @@ -548,6 +552,9 @@ struct perf_event_mmap_page {
> * if (pc->cap_user_rdpmc && index) {
> * width = pc->pmc_width;
> * pmc = rdpmc(index - 1);
> + * pmc <<= 64 - width;
> + * pmc >>= 64 - width;
> + * count += pmc;
> * }
> *
> * barrier();
> @@ -590,25 +597,27 @@ struct perf_event_mmap_page {
> * If cap_usr_time the below fields can be used to compute the time
> * delta since time_enabled (in ns) using rdtsc or similar.
> *
> - * u64 quot, rem;
> - * u64 delta;
> - *
> - * quot = (cyc >> time_shift);
> - * rem = cyc & (((u64)1 << time_shift) - 1);
> - * delta = time_offset + quot * time_mult +
> - * ((rem * time_mult) >> time_shift);
> + * cyc = time_cycles + ((cyc - time_cycles) & time_mask);
> + * delta = time_offset + mul_u64_u32_shr(cyc, time_mult, time_shift);
> *
> * Where time_offset,time_mult,time_shift and cyc are read in the
> - * seqcount loop described above. This delta can then be added to
> - * enabled and possible running (if index), improving the scaling:
> + * seqcount loop described above. mul_u64_u32_shr will compute:
> + *
> + * (u64)(((unsigned __int128)cyc * time_mult) >> time_shift)
> + *
> + * This delta can then be added to enabled and possible running (if
> + * index) to improve the scaling. Due to event multiplexing, running
> + * may be zero and so care is needed to avoid division by zero.
> *
> * enabled += delta;
> * if (index)
> * running += delta;
> *
> - * quot = count / running;
> - * rem = count % running;
> - * count = quot * enabled + (rem * enabled) / running;
> + * if (running != 0) {
> + * quot = count / running;
> + * rem = count % running;
> + * count = quot * enabled + (rem * enabled) / running;
> + * }
> */
> __u16 time_shift;
> __u32 time_mult;
> --
> 2.37.0.170.g444d1eabd0-goog
--
- Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/3] perf test: Remove x86 rdpmc test
2022-07-19 22:39 ` [PATCH v3 2/3] perf test: Remove x86 rdpmc test Ian Rogers
@ 2022-08-01 12:19 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-08-01 12:19 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Kajol Jain, Andi Kleen, Adrian Hunter,
Anshuman Khandual, linux-kernel, linux-perf-users, Rob Herring,
Stephane Eranian
Em Tue, Jul 19, 2022 at 03:39:45PM -0700, Ian Rogers escreveu:
> This test has been superseded by test_stat_user_read in:
> tools/lib/perf/tests/test-evsel.c
> The updated test doesn't divide-by-0 when running time of a counter is
> 0. It also supports ARM64.
Thanks, applied.
- Arnaldo
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/arch/x86/tests/Build | 1 -
> tools/perf/arch/x86/tests/arch-tests.c | 2 -
> tools/perf/arch/x86/tests/rdpmc.c | 182 -------------------------
> 3 files changed, 185 deletions(-)
> delete mode 100644 tools/perf/arch/x86/tests/rdpmc.c
>
> diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
> index 28d793390198..70b5bcbc15df 100644
> --- a/tools/perf/arch/x86/tests/Build
> +++ b/tools/perf/arch/x86/tests/Build
> @@ -2,7 +2,6 @@ perf-$(CONFIG_DWARF_UNWIND) += regs_load.o
> perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
>
> perf-y += arch-tests.o
> -perf-y += rdpmc.o
> perf-y += sample-parsing.o
> perf-$(CONFIG_AUXTRACE) += insn-x86.o intel-pt-pkt-decoder-test.o
> perf-$(CONFIG_X86_64) += bp-modify.o
> diff --git a/tools/perf/arch/x86/tests/arch-tests.c b/tools/perf/arch/x86/tests/arch-tests.c
> index 64fb73d14d2f..04018b8aa85b 100644
> --- a/tools/perf/arch/x86/tests/arch-tests.c
> +++ b/tools/perf/arch/x86/tests/arch-tests.c
> @@ -3,7 +3,6 @@
> #include "tests/tests.h"
> #include "arch-tests.h"
>
> -DEFINE_SUITE("x86 rdpmc", rdpmc);
> #ifdef HAVE_AUXTRACE_SUPPORT
> DEFINE_SUITE("x86 instruction decoder - new instructions", insn_x86);
> DEFINE_SUITE("Intel PT packet decoder", intel_pt_pkt_decoder);
> @@ -14,7 +13,6 @@ DEFINE_SUITE("x86 bp modify", bp_modify);
> DEFINE_SUITE("x86 Sample parsing", x86_sample_parsing);
>
> struct test_suite *arch_tests[] = {
> - &suite__rdpmc,
> #ifdef HAVE_DWARF_UNWIND_SUPPORT
> &suite__dwarf_unwind,
> #endif
> diff --git a/tools/perf/arch/x86/tests/rdpmc.c b/tools/perf/arch/x86/tests/rdpmc.c
> deleted file mode 100644
> index 498413ad9c97..000000000000
> --- a/tools/perf/arch/x86/tests/rdpmc.c
> +++ /dev/null
> @@ -1,182 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <errno.h>
> -#include <unistd.h>
> -#include <stdlib.h>
> -#include <signal.h>
> -#include <sys/mman.h>
> -#include <sys/types.h>
> -#include <sys/wait.h>
> -#include <linux/string.h>
> -#include <linux/types.h>
> -#include "perf-sys.h"
> -#include "debug.h"
> -#include "tests/tests.h"
> -#include "cloexec.h"
> -#include "event.h"
> -#include <internal/lib.h> // page_size
> -#include "arch-tests.h"
> -
> -static u64 rdpmc(unsigned int counter)
> -{
> - unsigned int low, high;
> -
> - asm volatile("rdpmc" : "=a" (low), "=d" (high) : "c" (counter));
> -
> - return low | ((u64)high) << 32;
> -}
> -
> -static u64 rdtsc(void)
> -{
> - unsigned int low, high;
> -
> - asm volatile("rdtsc" : "=a" (low), "=d" (high));
> -
> - return low | ((u64)high) << 32;
> -}
> -
> -static u64 mmap_read_self(void *addr)
> -{
> - struct perf_event_mmap_page *pc = addr;
> - u32 seq, idx, time_mult = 0, time_shift = 0;
> - u64 count, cyc = 0, time_offset = 0, enabled, running, delta;
> -
> - do {
> - seq = pc->lock;
> - barrier();
> -
> - enabled = pc->time_enabled;
> - running = pc->time_running;
> -
> - if (enabled != running) {
> - cyc = rdtsc();
> - time_mult = pc->time_mult;
> - time_shift = pc->time_shift;
> - time_offset = pc->time_offset;
> - }
> -
> - idx = pc->index;
> - count = pc->offset;
> - if (idx)
> - count += rdpmc(idx - 1);
> -
> - barrier();
> - } while (pc->lock != seq);
> -
> - if (enabled != running) {
> - u64 quot, rem;
> -
> - quot = (cyc >> time_shift);
> - rem = cyc & (((u64)1 << time_shift) - 1);
> - delta = time_offset + quot * time_mult +
> - ((rem * time_mult) >> time_shift);
> -
> - enabled += delta;
> - if (idx)
> - running += delta;
> -
> - quot = count / running;
> - rem = count % running;
> - count = quot * enabled + (rem * enabled) / running;
> - }
> -
> - return count;
> -}
> -
> -/*
> - * If the RDPMC instruction faults then signal this back to the test parent task:
> - */
> -static void segfault_handler(int sig __maybe_unused,
> - siginfo_t *info __maybe_unused,
> - void *uc __maybe_unused)
> -{
> - exit(-1);
> -}
> -
> -static int __test__rdpmc(void)
> -{
> - volatile int tmp = 0;
> - u64 i, loops = 1000;
> - int n;
> - int fd;
> - void *addr;
> - struct perf_event_attr attr = {
> - .type = PERF_TYPE_HARDWARE,
> - .config = PERF_COUNT_HW_INSTRUCTIONS,
> - .exclude_kernel = 1,
> - };
> - u64 delta_sum = 0;
> - struct sigaction sa;
> - char sbuf[STRERR_BUFSIZE];
> -
> - sigfillset(&sa.sa_mask);
> - sa.sa_sigaction = segfault_handler;
> - sa.sa_flags = 0;
> - sigaction(SIGSEGV, &sa, NULL);
> -
> - fd = sys_perf_event_open(&attr, 0, -1, -1,
> - perf_event_open_cloexec_flag());
> - if (fd < 0) {
> - pr_err("Error: sys_perf_event_open() syscall returned "
> - "with %d (%s)\n", fd,
> - str_error_r(errno, sbuf, sizeof(sbuf)));
> - return -1;
> - }
> -
> - addr = mmap(NULL, page_size, PROT_READ, MAP_SHARED, fd, 0);
> - if (addr == (void *)(-1)) {
> - pr_err("Error: mmap() syscall returned with (%s)\n",
> - str_error_r(errno, sbuf, sizeof(sbuf)));
> - goto out_close;
> - }
> -
> - for (n = 0; n < 6; n++) {
> - u64 stamp, now, delta;
> -
> - stamp = mmap_read_self(addr);
> -
> - for (i = 0; i < loops; i++)
> - tmp++;
> -
> - now = mmap_read_self(addr);
> - loops *= 10;
> -
> - delta = now - stamp;
> - pr_debug("%14d: %14Lu\n", n, (long long)delta);
> -
> - delta_sum += delta;
> - }
> -
> - munmap(addr, page_size);
> - pr_debug(" ");
> -out_close:
> - close(fd);
> -
> - if (!delta_sum)
> - return -1;
> -
> - return 0;
> -}
> -
> -int test__rdpmc(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
> -{
> - int status = 0;
> - int wret = 0;
> - int ret;
> - int pid;
> -
> - pid = fork();
> - if (pid < 0)
> - return -1;
> -
> - if (!pid) {
> - ret = __test__rdpmc();
> -
> - exit(ret);
> - }
> -
> - wret = waitpid(pid, &status, 0);
> - if (wret < 0 || status)
> - return -1;
> -
> - return 0;
> -}
> --
> 2.37.0.170.g444d1eabd0-goog
--
- Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] perf test: Add user space counter reading tests
2022-07-19 22:39 ` [PATCH v3 3/3] perf test: Add user space counter reading tests Ian Rogers
2022-07-20 14:57 ` Vince Weaver
@ 2022-08-01 12:23 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-08-01 12:23 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Kajol Jain, Andi Kleen, Adrian Hunter,
Anshuman Khandual, linux-kernel, linux-perf-users, Rob Herring,
Stephane Eranian
Em Tue, Jul 19, 2022 at 03:39:46PM -0700, Ian Rogers escreveu:
> These tests are based on test_stat_user_read in
> tools/lib/perf/tests/test-evsel.c. The tests are modified to skip if
> perf_event_open fails or rdpmc isn't supported.
Thanks, tested and applied.
- Arnaldo
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/tests/mmap-basic.c | 127 +++++++++++++++++++++++++++++++++-
> 1 file changed, 126 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
> index 30bbe144648a..dfb6173b2a82 100644
> --- a/tools/perf/tests/mmap-basic.c
> +++ b/tools/perf/tests/mmap-basic.c
> @@ -170,14 +170,139 @@ static int test__basic_mmap(struct test_suite *test __maybe_unused, int subtest
> return err;
> }
>
> +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,
> +#ifdef __aarch64__
> + .config1 = 0x2, /* Request user access */
> +#endif
> + };
> + int err, i, ret = TEST_FAIL;
> + bool opened = false, mapped = false;
> +
> + threads = perf_thread_map__new_dummy();
> + TEST_ASSERT_VAL("failed to create threads", threads);
> +
> + perf_thread_map__set_pid(threads, 0, 0);
> +
> + evsel = perf_evsel__new(&attr);
> + TEST_ASSERT_VAL("failed to create evsel", evsel);
> +
> + err = perf_evsel__open(evsel, NULL, threads);
> + if (err) {
> + pr_err("failed to open evsel: %s\n", strerror(-err));
> + ret = TEST_SKIP;
> + goto out;
> + }
> + opened = true;
> +
> + err = perf_evsel__mmap(evsel, 0);
> + if (err) {
> + pr_err("failed to mmap evsel: %s\n", strerror(-err));
> + goto out;
> + }
> + mapped = true;
> +
> + pc = perf_evsel__mmap_base(evsel, 0, 0);
> + if (!pc) {
> + pr_err("failed to get mmapped address\n");
> + goto out;
> + }
> +
> + if (!pc->cap_user_rdpmc || !pc->index) {
> + pr_err("userspace counter access not %s\n",
> + !pc->cap_user_rdpmc ? "supported" : "enabled");
> + ret = TEST_SKIP;
> + goto out;
> + }
> + if (pc->pmc_width < 32) {
> + pr_err("userspace counter width not set (%d)\n", pc->pmc_width);
> + goto out;
> + }
> +
> + perf_evsel__read(evsel, 0, 0, &counts);
> + if (counts.val == 0) {
> + pr_err("failed to read value for evsel\n");
> + goto out;
> + }
> +
> + for (i = 0; i < 5; i++) {
> + volatile int count = 0x10000 << i;
> + __u64 start, end, last = 0;
> +
> + pr_debug("\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;
> +
> + if ((end - start) < last) {
> + pr_err("invalid counter data: end=%llu start=%llu last= %llu\n",
> + end, start, last);
> + goto out;
> + }
> + last = end - start;
> + pr_debug("count = %llu\n", end - start);
> + }
> + ret = TEST_OK;
> +
> +out:
> + if (mapped)
> + perf_evsel__munmap(evsel);
> + if (opened)
> + perf_evsel__close(evsel);
> + perf_evsel__delete(evsel);
> +
> + perf_thread_map__put(threads);
> + return ret;
> +}
> +
> +static int test__mmap_user_read_instr(struct test_suite *test __maybe_unused,
> + int subtest __maybe_unused)
> +{
> + return test_stat_user_read(PERF_COUNT_HW_INSTRUCTIONS);
> +}
> +
> +static int test__mmap_user_read_cycles(struct test_suite *test __maybe_unused,
> + int subtest __maybe_unused)
> +{
> + return test_stat_user_read(PERF_COUNT_HW_CPU_CYCLES);
> +}
> +
> static struct test_case tests__basic_mmap[] = {
> TEST_CASE_REASON("Read samples using the mmap interface",
> basic_mmap,
> "permissions"),
> + TEST_CASE_REASON("User space counter reading of instructions",
> + mmap_user_read_instr,
> +#if defined(__i386__) || defined(__x86_64__) || defined(__aarch64__)
> + "permissions"
> +#else
> + "unsupported"
> +#endif
> + ),
> + TEST_CASE_REASON("User space counter reading of cycles",
> + mmap_user_read_cycles,
> +#if defined(__i386__) || defined(__x86_64__) || defined(__aarch64__)
> + "permissions"
> +#else
> + "unsupported"
> +#endif
> + ),
> { .name = NULL, }
> };
>
> struct test_suite suite__basic_mmap = {
> - .desc = "Read samples using the mmap interface",
> + .desc = "mmap interface tests",
> .test_cases = tests__basic_mmap,
> };
> --
> 2.37.0.170.g444d1eabd0-goog
--
- Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] perf: Align user space counter reading with code
2022-07-20 15:06 ` Vince Weaver
2022-07-20 15:18 ` Ian Rogers
@ 2022-08-04 8:49 ` Ingo Molnar
1 sibling, 0 replies; 13+ messages in thread
From: Ingo Molnar @ 2022-08-04 8:49 UTC (permalink / raw)
To: Vince Weaver
Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Kajol Jain, Andi Kleen, Adrian Hunter,
Anshuman Khandual, linux-kernel, linux-perf-users, Rob Herring,
Stephane Eranian
* Vince Weaver <vincent.weaver@maine.edu> wrote:
> On Tue, 19 Jul 2022, Ian Rogers wrote:
>
> > Align the user space counter reading documentation with the code in
> > perf_mmap__read_self. Previously the documentation was based on the perf
> > rdpmc test, but now general purpose code is provided by libperf.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
> > tools/include/uapi/linux/perf_event.h | 35 +++++++++++++++++----------
> > 2 files changed, 44 insertions(+), 26 deletions(-)
> >
> > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> > index d37629dbad72..6826dabb7e03 100644
> > --- a/include/uapi/linux/perf_event.h
> > +++ b/include/uapi/linux/perf_event.h
> > @@ -538,9 +538,13 @@ struct perf_event_mmap_page {
> > *
> > * if (pc->cap_usr_time && enabled != running) {
> > * cyc = rdtsc();
> > - * time_offset = pc->time_offset;
> > * time_mult = pc->time_mult;
> > * time_shift = pc->time_shift;
> > + * time_offset = pc->time_offset;
> > + * if (pc->cap_user_time_short) {
> > + * time_cycles = pc->time_cycles;
> > + * time_mask = pc->time_mask;
> > + * }
>
> From what I've been told, and from what perf_mmap__read_self() actually
> does, many of these MMAP fields need to be accessed by READ_ONCE()
> (a GPLv2 only interface) to be correct.
BTW., for this purpose I guess we could add a READ_ONCE() variant to perf
tooling that reimplements it with more relaxed licensing, so that the
headers & sample code can be included in GPL-incompatible projects?
READ_ONCE() isn't a super complicated primitive, and we've always been
permissive with simple primitives.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-08-04 8:49 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-19 22:39 [PATCH v3 0/3] Tidy user rdpmc documentation and testing Ian Rogers
2022-07-19 22:39 ` [PATCH v3 1/3] perf: Align user space counter reading with code Ian Rogers
2022-07-19 23:05 ` Rob Herring
2022-07-20 15:06 ` Vince Weaver
2022-07-20 15:18 ` Ian Rogers
2022-08-04 8:49 ` Ingo Molnar
2022-08-01 12:17 ` Arnaldo Carvalho de Melo
2022-07-19 22:39 ` [PATCH v3 2/3] perf test: Remove x86 rdpmc test Ian Rogers
2022-08-01 12:19 ` Arnaldo Carvalho de Melo
2022-07-19 22:39 ` [PATCH v3 3/3] perf test: Add user space counter reading tests Ian Rogers
2022-07-20 14:57 ` Vince Weaver
2022-07-20 15:09 ` Ian Rogers
2022-08-01 12:23 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).