Re: [RFC PATCH V4 5/5] cpuidle: cpuidle driver for apm

From: Trinabh Gupta
Date: Wed Mar 23 2011 - 06:25:15 EST

Hi Stephen,

Thanks for reviewing.

On 03/23/2011 06:44 AM, Stephen Rothwell wrote:

On Tue, 22 Mar 2011 18:03:40 +0530 Trinabh Gupta<trinabh@xxxxxxxxxxxxxxxxxx> wrote:

+static int apm_setup_cpuidle(int cpu)
+ struct cpuidle_device *dev = kzalloc(sizeof(struct cpuidle_device),

Same as xen comments: no NULL check.

+ dev->cpu = cpu;
+ dev->drv =&apm_idle_driver;

Also wondering why you would ever have a different idle routine on
different cpus?

Yes, this is an ongoing debate. Apparently it is a possibility
because of ACPI bugs. CPU's can have asymmetric C-states
and overall different idle routines on different cpus. Please
refer to and for a discussion around this.

I have posted a patch series that does global registration
i.e same idle routines for each cpu. Please check . That series applies on
top of this series. Global registration significantly
simplifies the design, but still we are not sure about the
direction to take.

+ dev->states[count] = state_apm_idle;
+ count++;
+ dev->state_count = count;
+ if (cpuidle_register_device(dev))
+ return -EIO;

And we leak dev.

+static void apm_idle_exit(void)
+ cpuidle_unregister_driver(&apm_idle_driver);

Do we leak the per cpu device structure here?

I will see how we can save
per cpu device structure pointers so that we can free them.

+ return;

Unnecessary return statement.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at