Messages in this thread | | | Date | Mon, 29 Feb 2016 12:30:47 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH v5] perf/x86/amd/power: Add AMD accumulated power reporting mechanism |
| |
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
| |