lkml.org 
[lkml]   [2019]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/9] perf/x86/intel: Support hardware TopDown metrics
On Tue, May 28, 2019 at 02:24:14PM -0400, Liang, Kan wrote:

> > > + if (!(txn_flags & PERF_PMU_TXN_REMOVE)) {
> > > + /* Reset the metric value when reading
> > > + * The SLOTS register must be reset when PERF_METRICS reset,
> > > + * otherwise PERF_METRICS may has wrong output.
> > > + */
> >
> > broken comment style.. (and grammer)
>
> Missed a full stop.
> Should be "Reset the metric value for each read."

s/may has wrong/may have wrong/

> > > + wrmsrl(MSR_PERF_METRICS, 0);
> > > + wrmsrl(MSR_CORE_PERF_FIXED_CTR3, 0);
> >
> > I don't get this, overflow happens on when we flip sign, so why is
> > programming 0 a sane thing to do?
>
> Reset the counters (programming 0) don't trigger overflow.

Right, so why then do you allow creating this thing as
is_sampling_event() ?

> We have to reset both registers for each read to avoid the known
> PERF_METRICS issue.

'the known PERF_METRICS issue' is unknown to me and any other reader.

> > > + metric = (cpuc->last_metric >> ((hwc->idx - INTEL_PMC_IDX_FIXED_METRIC_BASE)*8)) & 0xff;
> > > + last = (((metric * 0xffff) >> 8) * cpuc->last_slots) >> 16;
> >
> > How is that cpuc->last_* crap not broken for NMIs ?
>
> There should be no NMI for slots or metric events at the moment, because the
> MSR_PERF_METRICS and MSR_CORE_PERF_FIXED_CTR3 are reset in first read.
> Other NMIs will not touch the codes here.

What happens if someone does: read(perf_fd) and then has the NMI hit?

\
 
 \ /
  Last update: 2019-05-29 09:35    [W:0.046 / U:2.696 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site