lkml.org 
[lkml]   [2016]   [Feb]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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

    \
     
     \ /
      Last update: 2016-02-29 13:21    [W:3.812 / U:0.836 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site