[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [Xen-devel] [PATCH 1/9] Add cpu idle pwr mgmt to xen


  • To: "Keir Fraser" <keir.fraser@xxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • From: "Wei, Gang" <gang.wei@xxxxxxxxx>
  • Date: Fri, 25 Apr 2008 21:29:04 +0800
  • Delivery-date: Fri, 25 Apr 2008 06:29:42 -0700
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AcimklJCPOH/MtO7S+K/FieFl7t35AAQgZa+AACSzuA=
  • Thread-topic: [Xen-devel] [PATCH 1/9] Add cpu idle pwr mgmt to xen

On Friday, April 25, 2008 9:01 PM, Keir Fraser wrote:
> On 25/4/08 06:07, "Wei, Gang" <gang.wei@xxxxxxxxx> wrote:
> 
>> Add basic acpi C-states based cpu idle power mgmt in xen for x86.
> 
> Comments:
>  1. In the idle loop you can just replace default_idle() with
(*pm_idle)()
> directly.
>  2. Do not modify common/keyhandler.c. Instead add an __initcall() in
> cpu_idle.c to register your keyhandler. The initcall function and your
> keyhandler can both be 'static' functions.
>  3. I don't like ifdef COMPAT all over new files. Define a separate
compat
> shim file, built only for x86_64, which converts compat structures to
> native 64-bit structures. Then cpu_idle.c need know nothing about
compat
> issues and will be cleaner for it.
>  4. Don't define placeholders for Px and Tx info in the platform
hypercall
> header. They should be introduced when they are actually implemented.
> 
> That's it. I haven't looked at the other patches yet, but you can
probably
> make the above fixes and resubmit just this one patch without
affecting
> the others. I'll look at them after this one goes in.

It is great idea to do it one after one. I will focus on above 4
comments for this patch and come back soon. Thanks for quick comments.

> 
>  -- Keir

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.