* [PATCH v3] perf tests: mmap-basic: fix user rdpmc detection logic
@ 2026-03-02 5:55 Qiao Zhao
2026-03-05 3:08 ` Qiao Zhao
0 siblings, 1 reply; 3+ messages in thread
From: Qiao Zhao @ 2026-03-02 5:55 UTC (permalink / raw)
To: james.clark, irogers
Cc: linux-perf-users, leo.yan, namhyung, acme, mpetlan, Qiao Zhao
The mmap-basic test incorrectly determined rdpmc availability in
several environments, leading to unexpected failures on arm64 and
other architectures.
Previously the rdpmc capability decision lived in test_stat_user_read(),
which caused inconsistent behaviour when:
- perf_user_access sysctl state is unknown
- architectures expose cap_user_rdpmc differently
- arm64 platforms where rdpmc semantics differ from x86
As suggested during review, move the rdpmc capability decision into
set_user_read() so that the user_read state and expected behavior
are decided in a single place.
Changes in v3:
- Moves perf_user_access handling into set_user_read()
- Simplify test_stat_user_read() expectation logic
- Use unified rdpmc_supported calculation
- Handle USER_READ_UNKNOWN consistently
v2:
https://lore.kernel.org/linux-perf-users/20260203141608.14128-1-qzhao@redhat.com/
Note:
A potential cleanup around perf_event.h mentioned during review is
intentionally deferred and will be handled in a follow-up change
to keep this fix minimal and focused.
Tested on:
- ARM64 (armv8_pmuv3): all mmap-basic user-space counter tests pass
- X86(include hybrid): all mmap-basic tests pass
- IBM Power9: all mmap-basic tests pass
Signed-off-by: Qiao Zhao <qzhao@redhat.com>
---
tools/perf/tests/mmap-basic.c | 55 +++++++++++++++++++++++++----------
1 file changed, 39 insertions(+), 16 deletions(-)
diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
index 3313c236104e..3633e7c87dc1 100644
--- a/tools/perf/tests/mmap-basic.c
+++ b/tools/perf/tests/mmap-basic.c
@@ -185,38 +185,57 @@ static enum user_read_state set_user_read(struct perf_pmu *pmu, enum user_read_s
{
char buf[2] = {0, '\n'};
ssize_t len;
- int events_fd, rdpmc_fd;
+ int events_fd, fd;
enum user_read_state old_user_read = USER_READ_UNKNOWN;
if (enabled == USER_READ_UNKNOWN)
return USER_READ_UNKNOWN;
+ // Try the PMU rdpmc sysfs interface or similar
events_fd = perf_pmu__event_source_devices_fd();
- if (events_fd < 0)
- return USER_READ_UNKNOWN;
-
- rdpmc_fd = perf_pmu__pathname_fd(events_fd, pmu->name, "rdpmc", O_RDWR);
- if (rdpmc_fd < 0) {
+ if (events_fd >= 0) {
+ fd = perf_pmu__pathname_fd(events_fd, pmu->name, "rdpmc", O_RDWR);
+ if (fd >= 0) {
+ len = read(fd, buf, sizeof(buf));
+ if (len == sizeof(buf))
+ old_user_read = (buf[0] == '1') ?
+ USER_READ_ENABLED :
+ USER_READ_DISABLED;
+
+ if (enabled != old_user_read) {
+ buf[0] = (enabled == USER_READ_ENABLED) ? '1' : '0';
+ len = write(fd, buf, sizeof(buf));
+ if (len != sizeof(buf))
+ pr_debug("%s write failed\n", __func__);
+ }
+ close(fd);
+ close(events_fd);
+ return old_user_read;
+ }
close(events_fd);
- return USER_READ_UNKNOWN;
}
- len = read(rdpmc_fd, buf, sizeof(buf));
- if (len != sizeof(buf))
- pr_debug("%s read failed\n", __func__);
+ // Fallback: perf_user_access interface (arm64, riscv, or similar)
+ fd = open("/proc/sys/kernel/perf_user_access", O_RDWR);
+ if (fd < 0)
+ return USER_READ_UNKNOWN;
// Note, on Intel hybrid disabling on 1 PMU will implicitly disable on
// all the core PMUs.
- old_user_read = (buf[0] == '1') ? USER_READ_ENABLED : USER_READ_DISABLED;
+ len = read(fd, buf, sizeof(buf));
+ if (len == sizeof(buf))
+ old_user_read = (buf[0] == '1') ?
+ USER_READ_ENABLED :
+ USER_READ_DISABLED;
if (enabled != old_user_read) {
buf[0] = (enabled == USER_READ_ENABLED) ? '1' : '0';
- len = write(rdpmc_fd, buf, sizeof(buf));
+ len = write(fd, buf, sizeof(buf));
if (len != sizeof(buf))
pr_debug("%s write failed\n", __func__);
}
- close(rdpmc_fd);
- close(events_fd);
+
+ close(fd);
return old_user_read;
}
@@ -295,12 +314,16 @@ static int test_stat_user_read(u64 event, enum user_read_state enabled)
goto cleanup;
}
+#if defined(__aarch64__) || defined(__riscv)
+ rdpmc_supported = pc->cap_user_rdpmc;
+#else
if (saved_user_read_state == USER_READ_UNKNOWN)
- rdpmc_supported = pc->cap_user_rdpmc && pc->index;
+ rdpmc_supported = pc->cap_user_rdpmc;
else
rdpmc_supported = (enabled == USER_READ_ENABLED);
+#endif
- if (rdpmc_supported && (!pc->cap_user_rdpmc || !pc->index)) {
+ if (rdpmc_supported && !pc->cap_user_rdpmc) {
pr_err("User space counter reading for PMU %s [Failed unexpected supported counter access %d %d]\n",
pmu->name, pc->cap_user_rdpmc, pc->index);
ret = TEST_FAIL;
--
2.49.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3] perf tests: mmap-basic: fix user rdpmc detection logic
2026-03-02 5:55 [PATCH v3] perf tests: mmap-basic: fix user rdpmc detection logic Qiao Zhao
@ 2026-03-05 3:08 ` Qiao Zhao
2026-03-11 6:41 ` Qiao Zhao
0 siblings, 1 reply; 3+ messages in thread
From: Qiao Zhao @ 2026-03-05 3:08 UTC (permalink / raw)
To: james.clark, irogers; +Cc: linux-perf-users, leo.yan, namhyung, acme, mpetlan
Hi James, Ian,
Do you mind reviewing this v3 change? Thanks a lot!
- Qiao
On Mon, Mar 2, 2026 at 1:56 PM Qiao Zhao <qzhao@redhat.com> wrote:
>
> The mmap-basic test incorrectly determined rdpmc availability in
> several environments, leading to unexpected failures on arm64 and
> other architectures.
>
> Previously the rdpmc capability decision lived in test_stat_user_read(),
> which caused inconsistent behaviour when:
> - perf_user_access sysctl state is unknown
> - architectures expose cap_user_rdpmc differently
> - arm64 platforms where rdpmc semantics differ from x86
>
> As suggested during review, move the rdpmc capability decision into
> set_user_read() so that the user_read state and expected behavior
> are decided in a single place.
>
> Changes in v3:
> - Moves perf_user_access handling into set_user_read()
> - Simplify test_stat_user_read() expectation logic
> - Use unified rdpmc_supported calculation
> - Handle USER_READ_UNKNOWN consistently
>
> v2:
> https://lore.kernel.org/linux-perf-users/20260203141608.14128-1-qzhao@redhat.com/
>
> Note:
> A potential cleanup around perf_event.h mentioned during review is
> intentionally deferred and will be handled in a follow-up change
> to keep this fix minimal and focused.
>
> Tested on:
> - ARM64 (armv8_pmuv3): all mmap-basic user-space counter tests pass
> - X86(include hybrid): all mmap-basic tests pass
> - IBM Power9: all mmap-basic tests pass
>
> Signed-off-by: Qiao Zhao <qzhao@redhat.com>
> ---
> tools/perf/tests/mmap-basic.c | 55 +++++++++++++++++++++++++----------
> 1 file changed, 39 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
> index 3313c236104e..3633e7c87dc1 100644
> --- a/tools/perf/tests/mmap-basic.c
> +++ b/tools/perf/tests/mmap-basic.c
> @@ -185,38 +185,57 @@ static enum user_read_state set_user_read(struct perf_pmu *pmu, enum user_read_s
> {
> char buf[2] = {0, '\n'};
> ssize_t len;
> - int events_fd, rdpmc_fd;
> + int events_fd, fd;
> enum user_read_state old_user_read = USER_READ_UNKNOWN;
>
> if (enabled == USER_READ_UNKNOWN)
> return USER_READ_UNKNOWN;
>
> + // Try the PMU rdpmc sysfs interface or similar
> events_fd = perf_pmu__event_source_devices_fd();
> - if (events_fd < 0)
> - return USER_READ_UNKNOWN;
> -
> - rdpmc_fd = perf_pmu__pathname_fd(events_fd, pmu->name, "rdpmc", O_RDWR);
> - if (rdpmc_fd < 0) {
> + if (events_fd >= 0) {
> + fd = perf_pmu__pathname_fd(events_fd, pmu->name, "rdpmc", O_RDWR);
> + if (fd >= 0) {
> + len = read(fd, buf, sizeof(buf));
> + if (len == sizeof(buf))
> + old_user_read = (buf[0] == '1') ?
> + USER_READ_ENABLED :
> + USER_READ_DISABLED;
> +
> + if (enabled != old_user_read) {
> + buf[0] = (enabled == USER_READ_ENABLED) ? '1' : '0';
> + len = write(fd, buf, sizeof(buf));
> + if (len != sizeof(buf))
> + pr_debug("%s write failed\n", __func__);
> + }
> + close(fd);
> + close(events_fd);
> + return old_user_read;
> + }
> close(events_fd);
> - return USER_READ_UNKNOWN;
> }
>
> - len = read(rdpmc_fd, buf, sizeof(buf));
> - if (len != sizeof(buf))
> - pr_debug("%s read failed\n", __func__);
> + // Fallback: perf_user_access interface (arm64, riscv, or similar)
> + fd = open("/proc/sys/kernel/perf_user_access", O_RDWR);
> + if (fd < 0)
> + return USER_READ_UNKNOWN;
>
> // Note, on Intel hybrid disabling on 1 PMU will implicitly disable on
> // all the core PMUs.
> - old_user_read = (buf[0] == '1') ? USER_READ_ENABLED : USER_READ_DISABLED;
> + len = read(fd, buf, sizeof(buf));
> + if (len == sizeof(buf))
> + old_user_read = (buf[0] == '1') ?
> + USER_READ_ENABLED :
> + USER_READ_DISABLED;
>
> if (enabled != old_user_read) {
> buf[0] = (enabled == USER_READ_ENABLED) ? '1' : '0';
> - len = write(rdpmc_fd, buf, sizeof(buf));
> + len = write(fd, buf, sizeof(buf));
> if (len != sizeof(buf))
> pr_debug("%s write failed\n", __func__);
> }
> - close(rdpmc_fd);
> - close(events_fd);
> +
> + close(fd);
> return old_user_read;
> }
>
> @@ -295,12 +314,16 @@ static int test_stat_user_read(u64 event, enum user_read_state enabled)
> goto cleanup;
> }
>
> +#if defined(__aarch64__) || defined(__riscv)
> + rdpmc_supported = pc->cap_user_rdpmc;
> +#else
> if (saved_user_read_state == USER_READ_UNKNOWN)
> - rdpmc_supported = pc->cap_user_rdpmc && pc->index;
> + rdpmc_supported = pc->cap_user_rdpmc;
> else
> rdpmc_supported = (enabled == USER_READ_ENABLED);
> +#endif
>
> - if (rdpmc_supported && (!pc->cap_user_rdpmc || !pc->index)) {
> + if (rdpmc_supported && !pc->cap_user_rdpmc) {
> pr_err("User space counter reading for PMU %s [Failed unexpected supported counter access %d %d]\n",
> pmu->name, pc->cap_user_rdpmc, pc->index);
> ret = TEST_FAIL;
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3] perf tests: mmap-basic: fix user rdpmc detection logic
2026-03-05 3:08 ` Qiao Zhao
@ 2026-03-11 6:41 ` Qiao Zhao
0 siblings, 0 replies; 3+ messages in thread
From: Qiao Zhao @ 2026-03-11 6:41 UTC (permalink / raw)
To: james.clark, irogers; +Cc: linux-perf-users, leo.yan, namhyung, acme, mpetlan
Hi James, Ian,
Just a gentle ping on this patch.
Could you take a look when convenient?
Thanks,
Qiao
On Thu, Mar 5, 2026 at 11:08 AM Qiao Zhao <qzhao@redhat.com> wrote:
>
> Hi James, Ian,
>
> Do you mind reviewing this v3 change? Thanks a lot!
>
> - Qiao
>
> On Mon, Mar 2, 2026 at 1:56 PM Qiao Zhao <qzhao@redhat.com> wrote:
> >
> > The mmap-basic test incorrectly determined rdpmc availability in
> > several environments, leading to unexpected failures on arm64 and
> > other architectures.
> >
> > Previously the rdpmc capability decision lived in test_stat_user_read(),
> > which caused inconsistent behaviour when:
> > - perf_user_access sysctl state is unknown
> > - architectures expose cap_user_rdpmc differently
> > - arm64 platforms where rdpmc semantics differ from x86
> >
> > As suggested during review, move the rdpmc capability decision into
> > set_user_read() so that the user_read state and expected behavior
> > are decided in a single place.
> >
> > Changes in v3:
> > - Moves perf_user_access handling into set_user_read()
> > - Simplify test_stat_user_read() expectation logic
> > - Use unified rdpmc_supported calculation
> > - Handle USER_READ_UNKNOWN consistently
> >
> > v2:
> > https://lore.kernel.org/linux-perf-users/20260203141608.14128-1-qzhao@redhat.com/
> >
> > Note:
> > A potential cleanup around perf_event.h mentioned during review is
> > intentionally deferred and will be handled in a follow-up change
> > to keep this fix minimal and focused.
> >
> > Tested on:
> > - ARM64 (armv8_pmuv3): all mmap-basic user-space counter tests pass
> > - X86(include hybrid): all mmap-basic tests pass
> > - IBM Power9: all mmap-basic tests pass
> >
> > Signed-off-by: Qiao Zhao <qzhao@redhat.com>
> > ---
> > tools/perf/tests/mmap-basic.c | 55 +++++++++++++++++++++++++----------
> > 1 file changed, 39 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
> > index 3313c236104e..3633e7c87dc1 100644
> > --- a/tools/perf/tests/mmap-basic.c
> > +++ b/tools/perf/tests/mmap-basic.c
> > @@ -185,38 +185,57 @@ static enum user_read_state set_user_read(struct perf_pmu *pmu, enum user_read_s
> > {
> > char buf[2] = {0, '\n'};
> > ssize_t len;
> > - int events_fd, rdpmc_fd;
> > + int events_fd, fd;
> > enum user_read_state old_user_read = USER_READ_UNKNOWN;
> >
> > if (enabled == USER_READ_UNKNOWN)
> > return USER_READ_UNKNOWN;
> >
> > + // Try the PMU rdpmc sysfs interface or similar
> > events_fd = perf_pmu__event_source_devices_fd();
> > - if (events_fd < 0)
> > - return USER_READ_UNKNOWN;
> > -
> > - rdpmc_fd = perf_pmu__pathname_fd(events_fd, pmu->name, "rdpmc", O_RDWR);
> > - if (rdpmc_fd < 0) {
> > + if (events_fd >= 0) {
> > + fd = perf_pmu__pathname_fd(events_fd, pmu->name, "rdpmc", O_RDWR);
> > + if (fd >= 0) {
> > + len = read(fd, buf, sizeof(buf));
> > + if (len == sizeof(buf))
> > + old_user_read = (buf[0] == '1') ?
> > + USER_READ_ENABLED :
> > + USER_READ_DISABLED;
> > +
> > + if (enabled != old_user_read) {
> > + buf[0] = (enabled == USER_READ_ENABLED) ? '1' : '0';
> > + len = write(fd, buf, sizeof(buf));
> > + if (len != sizeof(buf))
> > + pr_debug("%s write failed\n", __func__);
> > + }
> > + close(fd);
> > + close(events_fd);
> > + return old_user_read;
> > + }
> > close(events_fd);
> > - return USER_READ_UNKNOWN;
> > }
> >
> > - len = read(rdpmc_fd, buf, sizeof(buf));
> > - if (len != sizeof(buf))
> > - pr_debug("%s read failed\n", __func__);
> > + // Fallback: perf_user_access interface (arm64, riscv, or similar)
> > + fd = open("/proc/sys/kernel/perf_user_access", O_RDWR);
> > + if (fd < 0)
> > + return USER_READ_UNKNOWN;
> >
> > // Note, on Intel hybrid disabling on 1 PMU will implicitly disable on
> > // all the core PMUs.
> > - old_user_read = (buf[0] == '1') ? USER_READ_ENABLED : USER_READ_DISABLED;
> > + len = read(fd, buf, sizeof(buf));
> > + if (len == sizeof(buf))
> > + old_user_read = (buf[0] == '1') ?
> > + USER_READ_ENABLED :
> > + USER_READ_DISABLED;
> >
> > if (enabled != old_user_read) {
> > buf[0] = (enabled == USER_READ_ENABLED) ? '1' : '0';
> > - len = write(rdpmc_fd, buf, sizeof(buf));
> > + len = write(fd, buf, sizeof(buf));
> > if (len != sizeof(buf))
> > pr_debug("%s write failed\n", __func__);
> > }
> > - close(rdpmc_fd);
> > - close(events_fd);
> > +
> > + close(fd);
> > return old_user_read;
> > }
> >
> > @@ -295,12 +314,16 @@ static int test_stat_user_read(u64 event, enum user_read_state enabled)
> > goto cleanup;
> > }
> >
> > +#if defined(__aarch64__) || defined(__riscv)
> > + rdpmc_supported = pc->cap_user_rdpmc;
> > +#else
> > if (saved_user_read_state == USER_READ_UNKNOWN)
> > - rdpmc_supported = pc->cap_user_rdpmc && pc->index;
> > + rdpmc_supported = pc->cap_user_rdpmc;
> > else
> > rdpmc_supported = (enabled == USER_READ_ENABLED);
> > +#endif
> >
> > - if (rdpmc_supported && (!pc->cap_user_rdpmc || !pc->index)) {
> > + if (rdpmc_supported && !pc->cap_user_rdpmc) {
> > pr_err("User space counter reading for PMU %s [Failed unexpected supported counter access %d %d]\n",
> > pmu->name, pc->cap_user_rdpmc, pc->index);
> > ret = TEST_FAIL;
> > --
> > 2.49.0
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-03-11 6:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-02 5:55 [PATCH v3] perf tests: mmap-basic: fix user rdpmc detection logic Qiao Zhao
2026-03-05 3:08 ` Qiao Zhao
2026-03-11 6:41 ` Qiao Zhao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox