Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756723AbbGQMMI (ORCPT ); Fri, 17 Jul 2015 08:12:08 -0400 Received: from foss.arm.com ([217.140.101.70]:48679 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753268AbbGQMMG (ORCPT ); Fri, 17 Jul 2015 08:12:06 -0400 Date: Fri, 17 Jul 2015 13:11:41 +0100 From: Mark Rutland To: "kan.liang@intel.com" Cc: "a.p.zijlstra@chello.nl" , "mingo@redhat.com" , "acme@kernel.org" , "eranian@google.com" , "ak@linux.intel.com" , "adrian.hunter@intel.com" , "dsahern@gmail.com" , "jolsa@kernel.org" , "namhyung@kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 2/9] perf/x86: core_misc PMU disable and enable support Message-ID: <20150717121141.GC26091@leverpostej> References: <1437078831-10152-1-git-send-email-kan.liang@intel.com> <1437078831-10152-3-git-send-email-kan.liang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1437078831-10152-3-git-send-email-kan.liang@intel.com> Thread-Topic: [PATCH 2/9] perf/x86: core_misc PMU disable and enable support Accept-Language: en-GB, en-US Content-Language: en-US acceptlanguage: en-GB, en-US User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2971 Lines: 89 On Thu, Jul 16, 2015 at 09:33:44PM +0100, kan.liang@intel.com wrote: > From: Kan Liang > > This patch implements core_misc PMU disable and enable functions. > core_misc PMU counters are free running counters, so it's impossible to > stop/start them. Doesn't that effectively mean you can't group them? You'll get arbitrary noise because counters will be incrementing as you read them. [...] > @@ -927,6 +933,10 @@ int p6_pmu_init(void); > > int knc_pmu_init(void); > > +void intel_core_misc_pmu_enable(void); > + > +void intel_core_misc_pmu_disable(void); > + > ssize_t events_sysfs_show(struct device *dev, struct device_attribute *attr, > char *page); > > diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c > index b9826a9..651a86d 100644 > --- a/arch/x86/kernel/cpu/perf_event_intel.c > +++ b/arch/x86/kernel/cpu/perf_event_intel.c > @@ -1586,6 +1586,8 @@ static int intel_pmu_handle_irq(struct pt_regs *regs) > if (!x86_pmu.late_ack) > apic_write(APIC_LVTPC, APIC_DM_NMI); > __intel_pmu_disable_all(); > + if (cpuc->core_misc_active_mask) > + intel_core_misc_pmu_disable(); Huh? Free running counters have nothing to do with the PMU interrupt; there's nothing they can do to trigger it. This feels very hacky. If this is necessary, surely it should live in __intel_pmu_disable_all? [...] > +void intel_core_misc_pmu_enable(void) > +{ > + struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events); > + struct perf_event *event; > + u64 start; > + int bit; > + > + for_each_set_bit(bit, cpuc->core_misc_active_mask, > + X86_CORE_MISC_COUNTER_MAX) { > + event = cpuc->core_misc_events[bit]; > + start = core_misc_pmu_read_counter(event); > + local64_set(&event->hw.prev_count, start); > + } > +} > + > +void intel_core_misc_pmu_disable(void) > +{ > + struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events); > + int bit; > + > + for_each_set_bit(bit, cpuc->core_misc_active_mask, > + X86_CORE_MISC_COUNTER_MAX) { > + core_misc_pmu_event_update(cpuc->core_misc_events[bit]); > + } > +} > + > static void core_misc_pmu_event_del(struct perf_event *event, int mode) > { > core_misc_pmu_event_stop(event, PERF_EF_UPDATE); > @@ -863,6 +899,11 @@ static void __init core_misc_pmus_register(void) > .capabilities = PERF_PMU_CAP_NO_INTERRUPT, > }; > > + if (type->type == perf_intel_core_misc_thread) { > + type->pmu.pmu_disable = (void *) intel_core_misc_pmu_disable; > + type->pmu.pmu_enable = (void *) intel_core_misc_pmu_enable; Why are you suprressing an entirely valid compiler warning here? The signatures of intel_core_misc_pmu_{enable,disable} aren't right. Fix them to take a struct pmu *. Mark. -- 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/