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

[Xen-changelog] [xen master] tmem: Batch and squash XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_[FLAGS, NPAGES, UUID]



commit b3d54cead6459567d9786ad415149868ee7f2f5b
Author:     Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
AuthorDate: Fri Sep 30 15:10:22 2016 -0400
Commit:     Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CommitDate: Fri Sep 30 15:27:05 2016 -0400

    tmem: Batch and squash XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_[FLAGS,NPAGES,UUID]
    in one sub-call: XEN_SYSCTL_TMEM_OP_GET_POOLS.
    
    These operations are used during the save process of migration.
    Instead of doing 64 hypercalls lets do just one. We modify
    the 'struct xen_tmem_client' structure (used in
    XEN_SYSCTL_TMEM_OP_[GET|SET]_CLIENT_INFO) to have an extra field
    'nr_pools'. Armed with that the code slurping up pages from the
    hypervisor can allocate a big enough structure (struct tmem_pool_info)
    to contain all the active pools. And then just iterate over each
    one and save it in the stream.
    
    We are also re-using one of the subcommands numbers for this,
    as such the XEN_SYSCTL_INTERFACE_VERSION should be incremented
    and that was done in the patch titled:
    "tmem/libxc: Squash XEN_SYSCTL_TMEM_OP_[SET|SAVE].."
    
    In the xc_tmem_[save|restore] we also added proper memory handling
    of the 'buf' and 'pools'. Because of the loops and to make it as
    easy as possible to review we add a goto label and for almost
    all error conditions jump in it.
    
    The include for inttypes is required for the PRId64 macro to
    work (which is needed to compile this code under 32-bit).
    
    Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
 tools/libxc/xc_tmem.c       | 159 +++++++++++++++++++++++++-------------------
 xen/common/tmem.c           |   3 +
 xen/common/tmem_control.c   |  78 +++++++++++++---------
 xen/include/public/sysctl.h |  33 ++++++++-
 4 files changed, 172 insertions(+), 101 deletions(-)

diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 4a74be9..51d11ef 100644
--- a/tools/libxc/xc_tmem.c
+++ b/tools/libxc/xc_tmem.c
@@ -18,6 +18,7 @@
  */
 
 #include "xc_private.h"
+#include <inttypes.h>
 #include <assert.h>
 #include <xen/tmem.h>
 
@@ -214,11 +215,11 @@ int xc_tmem_save(xc_interface *xch,
 {
     int marker = field_marker;
     int i, j, rc;
-    uint32_t flags;
     uint32_t minusone = -1;
-    uint32_t pool_id;
     struct tmem_handle *h;
     xen_tmem_client_t info;
+    xen_tmem_pool_info_t *pools;
+    char *buf = NULL;
 
     rc = xc_tmem_control(xch, 0, XEN_SYSCTL_TMEM_OP_SAVE_BEGIN,
                          dom, 0 /* len*/ , live, NULL);
@@ -231,72 +232,82 @@ int xc_tmem_save(xc_interface *xch,
         return rc;
     }
 
-    if ( write_exact(io_fd, &marker, sizeof(marker)) )
-        return -1;
-
     if ( xc_tmem_control(xch, 0 /* pool_id */,
                          XEN_SYSCTL_TMEM_OP_GET_CLIENT_INFO,
                          dom /* cli_id */, sizeof(info), 0 /* arg */,
                          &info) < 0 )
         return -1;
 
-    if ( write_exact(io_fd, &info, sizeof(info)) )
+    /* Nothing to do. */
+    if ( !info.nr_pools )
+        return 0;
+
+    pools = calloc(info.nr_pools, sizeof(*pools));
+    if ( !pools )
         return -1;
+
+    rc = xc_tmem_control(xch, 0 /* pool_id is ignored. */,
+                         XEN_SYSCTL_TMEM_OP_GET_POOLS,
+                         dom /* cli_id */, sizeof(*pools) * info.nr_pools,
+                         0 /* arg */, pools);
+
+    if ( rc < 0 || (uint32_t)rc > info.nr_pools )
+        goto out_memory;
+
+    /* Update it - as we have less pools between the two hypercalls. */
+    info.nr_pools = (uint32_t)rc;
+
+    if ( write_exact(io_fd, &marker, sizeof(marker)) )
+        goto out_memory;
+
+    if ( write_exact(io_fd, &info, sizeof(info)) )
+        goto out_memory;
+
     if ( write_exact(io_fd, &minusone, sizeof(minusone)) )
-        return -1;
-    for ( i = 0; i < info.maxpools; i++ )
+        goto out_memory;
+
+    for ( i = 0; i < info.nr_pools; i++ )
     {
-        uint64_t uuid[2];
-        uint32_t n_pages;
         uint32_t pagesize;
-        char *buf = NULL;
         int bufsize = 0;
         int checksum = 0;
+        xen_tmem_pool_info_t *pool = &pools[i];
 
-        /* get pool id, flags, pagesize, n_pages, uuid */
-        flags = 
xc_tmem_control(xch,i,XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_FLAGS,dom,0,0,NULL);
-        if ( flags != -1 )
+        if ( pool->flags.raw != -1 )
         {
-            pool_id = i;
-            n_pages = 
xc_tmem_control(xch,i,XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_NPAGES,dom,0,0,NULL);
-            if ( !(flags & TMEM_POOL_PERSIST) )
-                n_pages = 0;
-            
(void)xc_tmem_control(xch,i,XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_UUID,dom,sizeof(uuid),0,&uuid);
-            if ( write_exact(io_fd, &pool_id, sizeof(pool_id)) )
-                return -1;
-            if ( write_exact(io_fd, &flags, sizeof(flags)) )
-                return -1;
-            if ( write_exact(io_fd, &n_pages, sizeof(n_pages)) )
-                return -1;
-            if ( write_exact(io_fd, &uuid, sizeof(uuid)) )
-                return -1;
-            if ( n_pages == 0 )
+            if ( !pool->flags.u.persist )
+                pool->n_pages = 0;
+
+            if ( write_exact(io_fd, pool, sizeof(*pool)) )
+                goto out_memory;
+
+            if ( !pool->flags.u.persist )
                 continue;
 
-            pagesize = 1 << (((flags >> TMEM_POOL_PAGESIZE_SHIFT) &
-                              TMEM_POOL_PAGESIZE_MASK) + 12);
+            pagesize = 1 << (pool->flags.u.pagebits + 12);
             if ( pagesize > bufsize )
             {
                 bufsize = pagesize + sizeof(struct tmem_handle);
                 if ( (buf = realloc(buf,bufsize)) == NULL )
-                    return -1;
+                    goto out_memory;
             }
-            for ( j = n_pages; j > 0; j-- )
+            for ( j = pool->n_pages; j > 0; j-- )
             {
                 int ret;
-                if ( (ret = xc_tmem_control(xch, pool_id,
+                if ( (ret = xc_tmem_control(xch, pool->id,
                                             
XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_PAGE, dom,
                                             bufsize, 0, buf)) > 0 )
                 {
                     h = (struct tmem_handle *)buf;
                     if ( write_exact(io_fd, &h->oid, sizeof(h->oid)) )
-                        return -1;
+                        goto out_memory;
+
                     if ( write_exact(io_fd, &h->index, sizeof(h->index)) )
-                        return -1;
+                        goto out_memory;
                     h++;
                     checksum += *(char *)h;
                     if ( write_exact(io_fd, h, pagesize) )
-                        return -1;
+                        goto out_memory;
                 } else if ( ret == 0 ) {
                     continue;
                 } else {
@@ -304,14 +315,22 @@ int xc_tmem_save(xc_interface *xch,
                     h = (struct tmem_handle *)buf;
                     h->oid.oid[0] = h->oid.oid[1] = h->oid.oid[2] = -1L;
                     if ( write_exact(io_fd, &h->oid, sizeof(h->oid)) )
+                    {
+ out_memory:
+                        free(pools);
+                        free(buf);
                         return -1;
+                    }
                     break;
                 }
             }
-            DPRINTF("saved %d tmem pages for dom=%d pool=%d, checksum=%x\n",
-                         n_pages-j,dom,pool_id,checksum);
+            DPRINTF("saved %"PRId64" tmem pages for dom=%d pool=%d, 
checksum=%x\n",
+                    pool->n_pages - j, dom, pool->id, checksum);
         }
     }
+    free(pools);
+    free(buf);
+
     /* pool list terminator */
     minusone = -1;
     if ( write_exact(io_fd, &minusone, sizeof(minusone)) )
@@ -380,14 +399,19 @@ static int xc_tmem_restore_new_pool(
 
 int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
 {
-    uint32_t pool_id;
     uint32_t minusone;
-    uint32_t flags;
     xen_tmem_client_t info;
     int checksum = 0;
+    unsigned int i;
+    char *buf = NULL;
 
     if ( read_exact(io_fd, &info, sizeof(info)) )
         return -1;
+
+    /* We would never save if there weren't any pools! */
+    if ( !info.nr_pools )
+        return -1;
+
     if ( xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_RESTORE_BEGIN,dom,0,0,NULL) 
< 0 )
         return -1;
 
@@ -399,62 +423,63 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
 
     if ( read_exact(io_fd, &minusone, sizeof(minusone)) )
         return -1;
-    while ( read_exact(io_fd, &pool_id, sizeof(pool_id)) == 0 && pool_id != -1 
)
+
+    for ( i = 0; i < info.nr_pools; i++ )
     {
-        uint64_t uuid[2];
-        uint32_t n_pages;
-        char *buf = NULL;
         int bufsize = 0, pagesize;
         int j;
+        xen_tmem_pool_info_t pool;
 
-        if ( read_exact(io_fd, &flags, sizeof(flags)) )
-            return -1;
-        if ( read_exact(io_fd, &n_pages, sizeof(n_pages)) )
-            return -1;
-        if ( read_exact(io_fd, &uuid, sizeof(uuid)) )
-            return -1;
-        if ( xc_tmem_restore_new_pool(xch, dom, pool_id,
-                                 flags, uuid[0], uuid[1]) < 0)
-            return -1;
-        if ( n_pages <= 0 )
+        if ( read_exact(io_fd, &pool, sizeof(pool)) )
+            goto out_memory;
+
+        if ( xc_tmem_restore_new_pool(xch, dom, pool.id, pool.flags.raw,
+                                      pool.uuid[0], pool.uuid[1]) < 0 )
+            goto out_memory;
+
+        if ( pool.n_pages <= 0 )
             continue;
 
-        pagesize = 1 << (((flags >> TMEM_POOL_PAGESIZE_SHIFT) &
-                              TMEM_POOL_PAGESIZE_MASK) + 12);
+        pagesize = 1 << (pool.flags.u.pagebits + 12);
         if ( pagesize > bufsize )
         {
             bufsize = pagesize;
             if ( (buf = realloc(buf,bufsize)) == NULL )
-                return -1;
+                goto out_memory;
         }
-        for ( j = n_pages; j > 0; j-- )
+        for ( j = pool.n_pages; j > 0; j-- )
         {
             struct xen_tmem_oid oid;
             uint32_t index;
             int rc;
+
             if ( read_exact(io_fd, &oid, sizeof(oid)) )
-                return -1;
+                goto out_memory;
+
             if ( oid.oid[0] == -1L && oid.oid[1] == -1L && oid.oid[2] == -1L )
                 break;
             if ( read_exact(io_fd, &index, sizeof(index)) )
-                return -1;
+                goto out_memory;
+
             if ( read_exact(io_fd, buf, pagesize) )
-                return -1;
+                goto out_memory;
+
             checksum += *buf;
-            if ( (rc = xc_tmem_control_oid(xch, pool_id,
+            if ( (rc = xc_tmem_control_oid(xch, pool.id,
                                            
XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE, dom,
                                            bufsize, index, oid, buf)) <= 0 )
             {
                 DPRINTF("xc_tmem_restore: putting page failed, rc=%d\n",rc);
+ out_memory:
+                free(buf);
                 return -1;
             }
         }
-        if ( n_pages )
-            DPRINTF("restored %d tmem pages for dom=%d pool=%d, check=%x\n",
-                    n_pages-j,dom,pool_id,checksum);
+        if ( pool.n_pages )
+            DPRINTF("restored %"PRId64" tmem pages for dom=%d pool=%d, 
check=%x\n",
+                    pool.n_pages - j, dom, pool.id, checksum);
     }
-    if ( pool_id != -1 )
-        return -1;
+    free(buf);
 
     return 0;
 }
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index 14a12ca..6d5de5b 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -880,6 +880,7 @@ static void client_flush(struct client *client)
             continue;
         pool_flush(pool, client->cli_id);
         client->pools[i] = NULL;
+        client->info.nr_pools--;
     }
     client_free(client);
 }
@@ -1430,6 +1431,7 @@ static int do_tmem_destroy_pool(uint32_t pool_id)
         return 0;
     client->pools[pool_id] = NULL;
     pool_flush(pool, client->cli_id);
+    client->info.nr_pools--;
     return 1;
 }
 
@@ -1592,6 +1594,7 @@ static int do_tmem_new_pool(domid_t this_cli_id,
 
 out:
     tmem_client_info("pool_id=%d\n", d_poolid);
+    client->info.nr_pools++;
     return d_poolid;
 
 fail:
diff --git a/xen/common/tmem_control.c b/xen/common/tmem_control.c
index 3ec105f..ddd9cfe 100644
--- a/xen/common/tmem_control.c
+++ b/xen/common/tmem_control.c
@@ -276,7 +276,9 @@ static int __tmemc_set_client_info(struct client *client,
     if ( info.maxpools > MAX_POOLS_PER_DOMAIN )
         return -ERANGE;
 
+    /* Ignore info.nr_pools. */
     cli_id = client->cli_id;
+
     if ( info.weight != client->info.weight )
     {
         old_weight = client->info.weight;
@@ -346,46 +348,63 @@ static int tmemc_get_client_info(int cli_id,
     return 0;
 }
 
-static int tmemc_save_subop(int cli_id, uint32_t pool_id, uint32_t subop,
-                            XEN_GUEST_HANDLE_PARAM(char) buf, uint32_t arg)
+static int tmemc_get_pool(int cli_id,
+                          XEN_GUEST_HANDLE(xen_tmem_pool_info_t) pools,
+                          uint32_t len)
 {
     struct client *client = tmem_client_from_cli_id(cli_id);
-    struct tmem_pool *pool = (client == NULL || pool_id >= 
MAX_POOLS_PER_DOMAIN)
-                   ? NULL : client->pools[pool_id];
-    int rc = -1;
+    unsigned int i, idx;
+    int rc = 0;
+    unsigned int nr = len / sizeof(xen_tmem_pool_info_t);
+
+    if ( len % sizeof(xen_tmem_pool_info_t) )
+        return -EINVAL;
+
+    if ( nr > MAX_POOLS_PER_DOMAIN )
+        return -E2BIG;
+
+    if ( !guest_handle_okay(pools, nr) )
+        return -EINVAL;
 
-    switch(subop)
+    if ( !client )
+        return -EINVAL;
+
+    for ( idx = 0, i = 0; i < MAX_POOLS_PER_DOMAIN; i++ )
     {
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_FLAGS:
-         if ( pool == NULL )
-             break;
-         rc = (pool->persistent ? TMEM_POOL_PERSIST : 0) |
+        struct tmem_pool *pool = client->pools[i];
+        xen_tmem_pool_info_t out;
+
+        if ( pool == NULL )
+            continue;
+
+        out.flags.raw = (pool->persistent ? TMEM_POOL_PERSIST : 0) |
               (pool->shared ? TMEM_POOL_SHARED : 0) |
               (POOL_PAGESHIFT << TMEM_POOL_PAGESIZE_SHIFT) |
               (TMEM_SPEC_VERSION << TMEM_POOL_VERSION_SHIFT);
-        break;
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_NPAGES:
-         if ( pool == NULL )
-             break;
-        rc = _atomic_read(pool->pgp_count);
-        break;
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_UUID:
-         if ( pool == NULL )
-             break;
-        rc = 0;
-        if ( copy_to_guest(guest_handle_cast(buf, void), pool->uuid, 2) )
+        out.n_pages = _atomic_read(pool->pgp_count);
+        out.uuid[0] = pool->uuid[0];
+        out.uuid[1] = pool->uuid[1];
+        out.id = i;
+
+        /* N.B. 'idx' != 'i'. */
+        if ( __copy_to_guest_offset(pools, idx, &out, 1) )
+        {
             rc = -EFAULT;
-        break;
-    default:
-        rc = -1;
+            break;
+        }
+        idx++;
+        /* Don't try to put more than what was requested. */
+        if ( idx >= nr )
+            break;
     }
-    return rc;
+
+    /* And how many we have processed. */
+    return rc ? : idx;
 }
 
 int tmem_control(struct xen_sysctl_tmem_op *op)
 {
     int ret;
-    uint32_t pool_id = op->pool_id;
     uint32_t cmd = op->cmd;
 
     if ( op->pad != 0 )
@@ -416,11 +435,8 @@ int tmem_control(struct xen_sysctl_tmem_op *op)
     case XEN_SYSCTL_TMEM_OP_GET_CLIENT_INFO:
         ret = tmemc_get_client_info(op->cli_id, op->u.client);
         break;
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_FLAGS:
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_NPAGES:
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_UUID:
-        ret = tmemc_save_subop(op->cli_id, pool_id, cmd,
-                               guest_handle_cast(op->u.buf, char), op->arg);
+    case XEN_SYSCTL_TMEM_OP_GET_POOLS:
+        ret = tmemc_get_pool(op->cli_id, op->u.pool, op->len);
         break;
     default:
         ret = do_tmem_control(op);
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 0b8afaf..418f4bb 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -769,11 +769,9 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
 #define XEN_SYSCTL_TMEM_OP_LIST                   4
 #define XEN_SYSCTL_TMEM_OP_GET_CLIENT_INFO        5
 #define XEN_SYSCTL_TMEM_OP_SET_CLIENT_INFO        6
+#define XEN_SYSCTL_TMEM_OP_GET_POOLS              7
 #define XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB      8
 #define XEN_SYSCTL_TMEM_OP_SAVE_BEGIN             10
-#define XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_FLAGS    16
-#define XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_NPAGES   17
-#define XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_UUID     18
 #define XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_PAGE     19
 #define XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_INV      20
 #define XEN_SYSCTL_TMEM_OP_SAVE_END               21
@@ -800,6 +798,7 @@ struct xen_tmem_client {
     uint32_t version;   /* If mismatched we will get XEN_EOPNOTSUPP. */
     uint32_t maxpools;  /* If greater than what hypervisor supports, will get
                            XEN_ERANGE. */
+    uint32_t nr_pools;  /* Current amount of pools. Ignored on SET*/
     union {             /* See TMEM_CLIENT_[COMPRESS,FROZEN] */
         uint32_t raw;
         struct {
@@ -813,6 +812,31 @@ struct xen_tmem_client {
 typedef struct xen_tmem_client xen_tmem_client_t;
 DEFINE_XEN_GUEST_HANDLE(xen_tmem_client_t);
 
+/*
+ * XEN_SYSCTL_TMEM_OP_GET_POOLS uses the 'pool' array in
+ * xen_sysctl_tmem_op with this structure. The hypercall will
+ * return the number of entries in 'pool' or a negative value
+ * if an error was encountered.
+ */
+struct xen_tmem_pool_info {
+    union {
+        uint32_t raw;
+        struct {
+            uint32_t persist:1,    /* See TMEM_POOL_PERSIST. */
+                     shared:1,     /* See TMEM_POOL_SHARED. */
+                     rsv:2,
+                     pagebits:8,   /* TMEM_POOL_PAGESIZE_[SHIFT,MASK]. */
+                     rsv2:12,
+                     version:8;    /* TMEM_POOL_VERSION_[SHIFT,MASK]. */
+        } u;
+    } flags;
+    uint32_t id;                  /* Less than tmem_client.maxpools. */
+    uint64_t n_pages;
+    uint64_aligned_t uuid[2];
+};
+typedef struct xen_tmem_pool_info xen_tmem_pool_info_t;
+DEFINE_XEN_GUEST_HANDLE(xen_tmem_pool_info_t);
+
 struct xen_sysctl_tmem_op {
     uint32_t cmd;       /* IN: XEN_SYSCTL_TMEM_OP_* . */
     int32_t pool_id;    /* IN: 0 by default unless _SAVE_*, RESTORE_* .*/
@@ -827,6 +851,9 @@ struct xen_sysctl_tmem_op {
         XEN_GUEST_HANDLE_64(char) buf; /* IN/OUT: Buffer to save/restore */
         XEN_GUEST_HANDLE_64(xen_tmem_client_t) client; /* IN/OUT for */
                         /*  XEN_SYSCTL_TMEM_OP_[GET,SAVE]_CLIENT. */
+        XEN_GUEST_HANDLE_64(xen_tmem_pool_info_t) pool; /* OUT for */
+                        /* XEN_SYSCTL_TMEM_OP_GET_POOLS. Must have 'len' */
+                        /* of them. */
     } u;
 };
 typedef struct xen_sysctl_tmem_op xen_sysctl_tmem_op_t;
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog

 


Rackspace

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