From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933522AbbDUVA1 (ORCPT ); Tue, 21 Apr 2015 17:00:27 -0400 Received: from casper.infradead.org ([85.118.1.10]:47878 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932289AbbDUVAZ (ORCPT ); Tue, 21 Apr 2015 17:00:25 -0400 Date: Tue, 21 Apr 2015 23:00:16 +0200 From: Peter Zijlstra To: Jiri Olsa Cc: Jiri Olsa , lkml , Andi Kleen , Arnaldo Carvalho de Melo , Ingo Molnar , Paul Mackerras , Stephane Eranian Subject: Re: [PATCHv3] perf/x86/intel: Add cpu_(prepare|starting|dying) for core_pmu Message-ID: <20150421210016.GC2898@worktop.Skamania.guest> References: <1429606465-10271-1-git-send-email-jolsa@kernel.org> <20150421151216.GD3182@worktop.meeting.verilan.com> <20150421151827.GB13169@krava.redhat.com> <20150421152623.GC13169@krava.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150421152623.GC13169@krava.redhat.com> User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 21, 2015 at 05:26:23PM +0200, 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 > > SNIP > > [] __intel_shared_reg_get_constraints+0x69/0x1e0 > [] intel_get_event_constraints+0x9b/0x180 > [] x86_schedule_events+0x75/0x1d0 > [] ? check_preempt_curr+0x7c/0x90 > [] ? try_to_wake_up+0x24e/0x3e0 > [] ? default_wake_function+0x12/0x20 > [] ? autoremove_wake_function+0x16/0x40 > [] ? __wake_up_common+0x59/0x90 > [] ? __d_lookup+0xa7/0x150 > [] ? do_lookup+0x9f/0x230 > [] ? dput+0x9a/0x150 > [] ? path_to_nameidata+0x25/0x60 > [] ? __link_path_walk+0x7da/0x1000 > [] ? x86_pmu_add+0xb9/0x170 > [] x86_pmu_commit_txn+0x67/0xc0 > [] ? mntput_no_expire+0x30/0x110 > [] ? path_put+0x31/0x40 > [] ? current_fs_time+0x27/0x30 > [] ? mem_cgroup_get_reclaim_stat_from_page+0x20/0x70 > [] group_sched_in+0x13a/0x170 > [] ? sched_clock+0x9/0x10 > [] ctx_sched_in+0x2e8/0x330 > [] perf_event_sched_in+0x6b/0xb0 > [] perf_event_context_sched_in+0x76/0xc0 > [] perf_event_comm+0x1bb/0x2e0 > [] set_task_comm+0x69/0x80 > [] setup_new_exec+0xe1/0x2e0 > [] load_elf_binary+0x3ce/0x1ab0 > > Adding cpu_(prepare|starting|dying) for core_pmu to have shared_regs > data allocated for core_pmu. AFAICS there's no harm to initialize > debug store and PMU_FL_EXCL_CNTRS either for core_pmu. > > Signed-off-by: Jiri Olsa Acked-by: Peter Zijlstra (Intel)