[PATCH 0 of 5] Memory sharing overhaul part 2

[PATCH 0 of 5] Memory sharing overhaul part 2

[Andres Lagar-Cavilla]

(Sigh, the previous pachbomb got truncated by an smtp quota in my 
provider... remaining patches in this series)

This patch series proposes an overhaul of the memory sharing code.

Aside from bug fixes and cleanups, the main features are:
- Polling of stats via libxc, libxl and console
- Removal of global sharing hashtable and global sharing lock 
(if audit disabled)
- Turned sharing audits into a domctl
- New domctl to populate vacant physmap entries with shared 
pages.

As a result, the domctl interface to sharing changes. The only in-tree
consumer of this interface is updated in the current series. It is 
important that if any out-of-tree consumer exists, that they state
their opinion on this interface change.

Patches 5 to 8, 10, 11, 15 and 18 are tools patches.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
Signed-off-by: Adin Scannell <adin@scannell.ca>

 xen/arch/x86/mm/mem_sharing.c |  106 ++++++++++++++++++++++++++++++++++++++++++
 xen/include/public/domctl.h   |    3 +-
 tools/libxc/xc_memshr.c       |   23 +++++++++
 tools/libxc/xenctrl.h         |    6 ++
 xen/arch/ia64/xen/mm.c        |    6 ++
 xen/arch/x86/mm/mem_sharing.c |    8 +++
 xen/common/keyhandler.c       |    7 +-
 xen/include/xen/mm.h          |    3 +
 xen/arch/x86/mm/mem_sharing.c |   17 ++++-
 xen/include/public/domctl.h   |    1 +
 tools/libxc/xc_memshr.c       |   14 +++++
 tools/libxc/xenctrl.h         |    2 +
 12 files changed, 188 insertions(+), 8 deletions(-)

[PATCH 1 of 5] Add a shared page to the physmap

[Andres Lagar-Cavilla]

 xen/arch/x86/mm/mem_sharing.c |  106 ++++++++++++++++++++++++++++++++++++++++++
 xen/include/public/domctl.h   |    3 +-
 2 files changed, 108 insertions(+), 1 deletions(-)


This domctl is useful to, for example, populate parts of a domain's
physmap with shared frames, directly.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>

diff -r 62ea5c05ae7b -r b8be3a08e628 xen/arch/x86/mm/mem_sharing.c
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -734,6 +734,76 @@ err_out:
     return ret;
 }
 
+int mem_sharing_add_to_physmap(struct domain *sd, unsigned long sgfn, shr_handle_t sh,
+                            struct domain *cd, unsigned long cgfn) 
+{
+    struct page_info *spage;
+    int ret = -EINVAL;
+    mfn_t smfn, cmfn;
+    p2m_type_t smfn_type, cmfn_type;
+    struct gfn_info *gfn_info;
+    struct p2m_domain *p2m = p2m_get_hostp2m(cd);
+    p2m_access_t a;
+
+    shr_lock();
+    p2m_lock(p2m);
+    
+    /* XXX if sd == cd handle potential deadlock by ordering
+     * the get_ and put_gfn's */
+    smfn = get_gfn_query(sd, sgfn, &smfn_type);
+    cmfn = get_gfn_type_access(p2m, cgfn, &cmfn_type, &a, p2m_query, NULL);
+
+    /* Get the source shared page, check and lock */
+    ret = XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID;
+    spage = __grab_shared_page(smfn);
+    if ( spage == NULL )
+        goto err_out;
+    ASSERT(smfn_type == p2m_ram_shared);
+
+    /* Check that the handles match */
+    if ( spage->shared_info->handle != sh )
+        goto err_unlock;
+
+    /* Make sure the target page is a hole in the physmap */
+    if ( mfn_valid(cmfn) ||
+         (!(p2m_is_ram(cmfn_type))) )
+    {
+        ret = XEN_DOMCTL_MEM_SHARING_C_HANDLE_INVALID;
+        goto err_unlock;
+    }
+
+    /* This is simpler than regular sharing */
+    BUG_ON(!get_page_and_type(spage, dom_cow, PGT_shared_page));
+    if ( (gfn_info = mem_sharing_gfn_alloc(spage, cd, cgfn)) == NULL )
+    {
+        put_page_and_type(spage);
+        ret = -ENOMEM;
+        goto err_unlock;
+    }
+
+    ret = set_p2m_entry(p2m, cgfn, smfn, PAGE_ORDER_4K, p2m_ram_shared, a);
+
+    /* Tempted to turn this into an assert */
+    if ( !ret ) {
+        ret = -ENOENT;
+        mem_sharing_gfn_destroy(gfn_info, 0);
+        put_page_and_type(spage);
+    } else {
+        atomic_inc(&cd->shr_pages);
+        atomic_inc(&nr_shared_mfns);
+        ret = 0;
+    }
+
+err_unlock:
+    mem_sharing_page_unlock(spage);
+err_out:
+    put_gfn(cd, cgfn);
+    put_gfn(sd, sgfn);
+    p2m_unlock(p2m);
+    shr_unlock();
+    return ret;
+}
+
 int mem_sharing_unshare_page(struct domain *d,
                              unsigned long gfn, 
                              uint16_t flags)
@@ -982,6 +1052,42 @@ int mem_sharing_domctl(struct domain *d,
         }
         break;
 
+        case XEN_DOMCTL_MEM_EVENT_OP_SHARING_ADD_PHYSMAP:
+        {
+            unsigned long sgfn, cgfn;
+            struct domain *cd;
+            shr_handle_t sh;
+
+            if ( !mem_sharing_enabled(d) )
+                return -EINVAL;
+
+            cd = get_domain_by_id(mec->u.share.client_domain);
+            if ( !cd )
+                return -ESRCH;
+
+            if ( !mem_sharing_enabled(cd) )
+            {
+                put_domain(cd);
+                return -EINVAL;
+            }
+
+            if ( XEN_DOMCTL_MEM_SHARING_FIELD_IS_GREF(mec->u.share.source_gfn) )
+            {
+                /* Cannot add a gref to the physmap */
+                put_domain(cd);
+                return -EINVAL;
+            }
+
+            sgfn    = mec->u.share.source_gfn;
+            sh      = mec->u.share.source_handle;
+            cgfn    = mec->u.share.client_gfn;
+
+            rc = mem_sharing_add_to_physmap(d, sgfn, sh, cd, cgfn); 
+
+            put_domain(cd);
+        }
+        break;
+
         case XEN_DOMCTL_MEM_EVENT_OP_SHARING_RESUME:
         {
             if ( !mem_sharing_enabled(d) )
diff -r 62ea5c05ae7b -r b8be3a08e628 xen/include/public/domctl.h
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -771,6 +771,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_mem_e
 #define XEN_DOMCTL_MEM_EVENT_OP_SHARING_DEBUG_GFN      5
 #define XEN_DOMCTL_MEM_EVENT_OP_SHARING_DEBUG_MFN      6
 #define XEN_DOMCTL_MEM_EVENT_OP_SHARING_DEBUG_GREF     7
+#define XEN_DOMCTL_MEM_EVENT_OP_SHARING_ADD_PHYSMAP    8
 
 #define XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID  (-10)
 #define XEN_DOMCTL_MEM_SHARING_C_HANDLE_INVALID  (-9)
@@ -797,7 +798,7 @@ struct xen_domctl_mem_sharing_op {
             } u;
             uint64_aligned_t  handle;     /* OUT: the handle           */
         } nominate;
-        struct mem_sharing_op_share {     /* OP_SHARE */
+        struct mem_sharing_op_share {     /* OP_SHARE/ADD_PHYSMAP */
             uint64_aligned_t source_gfn;    /* IN: the gfn of the source page */
             uint64_aligned_t source_handle; /* IN: handle to the source page */
             uint64_aligned_t client_domain; /* IN: the client domain id */

[PATCH 2 of 5] Tools: Libxc wrappers to add shared pages to physmap

[Andres Lagar-Cavilla]

 tools/libxc/xc_memshr.c |  23 +++++++++++++++++++++++
 tools/libxc/xenctrl.h   |   6 ++++++
 2 files changed, 29 insertions(+), 0 deletions(-)


Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>

diff -r b8be3a08e628 -r 14f08673af6a tools/libxc/xc_memshr.c
--- a/tools/libxc/xc_memshr.c
+++ b/tools/libxc/xc_memshr.c
@@ -86,6 +86,29 @@ int xc_memshr_nominate_gref(xc_interface
     return ret;
 }
 
+int xc_memshr_add_to_physmap(xc_interface *xch,
+                    uint32_t source_domain,
+                    uint64_t source_gfn,
+                    uint64_t source_handle,
+                    uint32_t client_domain,
+                    uint64_t client_gfn)
+{
+    DECLARE_DOMCTL;
+    struct xen_domctl_mem_sharing_op *op;
+
+    domctl.cmd                  = XEN_DOMCTL_mem_sharing_op;
+    domctl.interface_version    = XEN_DOMCTL_INTERFACE_VERSION;
+    domctl.domain               = (domid_t) source_domain;
+    op = &(domctl.u.mem_sharing_op);
+    op->op = XEN_DOMCTL_MEM_EVENT_OP_SHARING_ADD_PHYSMAP;
+    op->u.share.source_gfn      = source_gfn;
+    op->u.share.source_handle   = source_handle;
+    op->u.share.client_gfn      = client_gfn;
+    op->u.share.client_domain   = (uint64_t) client_domain;
+
+    return do_domctl(xch, &domctl);
+}
+
 int xc_memshr_share(xc_interface *xch,
                     uint32_t source_domain,
                     uint64_t source_gfn,
diff -r b8be3a08e628 -r 14f08673af6a tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -1895,6 +1895,12 @@ int xc_memshr_nominate_gref(xc_interface
                             uint32_t domid,
                             grant_ref_t gref,
                             uint64_t *handle);
+int xc_memshr_add_to_physmap(xc_interface *xch,
+                    uint32_t source_domain,
+                    uint64_t source_gfn,
+                    uint64_t source_handle,
+                    uint32_t client_domain,
+                    uint64_t client_gfn);
 int xc_memshr_share(xc_interface *xch,
                     uint32_t source_domain,
                     uint64_t source_gfn,

[PATCH 3 of 5] Add the ability to poll stats about shared memory via the console

[Andres Lagar-Cavilla]

 xen/arch/ia64/xen/mm.c        |  6 ++++++
 xen/arch/x86/mm/mem_sharing.c |  8 ++++++++
 xen/common/keyhandler.c       |  7 +++++--
 xen/include/xen/mm.h          |  3 +++
 4 files changed, 22 insertions(+), 2 deletions(-)


Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>

diff -r 14f08673af6a -r 536da18b7ab6 xen/arch/ia64/xen/mm.c
--- a/xen/arch/ia64/xen/mm.c
+++ b/xen/arch/ia64/xen/mm.c
@@ -3565,6 +3565,12 @@ p2m_pod_decrease_reservation(struct doma
     return 0;
 }
 
+/* Simple no-op */
+void arch_dump_shared_mem_info(void)
+{
+    printk("Shared memory not supported yet\n");
+}
+
 /*
  * Local variables:
  * mode: C
diff -r 14f08673af6a -r 536da18b7ab6 xen/arch/x86/mm/mem_sharing.c
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1129,6 +1129,14 @@ int mem_sharing_domctl(struct domain *d,
     return rc;
 }
 
+void arch_dump_shared_mem_info(void)
+{
+    printk("Shared pages %u -- Saved frames %u -- Dom CoW footprintf %u\n",
+            mem_sharing_get_nr_shared_mfns(),
+            mem_sharing_get_nr_saved_mfns(),
+            dom_cow->tot_pages);
+}
+
 void __init mem_sharing_init(void)
 {
     printk("Initing memory sharing.\n");
diff -r 14f08673af6a -r 536da18b7ab6 xen/common/keyhandler.c
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -15,6 +15,7 @@
 #include <xen/compat.h>
 #include <xen/ctype.h>
 #include <xen/perfc.h>
+#include <xen/mm.h>
 #include <asm/debugger.h>
 #include <asm/div64.h>
 
@@ -248,8 +249,8 @@ static void dump_domains(unsigned char k
         printk("    refcnt=%d dying=%d pause_count=%d\n",
                atomic_read(&d->refcnt), d->is_dying,
                atomic_read(&d->pause_count));
-        printk("    nr_pages=%d xenheap_pages=%d dirty_cpus=%s max_pages=%u\n",
-               d->tot_pages, d->xenheap_pages, tmpstr, d->max_pages);
+        printk("    nr_pages=%d xenheap_pages=%d shared_pages=%u dirty_cpus=%s max_pages=%u\n",
+               d->tot_pages, d->xenheap_pages, atomic_read(&d->shr_pages), tmpstr, d->max_pages);
         printk("    handle=%02x%02x%02x%02x-%02x%02x-%02x%02x-"
                "%02x%02x-%02x%02x%02x%02x%02x%02x vm_assist=%08lx\n",
                d->handle[ 0], d->handle[ 1], d->handle[ 2], d->handle[ 3],
@@ -308,6 +309,8 @@ static void dump_domains(unsigned char k
         }
     }
 
+    arch_dump_shared_mem_info();
+
     rcu_read_unlock(&domlist_read_lock);
 #undef tmpstr
 }
diff -r 14f08673af6a -r 536da18b7ab6 xen/include/xen/mm.h
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -73,6 +73,9 @@ int assign_pages(
     unsigned int order,
     unsigned int memflags);
 
+/* Dump info to serial console */
+void arch_dump_shared_mem_info(void);
+
 /* memflags: */
 #define _MEMF_no_refcount 0
 #define  MEMF_no_refcount (1U<<_MEMF_no_refcount)

[PATCH 4 of 5] Adds a separate domctl for performing sharing audits

[Andres Lagar-Cavilla]

 xen/arch/x86/mm/mem_sharing.c |  17 ++++++++++++-----
 xen/include/public/domctl.h   |   1 +
 2 files changed, 13 insertions(+), 5 deletions(-)


Sharing audits are heavyweight, so instead of performing them inline,
we make them callable via a domctl.

Signed-off-by: Adin Scannell <adin@scannell.ca>

diff -r 536da18b7ab6 -r a79bb54cb4dc xen/arch/x86/mm/mem_sharing.c
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -820,7 +820,6 @@ int mem_sharing_unshare_page(struct doma
      * between shr_lock and p2m fine-grained locks in mm-lock. 
      * Callers may walk in here already holding the lock for this gfn */
     shr_lock();
-    mem_sharing_audit();
     mfn = get_gfn(d, gfn, &p2mt);
     
     /* Has someone already unshared it? */
@@ -1117,15 +1116,23 @@ int mem_sharing_domctl(struct domain *d,
         }
         break;
 
+        case XEN_DOMCTL_MEM_EVENT_OP_SHARING_AUDIT:
+        {
+#if MEM_SHARING_AUDIT
+            shr_lock();
+            rc = mem_sharing_audit();
+            shr_unlock();
+#else
+            rc = -ENOSYS;
+#endif
+            break;
+        }
+
         default:
             rc = -ENOSYS;
             break;
     }
 
-    shr_lock();
-    mem_sharing_audit();
-    shr_unlock();
-
     return rc;
 }
 
diff -r 536da18b7ab6 -r a79bb54cb4dc xen/include/public/domctl.h
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -772,6 +772,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_mem_e
 #define XEN_DOMCTL_MEM_EVENT_OP_SHARING_DEBUG_MFN      6
 #define XEN_DOMCTL_MEM_EVENT_OP_SHARING_DEBUG_GREF     7
 #define XEN_DOMCTL_MEM_EVENT_OP_SHARING_ADD_PHYSMAP    8
+#define XEN_DOMCTL_MEM_EVENT_OP_SHARING_AUDIT          9
 
 #define XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID  (-10)
 #define XEN_DOMCTL_MEM_SHARING_C_HANDLE_INVALID  (-9)

[PATCH 5 of 5] Tools: Libxc wrapper for the new sharing audit domctl

[Andres Lagar-Cavilla]

 tools/libxc/xc_memshr.c |  14 ++++++++++++++
 tools/libxc/xenctrl.h   |   2 ++
 2 files changed, 16 insertions(+), 0 deletions(-)


Signed-off-by: Adin Scannell <adin@scannell.ca>

diff -r a79bb54cb4dc -r 6da10b110204 tools/libxc/xc_memshr.c
--- a/tools/libxc/xc_memshr.c
+++ b/tools/libxc/xc_memshr.c
@@ -211,3 +211,17 @@ int xc_memshr_debug_gref(xc_interface *x
     return do_domctl(xch, &domctl);
 }
 
+int xc_memshr_audit(xc_interface *xch,
+                    uint32_t domid)
+{
+    DECLARE_DOMCTL;
+    struct xen_domctl_mem_sharing_op *op;
+
+    domctl.cmd = XEN_DOMCTL_mem_sharing_op;
+    domctl.interface_version = XEN_DOMCTL_INTERFACE_VERSION;
+    domctl.domain = (domid_t)domid;
+    op = &(domctl.u.mem_sharing_op);
+    op->op = XEN_DOMCTL_MEM_EVENT_OP_SHARING_AUDIT;
+
+    return do_domctl(xch, &domctl);
+}
diff -r a79bb54cb4dc -r 6da10b110204 tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -1921,6 +1921,8 @@ int xc_memshr_debug_mfn(xc_interface *xc
 int xc_memshr_debug_gref(xc_interface *xch,
                          uint32_t domid,
                          grant_ref_t gref);
+int xc_memshr_audit(xc_interface *xch,
+                    uint32_t domid);
 
 int xc_flask_load(xc_interface *xc_handle, char *buf, uint32_t size);
 int xc_flask_context_to_sid(xc_interface *xc_handle, char *buf, uint32_t size, uint32_t *sid);

Re: [PATCH 0 of 5] Memory sharing overhaul part 2

[Jan Beulich]

>>> On 08.12.11 at 14:54, Andres Lagar-Cavilla <andres@lagarcavilla.org> wrote:
>...
>  xen/arch/x86/mm/mem_sharing.c |  106 ++++++++++++++++++++++++++++++++++++++++++
>  xen/include/public/domctl.h   |    3 +-
>  tools/libxc/xc_memshr.c       |   23 +++++++++
>  tools/libxc/xenctrl.h         |    6 ++
>  xen/arch/ia64/xen/mm.c        |    6 ++
>  xen/arch/x86/mm/mem_sharing.c |    8 +++
>  xen/common/keyhandler.c       |    7 +-
>  xen/include/xen/mm.h          |    3 +
>  xen/arch/x86/mm/mem_sharing.c |   17 ++++-
>  xen/include/public/domctl.h   |    1 +
>  tools/libxc/xc_memshr.c       |   14 +++++
>  tools/libxc/xenctrl.h         |    2 +
>  12 files changed, 188 insertions(+), 8 deletions(-)

As this isn't the first time with your patch submissions - I'm not sure what
you use to generate these stats, but in order to be useful I would
generally expect this to be a list with duplicate entries folded rather than
a mere concatenation of those from the individual patches (leaving doing
the arithmetic to the reader/reviewer).

Jan

Re: [PATCH 0 of 5] Memory sharing overhaul part 2

[Jan Beulich]

>>> On 08.12.11 at 14:54, Andres Lagar-Cavilla <andres@lagarcavilla.org> wrote:
> (Sigh, the previous pachbomb got truncated by an smtp quota in my 
> provider... remaining patches in this series)

But you also didn't re-write the description for this shorter series:

> This patch series proposes an overhaul of the memory sharing code.
> 
> Aside from bug fixes and cleanups, the main features are:

I wasn't able to spot any bug fixes or cleanups, and apart from
patch 3 all others merely add dead code. I'm therefore tempted to
nak 1, 2, 4, and 5 unless you come forward with code that actually
uses these new bits.

> - Polling of stats via libxc, libxl and console
> - Removal of global sharing hashtable and global sharing lock 
> (if audit disabled)
> - Turned sharing audits into a domctl
> - New domctl to populate vacant physmap entries with shared 
> pages.
> 
> As a result, the domctl interface to sharing changes. The only in-tree
> consumer of this interface is updated in the current series. It is 
> important that if any out-of-tree consumer exists, that they state
> their opinion on this interface change.
> 
> Patches 5 to 8, 10, 11, 15 and 18 are tools patches.

There's no patch 8 and higher here.

Jan

Re: [PATCH 0 of 5] Memory sharing overhaul part 2

[Andres Lagar-Cavilla]

>>>> On 08.12.11 at 14:54, Andres Lagar-Cavilla <andres@lagarcavilla.org>
>>>> wrote:
>>...
>>  xen/arch/x86/mm/mem_sharing.c |  106
>> ++++++++++++++++++++++++++++++++++++++++++
>>  xen/include/public/domctl.h   |    3 +-
>>  tools/libxc/xc_memshr.c       |   23 +++++++++
>>  tools/libxc/xenctrl.h         |    6 ++
>>  xen/arch/ia64/xen/mm.c        |    6 ++
>>  xen/arch/x86/mm/mem_sharing.c |    8 +++
>>  xen/common/keyhandler.c       |    7 +-
>>  xen/include/xen/mm.h          |    3 +
>>  xen/arch/x86/mm/mem_sharing.c |   17 ++++-
>>  xen/include/public/domctl.h   |    1 +
>>  tools/libxc/xc_memshr.c       |   14 +++++
>>  tools/libxc/xenctrl.h         |    2 +
>>  12 files changed, 188 insertions(+), 8 deletions(-)
>
> As this isn't the first time with your patch submissions - I'm not sure
> what
> you use to generate these stats, but in order to be useful I would
> generally expect this to be a list with duplicate entries folded rather
> than
> a mere concatenation of those from the individual patches (leaving doing
> the arithmetic to the reader/reviewer).
I apologize. I am also frustrated by this. Hugely.
Andres
>
> Jan
>
>

Re: [PATCH 0 of 5] Memory sharing overhaul part 2

[Tim Deegan]

At 08:54 -0500 on 08 Dec (1323334476), Andres Lagar-Cavilla wrote:
> (Sigh, the previous pachbomb got truncated by an smtp quota in my 
> provider... remaining patches in this series)

I only got up to #12/18 in the previous round, so by my count I'm still
one short.  

Cheers,

Tim.