Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1586228 > unrolled thread

Re: [PATCH] perf/x86: fix event counter update issue

Started byVince Weaver <vincent.weaver@maine.edu>
First post2017-02-22 16:00 +0100
Last post2017-02-23 17:20 +0100
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] perf/x86: fix event counter update issue Vince Weaver <vincent.weaver@maine.edu> - 2017-02-22 16:00 +0100
    RE: [PATCH] perf/x86: fix event counter update issue "Liang, Kan" <kan.liang@intel.com> - 2017-02-22 16:40 +0100
      Re: [PATCH] perf/x86: fix event counter update issue Andi Kleen <ak@linux.intel.com> - 2017-02-22 20:20 +0100
      RE: [PATCH] perf/x86: fix event counter update issue Vince Weaver <vincent.weaver@maine.edu> - 2017-02-23 16:10 +0100
        RE: [PATCH] perf/x86: fix event counter update issue "Liang, Kan" <kan.liang@intel.com> - 2017-02-23 17:20 +0100

#1586228 — Re: [PATCH] perf/x86: fix event counter update issue

FromVince Weaver <vincent.weaver@maine.edu>
Date2017-02-22 16:00 +0100
SubjectRe: [PATCH] perf/x86: fix event counter update issue
Message-ID<tdGyD-3dk-37@gated-at.bofh.it>
On Mon, 5 Dec 2016, Peter Zijlstra wrote:

> ---
> Subject: perf,x86: Fix full width counter, counter overflow
> Date: Tue, 29 Nov 2016 20:33:28 +0000
> 
> Lukasz reported that perf stat counters overflow is broken on KNL/SLM.
> 
> Both these parts have full_width_write set, and that does indeed have
> a problem. In order to deal with counter wrap, we must sample the
> counter at at least half the counter period (see also the sampling
> theorem) such that we can unambiguously reconstruct the count.

I know I'm a bit late to this issue, but I suddenly have PAPI users being 
very worried that their results are going to be wrong ad I hadn't heard 
about this until recently.

I'm trying to make a reproducer test and want to make sure I understand 
the issue.  (And I don't have any of the easy trigger hardware either. 
And what is SLM?  Silvermont?  Are we really that short on Changelog space
that we can't spell out the abbreviations to make things clear for 
non-Intel employees?)

So from what I understand, the issue is if we have an architecture with 
full-width counters and we trigger a x86_perf_event_update() when bit
47 is set?

So if I have a test that runs in a loop for 2^48 retired instructions
(which takes ~12 hours on a recent machine) and then reads the results,
they might be wrong?

It sounds like this can also be triggered by a sampling event with a 
really long period, but I couldn't puzzle out from the Changelog exactly 
how to reproduce this (or even how serious the issue is).

Vince

[toc] | [next] | [standalone]


#1586261

From"Liang, Kan" <kan.liang@intel.com>
Date2017-02-22 16:40 +0100
Message-ID<tdHbj-3Iw-1@gated-at.bofh.it>
In reply to#1586228
> 
> On Mon, 5 Dec 2016, Peter Zijlstra wrote:
> 
> > ---
> > Subject: perf,x86: Fix full width counter, counter overflow
> > Date: Tue, 29 Nov 2016 20:33:28 +0000
> >
> > Lukasz reported that perf stat counters overflow is broken on KNL/SLM.
> >
> > Both these parts have full_width_write set, and that does indeed have
> > a problem. In order to deal with counter wrap, we must sample the
> > counter at at least half the counter period (see also the sampling
> > theorem) such that we can unambiguously reconstruct the count.
> 
> I know I'm a bit late to this issue, but I suddenly have PAPI users being very
> worried that their results are going to be wrong ad I hadn't heard about
> this until recently.
> 
> I'm trying to make a reproducer test and want to make sure I understand
> the issue.  (And I don't have any of the easy trigger hardware either.
> And what is SLM?  Silvermont?  

Yes.

> Are we really that short on Changelog space
> that we can't spell out the abbreviations to make things clear for non-Intel
> employees?)

Here is the original patch I posted for this issue, which include the test cases.
https://lkml.org/lkml/2016/11/28/540
After the discussion, my patch was discarded. Now, we use Peter's fix.

> 
> So from what I understand, the issue is if we have an architecture with full-
> width counters and we trigger a x86_perf_event_update() when bit
> 47 is set?

No. It related to the counter width. The number of bits we can use should be
1 bit less than the total width. Otherwise, there will be problem.
For big cores such as haswell, broadwell, skylake, the counter width is 48 bit.
So we can only use 47 bits.
For Silvermont and KNL, the counter width is only 32 bit I think. So we can only
use 31 bits.

> 
> So if I have a test that runs in a loop for 2^48 retired instructions (which
> takes ~12 hours on a recent machine) and then reads the results, they
> might be wrong?

It only needs several minutes to reproduce the issue on SLM/KNL.

Thanks,
Kan 

> 
> It sounds like this can also be triggered by a sampling event with a really
> long period, but I couldn't puzzle out from the Changelog exactly how to
> reproduce this (or even how serious the issue is).
> 
> Vince
> 

[toc] | [prev] | [next] | [standalone]


#1586427

FromAndi Kleen <ak@linux.intel.com>
Date2017-02-22 20:20 +0100
Message-ID<tdKCd-6rR-11@gated-at.bofh.it>
In reply to#1586261
> No. It related to the counter width. The number of bits we can use should be
> 1 bit less than the total width. Otherwise, there will be problem.
> For big cores such as haswell, broadwell, skylake, the counter width is 48 bit.
> So we can only use 47 bits.
> For Silvermont and KNL, the counter width is only 32 bit I think. So we can only
> use 31 bits.

It is 40 bits on these cores, so 39bits.

-Andi

[toc] | [prev] | [next] | [standalone]


#1586944

FromVince Weaver <vincent.weaver@maine.edu>
Date2017-02-23 16:10 +0100
Message-ID<te3bQ-2YE-31@gated-at.bofh.it>
In reply to#1586261
On Wed, 22 Feb 2017, Liang, Kan wrote:

> > So from what I understand, the issue is if we have an architecture with full-
> > width counters and we trigger a x86_perf_event_update() when bit
> > 47 is set?
> 
> No. It related to the counter width. The number of bits we can use should be
> 1 bit less than the total width. Otherwise, there will be problem.
> For big cores such as haswell, broadwell, skylake, the counter width is 48 bit.
> So we can only use 47 bits.
> For Silvermont and KNL, the counter width is only 32 bit I think. So we can only
> use 31 bits.

So on a machine with 48-bit counters I should just have a counting event
that counts to somewhere above 0x8000 0000 0001 and it should show 
problems?
Because I am unable to trigger this.

But I guess if anywhere along the line x86_perf_event_update() is run
then you start over?

I noticed your original reproducer bound the event to a core, is that 
needed to trigger this?

Can it happen on a fixed event or only a genearl purpose event?

> > So if I have a test that runs in a loop for 2^48 retired instructions (which
> > takes ~12 hours on a recent machine) and then reads the results, they
> > might be wrong?
> 
> It only needs several minutes to reproduce the issue on SLM/KNL.

Yes, but I only have machines with 48-bit counters.  So it's going to take 
256 times as long as on a machine with 40-bit counters.

I have an assembly loop that can consistently generate 2 instructions/cycle
(I'd be glad to hear suggestions for events that count faster) and on
a broadwell-ep machine it still takes at least 7 hours or so to get up
to 0x800000000000.

Vince

[toc] | [prev] | [next] | [standalone]


#1586990

From"Liang, Kan" <kan.liang@intel.com>
Date2017-02-23 17:20 +0100
Message-ID<te4hA-3BE-7@gated-at.bofh.it>
In reply to#1586944
> 
> On Wed, 22 Feb 2017, Liang, Kan wrote:
> 
> > > So from what I understand, the issue is if we have an architecture
> > > with full- width counters and we trigger a x86_perf_event_update()
> > > when bit
> > > 47 is set?
> >
> > No. It related to the counter width. The number of bits we can use
> > should be
> > 1 bit less than the total width. Otherwise, there will be problem.
> > For big cores such as haswell, broadwell, skylake, the counter width is 48
> bit.
> > So we can only use 47 bits.
> > For Silvermont and KNL, the counter width is only 32 bit I think. So
> > we can only use 31 bits.
> 
> So on a machine with 48-bit counters I should just have a counting event
> that counts to somewhere above 0x8000 0000 0001 and it should show
> problems?
Yes

> Because I am unable to trigger this.
> 
> But I guess if anywhere along the line x86_perf_event_update() is run then
> you start over?
> 

Probably. It depends on the left.

> I noticed your original reproducer bound the event to a core, is that needed
> to trigger this?

I don't think it's needed. But I didn't try anything without bound.

> 
> Can it happen on a fixed event or only a genearl purpose event?

I think it can happens on both. Because fixed counter and GP counter have
same counter width and code path.

> 
> > > So if I have a test that runs in a loop for 2^48 retired
> > > instructions (which takes ~12 hours on a recent machine) and then
> > > reads the results, they might be wrong?
> >
> > It only needs several minutes to reproduce the issue on SLM/KNL.
> 
> Yes, but I only have machines with 48-bit counters.  So it's going to take
> 256 times as long as on a machine with 40-bit counters.
> 
> I have an assembly loop that can consistently generate 2 instructions/cycle
> (I'd be glad to hear suggestions for events that count faster) and on a
> broadwell-ep machine it still takes at least 7 hours or so to get up to
> 0x800000000000.

I think you may use MSR tool to write a big number into IA32_PMC0
during your test. 
The writable IA32_PMC0 alias is 0x4C1.


Thanks,
Kan

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web