lkml.org 
[lkml]   [2021]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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

\
 
 \ /
  Last update: 2021-01-12 17:41    [W:0.064 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site