Re: [Xen-devel] [PATCH] Paging and memory sharing for HVM guests

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Wed, Dec 16, 2009 at 11:14:47PM +0000, Grzegorz Milos wrote:
> The series of 46 patches attached to this email contain the initial
> implementation of memory paging and sharing for Xen. Patrick Colp
> leads the work on the pager, and I am mostly responsible for memory
> sharing. We would be grateful for any comments/suggestions you might
> have. Individual patches are labeled with comments describing their
> purpose and a sign-off footnote. Of course we are happy to discuss
> them in more detail, as required. Assuming that there are no major
> objections against including them in the mainstream xen-unstable tree,
> we would like to move future development to that tree.


Congrats!

And now to the tiny review I did:

1). mem_event_xen_core.patch

"Copyright (c) 2009 Citrix (R)&D) Ltd. (Patrick Colp)"

The lawyers might not like it the copyright being assigned to a non-existent
entity :-(


2).  mem_paging_xen_pfec_page_paged.patch:

15 +    if ( p2m_is_paging(p2mt) )
 16 +    {
 17 +//        if ( p2m_is_paged(p2mt) )

I think you can safely remove the commented out code.

3). mem_sharing_xen_mm.patch:

 27 +    /*
 28 +     * Initialise our DOMID_IO domain.
                                ^^->COW
 29 +     * This domain owns sharable pages.
 30 +     */
 31 +    dom_cow = domain_create(DOMID_COW, DOMCRF_dummy, 0);


4), mem_sharing_tools_eagain.patch

The code spins with a 1 second timeout forever. Would it make sense
to include a retry counter so that, say after an hour (or maybe something
much smaller) you give up?

In the pv-ops patch series:

1). The  "*mfnp |= 0x80000000U;" and "*mfnp |= 0xf0000000U;" should
    use a #define. Maybe copy over the #defines from the xen tree ?

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