Re: [Xen-devel] [RFC PATCH] PVH: cleanup of p2m upon p2m destroy

[Tim Deegan <tim@xxxxxxx>]

At 18:01 -0800 on 18 Dec (1387386115), Mukesh Rathor wrote:
> On Wed, 18 Dec 2013 17:51:52 +0100
> Tim Deegan <tim@xxxxxxx> wrote:
> 
> > At 11:09 +0100 on 18 Dec (1387361398), Tim Deegan wrote:
> > > > An alternative might be to just create a link list then and walk
> > > > it. In general, foreign mappings should be very small, so the
> > > > overhead of 16 bytes per page for the link list might not be too
> > > > bad. I will code it if there is no disagreement from any
> > > > maintainer... everyone has different ideas :)...
> > > 
> > > I think it would be best to walk the p2m trie (i.e. bounded by
> > > amount of RAM, rather than max GFN) and do it preemptably.  I'll
> > > look into something like that for the mem_sharing loop today, and
> > > foreign mapping code can reuse it.
> > 
> > What I've ended up with is making p2m_change_entry_type_global()
> > preemptible (which is a bigger task but will be needed as domains get
> > bigger).  Do you think that using that function to switch all mappings
> > from p2m_foreign to p2m_invalid, appropriately late in the teardown,
> > will be good enough for what you need?
> 
> No, not quite, because I need to know which mfns are foreign and
> do put_page on them. 

Oh right, but you need to do that anyway -- once you've got refcounts
held by ept entries, you need to do _all_ entry writes through the
function that preserves the refcount invariant.  So
p2m_change_entry_type_global() has to use that function too, in case
either of the types it's called with is p2m_foreign.  And once that's
fixed, a call to s/foreign/invalid/ DTRT.

Also, this:

> +static int ept_put_foreign_mfn(ept_entry_t *entry, unsigned long data)
> +{
> +    put_page(mfn_to_page(entry->mfn));
> +    return 0;
> +}

gives me the heebie-jeebies, because it explcitly _breaks_ the
invariant, leaving the foreign mapping in place without its refcount.

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel