lkml.org 
[lkml]   [2015]   [Apr]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [RFC] x86, perf: Add an aperfmperf driver
On Apr 29, 2015 2:09 AM, "Peter Zijlstra" <peterz@infradead.org> wrote:
>
> On Tue, Apr 28, 2015 at 02:25:37PM -0700, Andy Lutomirski wrote:
> > diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
> > index 80091ae54c2b..fadc822efc90 100644
> > --- a/arch/x86/kernel/cpu/Makefile
> > +++ b/arch/x86/kernel/cpu/Makefile
> > @@ -45,6 +45,8 @@ obj-$(CONFIG_PERF_EVENTS_INTEL_UNCORE) += perf_event_intel_uncore.o \
> > perf_event_intel_uncore_snb.o \
> > perf_event_intel_uncore_snbep.o \
> > perf_event_intel_uncore_nhmex.o
> > +obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_aperf_mperf.o
> > +obj-$(CONFIG_CPU_SUP_AMD) += perf_event_aperf_mperf.o
>
> Does this actually work? I would expect it to go complain about having
> to build it twice if you have both set.

No, but only because I spelled the filename wrong while regenerating
the patch. Oops!

>
> > diff --git a/arch/x86/kernel/cpu/perf_event_aperfmperf.c b/arch/x86/kernel/cpu/perf_event_aperfmperf.c
> > new file mode 100644
> > index 000000000000..6e6d113bd9ce
> > --- /dev/null
> > +++ b/arch/x86/kernel/cpu/perf_event_aperfmperf.c
> > @@ -0,0 +1,119 @@
> > +#include <linux/perf_event.h>
> > +
> > +#define APERFMPERF_EVENT_APERF 0
> > +#define APERFMPERF_EVENT_MPERF 1
> > +
>
> > +static int aperfmperf_event_init(struct perf_event *event)
> > +{
> > + if (event->attr.type != event->pmu->type)
> > + return -ENOENT;
> > +
> > + if (event->attr.config != APERFMPERF_EVENT_APERF &&
> > + event->attr.config != APERFMPERF_EVENT_MPERF)
> > + return -ENOENT;
>
> Once we pass the type test we know its 'our' event, and we can go return
> fatal errors. No other PMU will pick this up.
>
> This could therefore turn into an -EINVAL.
>
> > +
> > + if (event->attr.config1 != 0)
> > + return -ENOENT;
>
> Idem.
>
> > + /* no sampling */
> > + if (event->hw.sample_period)
> > + return -EINVAL;
>
> You could have set pmu::capabilities =
> PERF_PMU_CAP_NO_INTERRUPT which would also have killed that dead.


That checks attr.sample_period. I'm a bit confused about the
relationship between event->hw and event->attr. Do I not need to
check hw.sample_period?

>
> > + /* unsupported modes and filters */
> > + if (event->attr.exclude_user ||
> > + event->attr.exclude_kernel ||
> > + event->attr.exclude_hv ||
> > + event->attr.exclude_idle ||
> > + event->attr.exclude_host ||
> > + event->attr.exclude_guest ||
> > + event->attr.freq ||
> > + event->attr.sample_period) /* no sampling */
> > + return -EINVAL;
> > +
> > + event->hw.idx = -1;
> > + event->hw.event_base = (event->attr.config == APERFMPERF_EVENT_APERF ?
> > + MSR_IA32_APERF : MSR_IA32_MPERF);
> > +
> > + return 0;
> > +}
>
> The rest looks about right. Very simple thing indeed ;-)

Before I submit v2, do you think this is actually worth doing? I can
see it being useful for answering questions like "did this workload
end up running at full speed".

--Andy


\
 
 \ /
  Last update: 2015-04-29 21:21    [W:0.089 / U:66.076 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site