This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


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

To: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>
Subject: [Xen-devel] Re: [RFC PATCH V4 5/5] cpuidle: cpuidle driver for apm
From: Trinabh Gupta <trinabh@xxxxxxxxxxxxxxxxxx>
Date: Wed, 23 Mar 2011 15:55:01 +0530
Cc: venki@xxxxxxxxxx, ak@xxxxxxxxxxxxxxx, suresh.b.siddha@xxxxxxxxx, peterz@xxxxxxxxxxxxx, benh@xxxxxxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxx, arjan@xxxxxxxxxxxxxxx, lenb@xxxxxxxxxx
Delivery-date: Thu, 24 Mar 2011 17:06:06 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20110323121458.ec7cdaf9.sfr@xxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20110322123208.28725.30945.stgit@xxxxxxxxxxxxxxxxxxx> <20110322123336.28725.29810.stgit@xxxxxxxxxxxxxxxxxxx> <20110323121458.ec7cdaf9.sfr@xxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20100621 Fedora/3.0.5-1.fc11 Thunderbird/3.0.5
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>  

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

Same as xen comments: no NULL check.

+       int count = CPUIDLE_DRIVER_STATE_START;
+       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 http://lkml.org/lkml/2009/9/24/132 and
https://lkml.org/lkml/2011/2/10/37 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
http://lkml.org/lkml/2011/3/22/161 . 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.


Xen-devel mailing list