Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757344AbcLOHPV (ORCPT ); Thu, 15 Dec 2016 02:15:21 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47164 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753353AbcLOHPU (ORCPT ); Thu, 15 Dec 2016 02:15:20 -0500 Date: Thu, 15 Dec 2016 08:14:33 +0100 From: Jiri Olsa To: Peter Zijlstra Cc: Andi Kleen , lkml , Alexander Shishkin , Vince Weaver , Ingo Molnar Subject: Re: [RFC] perf/x86/intel: Account interrupts for PEBS errors Message-ID: <20161215071433.GA16047@krava> References: <20161214165036.GB9180@krava> <20161214180730.GR3124@twins.programming.kicks-ass.net> <20161214181636.GA14741@krava> <20161214193239.GS3124@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161214193239.GS3124@twins.programming.kicks-ass.net> User-Agent: Mutt/1.7.1 (2016-10-04) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Thu, 15 Dec 2016 07:14:35 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1933 Lines: 59 On Wed, Dec 14, 2016 at 08:32:39PM +0100, Peter Zijlstra wrote: > On Wed, Dec 14, 2016 at 07:16:36PM +0100, Jiri Olsa wrote: > > > > > +++ b/arch/x86/events/intel/ds.c > > > > @@ -1389,9 +1389,13 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs) > > > > continue; > > > > > > > > /* log dropped samples number */ > > > > - if (error[bit]) > > > > + if (error[bit]) { > > > > perf_log_lost_samples(event, error[bit]); > > > > > > > > + if (perf_event_account_interrupt(event, 1)) > > > > > > Seems a bit daft to expose the .throttle argument, since that would be > > > the only point of calling this. > > > > there's also the other caller from __perf_event_overflow > > See the below patchlet ;-) ok, np ;-) > > > > > +static int __perf_event_overflow(struct perf_event *event, > > > > + int throttle, struct perf_sample_data *data, > > > > + struct pt_regs *regs) > > > > +{ > > > > + int events = atomic_read(&event->event_limit); > > > > + struct hw_perf_event *hwc = &event->hw; > > > > + int ret = 0; > > > > + > > > > + /* > > > > + * Non-sampling counters might still use the PMI to fold short > > > > + * hardware counters, ignore those. > > > > + */ > > > > + if (unlikely(!is_sampling_event(event))) > > > > + return 0; > > > > + > > > > + ret = perf_event_account_interrupt(event, throttle); > > > > + > > > > if (event->attr.freq) { > > > > u64 now = perf_clock(); > > > > s64 delta = now - hwc->freq_time_stamp; > > > > > > Arguably, everything in __perf_event_overflow() except for calling of > > > ->overflow_handler() should be done I think. > > > > well, I was wondering about that period adjustment bit > > > > but I wasn't sure about those pending_kill/pending_wakeup bits, > > they make sense to me only if we have some data to deliver > > Hmm, maybe. Please add a comment, that way we can at least rediscover we > thought about this. ook jirka