Re: [PATCH 2 of 3] x86/mem_sharing: modularize reverse map for shared frames

[Tim Deegan <tim@xen.org>]

At 10:16 -0400 on 12 Apr (1334225773), Andres Lagar-Cavilla wrote:
>      /* Do the accounting first. If anything fails below, we have bigger
>       * bigger fish to fry. First, remove the gfn from the list. */ 
> -    last_gfn = list_has_one_entry(&page->sharing->gfns);
> +    last_gfn = rmap_has_one_entry(page);
>      if ( last_gfn )
>      {
> -        /* Clean up shared state */
> -        audit_del_list(page);
> +        /* Clean up shared state. Get rid of the <domid, gfn> tuple
> +         * before destroying the rmap. */
> +        mem_sharing_gfn_destroy(page, d, gfn_info);

Moving this mem_sharing_gfn_destroy() around seems like it's unrelated
to the rest of this patch, which is basically code motion.  If so, can
you put it in a patch of its own?

Otherwise, pending patch 3/3 being OK too, 

Acked-by: Tim Deegan <tim@xen.org>

> +        page_sharing_dispose(page);
>          page->sharing = NULL;
>          atomic_dec(&nr_shared_mfns);
>      }
>      else
>          atomic_dec(&nr_saved_mfns);
> +
>      /* If the GFN is getting destroyed drop the references to MFN 
>       * (possibly freeing the page), and exit early */
>      if ( flags & MEM_SHARING_DESTROY_GFN )
>      {
> -        mem_sharing_gfn_destroy(d, gfn_info);
> +        if ( !last_gfn )
> +            mem_sharing_gfn_destroy(page, d, gfn_info);
>          put_page_and_type(page);
>          mem_sharing_page_unlock(page);
>          if ( last_gfn && 
> @@ -987,7 +1052,6 @@ gfn_found:
>   
>      if ( last_gfn )
>      {
> -        mem_sharing_gfn_destroy(d, gfn_info);
>          /* Making a page private atomically unlocks it */
>          BUG_ON(page_make_private(d, page) != 0);
>          goto private_page_found;
> @@ -1011,7 +1075,7 @@ gfn_found:
>      unmap_domain_page(t);
>  
>      BUG_ON(set_shared_p2m_entry(d, gfn, page_to_mfn(page)) == 0);
> -    mem_sharing_gfn_destroy(d, gfn_info);
> +    mem_sharing_gfn_destroy(old_page, d, gfn_info);
>      mem_sharing_page_unlock(old_page);
>      put_page_and_type(old_page);
>  
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel