On Wed, Sep 07, 2016 at 11:36:48AM -0400, Vince Weaver wrote: > On Wed, 7 Sep 2016, Alexander Shishkin wrote: > > > Also one unrelated thing in PEBS that Peter fixed.
--- Subject: perf,x86: Fix PEBSv3 record drain From: Peter Zijlstra <pet...@infradead.org> Date: Wed Sep 7 14:42:55 CEST 2016 Alexander hit the WARN_ON_ONCE(!event) on his Skylake while running the fuzzer. This means the PEBSv3 record included a status bit for an inactive event, something that _should_ not happen. Move the code that filters the status bits against our known PEBS events up a spot to guarantee we only deal with events we know about. Further add "continue" statements to the WARN_ON_ONCE()s such that we'll not die nor generate silly events in case we ever do hit them again. Cc: Vince Weaver <vi...@deater.net> Cc: Stephane Eranian <eran...@google.com> Cc: Andi Kleen <a...@linux.intel.com> Cc: Kan Liang <kan.li...@intel.com> Cc: sta...@vger.kernel.org Fixes: a3d86542de88 ("perf/x86/intel/pebs: Add PEBSv3 decoding") Reported-by: Alexander Shishkin <alexander.shish...@linux.intel.com> Tested-by: Alexander Shishkin <alexander.shish...@linux.intel.com> Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org> --- arch/x86/events/intel/ds.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) --- a/arch/x86/events/intel/ds.c +++ b/arch/x86/events/intel/ds.c @@ -1312,18 +1312,18 @@ static void intel_pmu_drain_pebs_nhm(str struct pebs_record_nhm *p = at; u64 pebs_status; - /* PEBS v3 has accurate status bits */ + pebs_status = p->status & cpuc->pebs_enabled; + pebs_status &= (1ULL << x86_pmu.max_pebs_events) - 1; + + /* PEBS v3 has more accurate status bits */ if (x86_pmu.intel_cap.pebs_format >= 3) { - for_each_set_bit(bit, (unsigned long *)&p->status, - MAX_PEBS_EVENTS) + for_each_set_bit(bit, (unsigned long *)&pebs_status, + x86_pmu.max_pebs_events) counts[bit]++; continue; } - pebs_status = p->status & cpuc->pebs_enabled; - pebs_status &= (1ULL << x86_pmu.max_pebs_events) - 1; - /* * On some CPUs the PEBS status can be zero when PEBS is * racing with clearing of GLOBAL_STATUS. @@ -1371,8 +1371,11 @@ static void intel_pmu_drain_pebs_nhm(str continue; event = cpuc->events[bit]; - WARN_ON_ONCE(!event); - WARN_ON_ONCE(!event->attr.precise_ip); + if (WARN_ON_ONCE(!event)) + continue; + + if (WARN_ON_ONCE(!event->attr.precise_ip)) + continue; /* log dropped samples number */ if (error[bit])