public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Cyrill Gorcunov <gorcunov@gmail.com>
To: Don Zickus <dzickus@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Jason Wessel <jason.wessel@windriver.com>,
	Ingo Molnar <mingo@elte.hu>,
	Robert Richter <robert.richter@amd.com>,
	ying.huang@intel.com, Andi Kleen <andi@firstfloor.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Frederic Weisbecker <fweisbec@gmail.com>
Subject: Re: [V2 PATCH 0/6] x86, NMI: give NMI handler a face-lift
Date: Fri, 19 Nov 2010 00:56:50 +0300	[thread overview]
Message-ID: <20101118215650.GL6028@lenovo> (raw)
In-Reply-To: <20101118193247.GF18100@redhat.com>

On Thu, Nov 18, 2010 at 02:32:47PM -0500, Don Zickus wrote:
... 
> On a side note I think I have a fix for the p4 problem but will probably
> need Cyril to look at it.  Basically in, p4_pmu_clear_cccr_ovf() it is
> using the high part of the cccr register to determine if the counter
> overflowed, when it probably wants to use the low bits of the cccr
> register and high bits of the event_base.
>

Thanks a hige Don for pointing to the problem. Here is the patch.
 
  Cyrill
---
perf, x86: P4 PMU - Fix unflagged overflows handling

Jason pointed out that kgdb no longer works with new
nmi-watchdog. Don found the reason -- P4 PMU reads CCCR
register instead of counter itself, it forces NMIs to
be eaten by perf subsystem.

Fix it by reading a proper register.

Reported-by: Jason Wessel <jason.wessel@windriver.com>
Reported-by: Don Zickus <dzickus@redhat.com>
Tested-by: Jason Wessel <jason.wessel@windriver.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/kernel/cpu/perf_event_p4.c |   21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Index: linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/cpu/perf_event_p4.c
+++ linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
@@ -753,19 +753,22 @@ out:
 
 static inline int p4_pmu_clear_cccr_ovf(struct hw_perf_event *hwc)
 {
-	int overflow = 0;
-	u32 low, high;
+	u32 overflow = 0;
+	u32 low, low_cccr, high;
 
-	rdmsr(hwc->config_base + hwc->idx, low, high);
+	/* an official way for overflow indication */
+	rdmsr(hwc->config_base + hwc->idx, low_cccr, high);
+	overflow |= (low_cccr & P4_CCCR_OVF);
+
+	/* unflagged overflows */
+	rdmsr(hwc->event_base + hwc->idx, low, high);
+	overflow |= high & 0x80000000;
 
-	/* we need to check high bit for unflagged overflows */
-	if ((low & P4_CCCR_OVF) || !(high & (1 << 31))) {
-		overflow = 1;
+	if (overflow)
 		(void)checking_wrmsrl(hwc->config_base + hwc->idx,
-			((u64)low) & ~P4_CCCR_OVF);
-	}
+			((u64)low_cccr) & ~P4_CCCR_OVF);
 
-	return overflow;
+	return overflow > 0;
 }
 
 static void p4_pmu_disable_pebs(void)

  parent reply	other threads:[~2010-11-18 21:56 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-12 14:43 [V2 PATCH 0/6] x86, NMI: give NMI handler a face-lift Don Zickus
2010-11-12 14:43 ` [PATCH 1/6] x86, NMI: Add NMI symbol constants and rename memory parity to PCI SERR Don Zickus
2010-11-12 14:43 ` [PATCH 2/6] x86, NMI: Add touch_nmi_watchdog to io_check_error delay Don Zickus
2010-11-12 14:43 ` [PATCH 3/6] x86, NMI: Rewrite NMI handler Don Zickus
2010-11-12 14:43 ` [PATCH 4/6] x86, NMI: Remove DIE_NMI_IPI and add priorties to handlers Don Zickus
2010-11-12 14:43 ` [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU Don Zickus
2010-11-12 14:43 ` [PATCH 6/6] x86, NMI: Remove do_nmi_callback logic Don Zickus
2010-11-12 15:05 ` [V2 PATCH 0/6] x86, NMI: give NMI handler a face-lift Jason Wessel
2010-11-12 15:42   ` Don Zickus
2010-11-12 15:55     ` Jason Wessel
2010-11-12 16:11       ` Don Zickus
2010-11-12 16:34         ` Jason Wessel
2010-11-12 17:27           ` Don Zickus
2010-11-16 18:43             ` Don Zickus
2010-11-16 20:04               ` Jason Wessel
2010-11-18  8:05                 ` Ingo Molnar
2010-11-18 12:47                   ` Jason Wessel
2010-11-18 13:17                     ` Peter Zijlstra
2010-11-18 14:32                       ` Don Zickus
2010-11-18 15:18                         ` Jason Wessel
2010-11-18 15:38                       ` Peter Zijlstra
2010-11-18 19:32                       ` Don Zickus
2010-11-18 19:51                         ` Jason Wessel
2010-11-18 20:04                           ` Peter Zijlstra
2010-11-18 20:08                           ` Don Zickus
2010-11-18 20:11                             ` Cyrill Gorcunov
2010-11-18 20:52                               ` Don Zickus
2010-11-18 21:01                                 ` Cyrill Gorcunov
2010-11-18 21:16                                   ` Don Zickus
2010-11-18 21:26                                     ` Cyrill Gorcunov
2010-11-18 20:28                             ` Cyrill Gorcunov
2010-11-18 20:39                               ` Cyrill Gorcunov
2010-11-18 21:02                                 ` Don Zickus
2010-11-18 21:19                                   ` Cyrill Gorcunov
2010-11-18 20:30                             ` Peter Zijlstra
2010-11-19 16:59                               ` Don Zickus
2010-11-19 18:25                                 ` Peter Zijlstra
2010-11-19 22:59                                   ` Don Zickus
2010-11-19 23:09                                     ` Peter Zijlstra
2010-11-19 23:30                                       ` Jason Wessel
2010-11-22 14:22                                         ` Don Zickus
2010-11-22 14:22                                       ` Don Zickus
2010-11-22 14:29                                         ` Peter Zijlstra
2010-11-18 20:04                         ` Cyrill Gorcunov
2010-11-18 21:56                         ` Cyrill Gorcunov [this message]
2010-11-18 21:58                           ` Cyrill Gorcunov
2010-11-18 22:15                           ` Cyrill Gorcunov
2010-11-18 22:24                             ` Jason Wessel
2010-11-18 22:27                               ` Cyrill Gorcunov

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=20101118215650.GL6028@lenovo \
    --to=gorcunov@gmail.com \
    --cc=andi@firstfloor.org \
    --cc=dzickus@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=jason.wessel@windriver.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=robert.richter@amd.com \
    --cc=ying.huang@intel.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