public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: "Odzioba, Lukasz" <lukasz.odzioba@intel.com>
Cc: "Liang, Kan" <kan.liang@intel.com>,
	Stephane Eranian <eranian@google.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	"ak@linux.intel.com" <ak@linux.intel.com>
Subject: Re: [PATCH] perf/x86: fix event counter update issue
Date: Mon, 5 Dec 2016 11:25:09 +0100	[thread overview]
Message-ID: <20161205102509.GH3124@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <D6EDEBF1F91015459DB866AC4EE162CC024DCFE5@IRSMSX103.ger.corp.intel.com>

On Fri, Dec 02, 2016 at 12:58:17PM +0000, Odzioba, Lukasz wrote:
> On Tuesday, November 29, 2016 9:33 PM, Liang, Kan wrote:
> > Yes, the patch as below fixes the issue on my SLM.
> 
> It works for me as well. 
> Can we still have it in 4.9?

I'll certainly, try. I've queued it as per the below.

---
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.

However commit:

  069e0c3c4058 ("perf/x86/intel: Support full width counting")

sets the sampling interval to the full period, not half.

Fixing that exposes another issue, in that we must not sign extend the
delta value when we shift it right; the counter cannot have
decremented after all.

With both these issues fixed, counter overflow functions correctly
again.

Cc: "mingo@redhat.com" <mingo@redhat.com>
Cc: "ak@linux.intel.com" <ak@linux.intel.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: stable@vger.kernel.org
Reported-by: Lukasz Odzioba <lukasz.odzioba@intel.com>
Tested-by: "Liang, Kan" <kan.liang@intel.com>
Tested-by: "Odzioba, Lukasz" <lukasz.odzioba@intel.com>
Fixes: 069e0c3c4058 ("perf/x86/intel: Support full width counting")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/events/core.c       |    2 +-
 arch/x86/events/intel/core.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -69,7 +69,7 @@ u64 x86_perf_event_update(struct perf_ev
 	int shift = 64 - x86_pmu.cntval_bits;
 	u64 prev_raw_count, new_raw_count;
 	int idx = hwc->idx;
-	s64 delta;
+	u64 delta;
 
 	if (idx == INTEL_PMC_IDX_FIXED_BTS)
 		return 0;
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4034,7 +4034,7 @@ __init int intel_pmu_init(void)
 
 	/* Support full width counters using alternative MSR range */
 	if (x86_pmu.intel_cap.full_width_write) {
-		x86_pmu.max_period = x86_pmu.cntval_mask;
+		x86_pmu.max_period = x86_pmu.cntval_mask >> 1;
 		x86_pmu.perfctr = MSR_IA32_PMC0;
 		pr_cont("full-width counters, ");
 	}

  reply	other threads:[~2016-12-05 10:25 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-28 19:26 [PATCH] perf/x86: fix event counter update issue kan.liang
2016-11-28 19:41 ` Stephane Eranian
2016-11-28 19:59   ` Liang, Kan
2016-11-28 20:18     ` Stephane Eranian
2016-11-28 20:23       ` Liang, Kan
2016-11-29  9:25 ` Peter Zijlstra
2016-11-29 14:46   ` Liang, Kan
2016-11-29 16:58     ` Peter Zijlstra
2016-11-29 17:06       ` Liang, Kan
2016-11-29 17:17         ` Peter Zijlstra
2016-11-29 17:20   ` Stephane Eranian
2016-11-29 17:30     ` Peter Zijlstra
2016-11-29 18:11       ` Stephane Eranian
2016-11-29 18:37       ` Andi Kleen
2016-11-29 19:07       ` Liang, Kan
2016-11-29 19:32         ` Peter Zijlstra
2016-11-29 20:33           ` Liang, Kan
2016-11-29 20:37             ` Stephane Eranian
2016-12-02 12:58             ` Odzioba, Lukasz
2016-12-05 10:25               ` Peter Zijlstra [this message]
2016-12-05 11:21                 ` Odzioba, Lukasz
2017-02-22 14:49                 ` Vince Weaver
2017-02-22 15:28                   ` Liang, Kan
2017-02-22 19:18                     ` Andi Kleen
2017-02-23 15:07                     ` Vince Weaver
2017-02-23 16:14                       ` Liang, Kan
2016-11-29 19:08     ` Odzioba, Lukasz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161205102509.GH3124@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=eranian@google.com \
    --cc=kan.liang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukasz.odzioba@intel.com \
    --cc=mingo@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox