From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753432AbbDUKwr (ORCPT ); Tue, 21 Apr 2015 06:52:47 -0400 Received: from mail-wg0-f49.google.com ([74.125.82.49]:33539 "EHLO mail-wg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751027AbbDUKwp (ORCPT ); Tue, 21 Apr 2015 06:52:45 -0400 Date: Tue, 21 Apr 2015 12:52:40 +0200 From: Ingo Molnar To: Jiri Olsa Cc: lkml , Andi Kleen , Arnaldo Carvalho de Melo , Paul Mackerras , Peter Zijlstra , Stephane Eranian Subject: Re: [PATCH] perf/x86/intel: Add cpu_(prepare|starting|dying) for core_pmu Message-ID: <20150421105239.GA26455@gmail.com> References: <1429606465-10271-1-git-send-email-jolsa@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1429606465-10271-1-git-send-email-jolsa@kernel.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Jiri Olsa wrote: > The core_pmu does not define cpu_* callbacks, which handles > allocation of 'struct cpu_hw_events::shared_regs' data, > initialization of debug store and PMU_FL_EXCL_CNTRS counters. > > While this probably won't happen on bare metal, virtual CPU can > define x86_pmu.extra_regs together with PMU version 1 and thus > be using core_pmu -> using shared_regs data without it being > allocated. That could could leave to following panic: > > BUG: unable to handle kernel NULL pointer dereference at (null) > IP: [] _spin_lock_irqsave+0x1f/0x40 ok. > diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c > index 9da2400c2ec3..0a61a9a021de 100644 > --- a/arch/x86/kernel/cpu/perf_event_intel.c > +++ b/arch/x86/kernel/cpu/perf_event_intel.c > @@ -2533,6 +2533,10 @@ ssize_t intel_event_sysfs_show(char *page, u64 config) > return x86_event_sysfs_show(page, config, event); > } > > +static int intel_pmu_cpu_prepare(int cpu); > +static void intel_pmu_cpu_starting(int cpu); > +static void intel_pmu_cpu_dying(int cpu); > + > static __initconst const struct x86_pmu core_pmu = { > .name = "core", > .handle_irq = x86_pmu_handle_irq, > @@ -2559,6 +2563,9 @@ static __initconst const struct x86_pmu core_pmu = { > .guest_get_msrs = core_guest_get_msrs, > .format_attrs = intel_arch_formats_attr, > .events_sysfs_show = intel_event_sysfs_show, > + .cpu_prepare = intel_pmu_cpu_prepare, > + .cpu_starting = intel_pmu_cpu_starting, > + .cpu_dying = intel_pmu_cpu_dying, > }; Instead of adding prototype declarations, please arrange the x86_pmu definition's position so that it comes after the required functions - so that no prototypes are needed. Thanks, Ingo