WARNING - OLD ARCHIVES

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/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Jan Beulich <jbeulich@xxxxxxxxxx>, Gang Wei <gang.wei@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 1/9] Add cpu idle pwr mgmt to xen
From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Date: Wed, 30 Apr 2008 09:54:15 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Wed, 30 Apr 2008 01:55:21 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <48183A3F.76E4.0078.0@xxxxxxxxxx>
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/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Aciqn8RNAuw9vxaTEd2yrAAX8io7RQ==
Thread-topic: [Xen-devel] [PATCH 1/9] Add cpu idle pwr mgmt to xen
User-agent: Microsoft-Entourage/11.4.0.080122
On 30/4/08 08:22, "Jan Beulich" <jbeulich@xxxxxxxxxx> wrote:

>>>> "Wei, Gang" <gang.wei@xxxxxxxxx> 30.04.08 05:27 >>>
>> Revising done according to Jan's comments. Resend.
> 
> Thanks. Unfortunately you now use a static (but not per-CPU) variable -
> while I understand that it is expected that the call is done just once, I
> don't think this is a good thing to do.

Why is the variable even non-local? Is it just to make the xlat_malloc*()
interfaces simpler? It's a false simplification if so, and I think you'd be
better making the variable an explicit parameter to those functions.

Also I agree with Jan regarding non-ISO C usage of loop-header variable
declarations (don't do it) and also you should check copy_from_guest*()
return values and return -EFAULT where appropriate. His comment regarding
explicit padding or use of uint32_t in your public bitfield also sounds good
to me.

 -- Keir



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