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: [RFC][PATCH] Basic support for page offline

To: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Subject: [Xen-devel] RE: [RFC][PATCH] Basic support for page offline
From: "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
Date: Sun, 15 Feb 2009 17:48:13 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Sun, 15 Feb 2009 01:48:53 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20090213170341.GC17060@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>
References: <E2263E4A5B2284449EEBD0AAB751098401C781605D@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20090213170341.GC17060@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcmN/Qv2l2LCkqSjSyyBudpWSZl9zABVIvrw
Thread-topic: [RFC][PATCH] Basic support for page offline
Tim Deegan <mailto:Tim.Deegan@xxxxxxxxxx> wrote:
> Hi,
> 
> So a few more comments on the detail of those patches.
> 
> I had imagined that you would suspend the domain, then update the p2m
> and pagetables in the guest memory from the _tools_.  That
> would involve
> less code (possibly none) in Xen, and is how I'd prefer it.  But your
> current approach probably catches more of the corner cases (grant tables
> &c) than the tools could, so that's OK.
> 
> update_pgtable_entry() needs a more descriptive name!  It updates
> potentially very many pagetable entries, and in a particular way.
> Also it probably ought to be static.

Tim, thanks for your feedback very much. Yes, the update_pgtable_entry() will 
update potentially very much page table entries, I'm not sure that's the right 
method to achieve it.

> 
> The reference counting in update_pgtable_entry() is confusing -- it
> should probably always do reference counting for both the old and new
> entries; that seems more robust than only doing the decrements
> there and
> manually setting count_info and type_info on the new page in replace_page.

Sure, I will do like this.

> 
> In replace_page(), your error paths are confused: the ENOMEM error case
> drops a ref that wasn't taken and if get_page() fails you
> don't free the
> allocated page.
> 
> Both of those functions need comments describing what they do and what
> their arguments are. 
> 
> memory_page_offline(): again, check your error and exit paths; I'm
> pretty sure you leak references to the domain.  Why does this take a
> domain, by the way?  can't it just take a range of MFNs and figure out
> the owning domain for each one as it goes?
> 
> Also, isn't the returned nr_offlined value always one less than was
> requested?  You write back the _index_ of the highest-numbered frame
> that you _attempted_ to offline, which is a pretty confusing number.
> 
> Other than that, the xen mm patch just needs a good scattering of comments.

Sure, I will update it.

> 
> The tools patch is enormous, and seems to copy big chunks of
> xc_domain_save into a new file.  And since Xen is now doing the hard
> work of pagetable manipulation, I don't think you even need to suspend
> the guest -- just pausing it should be enough and is much easier.

But I'm not sure if we can update the P2M table from Xen side, that's the 
reason I did the it in the user space.

-- Yunhong Jiang


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