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

[Xen-devel] Re: [PATCH] Support swap a page from user space tools -- Was

To: Tim Deegan <Tim.Deegan@xxxxxxxxxxxxx>, "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] Support swap a page from user space tools -- Was RE: [RFC][PATCH] Basic support for page offline
From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Date: Thu, 19 Mar 2009 09:45:54 +0000
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Thu, 19 Mar 2009 02:46:36 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20090319093245.GG11733@xxxxxxxxxxxxxxxxxxxxx>
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcmodaxGpRh5muaRTCCma6vBJRy3NQAAdKad
Thread-topic: [PATCH] Support swap a page from user space tools -- Was RE: [RFC][PATCH] Basic support for page offline
User-agent: Microsoft-Entourage/12.15.0.081119
On 19/03/2009 09:32, "Tim Deegan" <Tim.Deegan@xxxxxxxxxxxxx> wrote:

>>> - You're passing a physical address (of the PTE to update) in an MFN
>>>   field.  That's not going to be big enough on all platforms.  Also   it's
>>> pretty confusing.
>> 
>> Yes, fixed and now named pte_addr as a uint64.
> 
> You made it an unsigned long, which is still smaller than a paddr_t on
> PAE builds.  And you can't just make it 64 bits in that union without
> breaking the ABI; you'll need to add a new interface somewhere.  Maybe
> Keir can suggest a better place.

Firstly, the comment added to the header file is pretty rubbish. The
description fits existing update methods such as MMU_NORMAL_PT_UPDATE, so
based on that comment I could quite reasonably reject your patch on grounds
that it is redundant.

Secondly, the patch name says swap_page and a printk the patch adds refers
to 'swap page'. What's being swapped? That's not the name of the operation,
nor is swapping referred to in the description comment I mention above.

Thirdly, perhaps this makes more sense as a MMU_* op hanging off
mmu_update()? That call takes pairs of u64 values, which could give you the
space you require. Then you can add a nice comment explaining how your new
command differs from MMU_NORMAL_PT_UPDATE.

 -- Keir



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

<Prev in Thread] Current Thread [Next in Thread>