Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755628AbbGCS5R (ORCPT ); Fri, 3 Jul 2015 14:57:17 -0400 Received: from mail-oi0-f44.google.com ([209.85.218.44]:33876 "EHLO mail-oi0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755389AbbGCS47 (ORCPT ); Fri, 3 Jul 2015 14:56:59 -0400 MIME-Version: 1.0 Reply-To: eranian@gmail.com In-Reply-To: <20150703131336.GI19282@twins.programming.kicks-ass.net> References: <20150703131336.GI19282@twins.programming.kicks-ass.net> Date: Fri, 3 Jul 2015 20:56:57 +0200 Message-ID: Subject: Re: perf: fuzzer triggered warning in intel_pmu_drain_pebs_nhm() From: Stephane Eranian To: Peter Zijlstra Cc: Vince Weaver , LKML , Ingo Molnar , Arnaldo Carvalho de Melo , kan.liang@intel.com 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: 3599 Lines: 88 Peter, On Fri, Jul 3, 2015 at 3:13 PM, Peter Zijlstra wrote: > 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. > Where do you see that we use cpuc->pebs_enabled after clearing it in pebs_disable() to check for overflow or active in drain_pebs()? I only see it used in get_next_pebs_record_by_bit()? > 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/