Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965660AbcCJKnA (ORCPT ); Thu, 10 Mar 2016 05:43:00 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:48705 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965617AbcCJKmo (ORCPT ); Thu, 10 Mar 2016 05:42:44 -0500 Date: Thu, 10 Mar 2016 11:42:36 +0100 From: Peter Zijlstra To: Stephane Eranian Cc: Jiri Olsa , Andi Kleen , LKML , Arnaldo Carvalho de Melo , "mingo@elte.hu" , "Liang, Kan" , Namhyung Kim , Adrian Hunter Subject: Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status on HSW Message-ID: <20160310104236.GV6344@twins.programming.kicks-ass.net> References: <20160307102413.GB6356@twins.programming.kicks-ass.net> <20160307121840.GF6375@twins.programming.kicks-ass.net> <20160307182731.GA12153@krava.redhat.com> <20160307202556.GQ6344@twins.programming.kicks-ass.net> <20160308210707.GG6344@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2880 Lines: 69 On Wed, Mar 09, 2016 at 09:40:07AM -0800, Stephane Eranian wrote: > With your queue.tip perf/core branch, I run into another problem. > I am monitoring with 2 PEBS events and I have the NMI watchdog enabled. > > I see non-EXACT PEBS records again, despite my change (which is in). > I tracked it down to the following issue after the testing of bit 62: > > [31137.273061] CPU71 status=0x200000001 orig_status=0x200000001 bit62=0 > > The IRQ handler is called because the fixed counter for the NMI has overflowed > and it sees this in bit 33, but it also sees that one of the PEBS > events has also > overflowed, yet bit 62 is not set. Therefore both overflows are > treated as regular > and the drain_pebs() is not called generating a non-EXACT record for the PEBS > counter (counter 0). So something is wrong still and this is on Broadwell. > > First, I don't understand why the OVF bit for counter 0 is set. It > should not according > to specs because the counter is in PEBS mode. There must be a race there. So we > have to handle it by relying on cpuc->pebs_enabled. I will try that. > We likely also > need to force OVF bit 62 to 1 so we can ack it in the end (and in case > it gets set). How about we make the clear of pebs_enabled unconditional? --- arch/x86/events/intel/core.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c index 68fa55b4d42e..dc9579665425 100644 --- a/arch/x86/events/intel/core.c +++ b/arch/x86/events/intel/core.c @@ -1883,6 +1883,16 @@ static int intel_pmu_handle_irq(struct pt_regs *regs) status &= ~(GLOBAL_STATUS_COND_CHG | GLOBAL_STATUS_ASIF | GLOBAL_STATUS_LBRS_FROZEN); + /* + * There are cases where, even though, the PEBS ovfl bit is set + * in GLOBAL_OVF_STATUS, the PEBS events may also have their + * overflow bits set for their counters. We must clear them + * here because they have been processed as exact samples in + * the drain_pebs() routine. They must not be processed again + * in the for_each_bit_set() loop for regular samples below. + */ + status &= ~cpuc->pebs_enabled; + if (!status) goto done; @@ -1892,16 +1902,6 @@ static int intel_pmu_handle_irq(struct pt_regs *regs) if (__test_and_clear_bit(62, (unsigned long *)&status)) { handled++; x86_pmu.drain_pebs(regs); - /* - * There are cases where, even though, the PEBS ovfl bit is set - * in GLOBAL_OVF_STATUS, the PEBS events may also have their - * overflow bits set for their counters. We must clear them - * here because they have been processed as exact samples in - * the drain_pebs() routine. They must not be processed again - * in the for_each_bit_set() loop for regular samples below. - */ - status &= ~cpuc->pebs_enabled; - status &= x86_pmu.intel_ctrl | GLOBAL_STATUS_TRACE_TOPAPMI; } /*