From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758003AbcIGQib (ORCPT ); Wed, 7 Sep 2016 12:38:31 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:40604 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755717AbcIGQi2 (ORCPT ); Wed, 7 Sep 2016 12:38:28 -0400 Date: Wed, 7 Sep 2016 18:38:19 +0200 From: Peter Zijlstra To: Vince Weaver Cc: Alexander Shishkin , Ingo Molnar , Ingo Molnar , linux-kernel@vger.kernel.org, eranian@google.com, Arnaldo Carvalho de Melo , vincent.weaver@maine.edu Subject: Re: [PATCH v2 0/5] perf, bts: Fallout from the fuzzer for perf/urgent Message-ID: <20160907163819.GT10138@twins.programming.kicks-ass.net> References: <20160906132353.19887-1-alexander.shishkin@linux.intel.com> <20160906171915.GA8963@gmail.com> <87mvjjka7u.fsf@ashishki-desk.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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 Cc: Stephane Eranian Cc: Andi Kleen Cc: Kan Liang Cc: stable@vger.kernel.org Fixes: a3d86542de88 ("perf/x86/intel/pebs: Add PEBSv3 decoding") Reported-by: Alexander Shishkin Tested-by: Alexander Shishkin Signed-off-by: Peter Zijlstra (Intel) --- 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])