Re: [PATCH v6] x86/mem-sharing: mem-sharing a range of memory

[Wei Liu <wei.liu2@citrix.com>]

On Mon, Jul 11, 2016 at 01:57:50PM -0600, Tamas K Lengyel wrote:
> Currently mem-sharing can be performed on a page-by-page basis from the control
> domain. However, this process is quite wasteful when a range of pages have to
> be deduplicated.
> 
> This patch introduces a new mem_sharing memop for range sharing where
> the user doesn't have to separately nominate each page in both the source and
> destination domain, and the looping over all pages happen in the hypervisor.
> This significantly reduces the overhead of sharing a range of memory.
> 
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  tools/libxc/include/xenctrl.h        |  15 ++++
>  tools/libxc/xc_memshr.c              |  19 +++++
>  tools/tests/mem-sharing/memshrtool.c |  22 ++++++
>  xen/arch/x86/mm/mem_sharing.c        | 140 +++++++++++++++++++++++++++++++++++
>  xen/include/public/memory.h          |  10 ++-
>  5 files changed, 205 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 4a85b4a..650a763 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2333,6 +2333,21 @@ int xc_memshr_add_to_physmap(xc_interface *xch,
>                      domid_t client_domain,
>                      unsigned long client_gfn);
>  
> +/* Allows to deduplicate a range of memory of a client domain. Using
> + * this function is equivalent of calling xc_memshr_nominate_gfn for each gfn
> + * in the two domains followed by xc_memshr_share_gfns.
> + *
> + * May fail with -EINVAL if the source and client domain have different
> + * memory size or if memory sharing is not enabled on either of the domains.
> + * May also fail with -ENOMEM if there isn't enough memory available to store
> + * the sharing metadata before deduplication can happen.
> + */
> +int xc_memshr_range_share(xc_interface *xch,
> +                         domid_t source_domain,
> +                         domid_t client_domain,
> +                         unsigned long start,
> +                         unsigned long end);

Indentation seems wrong -- did you use tab here?

> +
>  /* Debug calls: return the number of pages referencing the shared frame backing
>   * the input argument. Should be one or greater. 
>   *
> diff --git a/tools/libxc/xc_memshr.c b/tools/libxc/xc_memshr.c
> index deb0aa4..6b26f3f 100644
> --- a/tools/libxc/xc_memshr.c
> +++ b/tools/libxc/xc_memshr.c
> @@ -181,6 +181,25 @@ int xc_memshr_add_to_physmap(xc_interface *xch,
>      return xc_memshr_memop(xch, source_domain, &mso);
>  }
>  
> +int xc_memshr_range_share(xc_interface *xch,
> +                         domid_t source_domain,
> +                         domid_t client_domain,
> +                         unsigned long start,
> +                         unsigned long end)

Ditto.

> +{
> +    xen_mem_sharing_op_t mso;
> +
> +    memset(&mso, 0, sizeof(mso));
> +
> +    mso.op = XENMEM_sharing_op_range_share;
> +
> +    mso.u.range.client_domain = client_domain;
> +    mso.u.range.start = start;
> +    mso.u.range.end = end;
> +
> +    return xc_memshr_memop(xch, source_domain, &mso);
> +}
> +
>  int xc_memshr_domain_resume(xc_interface *xch,
>                              domid_t domid)
>  {
> diff --git a/tools/tests/mem-sharing/memshrtool.c b/tools/tests/mem-sharing/memshrtool.c
> index 437c7c9..bd1f4e0 100644
> --- a/tools/tests/mem-sharing/memshrtool.c
> +++ b/tools/tests/mem-sharing/memshrtool.c
> @@ -24,6 +24,8 @@ static int usage(const char* prog)
>      printf("  nominate <domid> <gfn>  - Nominate a page for sharing.\n");
>      printf("  share <domid> <gfn> <handle> <source> <source-gfn> <source-handle>\n");
>      printf("                          - Share two pages.\n");
> +    printf("  range <source-domid> <destination-domid> <start-gfn> <end-gfn>\n");
> +    printf("                          - Share pages between domains in range.\n");
>      printf("  unshare <domid> <gfn>   - Unshare a page by grabbing a writable map.\n");
>      printf("  add-to-physmap <domid> <gfn> <source> <source-gfn> <source-handle>\n");
>      printf("                          - Populate a page in a domain with a shared page.\n");
> @@ -180,6 +182,26 @@ int main(int argc, const char** argv)
>          }
>          printf("Audit returned %d errors.\n", rc);
>      }
> +    else if( !strcasecmp(cmd, "range") )
> +    {
> +        domid_t sdomid, cdomid;
> +        int rc;
> +        unsigned long start, end;
> +
> +        if( argc != 6 )

Space after "if".

With all those fixed:

Acked-by: Wei Liu <wei.liu2@citrix.com>

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