From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753140AbbC3Nu6 (ORCPT ); Mon, 30 Mar 2015 09:50:58 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:54549 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752478AbbC3Nu5 (ORCPT ); Mon, 30 Mar 2015 09:50:57 -0400 Date: Mon, 30 Mar 2015 15:50:45 +0200 From: Peter Zijlstra To: Kan Liang Cc: linux-kernel@vger.kernel.org, mingo@kernel.org, acme@infradead.org, eranian@google.com, andi@firstfloor.org Subject: Re: [PATCH V5 5/6] perf, x86: drain PEBS buffer during context switch Message-ID: <20150330135045.GW23123@twins.programming.kicks-ass.net> References: <1424701556-28270-1-git-send-email-kan.liang@intel.com> <1424701556-28270-6-git-send-email-kan.liang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1424701556-28270-6-git-send-email-kan.liang@intel.com> 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 On Mon, Feb 23, 2015 at 09:25:55AM -0500, Kan Liang wrote: > From: Yan, Zheng > > Flush the PEBS buffer during context switch if PEBS interrupt threshold > is larger than one. This allows perf to supply TID for sample outputs. > > Signed-off-by: Yan, Zheng > Signed-off-by: Kan Liang > --- > arch/x86/kernel/cpu/perf_event.h | 3 +++ > arch/x86/kernel/cpu/perf_event_intel.c | 11 +++++++++- > arch/x86/kernel/cpu/perf_event_intel_ds.c | 33 ++++++++++++++++++++++++++++-- > arch/x86/kernel/cpu/perf_event_intel_lbr.c | 3 --- > 4 files changed, 44 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h > index bc4ae3b..b4f6431 100644 > --- a/arch/x86/kernel/cpu/perf_event.h > +++ b/arch/x86/kernel/cpu/perf_event.h > @@ -151,6 +151,7 @@ struct cpu_hw_events { > */ > struct debug_store *ds; > u64 pebs_enabled; > + bool pebs_sched_cb_enabled; > > /* > * Intel LBR bits Why do we need that extra state? I would've expected to see a inc/dec for every AUTO_RELOAD that gets added/removed. > @@ -704,13 +717,20 @@ void intel_pmu_pebs_enable(struct perf_event *event) > * When the event is constrained enough we can use a larger > * threshold and run the event with less frequent PMI. > */ > - if (0 && /* disable this temporarily */ > - (hwc->flags & PERF_X86_EVENT_AUTO_RELOAD) && > + if ((hwc->flags & PERF_X86_EVENT_AUTO_RELOAD) && > !(event->attr.sample_type & ~PEBS_FREERUNNING_FLAGS)) { > threshold = ds->pebs_absolute_maximum - > x86_pmu.max_pebs_events * x86_pmu.pebs_record_size; > + if (first_pebs) { > + perf_sched_cb_inc(event->ctx->pmu); > + cpuc->pebs_sched_cb_enabled = true; > + } > } else { > threshold = ds->pebs_buffer_base + x86_pmu.pebs_record_size; > + if (cpuc->pebs_sched_cb_enabled) { > + perf_sched_cb_dec(event->ctx->pmu); > + cpuc->pebs_sched_cb_enabled = false; > + } > } > if (first_pebs || ds->pebs_interrupt_threshold > threshold) > ds->pebs_interrupt_threshold = threshold; I'm confused, why do you do sched_cb_dec for every event that wasn't AUTO_RELOAD?