Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755730AbcCCXkw (ORCPT ); Thu, 3 Mar 2016 18:40:52 -0500 Received: from mail-wm0-f53.google.com ([74.125.82.53]:34692 "EHLO mail-wm0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750986AbcCCXku (ORCPT ); Thu, 3 Mar 2016 18:40:50 -0500 MIME-Version: 1.0 In-Reply-To: <20160303214312.GI23621@tassilo.jf.intel.com> References: <1457034642-21837-1-git-send-email-eranian@google.com> <1457034642-21837-3-git-send-email-eranian@google.com> <20160303214312.GI23621@tassilo.jf.intel.com> Date: Thu, 3 Mar 2016 15:40:49 -0800 Message-ID: Subject: Re: [PATCH 2/3] perf/x86/pebs: add workaround for broken OVFL status on HSW From: Stephane Eranian To: Andi Kleen Cc: LKML , Arnaldo Carvalho de Melo , Peter Zijlstra , "mingo@elte.hu" , "Liang, Kan" , Jiri Olsa , Namhyung Kim , Adrian Hunter Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1332 Lines: 34 On Thu, Mar 3, 2016 at 1:43 PM, Andi Kleen wrote: > > > + /* > > + * 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; > > If you just clear the bits here they will not be acked and stay around > forever in GLOBAL_STATUS, which causes other problems. > > You need a separate ack_status variable that contains all bits and is always > acked. I understand that. You mean I need to that has all the bits that were set to call intel_pmu_ack_status(). But if you look at the code, and where I made the change, there is no more intel_pmu_ack_status() BEFORE you read the status again via intel_pmu_get_status(). So why would I need to keep another variable around? > > > > -Andi > -- > ak@linux.intel.com -- Speaking for myself only