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 S1754597AbcB2LcS (ORCPT <rfc822;w@1wt.eu>);
	Mon, 29 Feb 2016 06:32:18 -0500
Received: from www.linutronix.de ([62.245.132.108]:39918 "EHLO
	Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752386AbcB2LcQ (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 29 Feb 2016 06:32:16 -0500
Date: Mon, 29 Feb 2016 12:30:47 +0100 (CET)
From: Thomas Gleixner <tglx@linutronix.de>
To: Huang Rui <ray.huang@amd.com>
cc: Borislav Petkov <bp@suse.de>, Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>, Andy Lutomirski <luto@amacapital.net>,
        Robert Richter <rric@kernel.org>, Jacob Shin <jacob.w.shin@gmail.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Kan Liang <kan.liang@intel.com>, linux-kernel@vger.kernel.org,
        spg_linux_kernel@amd.com, x86@kernel.org,
        Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
        Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>,
        Borislav Petkov <bp@alien8.de>, Fengguang Wu <fengguang.wu@intel.com>,
        Guenter Roeck <linux@roeck-us.net>
Subject: Re: [PATCH v5] perf/x86/amd/power: Add AMD accumulated power reporting
 mechanism
In-Reply-To: <20160229180048.GB20610@hr-amur2>
Message-ID: <alpine.DEB.2.11.1602291125510.3638@nanos>
References: <1456479650-4942-1-git-send-email-ray.huang@amd.com> <alpine.DEB.2.11.1602261100570.3638@nanos> <20160229180048.GB20610@hr-amur2>
User-Agent: Alpine 2.11 (DEB 23 2013-08-11)
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
X-Linutronix-Spam-Score: -1.0
X-Linutronix-Spam-Level: -
X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required,  ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, 1 Mar 2016, Huang Rui wrote:
> On Fri, Feb 26, 2016 at 11:18:28AM +0100, Thomas Gleixner wrote:
> > > +static int __init amd_power_pmu_init(void)
> > > +{
> > > +	int i, ret;
> > > +	u64 tmp;
> > > +
> > > +	if (!x86_match_cpu(cpu_match))
> > > +		return 0;
> > > +
> > > +	if (!boot_cpu_has(X86_FEATURE_ACC_POWER))
> > > +		return -ENODEV;
> > > +
> > > +	cores_per_cu = amd_get_cores_per_cu();
> > > +	cu_num = boot_cpu_data.x86_max_cores / cores_per_cu;
> > 
> > Please use the new package management functions which are on the way to tip.
> > 
> Can you give me some hints?

http://git.kernel.org/tip/1f12e32f4cd5243ae46d8b933181be0d022c6793

topology_max_packages() is what you want.
 
> > You cannot issue the CPU STARTING callback on present, but not online cpus.
> > 
> 
> Do you mean we should change for_each_present_cpu to
> for_each_online_cpu?
> 
> My orignal intent here, it's to allocate data structures of
> "power_pmu_masks" for each core.

Sure, you can allocate stuff for present cpus, but you cannot call the
CPU_STARTING callback for offline cpus.

> But now, I think we needn't below codes, right?

-ENOPARSE
 
> for_each_online_cpu(i)
>         power_cpu_init(i);

You still need that.

> > > +
> > > +	__perf_cpu_notifier(power_cpu_notifier);
> > > +
> > > +	ret = perf_pmu_register(&pmu_class, "power", -1);
> > > +	if (WARN_ON(ret)) {
> > > +		pr_warn("AMD Power PMU registration failed\n");
> > 
> > 
> > So that leaves the notifier installed and leaks all the allocated memory.
> > 
> 
> OK, do you mean "issue CPU_STARTING callback on present cpus" will
> cause the memory leak here? Could you please explain more?

Oh well, can you actually read your own code? You allocate a gazillion of
memory in power_cpu_prepare() and on error you just leak it.

Thanks,

	tglx