Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755082AbbGCNNv (ORCPT ); Fri, 3 Jul 2015 09:13:51 -0400 Received: from casper.infradead.org ([85.118.1.10]:55801 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754575AbbGCNNo (ORCPT ); Fri, 3 Jul 2015 09:13:44 -0400 Date: Fri, 3 Jul 2015 15:13:36 +0200 From: Peter Zijlstra To: Vince Weaver Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Arnaldo Carvalho de Melo , Stephane Eranian , kan.liang@intel.com Subject: Re: perf: fuzzer triggered warning in intel_pmu_drain_pebs_nhm() Message-ID: <20150703131336.GI19282@twins.programming.kicks-ass.net> References: 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: 3026 Lines: 81 On Thu, Jul 02, 2015 at 11:18:10AM -0400, Vince Weaver wrote: > > So sad to say the lack of fuzzer reports was because I was out of town for > a bit, not due to the kernel suddenly getting amazingly better. > > In any case I am running against current git and getting a lot of > warnings, but most of them seem to be old ones. This following one looks > new though. > > This is current linus-git on a Haswell machine with peterz's patch to fix > the aux buffer spinlock recursion (I can still crash the kernel if that > patch is not applied). > > It corresponds to: > > WARN_ON_ONCE(!event->attr.precise_ip); > > [ 584.352324] WARNING: CPU: 2 PID: 18924 at arch/x86/kernel/cpu/perf_event_intel_ds.c:1198 intel_pmu_drain_pebs_nhm+0x283/0x2e0() I've not yet tried to reproduce, but the below could explain things. On disabling an event we first clear our cpuc->pebs_enabled bits, only to then check them to see if there are any set, and if so, drain the buffer. If we just cleared the last bit, we'll fail to drain the buffer. If we then program another event on that counter and another PEBS event, we can hit the above WARN with the 'stale' entries left over from the previous event. --- arch/x86/kernel/cpu/perf_event_intel_ds.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c b/arch/x86/kernel/cpu/perf_event_intel_ds.c index 71fc40238843..041a30ba5654 100644 --- a/arch/x86/kernel/cpu/perf_event_intel_ds.c +++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c @@ -548,7 +548,7 @@ int intel_pmu_drain_bts_buffer(void) static inline void intel_pmu_drain_pebs_buffer(void) { - struct pt_regs regs; + struct pt_regs regs; /* SAMPLE_REGS_INTR must not be set for FREERUNNING */ x86_pmu.drain_pebs(®s); } @@ -755,13 +755,6 @@ void intel_pmu_pebs_disable(struct perf_event *event) struct hw_perf_event *hwc = &event->hw; struct debug_store *ds = cpuc->ds; - cpuc->pebs_enabled &= ~(1ULL << hwc->idx); - - if (event->hw.flags & PERF_X86_EVENT_PEBS_LDLAT) - cpuc->pebs_enabled &= ~(1ULL << (hwc->idx + 32)); - else if (event->hw.flags & PERF_X86_EVENT_PEBS_ST) - cpuc->pebs_enabled &= ~(1ULL << 63); - if (ds->pebs_interrupt_threshold > ds->pebs_buffer_base + x86_pmu.pebs_record_size) { intel_pmu_drain_pebs_buffer(); @@ -769,6 +762,13 @@ void intel_pmu_pebs_disable(struct perf_event *event) perf_sched_cb_dec(event->ctx->pmu); } + cpuc->pebs_enabled &= ~(1ULL << hwc->idx); + + if (event->hw.flags & PERF_X86_EVENT_PEBS_LDLAT) + cpuc->pebs_enabled &= ~(1ULL << (hwc->idx + 32)); + else if (event->hw.flags & PERF_X86_EVENT_PEBS_ST) + cpuc->pebs_enabled &= ~(1ULL << 63); + if (cpuc->enabled) wrmsrl(MSR_IA32_PEBS_ENABLE, cpuc->pebs_enabled); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/