mbox series

[0/1] perf: fix reset interface potential failure

Message ID 20221118020016.1571100-1-yamada.masahiko@fujitsu.com
Headers
Series perf: fix reset interface potential failure |

Message

Masahiko Yamada (Fujitsu) Nov. 18, 2022, 2 a.m. UTC 
  There is a potential bug where PERF_EVENT_IOC_RESET
does not work when accessing PMU registers directly
from userspace in the perf_event interface.

In the x86 environment, the kernel(perf_event reset handling) has a
potential failure, but it works with the papi library side workaround.
The PMU register direct access feature from user space was implemented in
the perf_event facility from linux-5.18 version in the arm64 environment,
but it does not work with the workaround on the papi library side in the
arm64 environment.
The workaround worked in the x86 environment and not in the arm64
environment because in the arm64 environment, only CPU_CYCLES was
a 64 bit counter and the rest were 32 bit counters, so the workaround
cleared the upper 32 bits of the value measured by CPU_CYCLES.

For this reason, we have created a patch on the kernel
that fixes a potential perf_event reset failure.

The motivation for the fix is to initialize pc->offset.
The perf_mmap__read_self function in tools/lib/perf/mmap.c is set by:.
cnt = READ_ONCE(pc->offset);
The pc->offset value is set in the following process
in the perf_event_update_userpage function:.
userpg->offset -= local64_read(&event->hw.prev_count);
hw->prev_count is set in the armpmu_event_set_period function
in drivers/perf/arm_pmu.c and in the x86_perf_event_set_period function
in arch/x86/events/core.c as follows:.
local64_set(&hwc->prev_count, (u64)-left);

Therefore, this patch was created to initialize hwc->prev_count
during reset processing.

We used
https://github.com/deater/perf_event_tests/blob/master/tests/rdpmc/\
rdpmc_reset.c for verification testing.

The mmap_read_ self function in
https://github.com/deater/perf_event_tests/blob/master/tests/rdpmc/\
rdpmc_lib.c comes with a fix (lines 158 to 159) similar to
the workaround in the papi library.

The mmap_read_self function implements the equivalent of
the perf_mmap__read_self function.

The following is a workaround for the mmap_read_self function.

-----SNIP-----
    158                 count<<=(64-width);
    159                 count>>=(64-width);
-----SNIP-----

If you do not apply the kernel patch to fix a potential perf_event
failure, removing the workaround from perf_event_tests will cause
both the x86/arm64 versions to fail.

$ ./rdpmc_reset
This test checks if userspace rdpmc() style reads work.

total start/read/stop latency: 71772447 cycles
 Event 0 -- Raw count: 100069445 enabled: 6636 running: 6636
 Event 0 -- Raw count: -281474876643918 enabled: 16399496 running: 16399496
 Event 1 -- Raw count: 100069412 enabled: 2071 running: 2071
 Event 1 -- Raw count: -281474876645181 enabled: 16394819 running: 16394819

 Expected: 100000000
 High: -281474876643918 Low: -281474876643918 Average: -281474876643918
 ( note, a small value above 100000000 may be expected due
   to overhead and interrupt noise, among other reasons)
 Average Error = -281474976.64%
Error out of range!
Testing if resetting while using rdpmc works...            FAILED

In the environment with the kernel patch to fix a potential perf_event
failure, removing the workaround from perf_event_tests still works with
the x86/arm64 version.

$ ./rdpmc_reset
This test checks if userspace rdpmc() style reads work.

total start/read/stop latency: 71863055 cycles
 Event 0 -- Raw count: 100081688 enabled: 10353 running: 10353
 Event 0 -- Raw count: 100078316 enabled: 16493263 running: 16493263
 Event 1 -- Raw count: 100081789 enabled: 4186 running: 4186
 Event 1 -- Raw count: 100076770 enabled: 16486696 running: 16486696

 Expected: 100000000
 High: 100078316   Low:  100078316   Average:  100078316
 ( note, a small value above 100000000 may be expected due
   to overhead and interrupt noise, among other reasons)
 Average Error = 0.08%
Testing if resetting while using rdpmc works...            PASSED

Masahiko, Yamada (1):
  perf: fix reset interface potential failure

 kernel/events/core.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Masahiko Yamada (Fujitsu) Jan. 17, 2023, 6:40 a.m. UTC | #1 
Dear all

> There is a potential bug where PERF_EVENT_IOC_RESET
> does not work when accessing PMU registers directly
> from userspace in the perf_event interface.
> 
> In the x86 environment, the kernel(perf_event reset handling) has a
> potential failure, but it works with the papi library side workaround.
> The PMU register direct access feature from user space was implemented in
> the perf_event facility from linux-5.18 version in the arm64 environment,
> but it does not work with the workaround on the papi library side in the
> arm64 environment.
> The workaround worked in the x86 environment and not in the arm64
> environment because in the arm64 environment, only CPU_CYCLES was
> a 64 bit counter and the rest were 32 bit counters, so the workaround
> cleared the upper 32 bits of the value measured by CPU_CYCLES.
> 
> For this reason, we have created a patch on the kernel
> that fixes a potential perf_event reset failure.

I have not received any comments or objections to this patch so far, 
but could you give me any comments on this patch?

Best regards,
Masahiko Yamada