[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v2 9/9] 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: Wei Liu <wei.liu2@xxxxxxxxxx> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> --- Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> Cc: Wei Liu <wei.liu2@xxxxxxxxxx> v1: First submission. v2: Added Wei's Ack. - Removed stray blanks - Remove sysctl from xen_sysctl_tmem_pool_info name - s/tmem_pool_info/xen_tmem_pool_info/ - Fix the >= one-off - Used uint64_aligned_t instead of uint64_t for the uuid. - Expand in the git commit description about the interface update - Reuse the subcommand number 7 instead of 16. --- 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 3b66d10..e72f2de 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> @@ -216,11 +217,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); @@ -233,72 +234,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 { @@ -306,14 +317,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)) ) @@ -382,14 +401,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; @@ -401,62 +425,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 6eb08aa..2528414 100644 --- a/xen/common/tmem_control.c +++ b/xen/common/tmem_control.c @@ -276,6 +276,8 @@ static int __tmemc_set_client_info(struct client *client, if ( info.maxpools > MAX_POOLS_PER_DOMAIN ) return -ERANGE; + /* Ignore info.nr_pools. */ + if ( info.weight != client->info.weight ) { old_weight = client->info.weight; @@ -343,46 +345,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 ) @@ -413,11 +432,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; -- 2.4.11 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |