lkml.org 
[lkml]   [2012]   [Feb]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] perf_event use rdpmc rather than rdmsr when possible in kernel
On Mon, 27 Feb 2012, Peter Zijlstra wrote:

> On Mon, 2012-02-20 at 17:38 -0500, Vince Weaver wrote:
>
> > diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
> > index 5adce10..5550047 100644
> > --- a/arch/x86/kernel/cpu/perf_event.c
> > +++ b/arch/x86/kernel/cpu/perf_event.c
> > @@ -85,7 +85,7 @@ u64 x86_perf_event_update(struct perf_event *event)
> > */
> > again:
> > prev_raw_count = local64_read(&hwc->prev_count);
> > - rdmsrl(hwc->event_base, new_raw_count);
> > + new_raw_count=native_read_pmc(hwc->event_base_rdpmc);
>
> That really wants to be rdpmc(), bypassing paravirt like that isn't
> nice.

I couldn't find another usable rdpmc() call in the kernel. Should I add
one? I admit I hadn't thought that this might break VMs not expecting
rdpmc calls from the kernel.

> > index abb2776..432ac69 100644
> > --- a/include/linux/perf_event.h
> > +++ b/include/linux/perf_event.h
> > @@ -562,6 +562,7 @@ struct hw_perf_event {
> > u64 last_tag;
> > unsigned long config_base;
> > unsigned long event_base;
> > + unsigned long event_base_rdpmc;
>
> We could make that unsigned int, the SDM explicitly states
> rdmsr/wrmsr/rdpmc take ECX (and ignore the upper 32bits RCX).

OK.

> > int idx;
> > int last_cpu;
> > struct hw_perf_event_extra extra_reg;
>
> But yeah, avoiding the extra variable will add a conditional and extra
> instructions to the rdpmc path.

yes, I couldn't think of a good way to get rid of the extra field
without adding extra conditional branches in key code paths.

Mostly that's the fault of the Intel fixed counters, otherwise you could
possibly fix things by cleverly adding or subtracting off the msr_base
to get the counter number.

Vince



\
 
 \ /
  Last update: 2012-02-27 17:09    [W:0.027 / U:0.556 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site