[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 2/5] hypervisor/xen/tools: Remove the XENMEM_get_oustanding_pages and provide the data via xc_phys_info



During the review of the patches it was noticed that there exists
a race wherein the 'free_memory' value consists of information from
two hypercalls. That is the XEN_SYSCTL_physinfo and 
XENMEM_get_outstanding_pages.

The free memory the host has available for guest is the difference between
the 'free_pages' (from XEN_SYSCTL_physinfo) and 'outstanding_pages'. As they
are two hypercalls many things can happen in between the execution of them.

This patch resolves this by eliminating the XENMEM_get_outstanding_pages
hypercall and providing the free_pages and outstanding_pages information
via the xc_phys_info structure.

It also removes the XSM hooks and adds locking as needed.

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Acked-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Reviewed-by: Tim Deegan <tim@xxxxxxx>
Acked-by: Keir Fraser <keir.xen@xxxxxxxxx>
[v1: Fix missing XSM hooks cleanups, fixed get_outstanding_claims fnc,
  add Acked/Reviewed-by]
[v2: s/%ld/PRIu64/ as we don't use 'long' anymore]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
 tools/libxc/xc_domain.c             |  9 ---------
 tools/libxc/xenctrl.h               |  2 --
 tools/libxl/libxl.c                 | 15 +--------------
 tools/libxl/libxl.h                 |  1 -
 tools/libxl/libxl_types.idl         |  1 +
 tools/libxl/xl_cmdimpl.c            | 16 +++-------------
 xen/common/memory.c                 |  8 --------
 xen/common/page_alloc.c             |  7 +++++--
 xen/common/sysctl.c                 |  3 ++-
 xen/include/public/memory.h         |  7 -------
 xen/include/public/sysctl.h         |  3 ++-
 xen/include/xen/mm.h                |  2 +-
 xen/include/xsm/dummy.h             |  6 ------
 xen/include/xsm/xsm.h               |  6 ------
 xen/xsm/dummy.c                     |  1 -
 xen/xsm/flask/hooks.c               |  7 -------
 xen/xsm/flask/policy/access_vectors |  2 +-
 17 files changed, 16 insertions(+), 80 deletions(-)

diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index bb71cca..3257e2a 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -873,15 +873,6 @@ int xc_domain_claim_pages(xc_interface *xch,
         err = errno = 0;
     return err;
 }
-unsigned long xc_domain_get_outstanding_pages(xc_interface *xch)
-{
-    long ret = do_memory_op(xch, XENMEM_get_outstanding_pages, NULL, 0);
-
-    /* Ignore it if the hypervisor does not support the call. */
-    if (ret == -1 && errno == ENOSYS)
-        ret = errno = 0;
-    return ret;
-}
 
 int xc_domain_populate_physmap(xc_interface *xch,
                                uint32_t domid,
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index c024af4..40ee8fc 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -1182,8 +1182,6 @@ int xc_domain_claim_pages(xc_interface *xch,
                                uint32_t domid,
                                unsigned long nr_pages);
 
-unsigned long xc_domain_get_outstanding_pages(xc_interface *xch);
-
 int xc_domain_memory_exchange_pages(xc_interface *xch,
                                     int domid,
                                     unsigned long nr_in_extents,
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 87bda72..dfba755 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3941,6 +3941,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo 
*physinfo)
     physinfo->total_pages = xcphysinfo.total_pages;
     physinfo->free_pages = xcphysinfo.free_pages;
     physinfo->scrub_pages = xcphysinfo.scrub_pages;
+    physinfo->outstanding_pages = xcphysinfo.outstanding_pages;
     l = xc_sharing_freed_pages(ctx->xch);
     if (l == -ENOSYS) {
         l = 0;
@@ -4104,20 +4105,6 @@ libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int 
*nr)
     return ret;
 }
 
-uint64_t libxl_get_claiminfo(libxl_ctx *ctx)
-{
-    long l;
-
-    l = xc_domain_get_outstanding_pages(ctx->xch);
-    if (l < 0) {
-        LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_WARNING, l,
-                            "xc_domain_get_outstanding_pages failed.");
-        return ERROR_FAIL;
-    }
-    /* In pages */
-    return l;
-}
-
 const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
 {
     union {
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index ef96bce..0bc005e 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -624,7 +624,6 @@ int libxl_wait_for_free_memory(libxl_ctx *ctx, uint32_t 
domid, uint32_t memory_k
 /* wait for the memory target of a domain to be reached */
 int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid, int 
wait_secs);
 
-uint64_t libxl_get_claiminfo(libxl_ctx *ctx);
 int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass);
 int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, 
libxl_console_type type);
 /* libxl_primary_console_exec finds the domid and console number
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index ecf1f0b..8262cba 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -487,6 +487,7 @@ libxl_physinfo = Struct("physinfo", [
     ("total_pages", uint64),
     ("free_pages", uint64),
     ("scrub_pages", uint64),
+    ("outstanding_pages", uint64),
     ("sharing_freed_pages", uint64),
     ("sharing_used_frames", uint64),
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c1a969b..edf0325 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4576,21 +4576,11 @@ static void output_physinfo(void)
     unsigned int i;
     libxl_bitmap cpumap;
     int n = 0;
-    long claims = 0;
 
     if (libxl_get_physinfo(ctx, &info) != 0) {
         fprintf(stderr, "libxl_physinfo failed.\n");
         return;
     }
-    /*
-     * Don't bother checking "claim_mode" as user might have turned it off
-     * and we have outstanding claims.
-     */
-    if ((claims = libxl_get_claiminfo(ctx)) < 0){
-        fprintf(stderr, "libxl_get_claiminfo failed. errno: %d (%s)\n",
-                errno, strerror(errno));
-        return;
-    }
     printf("nr_cpus                : %d\n", info.nr_cpus);
     printf("max_cpu_id             : %d\n", info.max_cpu_id);
     printf("nr_nodes               : %d\n", info.nr_nodes);
@@ -4610,15 +4600,15 @@ static void output_physinfo(void)
     if (vinfo) {
         i = (1 << 20) / vinfo->pagesize;
         printf("total_memory           : %"PRIu64"\n", info.total_pages / i);
-        printf("free_memory            : %"PRIu64"\n", (info.free_pages - 
claims) / i);
+        printf("free_memory            : %"PRIu64"\n", (info.free_pages - 
info.outstanding_pages) / i);
         printf("sharing_freed_memory   : %"PRIu64"\n", 
info.sharing_freed_pages / i);
         printf("sharing_used_memory    : %"PRIu64"\n", 
info.sharing_used_frames / i);
     }
     /*
      * Only if enabled (claim_mode=1) or there are outstanding claims.
      */
-    if (libxl_defbool_val(claim_mode) || claims)
-        printf("outstanding_claims     : %ld\n", claims / i);
+    if (libxl_defbool_val(claim_mode) || info.outstanding_pages)
+        printf("outstanding_claims     : %"PRIu64"\n", info.outstanding_pages 
/ i);
 
     if (!libxl_get_freecpus(ctx, &cpumap)) {
         libxl_for_each_bit(i, cpumap)
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 3239d53..06a0d0a 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -737,14 +737,6 @@ long do_memory_op(unsigned long cmd, 
XEN_GUEST_HANDLE_PARAM(void) arg)
 
         break;
 
-    case XENMEM_get_outstanding_pages:
-        rc = xsm_xenmem_get_outstanding_pages(XSM_PRIV);
-
-        if ( !rc )
-            rc = get_outstanding_claims();
-
-        break;
-
     default:
         rc = arch_memory_op(op, arg);
         break;
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 203f77a..2162ef1 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -380,9 +380,12 @@ out:
     return ret;
 }
 
-long get_outstanding_claims(void)
+void get_outstanding_claims(uint64_t *free_pages, uint64_t *outstanding_pages)
 {
-    return outstanding_claims;
+    spin_lock(&heap_lock);
+    *outstanding_pages = outstanding_claims;
+    *free_pages =  avail_domheap_pages();
+    spin_unlock(&heap_lock);
 }
 
 static unsigned long init_node_heap(int node, unsigned long mfn,
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 31f9650..117e095 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -264,7 +264,8 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
u_sysctl)
         pi->max_node_id = MAX_NUMNODES-1;
         pi->max_cpu_id = nr_cpu_ids - 1;
         pi->total_pages = total_pages;
-        pi->free_pages = avail_domheap_pages();
+        /* Protected by lock */
+        get_outstanding_claims(&pi->free_pages, &pi->outstanding_pages);
         pi->scrub_pages = 0;
         pi->cpu_khz = cpu_khz;
         arch_do_physinfo(pi);
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 51d5254..7a26dee 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -459,13 +459,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_sharing_op_t);
  * The zero value is appropiate.
  */
 
-/*
- * Get the number of pages currently claimed (but not yet "possessed")
- * across all domains.  The caller must be privileged but otherwise
- * the call never fails.
- */
-#define XENMEM_get_outstanding_pages        25
-
 #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
 
 #endif /* __XEN_PUBLIC_MEMORY_H__ */
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 03710d8..8437d31 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -34,7 +34,7 @@
 #include "xen.h"
 #include "domctl.h"
 
-#define XEN_SYSCTL_INTERFACE_VERSION 0x00000009
+#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000A
 
 /*
  * Read console content from Xen buffer ring.
@@ -101,6 +101,7 @@ struct xen_sysctl_physinfo {
     uint64_aligned_t total_pages;
     uint64_aligned_t free_pages;
     uint64_aligned_t scrub_pages;
+    uint64_aligned_t outstanding_pages;
     uint32_t hw_cap[8];
 
     /* XEN_SYSCTL_PHYSCAP_??? */
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 28512fb..ac6e4eb 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -52,7 +52,7 @@ void free_xenheap_pages(void *v, unsigned int order);
 /* Claim handling */
 unsigned long domain_adjust_tot_pages(struct domain *d, long pages);
 int domain_set_outstanding_pages(struct domain *d, unsigned long pages);
-long get_outstanding_claims(void);
+void get_outstanding_claims(uint64_t *free_pages, uint64_t *outstanding_pages);
 
 /* Domain suballocator. These functions are *not* interrupt-safe.*/
 void init_domheap_pages(paddr_t ps, paddr_t pe);
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index a872056..cc0a5a8 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -259,12 +259,6 @@ static XSM_INLINE int xsm_claim_pages(XSM_DEFAULT_ARG 
struct domain *d)
     return xsm_default_action(action, current->domain, d);
 }
 
-static XSM_INLINE int xsm_xenmem_get_outstanding_pages(XSM_DEFAULT_VOID)
-{
-    XSM_ASSERT_ACTION(XSM_PRIV);
-    return xsm_default_action(action, current->domain, NULL);
-}
-
 static XSM_INLINE int xsm_evtchn_unbound(XSM_DEFAULT_ARG struct domain *d, 
struct evtchn *chn,
                                          domid_t id2)
 {
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 58a4fbb..1939453 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -93,7 +93,6 @@ struct xsm_operations {
     int (*add_to_physmap) (struct domain *d1, struct domain *d2);
     int (*remove_from_physmap) (struct domain *d1, struct domain *d2);
     int (*claim_pages) (struct domain *d);
-    int (*xenmem_get_outstanding_pages) (void);
 
     int (*console_io) (struct domain *d, int cmd);
 
@@ -360,11 +359,6 @@ static inline int xsm_claim_pages(xsm_default_t def, 
struct domain *d)
     return xsm_ops->claim_pages(d);
 }
 
-static inline int xsm_xenmem_get_outstanding_pages(xsm_default_t def)
-{
-    return xsm_ops->xenmem_get_outstanding_pages();
-}
-
 static inline int xsm_console_io (xsm_default_t def, struct domain *d, int cmd)
 {
     return xsm_ops->console_io(d, cmd);
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 937761f..31e4f73 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -67,7 +67,6 @@ void xsm_fixup_ops (struct xsm_operations *ops)
     set_to_dummy_if_null(ops, memory_stat_reservation);
     set_to_dummy_if_null(ops, memory_pin_page);
     set_to_dummy_if_null(ops, claim_pages);
-    set_to_dummy_if_null(ops, xenmem_get_outstanding_pages);
 
     set_to_dummy_if_null(ops, console_io);
 
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index bb10de3..fa0589a 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -422,12 +422,6 @@ static int flask_claim_pages(struct domain *d)
     return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__SETCLAIM);
 }
 
-static int flask_xenmem_get_outstanding_pages(void)
-{
-    return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN,
-                                XEN__HEAP, NULL);
-}
-
 static int flask_console_io(struct domain *d, int cmd)
 {
     u32 perm;
@@ -1504,7 +1498,6 @@ static struct xsm_operations flask_ops = {
     .memory_stat_reservation = flask_memory_stat_reservation,
     .memory_pin_page = flask_memory_pin_page,
     .claim_pages = flask_claim_pages,
-    .xenmem_get_outstanding_pages = flask_xenmem_get_outstanding_pages,
 
     .console_io = flask_console_io,
 
diff --git a/xen/xsm/flask/policy/access_vectors 
b/xen/xsm/flask/policy/access_vectors
index 544c3ba..5dfe13b 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -54,7 +54,7 @@ class xen
     debug
 # XEN_SYSCTL_getcpuinfo, XENPF_get_cpu_version, XENPF_get_cpuinfo
     getcpuinfo
-# XEN_SYSCTL_availheap, XENMEM_get_outstanding_pages
+# XEN_SYSCTL_availheap
     heap
 # XEN_SYSCTL_get_pmstat, XEN_SYSCTL_pm_op, XENPF_set_processor_pminfo,
 # XENPF_core_parking
-- 
1.8.1.4


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.