From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751062AbdAXTyS (ORCPT <rfc822;w@1wt.eu>);
        Tue, 24 Jan 2017 14:54:18 -0500
Received: from Galois.linutronix.de ([146.0.238.70]:43602 "EHLO
        Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751024AbdAXTyQ (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 24 Jan 2017 14:54:16 -0500
Date: Tue, 24 Jan 2017 20:54:12 +0100 (CET)
From: Thomas Gleixner <tglx@linutronix.de>
To: Yasuaki Ishimatsu <yasu.isimatu@gmail.com>
cc: linux-kernel@vger.kernel.org, x86@kernel.org
Subject: Re: [PATCH] perf/x86/intel/rapl: Rename rapl_cpu_prepare() to
 rapl_cpu_starting()
In-Reply-To: <9063c543-880e-0b12-0671-1492182990e7@gmail.com>
Message-ID: <alpine.DEB.2.20.1701242052150.4828@nanos>
References: <9063c543-880e-0b12-0671-1492182990e7@gmail.com>
User-Agent: Alpine 2.20 (DEB 67 2015-01-07)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, 24 Jan 2017, Yasuaki Ishimatsu wrote:
> rapl_cpu_prepare() must be called after logical package id of CPU
> is set by topology_update_package_map().
> 
> But when onlining hot-added CPU, rapl_cpu_prepare() is called before
> setting logical package id of the hot-added CPU. So cpu_to_rapl_pmu()
> in rapl_cpu_prepare() finds a rapl_pmu of wrong logical package id and
> rapl_cpu_prepare() initializes the wrong rapl_pmu.
> 
> After that logical package id of the hot-added CPU is set by
> topology_update_package_map(). But rapl_cpu_prepare() does
> not initialize pmu of the logical package id of the hot-added CPU.
> So when calling rapl_cpu_online(), cpu_to_rapl_pmu() returns NULL and
> the following NULL pointer dereference occurs.
> 
>   BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
>   IP: rapl_cpu_online+0x8d/0xb0
>   <snip>
>   Call Trace:
>    ? rapl_cpu_offline+0xc0/0xc0
>    cpuhp_invoke_callback+0x8d/0x3f0
>    cpuhp_up_callbacks+0x37/0xb0
>    cpuhp_thread_fun+0xc9/0xe0
>    smpboot_thread_fn+0x110/0x160
>    kthread+0x101/0x140
>    ? sort_range+0x30/0x30
>    ? kthread_park+0x90/0x90
>    ret_from_fork+0x25/0x30
> 
> The patch renames rapl_cpu_prepare() to rapl_cpu_starting() and changes
> the position of cpuhp_state so that rapl_cpu_starting() is called
> after topology_update_package_map().

Does not work. You cannot call that callback in the starting context. It
does allocations. This needs be fixed in a different way. I'll have a look
tomorrow.

Thanks,

	tglx