Memory sharing overhaul part 2
PATCH 1/1 Memory sharing overhaul part 2
(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.
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/1 Add a shared page to the physmap
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.
xen/arch/x86/mm/mem_sharing.c +106 -0
@@ -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) )
xen/include/public/domctl.h +2 -1
@@ -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 1/1 Tools: Libxc wrappers to add shared pages to physmap
In reply to Andres Lagar-Cavillatools/libxc/xc_memshr.c | 23 +++++++++++++++++++++++ tools/libxc/xenctrl.h | 6 ++++++ 2 files changed, 29 insertions(+), 0 deletions(-)
tools/libxc/xc_memshr.c +23 -0
@@ -86,6 +86,29 @@ int xc_memshr_nominate_gref(xc_interfacereturn 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,
tools/libxc/xenctrl.h +6 -0
@@ -1895,6 +1895,12 @@ int xc_memshr_nominate_gref(xc_interfaceuint32_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 1/1 Add the ability to poll stats about shared memory via the console
In reply to Andres Lagar-Cavillaxen/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(-)
xen/arch/ia64/xen/mm.c +6 -0
@@ -3565,6 +3565,12 @@ p2m_pod_decrease_reservation(struct domareturn 0 ; } +/* Simple no-op */ +void arch_dump_shared_mem_info (void ) +{ +printk ("Shared memory not supported yet\n" ); +} +/* * Local variables: * mode: C
xen/arch/x86/mm/mem_sharing.c +8 -0
@@ -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" );
xen/common/keyhandler.c +5 -2
@@ -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 kprintk (" 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 }
xen/include/xen/mm.h +3 -0
@@ -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 1/1 Adds a separate domctl for performing sharing audits
In reply to Andres Lagar-Cavillaxen/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.
xen/arch/x86/mm/mem_sharing.c +12 -5
@@ -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; }
xen/include/public/domctl.h +1 -0
@@ -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 1/1 Tools: Libxc wrapper for the new sharing audit domctl
In reply to Andres Lagar-Cavillatools/libxc/xc_memshr.c | 14 ++++++++++++++ tools/libxc/xenctrl.h | 2 ++ 2 files changed, 16 insertions(+), 0 deletions(-)
tools/libxc/xc_memshr.c +14 -0
@@ -211,3 +211,17 @@ int xc_memshr_debug_gref(xc_interface *xreturn 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); +}
tools/libxc/xenctrl.h +2 -0
@@ -1921,6 +1921,8 @@ int xc_memshr_debug_mfn(xc_interface *xcint 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);
Quoted 15 lines
Quoted 1 line
Quoted 1 line · Andres Lagar-Cavilla
...
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
Quoted 3 lines
Quoted 1 line
Quoted 1 line · Andres Lagar-Cavilla
(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:
Quoted 3 lines
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.
Quoted 7 lines
- 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.
Quoted 6 lines
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
Quoted 18 lines
Quoted 17 lines
Quoted 2 lines
Quoted 2 lines
On 08.12.11 at 14:54, Andres Lagar-Cavilla <[email protected]> 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(-)
Quoted 7 lines
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
Quoted 4 lines
Jan
Quoted 2 lines · Andres Lagar-Cavilla
(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.