On Fri, 27 Aug 2021, Andy Lutomirski wrote:

On Thu, Aug 26, 2021, at 12:09 PM, Rob Herring wrote:

quoted

After testing some scenarios and finding perf_event_tests[1], this
series isn't going to work for x86 unless rdpmc is restricted to task
events only or allowed to segfault on CPU events when read on the
wrong CPU rather than just returning garbage. It's been discussed
before here[2].

Ultimately, I'm just trying to define the behavior for arm64 where we
don't have an existing ABI to maintain and don't have to recreate the
mistakes of x86 rdpmc ABI. Tying the access to mmap is messy. As we
explicitly request user access on perf_event_open(), I think it may be
better to just enable access when the event's context is active and
ignore mmap(). Maybe you have an opinion there since you added the
mmap() part?

That makes sense to me. The mmap() part was always a giant kludge.

There is fundamentally a race, at least if rseq isn’t used: if you check 
that you’re on the right CPU, do RDPMC, and throw out the result if you 
were on the wrong CPU (determined by looking at the mmap), you still 
would very much prefer not to fault.

Maybe rseq or a vDSO helper is the right solution for ARM.

as the author of those perf_event tests for rdpmc, I have to say if ARM 
comes up with a cleaner implementation I'd be glad to have x86 transition 
to something better.

The rdpmc code is a huge mess and has all kinds of corner cases.  I'm not 
sure anyone besides the PAPI library tries to use it, and while it's a 
nice performance improvement to use rdpmc it is really hard to get things 
working right.

As a PAPI developer we actually have run into the issue where the CPU 
switches and we were reporting the wrong results.  Also if I recall (it's 
been a while) we were having issues where the setup lets you attach to a 
process on another CPU for monitoring using the rdpmc interface and it 
returns results even though I think that will rarely ever work in 
practice.

Vince