Messages in this thread | | | From | Rob Herring <> | Date | Tue, 12 Jan 2021 10:16:50 -0600 | Subject | Re: [RFC] perf/x86: Only expose userspace rdpmc for events on current CPU |
| |
On Tue, Jan 12, 2021 at 9:33 AM Peter Zijlstra <peterz@infradead.org> wrote: > > On Thu, Jan 07, 2021 at 05:01:36PM -0700, Rob Herring wrote: > > Userspace access using rdpmc only makes sense if the event is valid for > > the current CPU. However, cap_user_rdpmc is currently set no matter which > > CPU the event is associated with. The result is userspace reading another > > CPU's event thinks it can use rdpmc to read the counter. In doing so, the > > wrong counter will be read. > > Don't do that then?
I could check this in userspace I suppose, but then it's yet another thing the rdpmc loop has to check. I think it's better to not add more overhead there.
Or we just ignore this. This issue came up in the referenced thread with Jiri. I'm not all that convinced that per CPU events and userspace rdpmc is too useful of a combination. It would only reduce the overhead on 1 out of N cpus. In this case, we'd have to limit the userspace read to per thread events (and ones on any cpu I suppose).
> > > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c > > index a88c94d65693..6e6d4c1d03ca 100644 > > --- a/arch/x86/events/core.c > > +++ b/arch/x86/events/core.c > > @@ -2490,7 +2490,8 @@ void arch_perf_update_userpage(struct perf_event *event, > > userpg->cap_user_time = 0; > > userpg->cap_user_time_zero = 0; > > userpg->cap_user_rdpmc = > > - !!(event->hw.flags & PERF_X86_EVENT_RDPMC_ALLOWED); > > + !!(event->hw.flags & PERF_X86_EVENT_RDPMC_ALLOWED) && > > + (event->oncpu == smp_processor_id()); > > userpg->pmc_width = x86_pmu.cntval_bits; > > > > if (!using_native_sched_clock() || !sched_clock_stable()) > > Isn't that a nop? That is, from the few sites I checked, we're always > calling this on the event's CPU.
If cpu0 opens and mmaps an event for cpu1, then cpu0 will see cap_user_rdpmc set and think it can use rdpmc.
Rob
| |