* [PATCH v3 01/11] tmem: Don't crash/hang/leak hypervisor when using shared pools within an guest.
2015-08-28 18:53 [PATCH v3] Tmem bug-fixes and cleanups Konrad Rzeszutek Wilk
@ 2015-08-28 18:53 ` Konrad Rzeszutek Wilk
2015-08-28 18:53 ` [PATCH v3 02/11] tmem: Add ASSERT in obj_rb_insert for pool->rwlock lock Konrad Rzeszutek Wilk
` (9 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-28 18:53 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2; +Cc: Konrad Rzeszutek Wilk
This is a regression introduced by a36590e1b5040838af19d2ca712a516f07a6062b
"tmem: reorg the shared pool allocate path".
When we are using shared pools we have an global array
(on which we put the pool), and an array of pools per domain.
We also have an shared list of clients (guests) _except_
for the very first domain that created the shared pool.
To deal with multiple guests using an shared pool we have an
ref count and a linked list. Whenever an new user of
a the shared pool joins we increase the ref count and add to
the linked list. Whenever an user quits the shared pool
we decrement and remove from the linked list.
Unfortunately this ref counting and linked list never
worked properly. There are multiple issues:
1) If we have one shared pool (and only one guest creating it)
- we do not add it to the shared list of clients. Which
means the logic in 'shared_pool_quit' never removed
the pool from global_shared_pools. That meant when the pool
was de-allocated - we still had an pointer to the pool
which would be accessed by tmemc_list_client (xl tmem-list -a)
and hit a NULL page!
2). If we have two shared pools in a domain - it (shared_pool_quit)
would remove the domain from the share_list linked list, decrements
the refcount to zero - and remove the pool from the global shared pool.
When done it would also clear the client->pools[] to NULL for itself.
Which is good. However since there are two shared pools in the domain
the next entry in the client->pools[] would have a stale pointer to
the just de-allocated pool. Accessing it and trying to de-allocate it
would lead to crashes or hypervisor hang - depending on the build.
Fun times!
To trigger this use
http://xenbits.xen.org/gitweb/?p=xentesttools/bootstrap.git;a=blob;f=root_image/drivers/tmem_test/tmem_test.c
This patch fixes it by making the very first domain that created
an shared pool to follow the same logic as every domain that is
joining a shared pool. That is increment the refcount and also
add itself to the shared list of domains using it.
We also remove an ASSERT that incorrectly assumed
that only one shared pool would exist for a domain.
And to mirror the reporting logic in shared_pool_join
we also add a printk to advertise inter-domain shared pool
joining.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
---
xen/common/tmem.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index f2dc26e..ed9b975 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -1037,6 +1037,9 @@ static int shared_pool_join(struct tmem_pool *pool, struct client *new_client)
tmem_client_info("adding new %s %d to shared pool owned by %s %d\n",
tmem_client_str, new_client->cli_id, tmem_client_str,
pool->client->cli_id);
+ else if ( pool->shared_count )
+ tmem_client_info("inter-guest sharing of shared pool %s by client %d\n",
+ tmem_client_str, pool->client->cli_id);
++pool->shared_count;
return 0;
}
@@ -1056,7 +1059,10 @@ static void shared_pool_reassign(struct tmem_pool *pool)
}
old_client->pools[pool->pool_id] = NULL;
sl = list_entry(pool->share_list.next, struct share_list, share_list);
- ASSERT(sl->client != old_client);
+ /*
+ * The sl->client can be old_client if there are multiple shared pools
+ * within an guest.
+ */
pool->client = new_client = sl->client;
for (poolid = 0; poolid < MAX_POOLS_PER_DOMAIN; poolid++)
if (new_client->pools[poolid] == pool)
@@ -1982,6 +1988,8 @@ static int do_tmem_new_pool(domid_t this_cli_id,
{
INIT_LIST_HEAD(&pool->share_list);
pool->shared_count = 0;
+ if ( shared_pool_join(pool, client) )
+ goto fail;
global_shared_pools[first_unused_s_poolid] = pool;
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 02/11] tmem: Add ASSERT in obj_rb_insert for pool->rwlock lock.
2015-08-28 18:53 [PATCH v3] Tmem bug-fixes and cleanups Konrad Rzeszutek Wilk
2015-08-28 18:53 ` [PATCH v3 01/11] tmem: Don't crash/hang/leak hypervisor when using shared pools within an guest Konrad Rzeszutek Wilk
@ 2015-08-28 18:53 ` Konrad Rzeszutek Wilk
2015-08-31 11:24 ` Jan Beulich
2015-08-28 18:53 ` [PATCH v3 03/11] tmem: Remove in xc_tmem_control_oid duplicate set_xen_guest_handle call Konrad Rzeszutek Wilk
` (8 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-28 18:53 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2; +Cc: Konrad Rzeszutek Wilk
Manipulating the obj-> structures requires us to hold the
pool->rwlock lock. Lets make that obvious in this function to
catch any errant users (none found, but we may in future).
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
xen/common/tmem.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index ed9b975..c1ffe79 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -916,6 +916,9 @@ static int obj_rb_insert(struct rb_root *root, struct tmem_object_root *obj)
struct rb_node **new, *parent = NULL;
struct tmem_object_root *this;
+ ASSERT(obj->pool && obj->pool != NULL);
+ ASSERT_WRITELOCK(&obj->pool->pool_rwlock);
+
new = &(root->rb_node);
while ( *new )
{
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 02/11] tmem: Add ASSERT in obj_rb_insert for pool->rwlock lock.
2015-08-28 18:53 ` [PATCH v3 02/11] tmem: Add ASSERT in obj_rb_insert for pool->rwlock lock Konrad Rzeszutek Wilk
@ 2015-08-31 11:24 ` Jan Beulich
0 siblings, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2015-08-31 11:24 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: andrew.cooper3, wei.liu2, xen-devel
>>> On 28.08.15 at 20:53, <konrad.wilk@oracle.com> wrote:
> Manipulating the obj-> structures requires us to hold the
> pool->rwlock lock. Lets make that obvious in this function to
> catch any errant users (none found, but we may in future).
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> xen/common/tmem.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/xen/common/tmem.c b/xen/common/tmem.c
> index ed9b975..c1ffe79 100644
> --- a/xen/common/tmem.c
> +++ b/xen/common/tmem.c
> @@ -916,6 +916,9 @@ static int obj_rb_insert(struct rb_root *root, struct tmem_object_root *obj)
> struct rb_node **new, *parent = NULL;
> struct tmem_object_root *this;
>
> + ASSERT(obj->pool && obj->pool != NULL);
Was this the same in v2 and I overlooked it? Anyway,
ASSERT(obj->pool);
is clearly enough.
Jan
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 03/11] tmem: Remove in xc_tmem_control_oid duplicate set_xen_guest_handle call
2015-08-28 18:53 [PATCH v3] Tmem bug-fixes and cleanups Konrad Rzeszutek Wilk
2015-08-28 18:53 ` [PATCH v3 01/11] tmem: Don't crash/hang/leak hypervisor when using shared pools within an guest Konrad Rzeszutek Wilk
2015-08-28 18:53 ` [PATCH v3 02/11] tmem: Add ASSERT in obj_rb_insert for pool->rwlock lock Konrad Rzeszutek Wilk
@ 2015-08-28 18:53 ` Konrad Rzeszutek Wilk
2015-08-28 18:53 ` [PATCH v3 04/11] tmem: Remove xc_tmem_control mystical arg3 Konrad Rzeszutek Wilk
` (7 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-28 18:53 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2; +Cc: Konrad Rzeszutek Wilk
We are doing another call to set_xen_guest_handle right
after the xc_hypercall_bounce_pre (the correct place to do it).
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
tools/libxc/xc_tmem.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 8352233..0d1bfb4 100644
--- a/tools/libxc/xc_tmem.c
+++ b/tools/libxc/xc_tmem.c
@@ -110,7 +110,6 @@ int xc_tmem_control_oid(xc_interface *xch,
op.pool_id = pool_id;
op.u.ctrl.subop = subop;
op.u.ctrl.cli_id = cli_id;
- set_xen_guest_handle(op.u.ctrl.buf,buf);
op.u.ctrl.arg1 = arg1;
op.u.ctrl.arg2 = arg2;
op.u.ctrl.oid[0] = oid.oid[0];
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 04/11] tmem: Remove xc_tmem_control mystical arg3
2015-08-28 18:53 [PATCH v3] Tmem bug-fixes and cleanups Konrad Rzeszutek Wilk
` (2 preceding siblings ...)
2015-08-28 18:53 ` [PATCH v3 03/11] tmem: Remove in xc_tmem_control_oid duplicate set_xen_guest_handle call Konrad Rzeszutek Wilk
@ 2015-08-28 18:53 ` Konrad Rzeszutek Wilk
2015-08-28 18:53 ` [PATCH v3 05/11] tmem: Move TMEM_CONTROL subop of tmem hypercall to sysctl Konrad Rzeszutek Wilk
` (6 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-28 18:53 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2; +Cc: Konrad Rzeszutek Wilk
It mentions it but it is never used. The hypercall interface
knows nothing of this sort of thing either. Lets just remove it.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Wei Liu <wei.liu2@citrix.com> [release + toolstack]
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
tools/libxc/include/xenctrl.h | 2 +-
tools/libxc/xc_tmem.c | 38 ++++++++++++++++------------------
tools/libxl/libxl.c | 10 ++++-----
tools/python/xen/lowlevel/xc/xc.c | 7 +++----
tools/xenstat/libxenstat/src/xenstat.c | 4 ++--
5 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index de3c0ad..9e34769 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2304,7 +2304,7 @@ int xc_tmem_control_oid(xc_interface *xch, int32_t pool_id, uint32_t subop,
struct tmem_oid oid, void *buf);
int xc_tmem_control(xc_interface *xch,
int32_t pool_id, uint32_t subop, uint32_t cli_id,
- uint32_t arg1, uint32_t arg2, uint64_t arg3, void *buf);
+ uint32_t arg1, uint32_t arg2, void *buf);
int xc_tmem_auth(xc_interface *xch, int cli_id, char *uuid_str, int arg1);
int xc_tmem_save(xc_interface *xch, int dom, int live, int fd, int field_marker);
int xc_tmem_save_extra(xc_interface *xch, int dom, int fd, int field_marker);
diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 0d1bfb4..467001b 100644
--- a/tools/libxc/xc_tmem.c
+++ b/tools/libxc/xc_tmem.c
@@ -51,7 +51,6 @@ int xc_tmem_control(xc_interface *xch,
uint32_t cli_id,
uint32_t arg1,
uint32_t arg2,
- uint64_t arg3,
void *buf)
{
tmem_op_t op;
@@ -64,7 +63,6 @@ int xc_tmem_control(xc_interface *xch,
op.u.ctrl.cli_id = cli_id;
op.u.ctrl.arg1 = arg1;
op.u.ctrl.arg2 = arg2;
- /* use xc_tmem_control_oid if arg3 is required */
op.u.ctrl.oid[0] = 0;
op.u.ctrl.oid[1] = 0;
op.u.ctrl.oid[2] = 0;
@@ -220,28 +218,28 @@ int xc_tmem_save(xc_interface *xch,
uint32_t minusone = -1;
struct tmem_handle *h;
- if ( xc_tmem_control(xch,0,TMEMC_SAVE_BEGIN,dom,live,0,0,NULL) <= 0 )
+ if ( xc_tmem_control(xch,0,TMEMC_SAVE_BEGIN,dom,live,0,NULL) <= 0 )
return 0;
if ( write_exact(io_fd, &marker, sizeof(marker)) )
return -1;
- version = xc_tmem_control(xch,0,TMEMC_SAVE_GET_VERSION,0,0,0,0,NULL);
+ version = xc_tmem_control(xch,0,TMEMC_SAVE_GET_VERSION,0,0,0,NULL);
if ( write_exact(io_fd, &version, sizeof(version)) )
return -1;
- max_pools = xc_tmem_control(xch,0,TMEMC_SAVE_GET_MAXPOOLS,0,0,0,0,NULL);
+ max_pools = xc_tmem_control(xch,0,TMEMC_SAVE_GET_MAXPOOLS,0,0,0,NULL);
if ( write_exact(io_fd, &max_pools, sizeof(max_pools)) )
return -1;
if ( version == -1 || max_pools == -1 )
return -1;
if ( write_exact(io_fd, &minusone, sizeof(minusone)) )
return -1;
- flags = xc_tmem_control(xch,0,TMEMC_SAVE_GET_CLIENT_FLAGS,dom,0,0,0,NULL);
+ flags = xc_tmem_control(xch,0,TMEMC_SAVE_GET_CLIENT_FLAGS,dom,0,0,NULL);
if ( write_exact(io_fd, &flags, sizeof(flags)) )
return -1;
- weight = xc_tmem_control(xch,0,TMEMC_SAVE_GET_CLIENT_WEIGHT,dom,0,0,0,NULL);
+ weight = xc_tmem_control(xch,0,TMEMC_SAVE_GET_CLIENT_WEIGHT,dom,0,0,NULL);
if ( write_exact(io_fd, &weight, sizeof(weight)) )
return -1;
- cap = xc_tmem_control(xch,0,TMEMC_SAVE_GET_CLIENT_CAP,dom,0,0,0,NULL);
+ cap = xc_tmem_control(xch,0,TMEMC_SAVE_GET_CLIENT_CAP,dom,0,0,NULL);
if ( write_exact(io_fd, &cap, sizeof(cap)) )
return -1;
if ( flags == -1 || weight == -1 || cap == -1 )
@@ -258,14 +256,14 @@ int xc_tmem_save(xc_interface *xch,
int checksum = 0;
/* get pool id, flags, pagesize, n_pages, uuid */
- flags = xc_tmem_control(xch,i,TMEMC_SAVE_GET_POOL_FLAGS,dom,0,0,0,NULL);
+ flags = xc_tmem_control(xch,i,TMEMC_SAVE_GET_POOL_FLAGS,dom,0,0,NULL);
if ( flags != -1 )
{
pool_id = i;
- n_pages = xc_tmem_control(xch,i,TMEMC_SAVE_GET_POOL_NPAGES,dom,0,0,0,NULL);
+ n_pages = xc_tmem_control(xch,i,TMEMC_SAVE_GET_POOL_NPAGES,dom,0,0,NULL);
if ( !(flags & TMEM_POOL_PERSIST) )
n_pages = 0;
- (void)xc_tmem_control(xch,i,TMEMC_SAVE_GET_POOL_UUID,dom,sizeof(uuid),0,0,&uuid);
+ (void)xc_tmem_control(xch,i,TMEMC_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)) )
@@ -290,7 +288,7 @@ int xc_tmem_save(xc_interface *xch,
int ret;
if ( (ret = xc_tmem_control(xch, pool_id,
TMEMC_SAVE_GET_NEXT_PAGE, dom,
- bufsize, 0, 0, buf)) > 0 )
+ bufsize, 0, buf)) > 0 )
{
h = (struct tmem_handle *)buf;
if ( write_exact(io_fd, &h->oid, sizeof(h->oid)) )
@@ -335,7 +333,7 @@ int xc_tmem_save_extra(xc_interface *xch, int dom, int io_fd, int field_marker)
if ( write_exact(io_fd, &marker, sizeof(marker)) )
return -1;
while ( xc_tmem_control(xch, 0, TMEMC_SAVE_GET_NEXT_INV, dom,
- sizeof(handle),0,0,&handle) > 0 ) {
+ sizeof(handle),0,&handle) > 0 ) {
if ( write_exact(io_fd, &handle.pool_id, sizeof(handle.pool_id)) )
return -1;
if ( write_exact(io_fd, &handle.oid, sizeof(handle.oid)) )
@@ -357,7 +355,7 @@ int xc_tmem_save_extra(xc_interface *xch, int dom, int io_fd, int field_marker)
/* only called for live migration */
void xc_tmem_save_done(xc_interface *xch, int dom)
{
- xc_tmem_control(xch,0,TMEMC_SAVE_END,dom,0,0,0,NULL);
+ xc_tmem_control(xch,0,TMEMC_SAVE_END,dom,0,0,NULL);
}
/* restore routines */
@@ -391,7 +389,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
uint32_t weight, cap, flags;
int checksum = 0;
- save_version = xc_tmem_control(xch,0,TMEMC_SAVE_GET_VERSION,dom,0,0,0,NULL);
+ save_version = xc_tmem_control(xch,0,TMEMC_SAVE_GET_VERSION,dom,0,0,NULL);
if ( save_version == -1 )
return -1; /* domain doesn't exist */
if ( read_exact(io_fd, &this_version, sizeof(this_version)) )
@@ -403,23 +401,23 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
return -1;
if ( minusone != -1 )
return -1;
- if ( xc_tmem_control(xch,0,TMEMC_RESTORE_BEGIN,dom,0,0,0,NULL) < 0 )
+ if ( xc_tmem_control(xch,0,TMEMC_RESTORE_BEGIN,dom,0,0,NULL) < 0 )
return -1;
if ( read_exact(io_fd, &flags, sizeof(flags)) )
return -1;
if ( flags & TMEM_CLIENT_COMPRESS )
- if ( xc_tmem_control(xch,0,TMEMC_SET_COMPRESS,dom,1,0,0,NULL) < 0 )
+ if ( xc_tmem_control(xch,0,TMEMC_SET_COMPRESS,dom,1,0,NULL) < 0 )
return -1;
if ( flags & TMEM_CLIENT_FROZEN )
- if ( xc_tmem_control(xch,0,TMEMC_FREEZE,dom,0,0,0,NULL) < 0 )
+ if ( xc_tmem_control(xch,0,TMEMC_FREEZE,dom,0,0,NULL) < 0 )
return -1;
if ( read_exact(io_fd, &weight, sizeof(weight)) )
return -1;
- if ( xc_tmem_control(xch,0,TMEMC_SET_WEIGHT,dom,0,0,0,NULL) < 0 )
+ if ( xc_tmem_control(xch,0,TMEMC_SET_WEIGHT,dom,0,0,NULL) < 0 )
return -1;
if ( read_exact(io_fd, &cap, sizeof(cap)) )
return -1;
- if ( xc_tmem_control(xch,0,TMEMC_SET_CAP,dom,0,0,0,NULL) < 0 )
+ if ( xc_tmem_control(xch,0,TMEMC_SET_CAP,dom,0,0,NULL) < 0 )
return -1;
if ( read_exact(io_fd, &minusone, sizeof(minusone)) )
return -1;
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 4f2eb24..e564c22 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -6046,7 +6046,7 @@ char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long)
char _buf[32768];
rc = xc_tmem_control(ctx->xch, -1, TMEMC_LIST, domid, 32768, use_long,
- 0, _buf);
+ _buf);
if (rc < 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
"Can not get tmem list");
@@ -6061,7 +6061,7 @@ int libxl_tmem_freeze(libxl_ctx *ctx, uint32_t domid)
int rc;
rc = xc_tmem_control(ctx->xch, -1, TMEMC_FREEZE, domid, 0, 0,
- 0, NULL);
+ NULL);
if (rc < 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
"Can not freeze tmem pools");
@@ -6076,7 +6076,7 @@ int libxl_tmem_thaw(libxl_ctx *ctx, uint32_t domid)
int rc;
rc = xc_tmem_control(ctx->xch, -1, TMEMC_THAW, domid, 0, 0,
- 0, NULL);
+ NULL);
if (rc < 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
"Can not thaw tmem pools");
@@ -6108,7 +6108,7 @@ int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name, uint32_t set)
"Invalid set, valid sets are <weight|cap|compress>");
return ERROR_INVAL;
}
- rc = xc_tmem_control(ctx->xch, -1, subop, domid, set, 0, 0, NULL);
+ rc = xc_tmem_control(ctx->xch, -1, subop, domid, set, 0, NULL);
if (rc < 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
"Can not set tmem %s", name);
@@ -6137,7 +6137,7 @@ int libxl_tmem_freeable(libxl_ctx *ctx)
{
int rc;
- rc = xc_tmem_control(ctx->xch, -1, TMEMC_QUERY_FREEABLE_MB, -1, 0, 0, 0, 0);
+ rc = xc_tmem_control(ctx->xch, -1, TMEMC_QUERY_FREEABLE_MB, -1, 0, 0, 0);
if (rc < 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
"Can not get tmem freeable memory");
diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index 2c36eb2..a0395dc 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -1798,21 +1798,20 @@ static PyObject *pyxc_tmem_control(XcObject *self,
uint32_t cli_id;
uint32_t arg1;
uint32_t arg2;
- uint64_t arg3;
char *buf;
char _buffer[32768], *buffer = _buffer;
int rc;
- static char *kwd_list[] = { "pool_id", "subop", "cli_id", "arg1", "arg2", "arg3", "buf", NULL };
+ static char *kwd_list[] = { "pool_id", "subop", "cli_id", "arg1", "arg2", "buf", NULL };
if ( !PyArg_ParseTupleAndKeywords(args, kwds, "iiiiiis", kwd_list,
- &pool_id, &subop, &cli_id, &arg1, &arg2, &arg3, &buf) )
+ &pool_id, &subop, &cli_id, &arg1, &arg2, &buf) )
return NULL;
if ( (subop == TMEMC_LIST) && (arg1 > 32768) )
arg1 = 32768;
- if ( (rc = xc_tmem_control(self->xc_handle, pool_id, subop, cli_id, arg1, arg2, arg3, buffer)) < 0 )
+ if ( (rc = xc_tmem_control(self->xc_handle, pool_id, subop, cli_id, arg1, arg2, buffer)) < 0 )
return Py_BuildValue("i", rc);
switch (subop) {
diff --git a/tools/xenstat/libxenstat/src/xenstat.c b/tools/xenstat/libxenstat/src/xenstat.c
index dfffbbc..24b57d0 100644
--- a/tools/xenstat/libxenstat/src/xenstat.c
+++ b/tools/xenstat/libxenstat/src/xenstat.c
@@ -150,7 +150,7 @@ void domain_get_tmem_stats(xenstat_handle * handle, xenstat_domain * domain)
char buffer[4096];
if (xc_tmem_control(handle->xc_handle,-1,TMEMC_LIST,domain->id,
- sizeof(buffer),-1,-1,buffer) < 0)
+ sizeof(buffer),-1,buffer) < 0)
return;
domain->tmem_stats.curr_eph_pages = parse(buffer,"Ec");
domain->tmem_stats.succ_eph_gets = parse(buffer,"Ge");
@@ -191,7 +191,7 @@ xenstat_node *xenstat_get_node(xenstat_handle * handle, unsigned int flags)
* handle->page_size;
rc = xc_tmem_control(handle->xc_handle, -1,
- TMEMC_QUERY_FREEABLE_MB, -1, 0, 0, 0, NULL);
+ TMEMC_QUERY_FREEABLE_MB, -1, 0, 0, NULL);
node->freeable_mb = (rc < 0) ? 0 : rc;
/* malloc(0) is not portable, so allocate a single domain. This will
* be resized below. */
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 05/11] tmem: Move TMEM_CONTROL subop of tmem hypercall to sysctl.
2015-08-28 18:53 [PATCH v3] Tmem bug-fixes and cleanups Konrad Rzeszutek Wilk
` (3 preceding siblings ...)
2015-08-28 18:53 ` [PATCH v3 04/11] tmem: Remove xc_tmem_control mystical arg3 Konrad Rzeszutek Wilk
@ 2015-08-28 18:53 ` Konrad Rzeszutek Wilk
2015-08-31 11:32 ` Jan Beulich
2015-08-28 18:53 ` [PATCH v3 06/11] tmem: Remove the old tmem control XSM checks as it is part of sysctl hypercall Konrad Rzeszutek Wilk
` (5 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-28 18:53 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2; +Cc: Konrad Rzeszutek Wilk
The operations are to be used by an control domain to set parameters,
list pools, clients, and to be used during migration.
There is no need to have them in the tmem hypercall path.
This patch moves code without adding fixes - and in fact in
some cases makes the parameters soo long that they hurt eyes - but
that is for another patch.
Note that in regards to existing users:
- Only the control domain could call it - which meant that if
a guest called it would get -EPERM, so we are OK there.
In practice no guests called this TMEM_CONTROL command.
- The spec: https://oss.oracle.com/projects/tmem/dist/documentation/api/tmemspec-v001.pdf
mentions: "TBD [Not sure if this is really needed.]"
which is a carte blanche as any to do this!
Note: The XSM check is the same - we just move it from do_tmem_op
to do_sysctl.
We also add an 32-bit pad to make the sysctl structure have the same
exact size under 32 and 64-bit toolstacks and not worry about aligment
issues.
And the XLAT does not need to deal with the buf as it has been
moved to another structure which is 32/64 fixed.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
tools/libxc/xc_tmem.c | 106 ++++++++++++-------------
tools/libxl/libxl.c | 14 ++--
tools/python/xen/lowlevel/xc/xc.c | 20 ++---
tools/xenstat/libxenstat/src/xenstat.c | 4 +-
xen/common/sysctl.c | 7 +-
xen/common/tmem.c | 137 ++++++++++++++++-----------------
xen/include/public/sysctl.h | 44 +++++++++++
xen/include/public/tmem.h | 43 ++---------
xen/include/xen/tmem.h | 3 +
xen/include/xen/tmem_xen.h | 4 -
xen/xsm/flask/hooks.c | 3 +
11 files changed, 203 insertions(+), 182 deletions(-)
diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 467001b..505595b 100644
--- a/tools/libxc/xc_tmem.c
+++ b/tools/libxc/xc_tmem.c
@@ -47,27 +47,28 @@ static int do_tmem_op(xc_interface *xch, tmem_op_t *op)
int xc_tmem_control(xc_interface *xch,
int32_t pool_id,
- uint32_t subop,
+ uint32_t cmd,
uint32_t cli_id,
uint32_t arg1,
uint32_t arg2,
void *buf)
{
- tmem_op_t op;
+ DECLARE_SYSCTL;
DECLARE_HYPERCALL_BOUNCE(buf, arg1, XC_HYPERCALL_BUFFER_BOUNCE_OUT);
int rc;
- op.cmd = TMEM_CONTROL;
- op.pool_id = pool_id;
- op.u.ctrl.subop = subop;
- op.u.ctrl.cli_id = cli_id;
- op.u.ctrl.arg1 = arg1;
- op.u.ctrl.arg2 = arg2;
- op.u.ctrl.oid[0] = 0;
- op.u.ctrl.oid[1] = 0;
- op.u.ctrl.oid[2] = 0;
-
- if ( subop == TMEMC_LIST && arg1 != 0 )
+ sysctl.cmd = XEN_SYSCTL_tmem_op;
+ sysctl.u.tmem_op.pool_id = pool_id;
+ sysctl.u.tmem_op.cmd = cmd;
+ sysctl.u.tmem_op.cli_id = cli_id;
+ sysctl.u.tmem_op.arg1 = arg1;
+ sysctl.u.tmem_op.arg2 = arg2;
+ sysctl.u.tmem_op.pad = 0;
+ sysctl.u.tmem_op.oid[0] = 0;
+ sysctl.u.tmem_op.oid[1] = 0;
+ sysctl.u.tmem_op.oid[2] = 0;
+
+ if ( cmd == XEN_SYSCTL_TMEM_OP_LIST && arg1 != 0 )
{
if ( buf == NULL )
{
@@ -81,11 +82,11 @@ int xc_tmem_control(xc_interface *xch,
}
}
- set_xen_guest_handle(op.u.ctrl.buf, buf);
+ set_xen_guest_handle(sysctl.u.tmem_op.buf, buf);
- rc = do_tmem_op(xch, &op);
+ rc = do_sysctl(xch, &sysctl);
- if (subop == TMEMC_LIST && arg1 != 0)
+ if ( cmd == XEN_SYSCTL_TMEM_OP_LIST && arg1 != 0 )
xc_hypercall_bounce_post(xch, buf);
return rc;
@@ -93,28 +94,29 @@ int xc_tmem_control(xc_interface *xch,
int xc_tmem_control_oid(xc_interface *xch,
int32_t pool_id,
- uint32_t subop,
+ uint32_t cmd,
uint32_t cli_id,
uint32_t arg1,
uint32_t arg2,
struct tmem_oid oid,
void *buf)
{
- tmem_op_t op;
+ DECLARE_SYSCTL;
DECLARE_HYPERCALL_BOUNCE(buf, arg1, XC_HYPERCALL_BUFFER_BOUNCE_OUT);
int rc;
- op.cmd = TMEM_CONTROL;
- op.pool_id = pool_id;
- op.u.ctrl.subop = subop;
- op.u.ctrl.cli_id = cli_id;
- op.u.ctrl.arg1 = arg1;
- op.u.ctrl.arg2 = arg2;
- op.u.ctrl.oid[0] = oid.oid[0];
- op.u.ctrl.oid[1] = oid.oid[1];
- op.u.ctrl.oid[2] = oid.oid[2];
-
- if ( subop == TMEMC_LIST && arg1 != 0 )
+ sysctl.cmd = XEN_SYSCTL_tmem_op;
+ sysctl.u.tmem_op.pool_id = pool_id;
+ sysctl.u.tmem_op.cmd = cmd;
+ sysctl.u.tmem_op.cli_id = cli_id;
+ sysctl.u.tmem_op.arg1 = arg1;
+ sysctl.u.tmem_op.arg2 = arg2;
+ sysctl.u.tmem_op.pad = 0;
+ sysctl.u.tmem_op.oid[0] = oid.oid[0];
+ sysctl.u.tmem_op.oid[1] = oid.oid[1];
+ sysctl.u.tmem_op.oid[2] = oid.oid[2];
+
+ if ( cmd == XEN_SYSCTL_TMEM_OP_LIST && arg1 != 0 )
{
if ( buf == NULL )
{
@@ -128,11 +130,11 @@ int xc_tmem_control_oid(xc_interface *xch,
}
}
- set_xen_guest_handle(op.u.ctrl.buf, buf);
+ set_xen_guest_handle(sysctl.u.tmem_op.buf, buf);
- rc = do_tmem_op(xch, &op);
+ rc = do_sysctl(xch, &sysctl);
- if (subop == TMEMC_LIST && arg1 != 0)
+ if ( cmd == XEN_SYSCTL_TMEM_OP_LIST && arg1 != 0 )
xc_hypercall_bounce_post(xch, buf);
return rc;
@@ -218,28 +220,28 @@ int xc_tmem_save(xc_interface *xch,
uint32_t minusone = -1;
struct tmem_handle *h;
- if ( xc_tmem_control(xch,0,TMEMC_SAVE_BEGIN,dom,live,0,NULL) <= 0 )
+ if ( xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_SAVE_BEGIN,dom,live,0,NULL) <= 0 )
return 0;
if ( write_exact(io_fd, &marker, sizeof(marker)) )
return -1;
- version = xc_tmem_control(xch,0,TMEMC_SAVE_GET_VERSION,0,0,0,NULL);
+ version = xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_SAVE_GET_VERSION,0,0,0,NULL);
if ( write_exact(io_fd, &version, sizeof(version)) )
return -1;
- max_pools = xc_tmem_control(xch,0,TMEMC_SAVE_GET_MAXPOOLS,0,0,0,NULL);
+ max_pools = xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_SAVE_GET_MAXPOOLS,0,0,0,NULL);
if ( write_exact(io_fd, &max_pools, sizeof(max_pools)) )
return -1;
if ( version == -1 || max_pools == -1 )
return -1;
if ( write_exact(io_fd, &minusone, sizeof(minusone)) )
return -1;
- flags = xc_tmem_control(xch,0,TMEMC_SAVE_GET_CLIENT_FLAGS,dom,0,0,NULL);
+ flags = xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_FLAGS,dom,0,0,NULL);
if ( write_exact(io_fd, &flags, sizeof(flags)) )
return -1;
- weight = xc_tmem_control(xch,0,TMEMC_SAVE_GET_CLIENT_WEIGHT,dom,0,0,NULL);
+ weight = xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_WEIGHT,dom,0,0,NULL);
if ( write_exact(io_fd, &weight, sizeof(weight)) )
return -1;
- cap = xc_tmem_control(xch,0,TMEMC_SAVE_GET_CLIENT_CAP,dom,0,0,NULL);
+ cap = xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_CAP,dom,0,0,NULL);
if ( write_exact(io_fd, &cap, sizeof(cap)) )
return -1;
if ( flags == -1 || weight == -1 || cap == -1 )
@@ -256,14 +258,14 @@ int xc_tmem_save(xc_interface *xch,
int checksum = 0;
/* get pool id, flags, pagesize, n_pages, uuid */
- flags = xc_tmem_control(xch,i,TMEMC_SAVE_GET_POOL_FLAGS,dom,0,0,NULL);
+ flags = xc_tmem_control(xch,i,XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_FLAGS,dom,0,0,NULL);
if ( flags != -1 )
{
pool_id = i;
- n_pages = xc_tmem_control(xch,i,TMEMC_SAVE_GET_POOL_NPAGES,dom,0,0,NULL);
+ 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,TMEMC_SAVE_GET_POOL_UUID,dom,sizeof(uuid),0,&uuid);
+ (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)) )
@@ -287,7 +289,7 @@ int xc_tmem_save(xc_interface *xch,
{
int ret;
if ( (ret = xc_tmem_control(xch, pool_id,
- TMEMC_SAVE_GET_NEXT_PAGE, dom,
+ XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_PAGE, dom,
bufsize, 0, buf)) > 0 )
{
h = (struct tmem_handle *)buf;
@@ -332,7 +334,7 @@ int xc_tmem_save_extra(xc_interface *xch, int dom, int io_fd, int field_marker)
if ( write_exact(io_fd, &marker, sizeof(marker)) )
return -1;
- while ( xc_tmem_control(xch, 0, TMEMC_SAVE_GET_NEXT_INV, dom,
+ while ( xc_tmem_control(xch, 0, XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_INV, dom,
sizeof(handle),0,&handle) > 0 ) {
if ( write_exact(io_fd, &handle.pool_id, sizeof(handle.pool_id)) )
return -1;
@@ -355,7 +357,7 @@ int xc_tmem_save_extra(xc_interface *xch, int dom, int io_fd, int field_marker)
/* only called for live migration */
void xc_tmem_save_done(xc_interface *xch, int dom)
{
- xc_tmem_control(xch,0,TMEMC_SAVE_END,dom,0,0,NULL);
+ xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_SAVE_END,dom,0,0,NULL);
}
/* restore routines */
@@ -389,7 +391,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
uint32_t weight, cap, flags;
int checksum = 0;
- save_version = xc_tmem_control(xch,0,TMEMC_SAVE_GET_VERSION,dom,0,0,NULL);
+ save_version = xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_SAVE_GET_VERSION,dom,0,0,NULL);
if ( save_version == -1 )
return -1; /* domain doesn't exist */
if ( read_exact(io_fd, &this_version, sizeof(this_version)) )
@@ -401,23 +403,23 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
return -1;
if ( minusone != -1 )
return -1;
- if ( xc_tmem_control(xch,0,TMEMC_RESTORE_BEGIN,dom,0,0,NULL) < 0 )
+ if ( xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_RESTORE_BEGIN,dom,0,0,NULL) < 0 )
return -1;
if ( read_exact(io_fd, &flags, sizeof(flags)) )
return -1;
if ( flags & TMEM_CLIENT_COMPRESS )
- if ( xc_tmem_control(xch,0,TMEMC_SET_COMPRESS,dom,1,0,NULL) < 0 )
+ if ( xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_SET_COMPRESS,dom,1,0,NULL) < 0 )
return -1;
if ( flags & TMEM_CLIENT_FROZEN )
- if ( xc_tmem_control(xch,0,TMEMC_FREEZE,dom,0,0,NULL) < 0 )
+ if ( xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_FREEZE,dom,0,0,NULL) < 0 )
return -1;
if ( read_exact(io_fd, &weight, sizeof(weight)) )
return -1;
- if ( xc_tmem_control(xch,0,TMEMC_SET_WEIGHT,dom,0,0,NULL) < 0 )
+ if ( xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_SET_WEIGHT,dom,0,0,NULL) < 0 )
return -1;
if ( read_exact(io_fd, &cap, sizeof(cap)) )
return -1;
- if ( xc_tmem_control(xch,0,TMEMC_SET_CAP,dom,0,0,NULL) < 0 )
+ if ( xc_tmem_control(xch,0,XEN_SYSCTL_TMEM_OP_SET_CAP,dom,0,0,NULL) < 0 )
return -1;
if ( read_exact(io_fd, &minusone, sizeof(minusone)) )
return -1;
@@ -464,7 +466,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
return -1;
checksum += *buf;
if ( (rc = xc_tmem_control_oid(xch, pool_id,
- TMEMC_RESTORE_PUT_PAGE, dom,
+ XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE, dom,
bufsize, index, oid, buf)) <= 0 )
{
DPRINTF("xc_tmem_restore: putting page failed, rc=%d\n",rc);
@@ -496,7 +498,7 @@ int xc_tmem_restore_extra(xc_interface *xch, int dom, int io_fd)
return -1;
if ( read_exact(io_fd, &index, sizeof(index)) )
return -1;
- if ( xc_tmem_control_oid(xch, pool_id, TMEMC_RESTORE_FLUSH_PAGE, dom,
+ if ( xc_tmem_control_oid(xch, pool_id, XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE, dom,
0,index,oid,NULL) <= 0 )
return -1;
count++;
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e564c22..10d1909 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -6045,7 +6045,7 @@ char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long)
int rc;
char _buf[32768];
- rc = xc_tmem_control(ctx->xch, -1, TMEMC_LIST, domid, 32768, use_long,
+ rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST, domid, 32768, use_long,
_buf);
if (rc < 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
@@ -6060,7 +6060,7 @@ int libxl_tmem_freeze(libxl_ctx *ctx, uint32_t domid)
{
int rc;
- rc = xc_tmem_control(ctx->xch, -1, TMEMC_FREEZE, domid, 0, 0,
+ rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_FREEZE, domid, 0, 0,
NULL);
if (rc < 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
@@ -6075,7 +6075,7 @@ int libxl_tmem_thaw(libxl_ctx *ctx, uint32_t domid)
{
int rc;
- rc = xc_tmem_control(ctx->xch, -1, TMEMC_THAW, domid, 0, 0,
+ rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_THAW, domid, 0, 0,
NULL);
if (rc < 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
@@ -6089,11 +6089,11 @@ int libxl_tmem_thaw(libxl_ctx *ctx, uint32_t domid)
static int32_t tmem_setop_from_string(char *set_name)
{
if (!strcmp(set_name, "weight"))
- return TMEMC_SET_WEIGHT;
+ return XEN_SYSCTL_TMEM_OP_SET_WEIGHT;
else if (!strcmp(set_name, "cap"))
- return TMEMC_SET_CAP;
+ return XEN_SYSCTL_TMEM_OP_SET_CAP;
else if (!strcmp(set_name, "compress"))
- return TMEMC_SET_COMPRESS;
+ return XEN_SYSCTL_TMEM_OP_SET_COMPRESS;
else
return -1;
}
@@ -6137,7 +6137,7 @@ int libxl_tmem_freeable(libxl_ctx *ctx)
{
int rc;
- rc = xc_tmem_control(ctx->xch, -1, TMEMC_QUERY_FREEABLE_MB, -1, 0, 0, 0);
+ rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB, -1, 0, 0, 0);
if (rc < 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
"Can not get tmem freeable memory");
diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index a0395dc..5264854 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -1808,25 +1808,25 @@ static PyObject *pyxc_tmem_control(XcObject *self,
&pool_id, &subop, &cli_id, &arg1, &arg2, &buf) )
return NULL;
- if ( (subop == TMEMC_LIST) && (arg1 > 32768) )
+ if ( (subop == XEN_SYSCTL_TMEM_OP_LIST) && (arg1 > 32768) )
arg1 = 32768;
if ( (rc = xc_tmem_control(self->xc_handle, pool_id, subop, cli_id, arg1, arg2, buffer)) < 0 )
return Py_BuildValue("i", rc);
switch (subop) {
- case TMEMC_LIST:
+ case XEN_SYSCTL_TMEM_OP_LIST:
return Py_BuildValue("s", buffer);
- case TMEMC_FLUSH:
+ case XEN_SYSCTL_TMEM_OP_FLUSH:
return Py_BuildValue("i", rc);
- case TMEMC_QUERY_FREEABLE_MB:
+ case XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB:
return Py_BuildValue("i", rc);
- case TMEMC_THAW:
- case TMEMC_FREEZE:
- case TMEMC_DESTROY:
- case TMEMC_SET_WEIGHT:
- case TMEMC_SET_CAP:
- case TMEMC_SET_COMPRESS:
+ case XEN_SYSCTL_TMEM_OP_THAW:
+ case XEN_SYSCTL_TMEM_OP_FREEZE:
+ case XEN_SYSCTL_TMEM_OP_DESTROY:
+ case XEN_SYSCTL_TMEM_OP_SET_WEIGHT:
+ case XEN_SYSCTL_TMEM_OP_SET_CAP:
+ case XEN_SYSCTL_TMEM_OP_SET_COMPRESS:
default:
break;
}
diff --git a/tools/xenstat/libxenstat/src/xenstat.c b/tools/xenstat/libxenstat/src/xenstat.c
index 24b57d0..3495f3f 100644
--- a/tools/xenstat/libxenstat/src/xenstat.c
+++ b/tools/xenstat/libxenstat/src/xenstat.c
@@ -149,7 +149,7 @@ void domain_get_tmem_stats(xenstat_handle * handle, xenstat_domain * domain)
{
char buffer[4096];
- if (xc_tmem_control(handle->xc_handle,-1,TMEMC_LIST,domain->id,
+ if (xc_tmem_control(handle->xc_handle,-1,XEN_SYSCTL_TMEM_OP_LIST,domain->id,
sizeof(buffer),-1,buffer) < 0)
return;
domain->tmem_stats.curr_eph_pages = parse(buffer,"Ec");
@@ -191,7 +191,7 @@ xenstat_node *xenstat_get_node(xenstat_handle * handle, unsigned int flags)
* handle->page_size;
rc = xc_tmem_control(handle->xc_handle, -1,
- TMEMC_QUERY_FREEABLE_MB, -1, 0, 0, NULL);
+ XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB, -1, 0, 0, NULL);
node->freeable_mb = (rc < 0) ? 0 : rc;
/* malloc(0) is not portable, so allocate a single domain. This will
* be resized below. */
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index f1c0c76..a4287c3 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -14,6 +14,7 @@
#include <xen/domain.h>
#include <xen/event.h>
#include <xen/domain_page.h>
+#include <xen/tmem.h>
#include <xen/trace.h>
#include <xen/console.h>
#include <xen/iocap.h>
@@ -68,7 +69,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
case XEN_SYSCTL_tbuf_op:
ret = tb_control(&op->u.tbuf_op);
break;
-
+
+ case XEN_SYSCTL_tmem_op:
+ ret = tmem_control(&op->u.tmem_op);
+ break;
+
case XEN_SYSCTL_sched_id:
op->u.sched_id.sched_id = sched_id();
break;
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index c1ffe79..b62b56e 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -18,6 +18,7 @@
#include <xen/tmem_xen.h> /* host-specific (eg Xen) code goes here */
#endif
+#include <public/sysctl.h>
#include <xen/tmem.h>
#include <xen/rbtree.h>
#include <xen/radix-tree.h>
@@ -2012,8 +2013,8 @@ fail:
static int tmemc_freeze_pools(domid_t cli_id, int arg)
{
struct client *client;
- bool_t freeze = (arg == TMEMC_FREEZE) ? 1 : 0;
- bool_t destroy = (arg == TMEMC_DESTROY) ? 1 : 0;
+ bool_t freeze = (arg == XEN_SYSCTL_TMEM_OP_FREEZE) ? 1 : 0;
+ bool_t destroy = (arg == XEN_SYSCTL_TMEM_OP_DESTROY) ? 1 : 0;
char *s;
s = destroy ? "destroyed" : ( freeze ? "frozen" : "thawed" );
@@ -2230,7 +2231,7 @@ static int __tmemc_set_var(struct client *client, uint32_t subop, uint32_t arg1)
switch (subop)
{
- case TMEMC_SET_WEIGHT:
+ case XEN_SYSCTL_TMEM_OP_SET_WEIGHT:
old_weight = client->weight;
client->weight = arg1;
tmem_client_info("tmem: weight set to %d for %s=%d\n",
@@ -2238,12 +2239,12 @@ static int __tmemc_set_var(struct client *client, uint32_t subop, uint32_t arg1)
atomic_sub(old_weight,&client_weight_total);
atomic_add(client->weight,&client_weight_total);
break;
- case TMEMC_SET_CAP:
+ case XEN_SYSCTL_TMEM_OP_SET_CAP:
client->cap = arg1;
tmem_client_info("tmem: cap set to %d for %s=%d\n",
arg1, tmem_cli_id_str, cli_id);
break;
- case TMEMC_SET_COMPRESS:
+ case XEN_SYSCTL_TMEM_OP_SET_COMPRESS:
if ( tmem_dedup_enabled() )
{
tmem_client_warn("tmem: compression %s for all %ss, cannot be changed when tmem_dedup is enabled\n",
@@ -2346,7 +2347,7 @@ static int tmemc_save_subop(int cli_id, uint32_t pool_id,
switch(subop)
{
- case TMEMC_SAVE_BEGIN:
+ case XEN_SYSCTL_TMEM_OP_SAVE_BEGIN:
if ( client == NULL )
return 0;
for (p = 0; p < MAX_POOLS_PER_DOMAIN; p++)
@@ -2363,33 +2364,33 @@ static int tmemc_save_subop(int cli_id, uint32_t pool_id,
client->live_migrating = 1;
rc = 1;
break;
- case TMEMC_RESTORE_BEGIN:
+ case XEN_SYSCTL_TMEM_OP_RESTORE_BEGIN:
if ( client == NULL && (client = client_create(cli_id)) != NULL )
return 1;
break;
- case TMEMC_SAVE_GET_VERSION:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_VERSION:
rc = TMEM_SPEC_VERSION;
break;
- case TMEMC_SAVE_GET_MAXPOOLS:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_MAXPOOLS:
rc = MAX_POOLS_PER_DOMAIN;
break;
- case TMEMC_SAVE_GET_CLIENT_WEIGHT:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_WEIGHT:
if ( client == NULL )
break;
rc = client->weight == -1 ? -2 : client->weight;
break;
- case TMEMC_SAVE_GET_CLIENT_CAP:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_CAP:
if ( client == NULL )
break;
rc = client->cap == -1 ? -2 : client->cap;
break;
- case TMEMC_SAVE_GET_CLIENT_FLAGS:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_FLAGS:
if ( client == NULL )
break;
rc = (client->compress ? TMEM_CLIENT_COMPRESS : 0 ) |
(client->was_frozen ? TMEM_CLIENT_FROZEN : 0 );
break;
- case TMEMC_SAVE_GET_POOL_FLAGS:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_FLAGS:
if ( pool == NULL )
break;
rc = (pool->persistent ? TMEM_POOL_PERSIST : 0) |
@@ -2397,19 +2398,19 @@ static int tmemc_save_subop(int cli_id, uint32_t pool_id,
(POOL_PAGESHIFT << TMEM_POOL_PAGESIZE_SHIFT) |
(TMEM_SPEC_VERSION << TMEM_POOL_VERSION_SHIFT);
break;
- case TMEMC_SAVE_GET_POOL_NPAGES:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_NPAGES:
if ( pool == NULL )
break;
rc = _atomic_read(pool->pgp_count);
break;
- case TMEMC_SAVE_GET_POOL_UUID:
+ 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) )
rc = -EFAULT;
break;
- case TMEMC_SAVE_END:
+ case XEN_SYSCTL_TMEM_OP_SAVE_END:
if ( client == NULL )
break;
client->live_migrating = 0;
@@ -2553,77 +2554,75 @@ static int tmemc_restore_flush_page(int cli_id, uint32_t pool_id, struct oid *oi
return do_tmem_flush_page(pool,oidp,index);
}
-static int do_tmem_control(struct tmem_op *op)
+int tmem_control(struct xen_sysctl_tmem_op *op)
{
int ret;
uint32_t pool_id = op->pool_id;
- uint32_t subop = op->u.ctrl.subop;
- struct oid *oidp = (struct oid *)(&op->u.ctrl.oid[0]);
+ uint32_t cmd = op->cmd;
+ struct oid *oidp = (struct oid *)(&op->oid[0]);
- if ( xsm_tmem_control(XSM_PRIV) )
- return -EPERM;
+ if ( op->pad != 0 )
+ return -EINVAL;
- switch(subop)
+ write_lock(&tmem_rwlock);
+
+ switch (cmd)
{
- case TMEMC_THAW:
- case TMEMC_FREEZE:
- case TMEMC_DESTROY:
- ret = tmemc_freeze_pools(op->u.ctrl.cli_id,subop);
+ case XEN_SYSCTL_TMEM_OP_THAW:
+ case XEN_SYSCTL_TMEM_OP_FREEZE:
+ case XEN_SYSCTL_TMEM_OP_DESTROY:
+ ret = tmemc_freeze_pools(op->cli_id, cmd);
break;
- case TMEMC_FLUSH:
- ret = tmemc_flush_mem(op->u.ctrl.cli_id,op->u.ctrl.arg1);
+ case XEN_SYSCTL_TMEM_OP_FLUSH:
+ ret = tmemc_flush_mem(op->cli_id,op->arg1);
break;
- case TMEMC_LIST:
- ret = tmemc_list(op->u.ctrl.cli_id,
- guest_handle_cast(op->u.ctrl.buf, char),
- op->u.ctrl.arg1,op->u.ctrl.arg2);
+ case XEN_SYSCTL_TMEM_OP_LIST:
+ ret = tmemc_list(op->cli_id,
+ guest_handle_cast(op->buf, char), op->arg1, op->arg2);
break;
- case TMEMC_SET_WEIGHT:
- case TMEMC_SET_CAP:
- case TMEMC_SET_COMPRESS:
- ret = tmemc_set_var(op->u.ctrl.cli_id,subop,op->u.ctrl.arg1);
+ case XEN_SYSCTL_TMEM_OP_SET_WEIGHT:
+ case XEN_SYSCTL_TMEM_OP_SET_CAP:
+ case XEN_SYSCTL_TMEM_OP_SET_COMPRESS:
+ ret = tmemc_set_var(op->cli_id, cmd, op->arg1);
break;
- case TMEMC_QUERY_FREEABLE_MB:
+ case XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB:
ret = tmem_freeable_pages() >> (20 - PAGE_SHIFT);
break;
- case TMEMC_SAVE_BEGIN:
- case TMEMC_RESTORE_BEGIN:
- case TMEMC_SAVE_GET_VERSION:
- case TMEMC_SAVE_GET_MAXPOOLS:
- case TMEMC_SAVE_GET_CLIENT_WEIGHT:
- case TMEMC_SAVE_GET_CLIENT_CAP:
- case TMEMC_SAVE_GET_CLIENT_FLAGS:
- case TMEMC_SAVE_GET_POOL_FLAGS:
- case TMEMC_SAVE_GET_POOL_NPAGES:
- case TMEMC_SAVE_GET_POOL_UUID:
- case TMEMC_SAVE_END:
- ret = tmemc_save_subop(op->u.ctrl.cli_id,pool_id,subop,
- guest_handle_cast(op->u.ctrl.buf, char),
- op->u.ctrl.arg1);
+ case XEN_SYSCTL_TMEM_OP_SAVE_BEGIN:
+ case XEN_SYSCTL_TMEM_OP_RESTORE_BEGIN:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_VERSION:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_MAXPOOLS:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_WEIGHT:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_CAP:
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_FLAGS:
+ 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:
+ case XEN_SYSCTL_TMEM_OP_SAVE_END:
+ ret = tmemc_save_subop(op->cli_id, pool_id, cmd,
+ guest_handle_cast(op->buf, char), op->arg1);
break;
- case TMEMC_SAVE_GET_NEXT_PAGE:
- ret = tmemc_save_get_next_page(op->u.ctrl.cli_id, pool_id,
- guest_handle_cast(op->u.ctrl.buf, char),
- op->u.ctrl.arg1);
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_PAGE:
+ ret = tmemc_save_get_next_page(op->cli_id, pool_id,
+ guest_handle_cast(op->buf, char), op->arg1);
break;
- case TMEMC_SAVE_GET_NEXT_INV:
- ret = tmemc_save_get_next_inv(op->u.ctrl.cli_id,
- guest_handle_cast(op->u.ctrl.buf, char),
- op->u.ctrl.arg1);
+ case XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_INV:
+ ret = tmemc_save_get_next_inv(op->cli_id,
+ guest_handle_cast(op->buf, char), op->arg1);
break;
- case TMEMC_RESTORE_PUT_PAGE:
- ret = tmemc_restore_put_page(op->u.ctrl.cli_id,pool_id,
- oidp, op->u.ctrl.arg2,
- guest_handle_cast(op->u.ctrl.buf, char),
- op->u.ctrl.arg1);
+ case XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE:
+ ret = tmemc_restore_put_page(op->cli_id, pool_id, oidp, op->arg2,
+ guest_handle_cast(op->buf, char), op->arg1);
break;
- case TMEMC_RESTORE_FLUSH_PAGE:
- ret = tmemc_restore_flush_page(op->u.ctrl.cli_id,pool_id,
- oidp, op->u.ctrl.arg2);
+ case XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE:
+ ret = tmemc_restore_flush_page(op->cli_id, pool_id, oidp, op->arg2);
break;
default:
ret = -1;
}
+
+ write_unlock(&tmem_rwlock);
+
return ret;
}
@@ -2666,7 +2665,7 @@ long do_tmem_op(tmem_cli_op_t uops)
if ( op.cmd == TMEM_CONTROL )
{
- rc = do_tmem_control(&op);
+ rc = -EOPNOTSUPP;
}
else if ( op.cmd == TMEM_AUTH )
{
@@ -2786,7 +2785,7 @@ void tmem_destroy(void *v)
write_unlock(&tmem_rwlock);
}
-#define MAX_EVICTS 10 /* should be variable or set via TMEMC_ ?? */
+#define MAX_EVICTS 10 /* should be variable or set via XEN_SYSCTL_TMEM_OP_ ?? */
void *tmem_relinquish_pages(unsigned int order, unsigned int memflags)
{
struct page_info *pfp;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 58c9be2..c1566e6 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -710,6 +710,48 @@ struct xen_sysctl_psr_cat_op {
typedef struct xen_sysctl_psr_cat_op xen_sysctl_psr_cat_op_t;
DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
+#define XEN_SYSCTL_TMEM_OP_ALL_CLIENTS 0xFFFFU
+
+#define XEN_SYSCTL_TMEM_OP_THAW 0
+#define XEN_SYSCTL_TMEM_OP_FREEZE 1
+#define XEN_SYSCTL_TMEM_OP_FLUSH 2
+#define XEN_SYSCTL_TMEM_OP_DESTROY 3
+#define XEN_SYSCTL_TMEM_OP_LIST 4
+#define XEN_SYSCTL_TMEM_OP_SET_WEIGHT 5
+#define XEN_SYSCTL_TMEM_OP_SET_CAP 6
+#define XEN_SYSCTL_TMEM_OP_SET_COMPRESS 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_VERSION 11
+#define XEN_SYSCTL_TMEM_OP_SAVE_GET_MAXPOOLS 12
+#define XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_WEIGHT 13
+#define XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_CAP 14
+#define XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_FLAGS 15
+#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
+#define XEN_SYSCTL_TMEM_OP_RESTORE_BEGIN 30
+#define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
+#define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
+
+struct xen_sysctl_tmem_op {
+ uint32_t cmd; /* IN: XEN_SYSCTL_TMEM_OP_* . */
+ int32_t pool_id; /* IN: 0 by default unless _SAVE_*, RESTORE_* .*/
+ uint32_t cli_id; /* IN: client id, 0 for XEN_SYSCTL_TMEM_QUERY_FREEABLE_MB
+ for all others can be the domain id or
+ XEN_SYSCTL_TMEM_OP_ALL_CLIENTS for all. */
+ uint32_t arg1; /* IN: If not applicable to command use 0. */
+ uint32_t arg2; /* IN: If not applicable to command use 0. */
+ uint32_t pad; /* Padding so structure is the same under 32 and 64. */
+ uint64_t oid[3]; /* IN: If not applicable to command use 0. */
+ XEN_GUEST_HANDLE_64(char) buf; /* IN/OUT: Buffer to save and restore ops. */
+};
+typedef struct xen_sysctl_tmem_op xen_sysctl_tmem_op_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_tmem_op_t);
+
struct xen_sysctl {
uint32_t cmd;
#define XEN_SYSCTL_readconsole 1
@@ -734,6 +776,7 @@ struct xen_sysctl {
#define XEN_SYSCTL_psr_cmt_op 21
#define XEN_SYSCTL_pcitopoinfo 22
#define XEN_SYSCTL_psr_cat_op 23
+#define XEN_SYSCTL_tmem_op 24
uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
union {
struct xen_sysctl_readconsole readconsole;
@@ -758,6 +801,7 @@ struct xen_sysctl {
struct xen_sysctl_coverage_op coverage_op;
struct xen_sysctl_psr_cmt_op psr_cmt_op;
struct xen_sysctl_psr_cat_op psr_cat_op;
+ struct xen_sysctl_tmem_op tmem_op;
uint8_t pad[128];
} u;
};
diff --git a/xen/include/public/tmem.h b/xen/include/public/tmem.h
index 4fd2fc6..e4ee704 100644
--- a/xen/include/public/tmem.h
+++ b/xen/include/public/tmem.h
@@ -33,7 +33,11 @@
#define TMEM_SPEC_VERSION 1
/* Commands to HYPERVISOR_tmem_op() */
-#define TMEM_CONTROL 0
+#ifdef __XEN__
+#define TMEM_CONTROL 0 /* Now called XEN_SYSCTL_tmem_op */
+#else
+#undef TMEM_CONTROL
+#endif
#define TMEM_NEW_POOL 1
#define TMEM_DESTROY_POOL 2
#define TMEM_PUT_PAGE 4
@@ -48,35 +52,9 @@
#endif
/* Privileged commands to HYPERVISOR_tmem_op() */
-#define TMEM_AUTH 101
+#define TMEM_AUTH 101
#define TMEM_RESTORE_NEW 102
-/* Subops for HYPERVISOR_tmem_op(TMEM_CONTROL) */
-#define TMEMC_THAW 0
-#define TMEMC_FREEZE 1
-#define TMEMC_FLUSH 2
-#define TMEMC_DESTROY 3
-#define TMEMC_LIST 4
-#define TMEMC_SET_WEIGHT 5
-#define TMEMC_SET_CAP 6
-#define TMEMC_SET_COMPRESS 7
-#define TMEMC_QUERY_FREEABLE_MB 8
-#define TMEMC_SAVE_BEGIN 10
-#define TMEMC_SAVE_GET_VERSION 11
-#define TMEMC_SAVE_GET_MAXPOOLS 12
-#define TMEMC_SAVE_GET_CLIENT_WEIGHT 13
-#define TMEMC_SAVE_GET_CLIENT_CAP 14
-#define TMEMC_SAVE_GET_CLIENT_FLAGS 15
-#define TMEMC_SAVE_GET_POOL_FLAGS 16
-#define TMEMC_SAVE_GET_POOL_NPAGES 17
-#define TMEMC_SAVE_GET_POOL_UUID 18
-#define TMEMC_SAVE_GET_NEXT_PAGE 19
-#define TMEMC_SAVE_GET_NEXT_INV 20
-#define TMEMC_SAVE_END 21
-#define TMEMC_RESTORE_BEGIN 30
-#define TMEMC_RESTORE_PUT_PAGE 32
-#define TMEMC_RESTORE_FLUSH_PAGE 33
-
/* Bits for HYPERVISOR_tmem_op(TMEM_NEW_POOL) */
#define TMEM_POOL_PERSIST 1
#define TMEM_POOL_SHARED 2
@@ -110,16 +88,7 @@ struct tmem_op {
uint32_t flags;
uint32_t arg1;
} creat; /* for cmd == TMEM_NEW_POOL, TMEM_AUTH, TMEM_RESTORE_NEW */
- struct {
- uint32_t subop;
- uint32_t cli_id;
- uint32_t arg1;
- uint32_t arg2;
- uint64_t oid[3];
- tmem_cli_va_t buf;
- } ctrl; /* for cmd == TMEM_CONTROL */
struct {
-
uint64_t oid[3];
uint32_t index;
uint32_t tmem_offset;
diff --git a/xen/include/xen/tmem.h b/xen/include/xen/tmem.h
index 5dbf9d5..32a542a 100644
--- a/xen/include/xen/tmem.h
+++ b/xen/include/xen/tmem.h
@@ -9,6 +9,9 @@
#ifndef __XEN_TMEM_H__
#define __XEN_TMEM_H__
+struct xen_sysctl_tmem_op;
+
+extern int tmem_control(struct xen_sysctl_tmem_op *op);
extern void tmem_destroy(void *);
extern void *tmem_relinquish_pages(unsigned int, unsigned int);
extern unsigned long tmem_freeable_pages(void);
diff --git a/xen/include/xen/tmem_xen.h b/xen/include/xen/tmem_xen.h
index c0d6831..0fdbf68 100644
--- a/xen/include/xen/tmem_xen.h
+++ b/xen/include/xen/tmem_xen.h
@@ -297,15 +297,11 @@ static inline int tmem_get_tmemop_from_client(tmem_op_t *op, tmem_cli_op_t uops)
switch ( cop.cmd )
{
case TMEM_NEW_POOL: u = XLAT_tmem_op_u_creat; break;
- case TMEM_CONTROL: u = XLAT_tmem_op_u_ctrl; break;
case TMEM_AUTH: u = XLAT_tmem_op_u_creat; break;
case TMEM_RESTORE_NEW:u = XLAT_tmem_op_u_creat; break;
default: u = XLAT_tmem_op_u_gen ; break;
}
-#define XLAT_tmem_op_HNDL_u_ctrl_buf(_d_, _s_) \
- guest_from_compat_handle((_d_)->u.ctrl.buf, (_s_)->u.ctrl.buf)
XLAT_tmem_op(op, &cop);
-#undef XLAT_tmem_op_HNDL_u_ctrl_buf
return 0;
}
#endif
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 7a4522e..666770a 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -801,6 +801,9 @@ static int flask_sysctl(int cmd)
return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
XEN2__PSR_CAT_OP, NULL);
+ case XEN_SYSCTL_tmem_op:
+ return domain_has_xen(current->domain, XEN__TMEM_CONTROL);
+
default:
printk("flask_sysctl: Unknown op %d\n", cmd);
return -EPERM;
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 05/11] tmem: Move TMEM_CONTROL subop of tmem hypercall to sysctl.
2015-08-28 18:53 ` [PATCH v3 05/11] tmem: Move TMEM_CONTROL subop of tmem hypercall to sysctl Konrad Rzeszutek Wilk
@ 2015-08-31 11:32 ` Jan Beulich
0 siblings, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2015-08-31 11:32 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: andrew.cooper3, wei.liu2, xen-devel
>>> On 28.08.15 at 20:53, <konrad.wilk@oracle.com> wrote:
> @@ -68,7 +69,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
> case XEN_SYSCTL_tbuf_op:
> ret = tb_control(&op->u.tbuf_op);
> break;
> -
> +
> + case XEN_SYSCTL_tmem_op:
> + ret = tmem_control(&op->u.tmem_op);
> + break;
> +
> case XEN_SYSCTL_sched_id:
Perhaps it was on a different part of the patch (or series), but ISTR
having pointed out strange placement within a switch() statement
on v2 already. This one logically also rather belongs at the end (the
case statements at least "try" to be sorted numerically).
> +struct xen_sysctl_tmem_op {
> + uint32_t cmd; /* IN: XEN_SYSCTL_TMEM_OP_* . */
> + int32_t pool_id; /* IN: 0 by default unless _SAVE_*, RESTORE_* .*/
> + uint32_t cli_id; /* IN: client id, 0 for XEN_SYSCTL_TMEM_QUERY_FREEABLE_MB
> + for all others can be the domain id or
> + XEN_SYSCTL_TMEM_OP_ALL_CLIENTS for all. */
> + uint32_t arg1; /* IN: If not applicable to command use 0. */
> + uint32_t arg2; /* IN: If not applicable to command use 0. */
> + uint32_t pad; /* Padding so structure is the same under 32 and 64. */
> + uint64_t oid[3]; /* IN: If not applicable to command use 0. */
Since this is an array, I'd generally prefer plural (0s) in the comment,
but I think a later patch is going to alter this again anyway.
With at least the earlier comment addressed, hypervisor side
Acked-by: Jen Beulich <jbeulich@suse.com>
Jan
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 06/11] tmem: Remove the old tmem control XSM checks as it is part of sysctl hypercall.
2015-08-28 18:53 [PATCH v3] Tmem bug-fixes and cleanups Konrad Rzeszutek Wilk
` (4 preceding siblings ...)
2015-08-28 18:53 ` [PATCH v3 05/11] tmem: Move TMEM_CONTROL subop of tmem hypercall to sysctl Konrad Rzeszutek Wilk
@ 2015-08-28 18:53 ` Konrad Rzeszutek Wilk
2015-08-28 18:53 ` [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid Konrad Rzeszutek Wilk
` (4 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-28 18:53 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2; +Cc: Konrad Rzeszutek Wilk
The sysctl is where the tmem control operations are done and the
XSM checks are done via there. The old mechanism (to check
for control tmem op XSM from do_tmem_op) is not needed anymore.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
xen/include/xsm/dummy.h | 6 ------
xen/include/xsm/xsm.h | 6 ------
xen/xsm/dummy.c | 1 -
xen/xsm/flask/hooks.c | 6 ------
xen/xsm/flask/policy/access_vectors | 2 +-
5 files changed, 1 insertion(+), 20 deletions(-)
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index bbbfce7..9fe372c 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -427,12 +427,6 @@ static XSM_INLINE int xsm_tmem_op(XSM_DEFAULT_VOID)
return xsm_default_action(action, current->domain, NULL);
}
-static XSM_INLINE int xsm_tmem_control(XSM_DEFAULT_VOID)
-{
- XSM_ASSERT_ACTION(XSM_PRIV);
- return xsm_default_action(action, current->domain, NULL);
-}
-
static XSM_INLINE long xsm_do_xsm_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
{
return -ENOSYS;
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 3678a93..ba3caed 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -137,7 +137,6 @@ struct xsm_operations {
int (*page_offline)(uint32_t cmd);
int (*tmem_op)(void);
- int (*tmem_control)(void);
long (*do_xsm_op) (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op);
#ifdef CONFIG_COMPAT
@@ -557,11 +556,6 @@ static inline int xsm_tmem_op(xsm_default_t def)
return xsm_ops->tmem_op();
}
-static inline int xsm_tmem_control(xsm_default_t def)
-{
- return xsm_ops->tmem_control();
-}
-
static inline long xsm_do_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
{
return xsm_ops->do_xsm_op(op);
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 21b1bf8..72eba40 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -112,7 +112,6 @@ void xsm_fixup_ops (struct xsm_operations *ops)
set_to_dummy_if_null(ops, page_offline);
set_to_dummy_if_null(ops, tmem_op);
- set_to_dummy_if_null(ops, tmem_control);
set_to_dummy_if_null(ops, hvm_param);
set_to_dummy_if_null(ops, hvm_control);
set_to_dummy_if_null(ops, hvm_param_nested);
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 666770a..fafb1a4 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1132,11 +1132,6 @@ static inline int flask_tmem_op(void)
return domain_has_xen(current->domain, XEN__TMEM_OP);
}
-static inline int flask_tmem_control(void)
-{
- return domain_has_xen(current->domain, XEN__TMEM_CONTROL);
-}
-
static int flask_add_to_physmap(struct domain *d1, struct domain *d2)
{
return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP);
@@ -1696,7 +1691,6 @@ static struct xsm_operations flask_ops = {
.page_offline = flask_page_offline,
.tmem_op = flask_tmem_op,
- .tmem_control = flask_tmem_control,
.hvm_param = flask_hvm_param,
.hvm_control = flask_hvm_param,
.hvm_param_nested = flask_hvm_param_nested,
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index 71495fd..0aa68f8 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -69,7 +69,7 @@ class xen
cpupool_op
# tmem hypercall (any access)
tmem_op
-# TMEM_CONTROL command of tmem hypercall
+# XEN_SYSCTL_tmem_op command of tmem (part of sysctl)
tmem_control
# XEN_SYSCTL_scheduler_op with XEN_DOMCTL_SCHEDOP_getinfo, XEN_SYSCTL_sched_id
getscheduler
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-08-28 18:53 [PATCH v3] Tmem bug-fixes and cleanups Konrad Rzeszutek Wilk
` (5 preceding siblings ...)
2015-08-28 18:53 ` [PATCH v3 06/11] tmem: Remove the old tmem control XSM checks as it is part of sysctl hypercall Konrad Rzeszutek Wilk
@ 2015-08-28 18:53 ` Konrad Rzeszutek Wilk
2015-08-31 11:38 ` Jan Beulich
2015-08-28 18:53 ` [PATCH v3 08/11] tmem/sysctl: Use 'struct tmem_oid' for every user Konrad Rzeszutek Wilk
` (3 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-28 18:53 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2; +Cc: Konrad Rzeszutek Wilk
And use it almost everywhere. It is easy to use it for the
sysctl since the hypervisor and toolstack are intertwined.
But for the tmem hypercall we need to be dilligient (as it
is guest facing) so delaying that to another patch:
"tmem/sysctl: Use 'struct tmem_oid' for every user" to help
with bisection issues.
We also move some of the parameters on functions to be within
the right location.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
tools/libxc/include/xenctrl.h | 4 ---
tools/libxc/xc_tmem.c | 10 +++-----
xen/common/tmem.c | 57 +++++++++++++++++++++++--------------------
xen/include/public/sysctl.h | 8 +++++-
4 files changed, 42 insertions(+), 37 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 9e34769..0ad6412 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2295,10 +2295,6 @@ int xc_disable_turbo(xc_interface *xch, int cpuid);
* tmem operations
*/
-struct tmem_oid {
- uint64_t oid[3];
-};
-
int xc_tmem_control_oid(xc_interface *xch, int32_t pool_id, uint32_t subop,
uint32_t cli_id, uint32_t arg1, uint32_t arg2,
struct tmem_oid oid, void *buf);
diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 505595b..425f51d 100644
--- a/tools/libxc/xc_tmem.c
+++ b/tools/libxc/xc_tmem.c
@@ -64,9 +64,9 @@ int xc_tmem_control(xc_interface *xch,
sysctl.u.tmem_op.arg1 = arg1;
sysctl.u.tmem_op.arg2 = arg2;
sysctl.u.tmem_op.pad = 0;
- sysctl.u.tmem_op.oid[0] = 0;
- sysctl.u.tmem_op.oid[1] = 0;
- sysctl.u.tmem_op.oid[2] = 0;
+ sysctl.u.tmem_op.oid.oid[0] = 0;
+ sysctl.u.tmem_op.oid.oid[1] = 0;
+ sysctl.u.tmem_op.oid.oid[2] = 0;
if ( cmd == XEN_SYSCTL_TMEM_OP_LIST && arg1 != 0 )
{
@@ -112,9 +112,7 @@ int xc_tmem_control_oid(xc_interface *xch,
sysctl.u.tmem_op.arg1 = arg1;
sysctl.u.tmem_op.arg2 = arg2;
sysctl.u.tmem_op.pad = 0;
- sysctl.u.tmem_op.oid[0] = oid.oid[0];
- sysctl.u.tmem_op.oid[1] = oid.oid[1];
- sysctl.u.tmem_op.oid[2] = oid.oid[2];
+ sysctl.u.tmem_op.oid = oid;
if ( cmd == XEN_SYSCTL_TMEM_OP_LIST && arg1 != 0 )
{
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index b62b56e..c9ff08c 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -125,12 +125,8 @@ struct tmem_pool {
#define is_persistent(_p) (_p->persistent)
#define is_shared(_p) (_p->shared)
-struct oid {
- uint64_t oid[3];
-};
-
struct tmem_object_root {
- struct oid oid;
+ struct tmem_oid oid;
struct rb_node rb_tree_node; /* protected by pool->pool_rwlock */
unsigned long objnode_count; /* atomicity depends on obj_spinlock */
long pgp_count; /* atomicity depends on obj_spinlock */
@@ -158,7 +154,7 @@ struct tmem_page_descriptor {
};
struct tmem_object_root *obj;
} us;
- struct oid inv_oid; /* used for invalid list only */
+ struct tmem_oid inv_oid; /* used for invalid list only */
};
pagesize_t size; /* 0 == PAGE_SIZE (pfp), -1 == data invalid,
else compressed data (cdata) */
@@ -815,7 +811,8 @@ static void rtn_free(struct radix_tree_node *rtn, void *arg)
/************ POOL OBJECT COLLECTION MANIPULATION ROUTINES *******************/
-static int oid_compare(struct oid *left, struct oid *right)
+static int oid_compare(struct tmem_oid *left,
+ struct tmem_oid *right)
{
if ( left->oid[2] == right->oid[2] )
{
@@ -839,19 +836,20 @@ static int oid_compare(struct oid *left, struct oid *right)
return 1;
}
-static void oid_set_invalid(struct oid *oidp)
+static void oid_set_invalid(struct tmem_oid *oidp)
{
oidp->oid[0] = oidp->oid[1] = oidp->oid[2] = -1UL;
}
-static unsigned oid_hash(struct oid *oidp)
+static unsigned oid_hash(struct tmem_oid *oidp)
{
return (tmem_hash(oidp->oid[0] ^ oidp->oid[1] ^ oidp->oid[2],
BITS_PER_LONG) & OBJ_HASH_BUCKETS_MASK);
}
/* searches for object==oid in pool, returns locked object if found */
-static struct tmem_object_root * obj_find(struct tmem_pool *pool, struct oid *oidp)
+static struct tmem_object_root * obj_find(struct tmem_pool *pool,
+ struct tmem_oid *oidp)
{
struct rb_node *node;
struct tmem_object_root *obj;
@@ -887,7 +885,7 @@ restart_find:
static void obj_free(struct tmem_object_root *obj)
{
struct tmem_pool *pool;
- struct oid old_oid;
+ struct tmem_oid old_oid;
ASSERT_SPINLOCK(&obj->obj_spinlock);
ASSERT(obj != NULL);
@@ -946,7 +944,8 @@ static int obj_rb_insert(struct rb_root *root, struct tmem_object_root *obj)
* allocate, initialize, and insert an tmem_object_root
* (should be called only if find failed)
*/
-static struct tmem_object_root * obj_alloc(struct tmem_pool *pool, struct oid *oidp)
+static struct tmem_object_root * obj_alloc(struct tmem_pool *pool,
+ struct tmem_oid *oidp)
{
struct tmem_object_root *obj;
@@ -1531,8 +1530,8 @@ cleanup:
}
static int do_tmem_put(struct tmem_pool *pool,
- struct oid *oidp, uint32_t index,
- xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
+ struct tmem_oid *oidp, uint32_t index,
+ xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
{
struct tmem_object_root *obj = NULL;
struct tmem_page_descriptor *pgp = NULL;
@@ -1696,8 +1695,9 @@ unlock_obj:
return ret;
}
-static int do_tmem_get(struct tmem_pool *pool, struct oid *oidp, uint32_t index,
- xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
+static int do_tmem_get(struct tmem_pool *pool,
+ struct tmem_oid *oidp, uint32_t index,
+ xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
{
struct tmem_object_root *obj;
struct tmem_page_descriptor *pgp;
@@ -1774,7 +1774,8 @@ bad_copy:
return rc;
}
-static int do_tmem_flush_page(struct tmem_pool *pool, struct oid *oidp, uint32_t index)
+static int do_tmem_flush_page(struct tmem_pool *pool,
+ struct tmem_oid *oidp, uint32_t index)
{
struct tmem_object_root *obj;
struct tmem_page_descriptor *pgp;
@@ -1807,7 +1808,8 @@ out:
return 1;
}
-static int do_tmem_flush_object(struct tmem_pool *pool, struct oid *oidp)
+static int do_tmem_flush_object(struct tmem_pool *pool,
+ struct tmem_oid *oidp)
{
struct tmem_object_root *obj;
@@ -2432,7 +2434,7 @@ static int tmemc_save_get_next_page(int cli_id, uint32_t pool_id,
struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
? NULL : client->pools[pool_id];
struct tmem_page_descriptor *pgp;
- struct oid oid;
+ struct tmem_oid oid;
int ret = 0;
struct tmem_handle h;
@@ -2525,8 +2527,10 @@ out:
return ret;
}
-static int tmemc_restore_put_page(int cli_id, uint32_t pool_id, struct oid *oidp,
- uint32_t index, tmem_cli_va_param_t buf, uint32_t bufsize)
+static int tmemc_restore_put_page(int cli_id, uint32_t pool_id,
+ struct tmem_oid *oidp,
+ uint32_t index, tmem_cli_va_param_t buf,
+ uint32_t bufsize)
{
struct client *client = tmem_client_from_cli_id(cli_id);
struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
@@ -2542,8 +2546,9 @@ static int tmemc_restore_put_page(int cli_id, uint32_t pool_id, struct oid *oidp
return do_tmem_put(pool, oidp, index, 0, buf);
}
-static int tmemc_restore_flush_page(int cli_id, uint32_t pool_id, struct oid *oidp,
- uint32_t index)
+static int tmemc_restore_flush_page(int cli_id, uint32_t pool_id,
+ struct tmem_oid *oidp,
+ uint32_t index)
{
struct client *client = tmem_client_from_cli_id(cli_id);
struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
@@ -2559,7 +2564,7 @@ int tmem_control(struct xen_sysctl_tmem_op *op)
int ret;
uint32_t pool_id = op->pool_id;
uint32_t cmd = op->cmd;
- struct oid *oidp = (struct oid *)(&op->oid[0]);
+ struct tmem_oid *oidp = &op->oid;
if ( op->pad != 0 )
return -EINVAL;
@@ -2633,7 +2638,7 @@ long do_tmem_op(tmem_cli_op_t uops)
struct tmem_op op;
struct client *client = current->domain->tmem_client;
struct tmem_pool *pool = NULL;
- struct oid *oidp;
+ struct tmem_oid *oidp;
int rc = 0;
bool_t succ_get = 0, succ_put = 0;
bool_t non_succ_get = 0, non_succ_put = 0;
@@ -2714,7 +2719,7 @@ long do_tmem_op(tmem_cli_op_t uops)
write_unlock(&tmem_rwlock);
read_lock(&tmem_rwlock);
- oidp = (struct oid *)&op.u.gen.oid[0];
+ oidp = (struct tmem_oid *)&op.u.gen.oid[0];
switch ( op.cmd )
{
case TMEM_NEW_POOL:
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index c1566e6..9a58bed 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -737,6 +737,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
#define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
#define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
+struct tmem_oid {
+ uint64_t oid[3];
+};
+typedef struct tmem_oid tmem_oid_t;
+DEFINE_XEN_GUEST_HANDLE(tmem_oid_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_* .*/
@@ -746,7 +752,7 @@ struct xen_sysctl_tmem_op {
uint32_t arg1; /* IN: If not applicable to command use 0. */
uint32_t arg2; /* IN: If not applicable to command use 0. */
uint32_t pad; /* Padding so structure is the same under 32 and 64. */
- uint64_t oid[3]; /* IN: If not applicable to command use 0. */
+ tmem_oid_t oid; /* IN: If not applicable to command use 0. */
XEN_GUEST_HANDLE_64(char) buf; /* IN/OUT: Buffer to save and restore ops. */
};
typedef struct xen_sysctl_tmem_op xen_sysctl_tmem_op_t;
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-08-28 18:53 ` [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid Konrad Rzeszutek Wilk
@ 2015-08-31 11:38 ` Jan Beulich
2015-08-31 15:37 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 31+ messages in thread
From: Jan Beulich @ 2015-08-31 11:38 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: andrew.cooper3, wei.liu2, xen-devel
>>> On 28.08.15 at 20:53, <konrad.wilk@oracle.com> wrote:
> @@ -2714,7 +2719,7 @@ long do_tmem_op(tmem_cli_op_t uops)
> write_unlock(&tmem_rwlock);
> read_lock(&tmem_rwlock);
>
> - oidp = (struct oid *)&op.u.gen.oid[0];
> + oidp = (struct tmem_oid *)&op.u.gen.oid[0];
AIUI this is going to go away later anyway, but generally I think it
would be better to hide explicit casts like this by using container_of()
when possible.
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -737,6 +737,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
> #define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
> #define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
>
> +struct tmem_oid {
> + uint64_t oid[3];
> +};
> +typedef struct tmem_oid tmem_oid_t;
> +DEFINE_XEN_GUEST_HANDLE(tmem_oid_t);
I know this is going to be a boring mechanical thing, but I'd really
like to see this to be xen_tmem_oid (and alike), especially since
you intend to also use the type for th non-tools part of the
interface.
Jan
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-08-31 11:38 ` Jan Beulich
@ 2015-08-31 15:37 ` Konrad Rzeszutek Wilk
2015-08-31 15:56 ` Jan Beulich
0 siblings, 1 reply; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-31 15:37 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel
On Mon, Aug 31, 2015 at 05:38:45AM -0600, Jan Beulich wrote:
> >>> On 28.08.15 at 20:53, <konrad.wilk@oracle.com> wrote:
> > @@ -2714,7 +2719,7 @@ long do_tmem_op(tmem_cli_op_t uops)
> > write_unlock(&tmem_rwlock);
> > read_lock(&tmem_rwlock);
> >
> > - oidp = (struct oid *)&op.u.gen.oid[0];
> > + oidp = (struct tmem_oid *)&op.u.gen.oid[0];
>
> AIUI this is going to go away later anyway, but generally I think it
> would be better to hide explicit casts like this by using container_of()
> when possible.
I am not sure if it will be possible as the structure that contains
the tmem_oid is an anonymous structure within an union:
struct tmem_op {
blah
union {
struct {
tmem_oid_t oid;
} gen;
} u;
}
And the 'container_of' macro looks to require only one level of
nesting.
>
> > --- a/xen/include/public/sysctl.h
> > +++ b/xen/include/public/sysctl.h
> > @@ -737,6 +737,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
> > #define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
> > #define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
> >
> > +struct tmem_oid {
> > + uint64_t oid[3];
> > +};
> > +typedef struct tmem_oid tmem_oid_t;
> > +DEFINE_XEN_GUEST_HANDLE(tmem_oid_t);
>
> I know this is going to be a boring mechanical thing, but I'd really
> like to see this to be xen_tmem_oid (and alike), especially since
> you intend to also use the type for th non-tools part of the
> interface.
This throws a wrench in the compat autogeneration tool.
I keep on getting:
Fields of 'compat_xen_tmem_oid' not found in 'compat/tmem.h'
and it failing to generate compat/.xlat/tmem.h file. Sticking an prefix of
xen to the 'common/compat/tmem_xen.c' or just leaving it as is did not help:
#include <xen/hypercall.h>
#include <compat/tmem.h>
-#define xen_tmem_op tmem_op
-/*CHECK_tmem_op;*/
-#undef xen_tmem_op
+#define xen_xen_tmem_oid xen_tmem_oid
+CHECK_xen_tmem_oid;
+#undef xen_xen_tmem_oid
Thoughts?
>
> Jan
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-08-31 15:37 ` Konrad Rzeszutek Wilk
@ 2015-08-31 15:56 ` Jan Beulich
2015-08-31 16:14 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 31+ messages in thread
From: Jan Beulich @ 2015-08-31 15:56 UTC (permalink / raw)
To: konrad.wilk; +Cc: andrew.cooper3, wei.liu2, xen-devel
>>> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> 08/31/15 5:38 PM >>>
>On Mon, Aug 31, 2015 at 05:38:45AM -0600, Jan Beulich wrote:
>> >>> On 28.08.15 at 20:53, <konrad.wilk@oracle.com> wrote:
>> > @@ -2714,7 +2719,7 @@ long do_tmem_op(tmem_cli_op_t uops)
>> > write_unlock(&tmem_rwlock);
>> > read_lock(&tmem_rwlock);
>> >
>> > - oidp = (struct oid *)&op.u.gen.oid[0];
>> > + oidp = (struct tmem_oid *)&op.u.gen.oid[0];
>>
>> AIUI this is going to go away later anyway, but generally I think it
>> would be better to hide explicit casts like this by using container_of()
>> when possible.
>
>I am not sure if it will be possible as the structure that contains
>the tmem_oid is an anonymous structure within an union:
>
>struct tmem_op {
>blah
>union {
>struct {
>tmem_oid_t oid;
>} gen;
>} u;
>}
>
>And the 'container_of' macro looks to require only one level of
>nesting.
I'm pretty sure the macro can deal with both.
>> > --- a/xen/include/public/sysctl.h
>> > +++ b/xen/include/public/sysctl.h
>> > @@ -737,6 +737,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
>> > #define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
>> > #define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
>> >
>> > +struct tmem_oid {
>> > + uint64_t oid[3];
>> > +};
>> > +typedef struct tmem_oid tmem_oid_t;
>> > +DEFINE_XEN_GUEST_HANDLE(tmem_oid_t);
>>
>> I know this is going to be a boring mechanical thing, but I'd really
>> like to see this to be xen_tmem_oid (and alike), especially since
>> you intend to also use the type for th non-tools part of the
>> interface.
>
>This throws a wrench in the compat autogeneration tool.
>
>I keep on getting:
>
>Fields of 'compat_xen_tmem_oid' not found in 'compat/tmem.h'
>and it failing to generate compat/.xlat/tmem.h file. Sticking an prefix of
>xen to the 'common/compat/tmem_xen.c' or just leaving it as is did not help:
Did you perhaps forget to adjust include/xlat.lst? I'm certain adding a prefix won't
break everything, albeit iirc there's not going to be a compat_xen_tmem_oid, but
the xen_ prefix in such cases gets replaced by compat_.
Jan
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-08-31 15:56 ` Jan Beulich
@ 2015-08-31 16:14 ` Konrad Rzeszutek Wilk
2015-09-01 7:04 ` Jan Beulich
2015-09-01 15:18 ` Konrad Rzeszutek Wilk
0 siblings, 2 replies; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-31 16:14 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel
> >And the 'container_of' macro looks to require only one level of
> >nesting.
>
> I'm pretty sure the macro can deal with both.
OK, let me experiement with it as at the first blush it does not work for me.
>
> >> > --- a/xen/include/public/sysctl.h
> >> > +++ b/xen/include/public/sysctl.h
> >> > @@ -737,6 +737,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
> >> > #define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
> >> > #define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
> >> >
> >> > +struct tmem_oid {
> >> > + uint64_t oid[3];
> >> > +};
> >> > +typedef struct tmem_oid tmem_oid_t;
> >> > +DEFINE_XEN_GUEST_HANDLE(tmem_oid_t);
> >>
> >> I know this is going to be a boring mechanical thing, but I'd really
> >> like to see this to be xen_tmem_oid (and alike), especially since
> >> you intend to also use the type for th non-tools part of the
> >> interface.
> >
> >This throws a wrench in the compat autogeneration tool.
> >
> >I keep on getting:
> >
> >Fields of 'compat_xen_tmem_oid' not found in 'compat/tmem.h'
> >and it failing to generate compat/.xlat/tmem.h file. Sticking an prefix of
> >xen to the 'common/compat/tmem_xen.c' or just leaving it as is did not help:
>
> Did you perhaps forget to adjust include/xlat.lst? I'm certain adding a prefix won't
Yes. And ran 'make distclean' before compiling.
> break everything, albeit iirc there's not going to be a compat_xen_tmem_oid, but
> the xen_ prefix in such cases gets replaced by compat_.
Which is part of the problem - in 'xen_tmem_oid' the 'xen' parts got replaced.
And the 'compat_tmem_oid' gets generated but the tool is trying to find
'compat_xen_tmem_oid' (at least that is the error) and can't find it now.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-08-31 16:14 ` Konrad Rzeszutek Wilk
@ 2015-09-01 7:04 ` Jan Beulich
2015-09-01 15:23 ` Konrad Rzeszutek Wilk
2015-09-01 15:18 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 31+ messages in thread
From: Jan Beulich @ 2015-09-01 7:04 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: andrew.cooper3, wei.liu2, xen-devel
>>> On 31.08.15 at 18:14, <konrad.wilk@oracle.com> wrote:
>> >> > --- a/xen/include/public/sysctl.h
>> >> > +++ b/xen/include/public/sysctl.h
>> >> > @@ -737,6 +737,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
>> >> > #define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
>> >> > #define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
>> >> >
>> >> > +struct tmem_oid {
>> >> > + uint64_t oid[3];
>> >> > +};
>> >> > +typedef struct tmem_oid tmem_oid_t;
>> >> > +DEFINE_XEN_GUEST_HANDLE(tmem_oid_t);
>> >>
>> >> I know this is going to be a boring mechanical thing, but I'd really
>> >> like to see this to be xen_tmem_oid (and alike), especially since
>> >> you intend to also use the type for th non-tools part of the
>> >> interface.
>> >
>> >This throws a wrench in the compat autogeneration tool.
>> >
>> >I keep on getting:
>> >
>> >Fields of 'compat_xen_tmem_oid' not found in 'compat/tmem.h'
>> >and it failing to generate compat/.xlat/tmem.h file. Sticking an prefix of
>> >xen to the 'common/compat/tmem_xen.c' or just leaving it as is did not help:
>>
>> Did you perhaps forget to adjust include/xlat.lst? I'm certain adding a
> prefix won't
>
> Yes. And ran 'make distclean' before compiling.
>
>> break everything, albeit iirc there's not going to be a compat_xen_tmem_oid,
> but
>> the xen_ prefix in such cases gets replaced by compat_.
>
> Which is part of the problem - in 'xen_tmem_oid' the 'xen' parts got
> replaced.
> And the 'compat_tmem_oid' gets generated but the tool is trying to find
> 'compat_xen_tmem_oid' (at least that is the error) and can't find it now.
Perhaps that's because the structure definition is public/sysctl.h
(note how the error message refers to public/tmem.h)? Since you
move it in a later patch, could you put it into public/tmem.h right
away?
Jan
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-09-01 7:04 ` Jan Beulich
@ 2015-09-01 15:23 ` Konrad Rzeszutek Wilk
2015-09-01 15:54 ` Jan Beulich
0 siblings, 1 reply; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-09-01 15:23 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel
[-- Attachment #1: Type: text/plain, Size: 3999 bytes --]
On Tue, Sep 01, 2015 at 01:04:11AM -0600, Jan Beulich wrote:
> >>> On 31.08.15 at 18:14, <konrad.wilk@oracle.com> wrote:
> >> >> > --- a/xen/include/public/sysctl.h
> >> >> > +++ b/xen/include/public/sysctl.h
> >> >> > @@ -737,6 +737,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
> >> >> > #define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
> >> >> > #define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
> >> >> >
> >> >> > +struct tmem_oid {
> >> >> > + uint64_t oid[3];
> >> >> > +};
> >> >> > +typedef struct tmem_oid tmem_oid_t;
> >> >> > +DEFINE_XEN_GUEST_HANDLE(tmem_oid_t);
> >> >>
> >> >> I know this is going to be a boring mechanical thing, but I'd really
> >> >> like to see this to be xen_tmem_oid (and alike), especially since
> >> >> you intend to also use the type for th non-tools part of the
> >> >> interface.
> >> >
> >> >This throws a wrench in the compat autogeneration tool.
> >> >
> >> >I keep on getting:
> >> >
> >> >Fields of 'compat_xen_tmem_oid' not found in 'compat/tmem.h'
> >> >and it failing to generate compat/.xlat/tmem.h file. Sticking an prefix of
> >> >xen to the 'common/compat/tmem_xen.c' or just leaving it as is did not help:
> >>
> >> Did you perhaps forget to adjust include/xlat.lst? I'm certain adding a
> > prefix won't
> >
> > Yes. And ran 'make distclean' before compiling.
> >
> >> break everything, albeit iirc there's not going to be a compat_xen_tmem_oid,
> > but
> >> the xen_ prefix in such cases gets replaced by compat_.
> >
> > Which is part of the problem - in 'xen_tmem_oid' the 'xen' parts got
> > replaced.
> > And the 'compat_tmem_oid' gets generated but the tool is trying to find
> > 'compat_xen_tmem_oid' (at least that is the error) and can't find it now.
>
> Perhaps that's because the structure definition is public/sysctl.h
> (note how the error message refers to public/tmem.h)? Since you
> move it in a later patch, could you put it into public/tmem.h right
> away?
There is no need for the compat layer with the patch
("tmem: Make the uint64_t oid[3] a proper structure: ") which
alters 'xen_sysctl_tmem_op':
+struct xen_tmem_oid {
+ uint64_t oid[3];
+};
+typedef struct xen_tmem_oid xen_tmem_oid_t;
+DEFINE_XEN_GUEST_HANDLE(xen_tmem_oid_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_* .*/
@@ -746,7 +752,7 @@ struct xen_sysctl_tmem_op {
uint32_t arg1; /* IN: If not applicable to command use 0. */
uint32_t arg2; /* IN: If not applicable to command use 0. */
uint32_t pad; /* Padding so structure is the same under 32 and 64. */
- uint64_t oid[3]; /* IN: If not applicable to command use 0s. */
+ xen_tmem_oid_t oid; /* IN: If not applicable to command use 0s. */
XEN_GUEST_HANDLE_64(char) buf; /* IN/OUT: Buffer to save and restore ops. */
};
typedef struct xen_sysctl_tmem_op xen_sysctl_tmem_op_t;
There is only need for the compat layer once I move the 'xen_tmem_oid_t' to
the tmem.h header file - and there is where the introduction in xlat.lst is done:
index 8cedee7..ace1d53 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -103,6 +103,7 @@
! sched_poll sched.h
? sched_remote_shutdown sched.h
? sched_shutdown sched.h
+? xen_tmem_oid tmem.h
! tmem_op tmem.h
? t_buf trace.h
? vcpu_get_physid vcpu.h
.. and which fails compilation.
I think I am confusing the matters by responding on the thread that
refers to the introduction of 'struct xen_tmem_oid' in sysctl (which works
fine btw), but I am tripping over problems with the next patch
('tmem: Use 'struct xen_tmem_oid' for every user')
which moves the 'struct xen_tmem_oid' in the tmem.h to be used by
both hypercalls (tmem.h and sysctl.h).
Attaching both patches.
>
> Jan
>
[-- Attachment #2: 0001-tmem-Make-the-uint64_t-oid-3-a-proper-structure-xen_.patch --]
[-- Type: text/plain, Size: 11891 bytes --]
>From 6a94354fbacf92d9c4ea3aeffcf0d36ba49a1e17 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 31 Aug 2015 10:57:27 -0400
Subject: [PATCH 1/2] tmem: Make the uint64_t oid[3] a proper structure:
xen_tmem_oid
And use it almost everywhere. It is easy to use it for the
sysctl since the hypervisor and toolstack are intertwined.
But for the tmem hypercall we need to be dilligient (as it
is guest facing) so delaying that to another patch:
"tmem: Use 'struct xen_tmem_oid' for every user" to help
with bisection issues.
We also move some of the parameters on functions to be within
the right location.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
tools/libxc/include/xenctrl.h | 6 +----
tools/libxc/xc_tmem.c | 16 ++++++------
xen/common/tmem.c | 57 +++++++++++++++++++++++--------------------
xen/include/public/sysctl.h | 8 +++++-
4 files changed, 46 insertions(+), 41 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 9e34769..2000f12 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2295,13 +2295,9 @@ int xc_disable_turbo(xc_interface *xch, int cpuid);
* tmem operations
*/
-struct tmem_oid {
- uint64_t oid[3];
-};
-
int xc_tmem_control_oid(xc_interface *xch, int32_t pool_id, uint32_t subop,
uint32_t cli_id, uint32_t arg1, uint32_t arg2,
- struct tmem_oid oid, void *buf);
+ struct xen_tmem_oid oid, void *buf);
int xc_tmem_control(xc_interface *xch,
int32_t pool_id, uint32_t subop, uint32_t cli_id,
uint32_t arg1, uint32_t arg2, void *buf);
diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 505595b..558d2ea 100644
--- a/tools/libxc/xc_tmem.c
+++ b/tools/libxc/xc_tmem.c
@@ -64,9 +64,9 @@ int xc_tmem_control(xc_interface *xch,
sysctl.u.tmem_op.arg1 = arg1;
sysctl.u.tmem_op.arg2 = arg2;
sysctl.u.tmem_op.pad = 0;
- sysctl.u.tmem_op.oid[0] = 0;
- sysctl.u.tmem_op.oid[1] = 0;
- sysctl.u.tmem_op.oid[2] = 0;
+ sysctl.u.tmem_op.oid.oid[0] = 0;
+ sysctl.u.tmem_op.oid.oid[1] = 0;
+ sysctl.u.tmem_op.oid.oid[2] = 0;
if ( cmd == XEN_SYSCTL_TMEM_OP_LIST && arg1 != 0 )
{
@@ -98,7 +98,7 @@ int xc_tmem_control_oid(xc_interface *xch,
uint32_t cli_id,
uint32_t arg1,
uint32_t arg2,
- struct tmem_oid oid,
+ struct xen_tmem_oid oid,
void *buf)
{
DECLARE_SYSCTL;
@@ -112,9 +112,7 @@ int xc_tmem_control_oid(xc_interface *xch,
sysctl.u.tmem_op.arg1 = arg1;
sysctl.u.tmem_op.arg2 = arg2;
sysctl.u.tmem_op.pad = 0;
- sysctl.u.tmem_op.oid[0] = oid.oid[0];
- sysctl.u.tmem_op.oid[1] = oid.oid[1];
- sysctl.u.tmem_op.oid[2] = oid.oid[2];
+ sysctl.u.tmem_op.oid = oid;
if ( cmd == XEN_SYSCTL_TMEM_OP_LIST && arg1 != 0 )
{
@@ -453,7 +451,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
}
for ( j = n_pages; j > 0; j-- )
{
- struct tmem_oid oid;
+ struct xen_tmem_oid oid;
uint32_t index;
int rc;
if ( read_exact(io_fd, &oid, sizeof(oid)) )
@@ -487,7 +485,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
int xc_tmem_restore_extra(xc_interface *xch, int dom, int io_fd)
{
uint32_t pool_id;
- struct tmem_oid oid;
+ struct xen_tmem_oid oid;
uint32_t index;
int count = 0;
int checksum = 0;
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index c73add5..eea3979 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -125,12 +125,8 @@ struct tmem_pool {
#define is_persistent(_p) (_p->persistent)
#define is_shared(_p) (_p->shared)
-struct oid {
- uint64_t oid[3];
-};
-
struct tmem_object_root {
- struct oid oid;
+ struct xen_tmem_oid oid;
struct rb_node rb_tree_node; /* protected by pool->pool_rwlock */
unsigned long objnode_count; /* atomicity depends on obj_spinlock */
long pgp_count; /* atomicity depends on obj_spinlock */
@@ -158,7 +154,7 @@ struct tmem_page_descriptor {
};
struct tmem_object_root *obj;
} us;
- struct oid inv_oid; /* used for invalid list only */
+ struct xen_tmem_oid inv_oid; /* used for invalid list only */
};
pagesize_t size; /* 0 == PAGE_SIZE (pfp), -1 == data invalid,
else compressed data (cdata) */
@@ -815,7 +811,8 @@ static void rtn_free(struct radix_tree_node *rtn, void *arg)
/************ POOL OBJECT COLLECTION MANIPULATION ROUTINES *******************/
-static int oid_compare(struct oid *left, struct oid *right)
+static int oid_compare(struct xen_tmem_oid *left,
+ struct xen_tmem_oid *right)
{
if ( left->oid[2] == right->oid[2] )
{
@@ -839,19 +836,20 @@ static int oid_compare(struct oid *left, struct oid *right)
return 1;
}
-static void oid_set_invalid(struct oid *oidp)
+static void oid_set_invalid(struct xen_tmem_oid *oidp)
{
oidp->oid[0] = oidp->oid[1] = oidp->oid[2] = -1UL;
}
-static unsigned oid_hash(struct oid *oidp)
+static unsigned oid_hash(struct xen_tmem_oid *oidp)
{
return (tmem_hash(oidp->oid[0] ^ oidp->oid[1] ^ oidp->oid[2],
BITS_PER_LONG) & OBJ_HASH_BUCKETS_MASK);
}
/* searches for object==oid in pool, returns locked object if found */
-static struct tmem_object_root * obj_find(struct tmem_pool *pool, struct oid *oidp)
+static struct tmem_object_root * obj_find(struct tmem_pool *pool,
+ struct xen_tmem_oid *oidp)
{
struct rb_node *node;
struct tmem_object_root *obj;
@@ -887,7 +885,7 @@ restart_find:
static void obj_free(struct tmem_object_root *obj)
{
struct tmem_pool *pool;
- struct oid old_oid;
+ struct xen_tmem_oid old_oid;
ASSERT_SPINLOCK(&obj->obj_spinlock);
ASSERT(obj != NULL);
@@ -946,7 +944,8 @@ static int obj_rb_insert(struct rb_root *root, struct tmem_object_root *obj)
* allocate, initialize, and insert an tmem_object_root
* (should be called only if find failed)
*/
-static struct tmem_object_root * obj_alloc(struct tmem_pool *pool, struct oid *oidp)
+static struct tmem_object_root * obj_alloc(struct tmem_pool *pool,
+ struct xen_tmem_oid *oidp)
{
struct tmem_object_root *obj;
@@ -1531,8 +1530,8 @@ cleanup:
}
static int do_tmem_put(struct tmem_pool *pool,
- struct oid *oidp, uint32_t index,
- xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
+ struct xen_tmem_oid *oidp, uint32_t index,
+ xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
{
struct tmem_object_root *obj = NULL;
struct tmem_page_descriptor *pgp = NULL;
@@ -1696,8 +1695,9 @@ unlock_obj:
return ret;
}
-static int do_tmem_get(struct tmem_pool *pool, struct oid *oidp, uint32_t index,
- xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
+static int do_tmem_get(struct tmem_pool *pool,
+ struct xen_tmem_oid *oidp, uint32_t index,
+ xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
{
struct tmem_object_root *obj;
struct tmem_page_descriptor *pgp;
@@ -1774,7 +1774,8 @@ bad_copy:
return rc;
}
-static int do_tmem_flush_page(struct tmem_pool *pool, struct oid *oidp, uint32_t index)
+static int do_tmem_flush_page(struct tmem_pool *pool,
+ struct xen_tmem_oid *oidp, uint32_t index)
{
struct tmem_object_root *obj;
struct tmem_page_descriptor *pgp;
@@ -1807,7 +1808,8 @@ out:
return 1;
}
-static int do_tmem_flush_object(struct tmem_pool *pool, struct oid *oidp)
+static int do_tmem_flush_object(struct tmem_pool *pool,
+ struct xen_tmem_oid *oidp)
{
struct tmem_object_root *obj;
@@ -2432,7 +2434,7 @@ static int tmemc_save_get_next_page(int cli_id, uint32_t pool_id,
struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
? NULL : client->pools[pool_id];
struct tmem_page_descriptor *pgp;
- struct oid oid;
+ struct xen_tmem_oid oid;
int ret = 0;
struct tmem_handle h;
@@ -2525,8 +2527,10 @@ out:
return ret;
}
-static int tmemc_restore_put_page(int cli_id, uint32_t pool_id, struct oid *oidp,
- uint32_t index, tmem_cli_va_param_t buf, uint32_t bufsize)
+static int tmemc_restore_put_page(int cli_id, uint32_t pool_id,
+ struct xen_tmem_oid *oidp,
+ uint32_t index, tmem_cli_va_param_t buf,
+ uint32_t bufsize)
{
struct client *client = tmem_client_from_cli_id(cli_id);
struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
@@ -2542,8 +2546,9 @@ static int tmemc_restore_put_page(int cli_id, uint32_t pool_id, struct oid *oidp
return do_tmem_put(pool, oidp, index, 0, buf);
}
-static int tmemc_restore_flush_page(int cli_id, uint32_t pool_id, struct oid *oidp,
- uint32_t index)
+static int tmemc_restore_flush_page(int cli_id, uint32_t pool_id,
+ struct xen_tmem_oid *oidp,
+ uint32_t index)
{
struct client *client = tmem_client_from_cli_id(cli_id);
struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
@@ -2559,7 +2564,7 @@ int tmem_control(struct xen_sysctl_tmem_op *op)
int ret;
uint32_t pool_id = op->pool_id;
uint32_t cmd = op->cmd;
- struct oid *oidp = (struct oid *)(&op->oid[0]);
+ struct xen_tmem_oid *oidp = &op->oid;
if ( op->pad != 0 )
return -EINVAL;
@@ -2633,7 +2638,7 @@ long do_tmem_op(tmem_cli_op_t uops)
struct tmem_op op;
struct client *client = current->domain->tmem_client;
struct tmem_pool *pool = NULL;
- struct oid *oidp;
+ struct xen_tmem_oid *oidp;
int rc = 0;
bool_t succ_get = 0, succ_put = 0;
bool_t non_succ_get = 0, non_succ_put = 0;
@@ -2714,7 +2719,7 @@ long do_tmem_op(tmem_cli_op_t uops)
write_unlock(&tmem_rwlock);
read_lock(&tmem_rwlock);
- oidp = (struct oid *)&op.u.gen.oid[0];
+ oidp = (struct xen_tmem_oid *)&op.u.gen.oid[0];
switch ( op.cmd )
{
case TMEM_NEW_POOL:
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index fc4709a..2d7580b 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -737,6 +737,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
#define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
#define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
+struct xen_tmem_oid {
+ uint64_t oid[3];
+};
+typedef struct xen_tmem_oid xen_tmem_oid_t;
+DEFINE_XEN_GUEST_HANDLE(xen_tmem_oid_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_* .*/
@@ -746,7 +752,7 @@ struct xen_sysctl_tmem_op {
uint32_t arg1; /* IN: If not applicable to command use 0. */
uint32_t arg2; /* IN: If not applicable to command use 0. */
uint32_t pad; /* Padding so structure is the same under 32 and 64. */
- uint64_t oid[3]; /* IN: If not applicable to command use 0s. */
+ xen_tmem_oid_t oid; /* IN: If not applicable to command use 0s. */
XEN_GUEST_HANDLE_64(char) buf; /* IN/OUT: Buffer to save and restore ops. */
};
typedef struct xen_sysctl_tmem_op xen_sysctl_tmem_op_t;
--
2.1.0
[-- Attachment #3: 0002-tmem-Use-struct-xen_tmem_oid-for-every-user.patch --]
[-- Type: text/plain, Size: 4328 bytes --]
>From 54601d2a8e3856621eb107694e7986fb2abcafad Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 31 Aug 2015 11:00:29 -0400
Subject: [PATCH 2/2] tmem: Use 'struct xen_tmem_oid' for every user.
Patch "tmem: Make the uint64_t oid[3] a proper structure:
xen_tmem_oid" converted the sysctl API to use an
proper structure. But it did not do it for the tmem hypercall.
This expands that and converts the tmem hypercall. For this
to work we define the struct in tmem.h and include it in
sysctl.h.
This change also included work to make the compat layer
happy. That was to declare the struct xen_tmem_oid to be
checked in xlat.lst - which will construct an typedef
in the compat file with the same type, hence allowing
copying of 'oid' member without type issues.
The layout (and size) of this structure in memory for the
'struct tmem_op' (so guest facing) is the same! Verified
via pahole and with 32/64 bit guests.
--- /tmp/old 2015-08-27 16:34:00.535638730 -0400
+++ /tmp/new 2015-08-27 16:34:10.447705328 -0400
@@ -8,7 +8,7 @@
uint32_t arg1; /* 28 4 */
} creat; /* 24 */
struct {
- uint64_t oid[3]; /* 8 24 */
+ xen_tmem_oid_t oid; /* 8 24 */
uint32_t index; /* 32 4 */
uint32_t tmem_offset; /* 36 4 */
uint32_t pfn_offset; /* 40 4 */
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
xen/common/compat/tmem_xen.c | 6 +++---
xen/include/public/sysctl.h | 7 +------
xen/include/public/tmem.h | 9 +++++++++
xen/include/xlat.lst | 1 +
4 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/xen/common/compat/tmem_xen.c b/xen/common/compat/tmem_xen.c
index 97c7ff2..6bd1ad4 100644
--- a/xen/common/compat/tmem_xen.c
+++ b/xen/common/compat/tmem_xen.c
@@ -11,9 +11,9 @@
#include <xen/hypercall.h>
#include <compat/tmem.h>
-#define xen_tmem_op tmem_op
-/*CHECK_tmem_op;*/
-#undef xen_tmem_op
+#define xen_xen_tmem_oid xen_tmem_oid
+CHECK_xen_tmem_oid;
+#undef xen_xen_tmem_oid
/*
* Local variables:
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 2d7580b..3bdf0e1 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -34,6 +34,7 @@
#include "xen.h"
#include "domctl.h"
#include "physdev.h"
+#include "tmem.h"
#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000C
@@ -737,12 +738,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
#define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
#define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
-struct xen_tmem_oid {
- uint64_t oid[3];
-};
-typedef struct xen_tmem_oid xen_tmem_oid_t;
-DEFINE_XEN_GUEST_HANDLE(xen_tmem_oid_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_* .*/
diff --git a/xen/include/public/tmem.h b/xen/include/public/tmem.h
index e4ee704..913566a 100644
--- a/xen/include/public/tmem.h
+++ b/xen/include/public/tmem.h
@@ -73,6 +73,11 @@
#define EFROZEN 1000
#define EEMPTY 1001
+struct xen_tmem_oid {
+ uint64_t oid[3];
+};
+typedef struct xen_tmem_oid xen_tmem_oid_t;
+DEFINE_XEN_GUEST_HANDLE(xen_tmem_oid_t);
#ifndef __ASSEMBLY__
#if __XEN_INTERFACE_VERSION__ < 0x00040400
@@ -89,7 +94,11 @@ struct tmem_op {
uint32_t arg1;
} creat; /* for cmd == TMEM_NEW_POOL, TMEM_AUTH, TMEM_RESTORE_NEW */
struct {
+#if __XEN_INTERFACE_VERSION__ < 0x00040600
uint64_t oid[3];
+#else
+ xen_tmem_oid_t oid;
+#endif
uint32_t index;
uint32_t tmem_offset;
uint32_t pfn_offset;
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 8cedee7..ace1d53 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -103,6 +103,7 @@
! sched_poll sched.h
? sched_remote_shutdown sched.h
? sched_shutdown sched.h
+? xen_tmem_oid tmem.h
! tmem_op tmem.h
? t_buf trace.h
? vcpu_get_physid vcpu.h
--
2.1.0
[-- Attachment #4: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-09-01 15:23 ` Konrad Rzeszutek Wilk
@ 2015-09-01 15:54 ` Jan Beulich
2015-09-01 16:55 ` Konrad Rzeszutek Wilk
2015-09-01 20:11 ` Konrad Rzeszutek Wilk
0 siblings, 2 replies; 31+ messages in thread
From: Jan Beulich @ 2015-09-01 15:54 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: andrew.cooper3, wei.liu2, xen-devel
>>> On 01.09.15 at 17:23, <konrad.wilk@oracle.com> wrote:
> There is only need for the compat layer once I move the 'xen_tmem_oid_t' to
> the tmem.h header file - and there is where the introduction in xlat.lst is
> done:
>
> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -103,6 +103,7 @@
> ! sched_poll sched.h
> ? sched_remote_shutdown sched.h
> ? sched_shutdown sched.h
> +? xen_tmem_oid tmem.h
Looks like your problem is the xen_ prefix here - no other entity has
it, despite there being a number of examples where the actual type
uses such a prefix.
Jan
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-09-01 15:54 ` Jan Beulich
@ 2015-09-01 16:55 ` Konrad Rzeszutek Wilk
2015-09-01 20:11 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-09-01 16:55 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel
On Tue, Sep 01, 2015 at 09:54:12AM -0600, Jan Beulich wrote:
> >>> On 01.09.15 at 17:23, <konrad.wilk@oracle.com> wrote:
> > There is only need for the compat layer once I move the 'xen_tmem_oid_t' to
> > the tmem.h header file - and there is where the introduction in xlat.lst is
> > done:
> >
> > --- a/xen/include/xlat.lst
> > +++ b/xen/include/xlat.lst
> > @@ -103,6 +103,7 @@
> > ! sched_poll sched.h
> > ? sched_remote_shutdown sched.h
> > ? sched_shutdown sched.h
> > +? xen_tmem_oid tmem.h
>
> Looks like your problem is the xen_ prefix here - no other entity has
> it, despite there being a number of examples where the actual type
> uses such a prefix.
Yup! This made it work:
diff --git a/xen/common/compat/tmem_xen.c b/xen/common/compat/tmem_xen.c
index 6bd1ad4..db08005 100644
--- a/xen/common/compat/tmem_xen.c
+++ b/xen/common/compat/tmem_xen.c
@@ -11,9 +11,7 @@
#include <xen/hypercall.h>
#include <compat/tmem.h>
-#define xen_xen_tmem_oid xen_tmem_oid
-CHECK_xen_tmem_oid;
-#undef xen_xen_tmem_oid
+CHECK_tmem_oid;
/*
* Local variables:
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index ace1d53..3795059 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -103,7 +103,7 @@
! sched_poll sched.h
? sched_remote_shutdown sched.h
? sched_shutdown sched.h
-? xen_tmem_oid tmem.h
+? tmem_oid tmem.h
! tmem_op tmem.h
? t_buf trace.h
? vcpu_get_physid vcpu.h
Thank you!
>
> Jan
>
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-09-01 15:54 ` Jan Beulich
2015-09-01 16:55 ` Konrad Rzeszutek Wilk
@ 2015-09-01 20:11 ` Konrad Rzeszutek Wilk
2015-09-02 6:43 ` Jan Beulich
1 sibling, 1 reply; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-09-01 20:11 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel
On Tue, Sep 01, 2015 at 09:54:12AM -0600, Jan Beulich wrote:
> >>> On 01.09.15 at 17:23, <konrad.wilk@oracle.com> wrote:
> > There is only need for the compat layer once I move the 'xen_tmem_oid_t' to
> > the tmem.h header file - and there is where the introduction in xlat.lst is
> > done:
> >
> > --- a/xen/include/xlat.lst
> > +++ b/xen/include/xlat.lst
> > @@ -103,6 +103,7 @@
> > ! sched_poll sched.h
> > ? sched_remote_shutdown sched.h
> > ? sched_shutdown sched.h
> > +? xen_tmem_oid tmem.h
>
> Looks like your problem is the xen_ prefix here - no other entity has
> it, despite there being a number of examples where the actual type
> uses such a prefix.
With your awesome hints (thank you!), this all now works. This is what
the patch ended up looking like:
>From 9a829efde9a47ca59a5c2029f3706a071342ea23 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 31 Aug 2015 10:57:27 -0400
Subject: [PATCH v3.1 07/10] tmem: Make the uint64_t oid[3] a proper structure:
xen_tmem_oid
And use it almost everywhere. It is easy to use it for the
sysctl since the hypervisor and toolstack are intertwined.
But for the tmem hypercall we need to be dilligient (as it
is guest facing) so delaying that to another patch:
"tmem: Use 'struct xen_tmem_oid' for every user" to help
with bisection issues.
We also move some of the parameters on functions to be within
the right location.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
tools/libxc/include/xenctrl.h | 6 +----
tools/libxc/xc_tmem.c | 16 ++++++------
xen/common/tmem.c | 57 +++++++++++++++++++++++--------------------
xen/include/public/sysctl.h | 8 +++++-
4 files changed, 46 insertions(+), 41 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 9e34769..2000f12 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2295,13 +2295,9 @@ int xc_disable_turbo(xc_interface *xch, int cpuid);
* tmem operations
*/
-struct tmem_oid {
- uint64_t oid[3];
-};
-
int xc_tmem_control_oid(xc_interface *xch, int32_t pool_id, uint32_t subop,
uint32_t cli_id, uint32_t arg1, uint32_t arg2,
- struct tmem_oid oid, void *buf);
+ struct xen_tmem_oid oid, void *buf);
int xc_tmem_control(xc_interface *xch,
int32_t pool_id, uint32_t subop, uint32_t cli_id,
uint32_t arg1, uint32_t arg2, void *buf);
diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 505595b..558d2ea 100644
--- a/tools/libxc/xc_tmem.c
+++ b/tools/libxc/xc_tmem.c
@@ -64,9 +64,9 @@ int xc_tmem_control(xc_interface *xch,
sysctl.u.tmem_op.arg1 = arg1;
sysctl.u.tmem_op.arg2 = arg2;
sysctl.u.tmem_op.pad = 0;
- sysctl.u.tmem_op.oid[0] = 0;
- sysctl.u.tmem_op.oid[1] = 0;
- sysctl.u.tmem_op.oid[2] = 0;
+ sysctl.u.tmem_op.oid.oid[0] = 0;
+ sysctl.u.tmem_op.oid.oid[1] = 0;
+ sysctl.u.tmem_op.oid.oid[2] = 0;
if ( cmd == XEN_SYSCTL_TMEM_OP_LIST && arg1 != 0 )
{
@@ -98,7 +98,7 @@ int xc_tmem_control_oid(xc_interface *xch,
uint32_t cli_id,
uint32_t arg1,
uint32_t arg2,
- struct tmem_oid oid,
+ struct xen_tmem_oid oid,
void *buf)
{
DECLARE_SYSCTL;
@@ -112,9 +112,7 @@ int xc_tmem_control_oid(xc_interface *xch,
sysctl.u.tmem_op.arg1 = arg1;
sysctl.u.tmem_op.arg2 = arg2;
sysctl.u.tmem_op.pad = 0;
- sysctl.u.tmem_op.oid[0] = oid.oid[0];
- sysctl.u.tmem_op.oid[1] = oid.oid[1];
- sysctl.u.tmem_op.oid[2] = oid.oid[2];
+ sysctl.u.tmem_op.oid = oid;
if ( cmd == XEN_SYSCTL_TMEM_OP_LIST && arg1 != 0 )
{
@@ -453,7 +451,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
}
for ( j = n_pages; j > 0; j-- )
{
- struct tmem_oid oid;
+ struct xen_tmem_oid oid;
uint32_t index;
int rc;
if ( read_exact(io_fd, &oid, sizeof(oid)) )
@@ -487,7 +485,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
int xc_tmem_restore_extra(xc_interface *xch, int dom, int io_fd)
{
uint32_t pool_id;
- struct tmem_oid oid;
+ struct xen_tmem_oid oid;
uint32_t index;
int count = 0;
int checksum = 0;
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index c73add5..5260c6c 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -125,12 +125,8 @@ struct tmem_pool {
#define is_persistent(_p) (_p->persistent)
#define is_shared(_p) (_p->shared)
-struct oid {
- uint64_t oid[3];
-};
-
struct tmem_object_root {
- struct oid oid;
+ struct xen_tmem_oid oid;
struct rb_node rb_tree_node; /* protected by pool->pool_rwlock */
unsigned long objnode_count; /* atomicity depends on obj_spinlock */
long pgp_count; /* atomicity depends on obj_spinlock */
@@ -158,7 +154,7 @@ struct tmem_page_descriptor {
};
struct tmem_object_root *obj;
} us;
- struct oid inv_oid; /* used for invalid list only */
+ struct xen_tmem_oid inv_oid; /* used for invalid list only */
};
pagesize_t size; /* 0 == PAGE_SIZE (pfp), -1 == data invalid,
else compressed data (cdata) */
@@ -815,7 +811,8 @@ static void rtn_free(struct radix_tree_node *rtn, void *arg)
/************ POOL OBJECT COLLECTION MANIPULATION ROUTINES *******************/
-static int oid_compare(struct oid *left, struct oid *right)
+static int oid_compare(struct xen_tmem_oid *left,
+ struct xen_tmem_oid *right)
{
if ( left->oid[2] == right->oid[2] )
{
@@ -839,19 +836,20 @@ static int oid_compare(struct oid *left, struct oid *right)
return 1;
}
-static void oid_set_invalid(struct oid *oidp)
+static void oid_set_invalid(struct xen_tmem_oid *oidp)
{
oidp->oid[0] = oidp->oid[1] = oidp->oid[2] = -1UL;
}
-static unsigned oid_hash(struct oid *oidp)
+static unsigned oid_hash(struct xen_tmem_oid *oidp)
{
return (tmem_hash(oidp->oid[0] ^ oidp->oid[1] ^ oidp->oid[2],
BITS_PER_LONG) & OBJ_HASH_BUCKETS_MASK);
}
/* searches for object==oid in pool, returns locked object if found */
-static struct tmem_object_root * obj_find(struct tmem_pool *pool, struct oid *oidp)
+static struct tmem_object_root * obj_find(struct tmem_pool *pool,
+ struct xen_tmem_oid *oidp)
{
struct rb_node *node;
struct tmem_object_root *obj;
@@ -887,7 +885,7 @@ restart_find:
static void obj_free(struct tmem_object_root *obj)
{
struct tmem_pool *pool;
- struct oid old_oid;
+ struct xen_tmem_oid old_oid;
ASSERT_SPINLOCK(&obj->obj_spinlock);
ASSERT(obj != NULL);
@@ -946,7 +944,8 @@ static int obj_rb_insert(struct rb_root *root, struct tmem_object_root *obj)
* allocate, initialize, and insert an tmem_object_root
* (should be called only if find failed)
*/
-static struct tmem_object_root * obj_alloc(struct tmem_pool *pool, struct oid *oidp)
+static struct tmem_object_root * obj_alloc(struct tmem_pool *pool,
+ struct xen_tmem_oid *oidp)
{
struct tmem_object_root *obj;
@@ -1531,8 +1530,8 @@ cleanup:
}
static int do_tmem_put(struct tmem_pool *pool,
- struct oid *oidp, uint32_t index,
- xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
+ struct xen_tmem_oid *oidp, uint32_t index,
+ xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
{
struct tmem_object_root *obj = NULL;
struct tmem_page_descriptor *pgp = NULL;
@@ -1696,8 +1695,9 @@ unlock_obj:
return ret;
}
-static int do_tmem_get(struct tmem_pool *pool, struct oid *oidp, uint32_t index,
- xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
+static int do_tmem_get(struct tmem_pool *pool,
+ struct xen_tmem_oid *oidp, uint32_t index,
+ xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
{
struct tmem_object_root *obj;
struct tmem_page_descriptor *pgp;
@@ -1774,7 +1774,8 @@ bad_copy:
return rc;
}
-static int do_tmem_flush_page(struct tmem_pool *pool, struct oid *oidp, uint32_t index)
+static int do_tmem_flush_page(struct tmem_pool *pool,
+ struct xen_tmem_oid *oidp, uint32_t index)
{
struct tmem_object_root *obj;
struct tmem_page_descriptor *pgp;
@@ -1807,7 +1808,8 @@ out:
return 1;
}
-static int do_tmem_flush_object(struct tmem_pool *pool, struct oid *oidp)
+static int do_tmem_flush_object(struct tmem_pool *pool,
+ struct xen_tmem_oid *oidp)
{
struct tmem_object_root *obj;
@@ -2432,7 +2434,7 @@ static int tmemc_save_get_next_page(int cli_id, uint32_t pool_id,
struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
? NULL : client->pools[pool_id];
struct tmem_page_descriptor *pgp;
- struct oid oid;
+ struct xen_tmem_oid oid;
int ret = 0;
struct tmem_handle h;
@@ -2525,8 +2527,10 @@ out:
return ret;
}
-static int tmemc_restore_put_page(int cli_id, uint32_t pool_id, struct oid *oidp,
- uint32_t index, tmem_cli_va_param_t buf, uint32_t bufsize)
+static int tmemc_restore_put_page(int cli_id, uint32_t pool_id,
+ struct xen_tmem_oid *oidp,
+ uint32_t index, tmem_cli_va_param_t buf,
+ uint32_t bufsize)
{
struct client *client = tmem_client_from_cli_id(cli_id);
struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
@@ -2542,8 +2546,9 @@ static int tmemc_restore_put_page(int cli_id, uint32_t pool_id, struct oid *oidp
return do_tmem_put(pool, oidp, index, 0, buf);
}
-static int tmemc_restore_flush_page(int cli_id, uint32_t pool_id, struct oid *oidp,
- uint32_t index)
+static int tmemc_restore_flush_page(int cli_id, uint32_t pool_id,
+ struct xen_tmem_oid *oidp,
+ uint32_t index)
{
struct client *client = tmem_client_from_cli_id(cli_id);
struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
@@ -2559,7 +2564,7 @@ int tmem_control(struct xen_sysctl_tmem_op *op)
int ret;
uint32_t pool_id = op->pool_id;
uint32_t cmd = op->cmd;
- struct oid *oidp = (struct oid *)(&op->oid[0]);
+ struct xen_tmem_oid *oidp = &op->oid;
if ( op->pad != 0 )
return -EINVAL;
@@ -2633,7 +2638,7 @@ long do_tmem_op(tmem_cli_op_t uops)
struct tmem_op op;
struct client *client = current->domain->tmem_client;
struct tmem_pool *pool = NULL;
- struct oid *oidp;
+ struct xen_tmem_oid *oidp;
int rc = 0;
bool_t succ_get = 0, succ_put = 0;
bool_t non_succ_get = 0, non_succ_put = 0;
@@ -2714,7 +2719,7 @@ long do_tmem_op(tmem_cli_op_t uops)
write_unlock(&tmem_rwlock);
read_lock(&tmem_rwlock);
- oidp = (struct oid *)&op.u.gen.oid[0];
+ oidp = container_of(&op.u.gen.oid[0], struct xen_tmem_oid, oid[0]);
switch ( op.cmd )
{
case TMEM_NEW_POOL:
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index fc4709a..2d7580b 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -737,6 +737,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
#define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
#define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
+struct xen_tmem_oid {
+ uint64_t oid[3];
+};
+typedef struct xen_tmem_oid xen_tmem_oid_t;
+DEFINE_XEN_GUEST_HANDLE(xen_tmem_oid_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_* .*/
@@ -746,7 +752,7 @@ struct xen_sysctl_tmem_op {
uint32_t arg1; /* IN: If not applicable to command use 0. */
uint32_t arg2; /* IN: If not applicable to command use 0. */
uint32_t pad; /* Padding so structure is the same under 32 and 64. */
- uint64_t oid[3]; /* IN: If not applicable to command use 0s. */
+ xen_tmem_oid_t oid; /* IN: If not applicable to command use 0s. */
XEN_GUEST_HANDLE_64(char) buf; /* IN/OUT: Buffer to save and restore ops. */
};
typedef struct xen_sysctl_tmem_op xen_sysctl_tmem_op_t;
--
2.1.0
>
> Jan
>
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-09-01 20:11 ` Konrad Rzeszutek Wilk
@ 2015-09-02 6:43 ` Jan Beulich
0 siblings, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2015-09-02 6:43 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: andrew.cooper3, wei.liu2, xen-devel
>>> On 01.09.15 at 22:11, <konrad.wilk@oracle.com> wrote:
> With your awesome hints (thank you!), this all now works. This is what
> the patch ended up looking like:
>
>
> From 9a829efde9a47ca59a5c2029f3706a071342ea23 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Mon, 31 Aug 2015 10:57:27 -0400
> Subject: [PATCH v3.1 07/10] tmem: Make the uint64_t oid[3] a proper
> structure:
> xen_tmem_oid
>
> And use it almost everywhere. It is easy to use it for the
> sysctl since the hypervisor and toolstack are intertwined.
>
> But for the tmem hypercall we need to be dilligient (as it
> is guest facing) so delaying that to another patch:
> "tmem: Use 'struct xen_tmem_oid' for every user" to help
> with bisection issues.
>
> We also move some of the parameters on functions to be within
> the right location.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-08-31 16:14 ` Konrad Rzeszutek Wilk
2015-09-01 7:04 ` Jan Beulich
@ 2015-09-01 15:18 ` Konrad Rzeszutek Wilk
2015-09-01 15:37 ` Jan Beulich
1 sibling, 1 reply; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-09-01 15:18 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel
[-- Attachment #1: Type: text/plain, Size: 760 bytes --]
On Mon, Aug 31, 2015 at 12:14:16PM -0400, Konrad Rzeszutek Wilk wrote:
> > >And the 'container_of' macro looks to require only one level of
> > >nesting.
> >
> > I'm pretty sure the macro can deal with both.
>
> OK, let me experiement with it as at the first blush it does not work for me.
I can't get it to work.
My understanding (and the test case - see attached) seem to agree that
the 'container_of' is to be used when you have the pointer to the nested
member (oid) and want the pointer to the structure in which it is
embedded in (op). We have the opposite case - we have the pointer to the
structure in which it was embedded (op):
struct tmem_op *op;
oidp = (struct tmem_oid *)&op.u.gen.oid[0];
Hence using the container_of macro won't work.
[-- Attachment #2: a.c --]
[-- Type: text/plain, Size: 1334 bytes --]
#include <stdio.h>
struct tmem_oid {
unsigned long oid[3];
};
struct internal {
unsigned int cmd;
struct tmem_oid oid;
};
struct tmem_op {
unsigned int x;
union {
struct {
unsigned int cmd;
} g;
struct internal c;
} u;
};
#define container_of(ptr, type, member) ({ \
typeof( ((type *)0)->member ) *__mptr = (ptr); \
(type *)( (char *)__mptr - offsetof(type,member) );})
#define offsetof(a,b) __builtin_offsetof(a,b)
void main(void) {
struct tmem_op op;
struct tmem_oid *_oid;
struct tmem_oid *_src;
struct internal *_i;
struct internal *_q;
printf("%d \n", offsetof(struct tmem_op, u.c.oid)); // 16
printf("%d \n", offsetof(struct tmem_op, u.c.oid.oid)); // 16
printf("%d \n", offsetof(struct internal, oid)); // 8
_i = &op.u.c;
printf("%d\n", (void *)_i - (void *)&op); // 8
_oid = &op.u.c.oid;
printf("%d\n", (void *)_oid - (void *)&op); // 16
/* Pointer to struct tmem_oid (_oid), getting the pointer to 'struct internal' */
_q = container_of(_oid, struct internal, oid);
printf("%d\n", (void *)_q - (void *)&op); // 8
/* Having the pointer to outer ('*op') we can get the pointer to the internal. */
_src = (struct tmem_oid *)((void *)&op + offsetof(struct tmem_op, u.c.Xoid.oid));
printf("%d\n", (void *)_src - (void *)&op); // 16
};
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-09-01 15:18 ` Konrad Rzeszutek Wilk
@ 2015-09-01 15:37 ` Jan Beulich
2015-09-01 15:53 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 31+ messages in thread
From: Jan Beulich @ 2015-09-01 15:37 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: andrew.cooper3, wei.liu2, xen-devel
>>> On 01.09.15 at 17:18, <konrad.wilk@oracle.com> wrote:
> On Mon, Aug 31, 2015 at 12:14:16PM -0400, Konrad Rzeszutek Wilk wrote:
>> > >And the 'container_of' macro looks to require only one level of
>> > >nesting.
>> >
>> > I'm pretty sure the macro can deal with both.
>>
>> OK, let me experiement with it as at the first blush it does not work for
> me.
>
> I can't get it to work.
>
> My understanding (and the test case - see attached) seem to agree that
> the 'container_of' is to be used when you have the pointer to the nested
> member (oid) and want the pointer to the structure in which it is
> embedded in (op). We have the opposite case - we have the pointer to the
> structure in which it was embedded (op):
>
> struct tmem_op *op;
>
> oidp = (struct tmem_oid *)&op.u.gen.oid[0];
>
> Hence using the container_of macro won't work.
All of these work and behave the same for me:
struct tmem_oid *test1(struct tmem_op *op) {
return (struct tmem_oid *)&op->u.gen.oid[0];
}
struct tmem_oid *test2(struct tmem_op *op) {
return container_of(&op->u.gen.oid[0], struct tmem_oid, oid[0]);
}
struct tmem_oid *test3(struct tmem_op *op) {
return container_of(op->u.gen.oid, struct tmem_oid, oid[0]);
}
Jan
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid
2015-09-01 15:37 ` Jan Beulich
@ 2015-09-01 15:53 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-09-01 15:53 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel
On Tue, Sep 01, 2015 at 09:37:48AM -0600, Jan Beulich wrote:
> >>> On 01.09.15 at 17:18, <konrad.wilk@oracle.com> wrote:
> > On Mon, Aug 31, 2015 at 12:14:16PM -0400, Konrad Rzeszutek Wilk wrote:
> >> > >And the 'container_of' macro looks to require only one level of
> >> > >nesting.
> >> >
> >> > I'm pretty sure the macro can deal with both.
> >>
> >> OK, let me experiement with it as at the first blush it does not work for
> > me.
> >
> > I can't get it to work.
> >
> > My understanding (and the test case - see attached) seem to agree that
> > the 'container_of' is to be used when you have the pointer to the nested
> > member (oid) and want the pointer to the structure in which it is
> > embedded in (op). We have the opposite case - we have the pointer to the
> > structure in which it was embedded (op):
> >
> > struct tmem_op *op;
> >
> > oidp = (struct tmem_oid *)&op.u.gen.oid[0];
> >
> > Hence using the container_of macro won't work.
>
> All of these work and behave the same for me:
>
> struct tmem_oid *test1(struct tmem_op *op) {
> return (struct tmem_oid *)&op->u.gen.oid[0];
> }
>
> struct tmem_oid *test2(struct tmem_op *op) {
> return container_of(&op->u.gen.oid[0], struct tmem_oid, oid[0]);
> }
>
> struct tmem_oid *test3(struct tmem_op *op) {
> return container_of(op->u.gen.oid, struct tmem_oid, oid[0]);
> }
Woot! That will definitly work. Thank you!
>
> Jan
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 08/11] tmem/sysctl: Use 'struct tmem_oid' for every user.
2015-08-28 18:53 [PATCH v3] Tmem bug-fixes and cleanups Konrad Rzeszutek Wilk
` (6 preceding siblings ...)
2015-08-28 18:53 ` [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid Konrad Rzeszutek Wilk
@ 2015-08-28 18:53 ` Konrad Rzeszutek Wilk
2015-08-31 11:42 ` Jan Beulich
2015-08-28 18:53 ` [PATCH v3 09/11] tmem: Use 'struct tmem_oid' in tmem_handle and move it to sysctl header Konrad Rzeszutek Wilk
` (2 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-28 18:53 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2; +Cc: Konrad Rzeszutek Wilk
Patch "tmem: Make the uint64_t oid[3] a proper structure:
tmem_oid" converted the sysctl API to use an
proper structure. But it did not do it for the tmem hypercall.
This expands that and converts the tmem hypercall. For this
to work we define the struct in tmem.h and include it in
sysctl.h.
This change also included work to make the compat layer
happy. That was to declare the struct tmem_oid to be
checked in xlat.lst - which will construct an typedef
in the compat file with the same type, hence allowing
copying of 'oid' member without type issues.
The layout (and size) of this structure in memory for the
'struct tmem_op' (so guest facing) is the same! Verified
via pahole and with 32/64 bit guests.
--- /tmp/old 2015-08-27 16:34:00.535638730 -0400
+++ /tmp/new 2015-08-27 16:34:10.447705328 -0400
@@ -8,7 +8,7 @@
uint32_t arg1; /* 28 4 */
} creat; /* 24 */
struct {
- uint64_t oid[3]; /* 8 24 */
+ xen_tmem_oid_t oid; /* 8 24 */
uint32_t index; /* 32 4 */
uint32_t tmem_offset; /* 36 4 */
uint32_t pfn_offset; /* 40 4 */
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
xen/common/compat/tmem_xen.c | 6 +++---
xen/common/tmem.c | 2 +-
xen/include/public/sysctl.h | 7 +------
xen/include/public/tmem.h | 9 +++++++++
xen/include/xlat.lst | 1 +
5 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/xen/common/compat/tmem_xen.c b/xen/common/compat/tmem_xen.c
index 97c7ff2..6310e35 100644
--- a/xen/common/compat/tmem_xen.c
+++ b/xen/common/compat/tmem_xen.c
@@ -11,9 +11,9 @@
#include <xen/hypercall.h>
#include <compat/tmem.h>
-#define xen_tmem_op tmem_op
-/*CHECK_tmem_op;*/
-#undef xen_tmem_op
+#define xen_tmem_oid tmem_oid
+CHECK_tmem_oid;
+#undef xen_tmem_oid
/*
* Local variables:
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index c9ff08c..f06823e 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -2719,7 +2719,7 @@ long do_tmem_op(tmem_cli_op_t uops)
write_unlock(&tmem_rwlock);
read_lock(&tmem_rwlock);
- oidp = (struct tmem_oid *)&op.u.gen.oid[0];
+ oidp = &op.u.gen.oid;
switch ( op.cmd )
{
case TMEM_NEW_POOL:
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 9a58bed..3656c7f 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -34,6 +34,7 @@
#include "xen.h"
#include "domctl.h"
#include "physdev.h"
+#include "tmem.h"
#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000C
@@ -737,12 +738,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
#define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
#define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
-struct tmem_oid {
- uint64_t oid[3];
-};
-typedef struct tmem_oid tmem_oid_t;
-DEFINE_XEN_GUEST_HANDLE(tmem_oid_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_* .*/
diff --git a/xen/include/public/tmem.h b/xen/include/public/tmem.h
index e4ee704..57d5601 100644
--- a/xen/include/public/tmem.h
+++ b/xen/include/public/tmem.h
@@ -73,6 +73,11 @@
#define EFROZEN 1000
#define EEMPTY 1001
+struct tmem_oid {
+ uint64_t oid[3];
+};
+typedef struct tmem_oid tmem_oid_t;
+DEFINE_XEN_GUEST_HANDLE(tmem_oid_t);
#ifndef __ASSEMBLY__
#if __XEN_INTERFACE_VERSION__ < 0x00040400
@@ -89,7 +94,11 @@ struct tmem_op {
uint32_t arg1;
} creat; /* for cmd == TMEM_NEW_POOL, TMEM_AUTH, TMEM_RESTORE_NEW */
struct {
+#if __XEN_INTERFACE_VERSION__ < 0x00040600
uint64_t oid[3];
+#else
+ tmem_oid_t oid;
+#endif
uint32_t index;
uint32_t tmem_offset;
uint32_t pfn_offset;
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 8cedee7..3795059 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -103,6 +103,7 @@
! sched_poll sched.h
? sched_remote_shutdown sched.h
? sched_shutdown sched.h
+? tmem_oid tmem.h
! tmem_op tmem.h
? t_buf trace.h
? vcpu_get_physid vcpu.h
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 08/11] tmem/sysctl: Use 'struct tmem_oid' for every user.
2015-08-28 18:53 ` [PATCH v3 08/11] tmem/sysctl: Use 'struct tmem_oid' for every user Konrad Rzeszutek Wilk
@ 2015-08-31 11:42 ` Jan Beulich
0 siblings, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2015-08-31 11:42 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: andrew.cooper3, wei.liu2, xen-devel
>>> On 28.08.15 at 20:53, <konrad.wilk@oracle.com> wrote:
> Patch "tmem: Make the uint64_t oid[3] a proper structure:
> tmem_oid" converted the sysctl API to use an
> proper structure. But it did not do it for the tmem hypercall.
>
> This expands that and converts the tmem hypercall. For this
> to work we define the struct in tmem.h and include it in
> sysctl.h.
>
> This change also included work to make the compat layer
> happy. That was to declare the struct tmem_oid to be
> checked in xlat.lst - which will construct an typedef
> in the compat file with the same type, hence allowing
> copying of 'oid' member without type issues.
>
> The layout (and size) of this structure in memory for the
> 'struct tmem_op' (so guest facing) is the same! Verified
> via pahole and with 32/64 bit guests.
>
> --- /tmp/old 2015-08-27 16:34:00.535638730 -0400
> +++ /tmp/new 2015-08-27 16:34:10.447705328 -0400
> @@ -8,7 +8,7 @@
> uint32_t arg1; /* 28 4 */
> } creat; /* 24 */
> struct {
> - uint64_t oid[3]; /* 8 24 */
> + xen_tmem_oid_t oid; /* 8 24 */
> uint32_t index; /* 32 4 */
> uint32_t tmem_offset; /* 36 4 */
> uint32_t pfn_offset; /* 40 4 */
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
With the suggestion to drop "sysctl" from the title (since this is
about everything other than sysctl)
Acked-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 09/11] tmem: Use 'struct tmem_oid' in tmem_handle and move it to sysctl header.
2015-08-28 18:53 [PATCH v3] Tmem bug-fixes and cleanups Konrad Rzeszutek Wilk
` (7 preceding siblings ...)
2015-08-28 18:53 ` [PATCH v3 08/11] tmem/sysctl: Use 'struct tmem_oid' for every user Konrad Rzeszutek Wilk
@ 2015-08-28 18:53 ` Konrad Rzeszutek Wilk
2015-08-31 11:44 ` Jan Beulich
2015-09-01 20:12 ` Konrad Rzeszutek Wilk
2015-08-28 18:53 ` [PATCH v3 10/11] tmem: Remove extra spaces at end and some hard tabbing Konrad Rzeszutek Wilk
2015-08-28 18:53 ` [PATCH v3 11/11] tmem: Spelling and full stop surgery Konrad Rzeszutek Wilk
10 siblings, 2 replies; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-28 18:53 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2; +Cc: Konrad Rzeszutek Wilk
Instead of the three member uint64_t structure.
The structure is used by the control stack for
XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_[PAGE|INV] only so
moving it to the sysctl header.
Also modified tmemc_save_get_next_page to deal with
the new type - and converted some of the on-stack
usage of the array to use an pointer.
Further work will be to make the xen_sysctl_tmem_op have
an union with proper type for the two: ..GET_NEXT_[PAGE|INV]
operations.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
tools/libxc/xc_tmem.c | 6 +++---
xen/common/tmem.c | 12 ++++++------
xen/include/public/sysctl.h | 11 +++++++++++
xen/include/public/tmem.h | 6 ------
4 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 425f51d..70a8e11 100644
--- a/tools/libxc/xc_tmem.c
+++ b/tools/libxc/xc_tmem.c
@@ -304,7 +304,7 @@ int xc_tmem_save(xc_interface *xch,
} else {
/* page list terminator */
h = (struct tmem_handle *)buf;
- h->oid[0] = h->oid[1] = h->oid[2] = -1L;
+ h->oid.oid[0] = h->oid.oid[1] = h->oid.oid[2] = -1L;
if ( write_exact(io_fd, &h->oid, sizeof(h->oid)) )
return -1;
break;
@@ -341,8 +341,8 @@ int xc_tmem_save_extra(xc_interface *xch, int dom, int io_fd, int field_marker)
if ( write_exact(io_fd, &handle.index, sizeof(handle.index)) )
return -1;
count++;
- checksum += handle.pool_id + handle.oid[0] + handle.oid[1] +
- handle.oid[2] + handle.index;
+ checksum += handle.pool_id + handle.oid.oid[0] + handle.oid.oid[1] +
+ handle.oid.oid[2] + handle.index;
}
if ( count )
DPRINTF("needed %d tmem invalidates, check=%d\n",count,checksum);
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index f06823e..4071dc0 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -2434,7 +2434,7 @@ static int tmemc_save_get_next_page(int cli_id, uint32_t pool_id,
struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
? NULL : client->pools[pool_id];
struct tmem_page_descriptor *pgp;
- struct tmem_oid oid;
+ struct tmem_oid *oid;
int ret = 0;
struct tmem_handle h;
@@ -2466,10 +2466,10 @@ static int tmemc_save_get_next_page(int cli_id, uint32_t pool_id,
pgp = list_entry((&pool->cur_pgp->us.pool_pers_pages)->next,
struct tmem_page_descriptor,us.pool_pers_pages);
pool->cur_pgp = pgp;
- oid = pgp->us.obj->oid;
+ oid = &pgp->us.obj->oid;
h.pool_id = pool_id;
- BUILD_BUG_ON(sizeof(h.oid) != sizeof(oid));
- memcpy(h.oid, oid.oid, sizeof(h.oid));
+ BUILD_BUG_ON(sizeof(h.oid) != sizeof(*oid));
+ memcpy(&(h.oid), oid, sizeof(h.oid));
h.index = pgp->index;
if ( copy_to_guest(guest_handle_cast(buf, void), &h, 1) )
{
@@ -2477,7 +2477,7 @@ static int tmemc_save_get_next_page(int cli_id, uint32_t pool_id,
goto out;
}
guest_handle_add_offset(buf, sizeof(h));
- ret = do_tmem_get(pool, &oid, pgp->index, 0, buf);
+ ret = do_tmem_get(pool, oid, pgp->index, 0, buf);
out:
spin_unlock(&pers_lists_spinlock);
@@ -2517,7 +2517,7 @@ static int tmemc_save_get_next_inv(int cli_id, tmem_cli_va_param_t buf,
}
h.pool_id = pgp->pool_id;
BUILD_BUG_ON(sizeof(h.oid) != sizeof(pgp->inv_oid));
- memcpy(h.oid, pgp->inv_oid.oid, sizeof(h.oid));
+ memcpy(&(h.oid), &(pgp->inv_oid), sizeof(h.oid));
h.index = pgp->index;
ret = 1;
if ( copy_to_guest(guest_handle_cast(buf, void), &h, 1) )
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 3656c7f..bbcc96f 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -738,6 +738,17 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
#define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
#define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
+/*
+ * XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_[PAGE|INV] override the 'buf' in
+ * xen_sysctl_tmem_op with this structure - sometimes with an extra
+ * page tackled on.
+ */
+struct tmem_handle {
+ uint32_t pool_id;
+ uint32_t index;
+ tmem_oid_t oid;
+};
+
struct xen_sysctl_tmem_op {
uint32_t cmd; /* IN: XEN_SYSCTL_TMEM_OP_* . */
int32_t pool_id; /* IN: 0 by default unless _SAVE_*, RESTORE_* .*/
diff --git a/xen/include/public/tmem.h b/xen/include/public/tmem.h
index 57d5601..5dd2399 100644
--- a/xen/include/public/tmem.h
+++ b/xen/include/public/tmem.h
@@ -109,12 +109,6 @@ struct tmem_op {
};
typedef struct tmem_op tmem_op_t;
DEFINE_XEN_GUEST_HANDLE(tmem_op_t);
-
-struct tmem_handle {
- uint32_t pool_id;
- uint32_t index;
- uint64_t oid[3];
-};
#endif
#endif /* __XEN_PUBLIC_TMEM_H__ */
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 09/11] tmem: Use 'struct tmem_oid' in tmem_handle and move it to sysctl header.
2015-08-28 18:53 ` [PATCH v3 09/11] tmem: Use 'struct tmem_oid' in tmem_handle and move it to sysctl header Konrad Rzeszutek Wilk
@ 2015-08-31 11:44 ` Jan Beulich
2015-09-01 20:12 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2015-08-31 11:44 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: andrew.cooper3, wei.liu2, xen-devel
>>> On 28.08.15 at 20:53, <konrad.wilk@oracle.com> wrote:
> Instead of the three member uint64_t structure.
>
> The structure is used by the control stack for
> XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_[PAGE|INV] only so
> moving it to the sysctl header.
>
> Also modified tmemc_save_get_next_page to deal with
> the new type - and converted some of the on-stack
> usage of the array to use an pointer.
>
> Further work will be to make the xen_sysctl_tmem_op have
> an union with proper type for the two: ..GET_NEXT_[PAGE|INV]
> operations.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Hypervisor side:
Acked-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 09/11] tmem: Use 'struct tmem_oid' in tmem_handle and move it to sysctl header.
2015-08-28 18:53 ` [PATCH v3 09/11] tmem: Use 'struct tmem_oid' in tmem_handle and move it to sysctl header Konrad Rzeszutek Wilk
2015-08-31 11:44 ` Jan Beulich
@ 2015-09-01 20:12 ` Konrad Rzeszutek Wilk
2015-09-02 6:46 ` Jan Beulich
1 sibling, 1 reply; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-09-01 20:12 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2
On Fri, Aug 28, 2015 at 02:53:18PM -0400, Konrad Rzeszutek Wilk wrote:
> Instead of the three member uint64_t structure.
>
> The structure is used by the control stack for
> XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_[PAGE|INV] only so
> moving it to the sysctl header.
>
> Also modified tmemc_save_get_next_page to deal with
> the new type - and converted some of the on-stack
> usage of the array to use an pointer.
>
> Further work will be to make the xen_sysctl_tmem_op have
> an union with proper type for the two: ..GET_NEXT_[PAGE|INV]
> operations.
.. with the changes that Jan requested the patch ended up looking
like:
>From 4fd1cf85b10565b7b7e3f73d19fa30de808ff60f Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 31 Aug 2015 11:00:29 -0400
Subject: [PATCH v3.1 08/10] tmem: Use 'struct xen_tmem_oid' for every user.
Patch "tmem: Make the uint64_t oid[3] a proper structure:
xen_tmem_oid" converted the sysctl API to use an
proper structure. But it did not do it for the tmem hypercall.
This expands that and converts the tmem hypercall. For this
to work we define the struct in tmem.h and include it in
sysctl.h.
This change also included work to make the compat layer
happy. That was to declare the struct xen_tmem_oid to be
checked in xlat.lst - which will construct an typedef
in the compat file with the same type, hence allowing
copying of 'oid' member without type issues. The kicker
is that the compat layer adds the prefix 'xen' and since
our structure already has it - we must not include it.
The layout (and size) of this structure in memory for the
'struct tmem_op' (so guest facing) is the same! Verified
via pahole and with 32/64 bit guests.
--- /tmp/old 2015-08-27 16:34:00.535638730 -0400
+++ /tmp/new 2015-08-27 16:34:10.447705328 -0400
@@ -8,7 +8,7 @@
uint32_t arg1; /* 28 4 */
} creat; /* 24 */
struct {
- uint64_t oid[3]; /* 8 24 */
+ xen_tmem_oid_t oid; /* 8 24 */
uint32_t index; /* 32 4 */
uint32_t tmem_offset; /* 36 4 */
uint32_t pfn_offset; /* 40 4 */
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
xen/common/compat/tmem_xen.c | 4 +---
xen/common/tmem.c | 2 +-
xen/include/public/sysctl.h | 7 +------
xen/include/public/tmem.h | 9 +++++++++
xen/include/xlat.lst | 1 +
5 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/xen/common/compat/tmem_xen.c b/xen/common/compat/tmem_xen.c
index 97c7ff2..db08005 100644
--- a/xen/common/compat/tmem_xen.c
+++ b/xen/common/compat/tmem_xen.c
@@ -11,9 +11,7 @@
#include <xen/hypercall.h>
#include <compat/tmem.h>
-#define xen_tmem_op tmem_op
-/*CHECK_tmem_op;*/
-#undef xen_tmem_op
+CHECK_tmem_oid;
/*
* Local variables:
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index 5260c6c..c5edab4 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -2719,7 +2719,7 @@ long do_tmem_op(tmem_cli_op_t uops)
write_unlock(&tmem_rwlock);
read_lock(&tmem_rwlock);
- oidp = container_of(&op.u.gen.oid[0], struct xen_tmem_oid, oid[0]);
+ oidp = &op.u.gen.oid;
switch ( op.cmd )
{
case TMEM_NEW_POOL:
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 2d7580b..3bdf0e1 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -34,6 +34,7 @@
#include "xen.h"
#include "domctl.h"
#include "physdev.h"
+#include "tmem.h"
#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000C
@@ -737,12 +738,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
#define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
#define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
-struct xen_tmem_oid {
- uint64_t oid[3];
-};
-typedef struct xen_tmem_oid xen_tmem_oid_t;
-DEFINE_XEN_GUEST_HANDLE(xen_tmem_oid_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_* .*/
diff --git a/xen/include/public/tmem.h b/xen/include/public/tmem.h
index e4ee704..913566a 100644
--- a/xen/include/public/tmem.h
+++ b/xen/include/public/tmem.h
@@ -73,6 +73,11 @@
#define EFROZEN 1000
#define EEMPTY 1001
+struct xen_tmem_oid {
+ uint64_t oid[3];
+};
+typedef struct xen_tmem_oid xen_tmem_oid_t;
+DEFINE_XEN_GUEST_HANDLE(xen_tmem_oid_t);
#ifndef __ASSEMBLY__
#if __XEN_INTERFACE_VERSION__ < 0x00040400
@@ -89,7 +94,11 @@ struct tmem_op {
uint32_t arg1;
} creat; /* for cmd == TMEM_NEW_POOL, TMEM_AUTH, TMEM_RESTORE_NEW */
struct {
+#if __XEN_INTERFACE_VERSION__ < 0x00040600
uint64_t oid[3];
+#else
+ xen_tmem_oid_t oid;
+#endif
uint32_t index;
uint32_t tmem_offset;
uint32_t pfn_offset;
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 8cedee7..3795059 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -103,6 +103,7 @@
! sched_poll sched.h
? sched_remote_shutdown sched.h
? sched_shutdown sched.h
+? tmem_oid tmem.h
! tmem_op tmem.h
? t_buf trace.h
? vcpu_get_physid vcpu.h
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 09/11] tmem: Use 'struct tmem_oid' in tmem_handle and move it to sysctl header.
2015-09-01 20:12 ` Konrad Rzeszutek Wilk
@ 2015-09-02 6:46 ` Jan Beulich
0 siblings, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2015-09-02 6:46 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: andrew.cooper3, wei.liu2, xen-devel
>>> On 01.09.15 at 22:12, <konrad.wilk@oracle.com> wrote:
> .. with the changes that Jan requested the patch ended up looking
> like:
>
> From 4fd1cf85b10565b7b7e3f73d19fa30de808ff60f Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Mon, 31 Aug 2015 11:00:29 -0400
> Subject: [PATCH v3.1 08/10] tmem: Use 'struct xen_tmem_oid' for every user.
>
> Patch "tmem: Make the uint64_t oid[3] a proper structure:
> xen_tmem_oid" converted the sysctl API to use an
> proper structure. But it did not do it for the tmem hypercall.
>
> This expands that and converts the tmem hypercall. For this
> to work we define the struct in tmem.h and include it in
> sysctl.h.
>
> This change also included work to make the compat layer
> happy. That was to declare the struct xen_tmem_oid to be
> checked in xlat.lst - which will construct an typedef
> in the compat file with the same type, hence allowing
> copying of 'oid' member without type issues. The kicker
> is that the compat layer adds the prefix 'xen' and since
> our structure already has it - we must not include it.
>
> The layout (and size) of this structure in memory for the
> 'struct tmem_op' (so guest facing) is the same! Verified
> via pahole and with 32/64 bit guests.
>
> --- /tmp/old 2015-08-27 16:34:00.535638730 -0400
> +++ /tmp/new 2015-08-27 16:34:10.447705328 -0400
> @@ -8,7 +8,7 @@
> uint32_t arg1; /* 28 4 */
> } creat; /* 24 */
> struct {
> - uint64_t oid[3]; /* 8 24 */
> + xen_tmem_oid_t oid; /* 8 24 */
> uint32_t index; /* 32 4 */
> uint32_t tmem_offset; /* 36 4 */
> uint32_t pfn_offset; /* 40 4 */
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
My ack (given for v3) stands.
Jan
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 10/11] tmem: Remove extra spaces at end and some hard tabbing.
2015-08-28 18:53 [PATCH v3] Tmem bug-fixes and cleanups Konrad Rzeszutek Wilk
` (8 preceding siblings ...)
2015-08-28 18:53 ` [PATCH v3 09/11] tmem: Use 'struct tmem_oid' in tmem_handle and move it to sysctl header Konrad Rzeszutek Wilk
@ 2015-08-28 18:53 ` Konrad Rzeszutek Wilk
2015-08-28 18:53 ` [PATCH v3 11/11] tmem: Spelling and full stop surgery Konrad Rzeszutek Wilk
10 siblings, 0 replies; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-28 18:53 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2; +Cc: Konrad Rzeszutek Wilk
My editor marks these in red glowing red so removing them to
make it easier to focus on code.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
xen/common/tmem.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index 4071dc0..1b9c5d7 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -111,7 +111,7 @@ struct tmem_pool {
atomic_t pgp_count;
int pgp_count_max;
long obj_count; /* atomicity depends on pool_rwlock held for write */
- long obj_count_max;
+ long obj_count_max;
unsigned long objnode_count, objnode_count_max;
uint64_t sum_life_cycles;
uint64_t sum_evicted_cycles;
@@ -1089,7 +1089,7 @@ static int shared_pool_quit(struct tmem_pool *pool, domid_t cli_id)
ASSERT(is_shared(pool));
ASSERT(pool->client != NULL);
-
+
ASSERT_WRITELOCK(&tmem_rwlock);
pool_destroy_objs(pool, cli_id);
list_for_each_entry(sl,&pool->share_list, share_list)
@@ -1177,7 +1177,7 @@ static struct client *client_create(domid_t cli_id)
}
if ( !d->is_dying ) {
d->tmem_client = client;
- client->domain = d;
+ client->domain = d;
}
rcu_unlock_domain(d);
@@ -1226,7 +1226,7 @@ static bool_t client_over_quota(struct client *client)
int total = _atomic_read(client_weight_total);
ASSERT(client != NULL);
- if ( (total == 0) || (client->weight == 0) ||
+ if ( (total == 0) || (client->weight == 0) ||
(client->eph_count == 0) )
return 0;
return ( ((global_eph_count*100L) / client->eph_count ) >
@@ -1411,7 +1411,7 @@ static int do_tmem_put_compress(struct tmem_page_descriptor *pgp, xen_pfn_t cmfn
void *dst, *p;
size_t size;
int ret = 0;
-
+
ASSERT(pgp != NULL);
ASSERT(pgp->us.obj != NULL);
ASSERT_SPINLOCK(&pgp->us.obj->obj_spinlock);
@@ -1556,7 +1556,7 @@ refind:
{
/* no puts allowed into a frozen pool (except dup puts) */
if ( client->frozen )
- goto unlock_obj;
+ goto unlock_obj;
}
}
else
@@ -1569,10 +1569,10 @@ refind:
write_lock(&pool->pool_rwlock);
/*
- * Parallel callers may already allocated obj and inserted to obj_rb_root
- * before us.
- */
- if (!obj_rb_insert(&pool->obj_rb_root[oid_hash(oidp)], obj))
+ * Parallel callers may already allocated obj and inserted to obj_rb_root
+ * before us.
+ */
+ if ( !obj_rb_insert(&pool->obj_rb_root[oid_hash(oidp)], obj) )
{
tmem_free(obj, pool);
write_unlock(&pool->pool_rwlock);
@@ -1945,7 +1945,7 @@ static int do_tmem_new_pool(domid_t this_cli_id,
(client->shared_auth_uuid[i][1] == uuid_hi) )
break;
if ( i == MAX_GLOBAL_SHARED_POOLS )
- {
+ {
tmem_client_info("Shared auth failed, create non shared pool instead!\n");
pool->shared = 0;
goto out;
@@ -2107,7 +2107,7 @@ static int tmemc_list_client(struct client *c, tmem_cli_va_param_t buf,
p->obj_count, p->obj_count_max,
p->objnode_count, p->objnode_count_max,
p->good_puts, p->puts,p->dup_puts_flushed, p->dup_puts_replaced,
- p->no_mem_puts,
+ p->no_mem_puts,
p->found_gets, p->gets,
p->flushs_found, p->flushs, p->flush_objs_found, p->flush_objs);
if ( sum + n >= len )
@@ -2146,7 +2146,7 @@ static int tmemc_list_shared(tmem_cli_va_param_t buf, int off, uint32_t len,
p->obj_count, p->obj_count_max,
p->objnode_count, p->objnode_count_max,
p->good_puts, p->puts,p->dup_puts_flushed, p->dup_puts_replaced,
- p->no_mem_puts,
+ p->no_mem_puts,
p->found_gets, p->gets,
p->flushs_found, p->flushs, p->flush_objs_found, p->flush_objs);
if ( sum + n >= len )
@@ -2456,7 +2456,7 @@ static int tmemc_save_get_next_page(int cli_id, uint32_t pool_id,
/* process the first one */
pool->cur_pgp = pgp = list_entry((&pool->persistent_page_list)->next,
struct tmem_page_descriptor,us.pool_pers_pages);
- } else if ( list_is_last(&pool->cur_pgp->us.pool_pers_pages,
+ } else if ( list_is_last(&pool->cur_pgp->us.pool_pers_pages,
&pool->persistent_page_list) )
{
/* already processed the last one in the list */
@@ -2504,7 +2504,7 @@ static int tmemc_save_get_next_inv(int cli_id, tmem_cli_va_param_t buf,
pgp = list_entry((&client->persistent_invalidated_list)->next,
struct tmem_page_descriptor,client_inv_pages);
client->cur_pgp = pgp;
- } else if ( list_is_last(&client->cur_pgp->client_inv_pages,
+ } else if ( list_is_last(&client->cur_pgp->client_inv_pages,
&client->persistent_invalidated_list) )
{
client->cur_pgp = NULL;
@@ -2541,7 +2541,7 @@ static int tmemc_restore_put_page(int cli_id, uint32_t pool_id,
if (bufsize != PAGE_SIZE) {
tmem_client_err("tmem: %s: invalid parameter bufsize(%d) != (%ld)\n",
__func__, bufsize, PAGE_SIZE);
- return -EINVAL;
+ return -EINVAL;
}
return do_tmem_put(pool, oidp, index, 0, buf);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 11/11] tmem: Spelling and full stop surgery.
2015-08-28 18:53 [PATCH v3] Tmem bug-fixes and cleanups Konrad Rzeszutek Wilk
` (9 preceding siblings ...)
2015-08-28 18:53 ` [PATCH v3 10/11] tmem: Remove extra spaces at end and some hard tabbing Konrad Rzeszutek Wilk
@ 2015-08-28 18:53 ` Konrad Rzeszutek Wilk
10 siblings, 0 replies; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-28 18:53 UTC (permalink / raw)
To: xen-devel, jbeulich, andrew.cooper3, wei.liu2; +Cc: Konrad Rzeszutek Wilk
I could not help myself.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
xen/common/tmem.c | 234 ++++++++++++++++++++++++++++--------------------------
1 file changed, 121 insertions(+), 113 deletions(-)
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index 1b9c5d7..861dabd 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -15,7 +15,7 @@
*/
#ifdef __XEN__
-#include <xen/tmem_xen.h> /* host-specific (eg Xen) code goes here */
+#include <xen/tmem_xen.h> /* host-specific (eg Xen) code goes here. */
#endif
#include <public/sysctl.h>
@@ -27,7 +27,7 @@
#define TMEM_SPEC_VERSION 1
-/* global statistics (none need to be locked) */
+/* Global statistics (none need to be locked). */
static unsigned long total_tmem_ops = 0;
static unsigned long errored_tmem_ops = 0;
static unsigned long total_flush_pool = 0;
@@ -69,18 +69,18 @@ struct client {
bool_t compress;
bool_t frozen;
bool_t shared_auth_required;
- /* for save/restore/migration */
+ /* For save/restore/migration. */
bool_t live_migrating;
bool_t was_frozen;
struct list_head persistent_invalidated_list;
struct tmem_page_descriptor *cur_pgp;
- /* statistics collection */
+ /* Statistics collection. */
unsigned long compress_poor, compress_nomem;
unsigned long compressed_pages;
uint64_t compressed_sum_size;
uint64_t total_cycles;
unsigned long succ_pers_puts, succ_eph_gets, succ_pers_gets;
- /* shared pool authentication */
+ /* Shared pool authentication. */
uint64_t shared_auth_uuid[MAX_GLOBAL_SHARED_POOLS][2];
};
@@ -90,7 +90,7 @@ struct share_list {
};
#define POOL_PAGESHIFT (PAGE_SHIFT - 12)
-#define OBJ_HASH_BUCKETS 256 /* must be power of two */
+#define OBJ_HASH_BUCKETS 256 /* Must be power of two. */
#define OBJ_HASH_BUCKETS_MASK (OBJ_HASH_BUCKETS-1)
struct tmem_pool {
@@ -98,19 +98,19 @@ struct tmem_pool {
bool_t persistent;
bool_t is_dying;
struct client *client;
- uint64_t uuid[2]; /* 0 for private, non-zero for shared */
+ uint64_t uuid[2]; /* 0 for private, non-zero for shared. */
uint32_t pool_id;
rwlock_t pool_rwlock;
- struct rb_root obj_rb_root[OBJ_HASH_BUCKETS]; /* protected by pool_rwlock */
- struct list_head share_list; /* valid if shared */
- int shared_count; /* valid if shared */
- /* for save/restore/migration */
+ struct rb_root obj_rb_root[OBJ_HASH_BUCKETS]; /* Protected by pool_rwlock. */
+ struct list_head share_list; /* Valid if shared. */
+ int shared_count; /* Valid if shared. */
+ /* For save/restore/migration. */
struct list_head persistent_page_list;
struct tmem_page_descriptor *cur_pgp;
- /* statistics collection */
+ /* Statistics collection. */
atomic_t pgp_count;
int pgp_count_max;
- long obj_count; /* atomicity depends on pool_rwlock held for write */
+ long obj_count; /* Atomicity depends on pool_rwlock held for write. */
long obj_count_max;
unsigned long objnode_count, objnode_count_max;
uint64_t sum_life_cycles;
@@ -127,10 +127,10 @@ struct tmem_pool {
struct tmem_object_root {
struct tmem_oid oid;
- struct rb_node rb_tree_node; /* protected by pool->pool_rwlock */
- unsigned long objnode_count; /* atomicity depends on obj_spinlock */
- long pgp_count; /* atomicity depends on obj_spinlock */
- struct radix_tree_root tree_root; /* tree of pages within object */
+ struct rb_node rb_tree_node; /* Protected by pool->pool_rwlock. */
+ unsigned long objnode_count; /* Atomicity depends on obj_spinlock. */
+ long pgp_count; /* Atomicity depends on obj_spinlock. */
+ struct radix_tree_root tree_root; /* Tree of pages within object. */
struct tmem_pool *pool;
domid_t last_client;
spinlock_t obj_spinlock;
@@ -154,23 +154,23 @@ struct tmem_page_descriptor {
};
struct tmem_object_root *obj;
} us;
- struct tmem_oid inv_oid; /* used for invalid list only */
+ struct tmem_oid inv_oid; /* Used for invalid list only. */
};
pagesize_t size; /* 0 == PAGE_SIZE (pfp), -1 == data invalid,
- else compressed data (cdata) */
+ else compressed data (cdata). */
uint32_t index;
- /* must hold pcd_tree_rwlocks[firstbyte] to use pcd pointer/siblings */
- uint16_t firstbyte; /* NON_SHAREABLE->pfp otherwise->pcd */
- bool_t eviction_attempted; /* CHANGE TO lifetimes? (settable) */
+ /* Must hold pcd_tree_rwlocks[firstbyte] to use pcd pointer/siblings. */
+ uint16_t firstbyte; /* NON_SHAREABLE->pfp otherwise->pcd. */
+ bool_t eviction_attempted; /* CHANGE TO lifetimes? (settable). */
struct list_head pcd_siblings;
union {
- struct page_info *pfp; /* page frame pointer */
- char *cdata; /* compressed data */
- struct tmem_page_content_descriptor *pcd; /* page dedup */
+ struct page_info *pfp; /* Page frame pointer. */
+ char *cdata; /* Compressed data. */
+ struct tmem_page_content_descriptor *pcd; /* Page dedup. */
};
union {
uint64_t timestamp;
- uint32_t pool_id; /* used for invalid list only */
+ uint32_t pool_id; /* Used for invalid list only. */
};
};
@@ -178,21 +178,21 @@ struct tmem_page_descriptor {
struct tmem_page_content_descriptor {
union {
- struct page_info *pfp; /* page frame pointer */
- char *cdata; /* if compression_enabled */
- char *tze; /* if !compression_enabled, trailing zeroes eliminated */
+ struct page_info *pfp; /* Page frame pointer. */
+ char *cdata; /* If compression_enabled. */
+ char *tze; /* If !compression_enabled, trailing zeroes eliminated. */
};
struct list_head pgp_list;
struct rb_node pcd_rb_tree_node;
uint32_t pgp_ref_count;
- pagesize_t size; /* if compression_enabled -> 0<size<PAGE_SIZE (*cdata)
+ pagesize_t size; /* If compression_enabled -> 0<size<PAGE_SIZE (*cdata)
* else if tze, 0<=size<PAGE_SIZE, rounded up to mult of 8
- * else PAGE_SIZE -> *pfp */
+ * else PAGE_SIZE -> *pfp. */
};
-struct rb_root pcd_tree_roots[256]; /* choose based on first byte of page */
-rwlock_t pcd_tree_rwlocks[256]; /* poor man's concurrency for now */
+struct rb_root pcd_tree_roots[256]; /* Choose based on first byte of page. */
+rwlock_t pcd_tree_rwlocks[256]; /* Poor man's concurrency for now. */
-static LIST_HEAD(global_ephemeral_page_list); /* all pages in ephemeral pools */
+static LIST_HEAD(global_ephemeral_page_list); /* All pages in ephemeral pools. */
static LIST_HEAD(global_client_list);
@@ -209,14 +209,14 @@ PAGE_LIST_HEAD(tmem_page_list);
unsigned long tmem_page_list_pages = 0;
DEFINE_RWLOCK(tmem_rwlock);
-static DEFINE_SPINLOCK(eph_lists_spinlock); /* protects global AND clients */
+static DEFINE_SPINLOCK(eph_lists_spinlock); /* Protects global AND clients. */
static DEFINE_SPINLOCK(pers_lists_spinlock);
#define ASSERT_SPINLOCK(_l) ASSERT(spin_is_locked(_l))
#define ASSERT_WRITELOCK(_l) ASSERT(rw_is_write_locked(_l))
-/* global counters (should use long_atomic_t access) */
-static long global_eph_count = 0; /* atomicity depends on eph_lists_spinlock */
+/* Global counters (should use long_atomic_t access). */
+static long global_eph_count = 0; /* Atomicity depends on eph_lists_spinlock. */
static atomic_t global_obj_count = ATOMIC_INIT(0);
static atomic_t global_pgp_count = ATOMIC_INIT(0);
static atomic_t global_pcd_count = ATOMIC_INIT(0);
@@ -341,7 +341,7 @@ static int __init tmem_mempool_init(void)
return tmem_mempool != NULL;
}
-/* persistent pools are per-domain */
+/* Persistent pools are per-domain. */
static void *tmem_persistent_pool_page_get(unsigned long size)
{
struct page_info *pi;
@@ -365,7 +365,7 @@ static void tmem_persistent_pool_page_put(void *page_va)
}
/*
- * Page content descriptor manipulation routines
+ * Page content descriptor manipulation routines.
*/
#define NOT_SHAREABLE ((uint16_t)-1UL)
@@ -390,8 +390,10 @@ static int pcd_copy_to_client(xen_pfn_t cmfn, struct tmem_page_descriptor *pgp)
return ret;
}
-/* ensure pgp no longer points to pcd, nor vice-versa */
-/* take pcd rwlock unless have_pcd_rwlock is set, always unlock when done */
+/*
+ * Ensure pgp no longer points to pcd, nor vice-versa.
+ * Take pcd rwlock unless have_pcd_rwlock is set, always unlock when done.
+ */
static void pcd_disassociate(struct tmem_page_descriptor *pgp, struct tmem_pool *pool, bool_t have_pcd_rwlock)
{
struct tmem_page_content_descriptor *pcd = pgp->pcd;
@@ -421,30 +423,30 @@ static void pcd_disassociate(struct tmem_page_descriptor *pgp, struct tmem_pool
return;
}
- /* no more references to this pcd, recycle it and the physical page */
+ /* No more references to this pcd, recycle it and the physical page. */
ASSERT(list_empty(&pcd->pgp_list));
pcd->pfp = NULL;
- /* remove pcd from rbtree */
+ /* Remove pcd from rbtree. */
rb_erase(&pcd->pcd_rb_tree_node,&pcd_tree_roots[firstbyte]);
- /* reinit the struct for safety for now */
+ /* Reinit the struct for safety for now. */
RB_CLEAR_NODE(&pcd->pcd_rb_tree_node);
- /* now free up the pcd memory */
+ /* Now free up the pcd memory. */
tmem_free(pcd, NULL);
atomic_dec_and_assert(global_pcd_count);
if ( pgp_size != 0 && pcd_size < PAGE_SIZE )
{
- /* compressed data */
+ /* Compressed data. */
tmem_free(pcd_cdata, pool);
pcd_tot_csize -= pcd_csize;
}
else if ( pcd_size != PAGE_SIZE )
{
- /* trailing zero data */
+ /* Trailing zero data. */
pcd_tot_tze_size -= pcd_size;
if ( pcd_size )
tmem_free(pcd_tze, pool);
} else {
- /* real physical page */
+ /* Real physical page. */
if ( tmem_tze_enabled() )
pcd_tot_tze_size -= PAGE_SIZE;
if ( tmem_compression_enabled() )
@@ -485,48 +487,50 @@ static int pcd_associate(struct tmem_page_descriptor *pgp, char *cdata, pagesize
}
write_lock(&pcd_tree_rwlocks[firstbyte]);
- /* look for page match */
+ /* Look for page match. */
root = &pcd_tree_roots[firstbyte];
new = &(root->rb_node);
while ( *new )
{
pcd = container_of(*new, struct tmem_page_content_descriptor, pcd_rb_tree_node);
parent = *new;
- /* compare new entry and rbtree entry, set cmp accordingly */
+ /* Compare new entry and rbtree entry, set cmp accordingly. */
if ( cdata != NULL )
{
if ( pcd->size < PAGE_SIZE )
- /* both new entry and rbtree entry are compressed */
+ /* Both new entry and rbtree entry are compressed. */
cmp = tmem_pcd_cmp(cdata,csize,pcd->cdata,pcd->size);
else
- /* new entry is compressed, rbtree entry is not */
+ /* New entry is compressed, rbtree entry is not. */
cmp = -1;
} else if ( pcd->size < PAGE_SIZE )
- /* rbtree entry is compressed, rbtree entry is not */
+ /* Rbtree entry is compressed, rbtree entry is not. */
cmp = 1;
else if ( tmem_tze_enabled() ) {
if ( pcd->size < PAGE_SIZE )
- /* both new entry and rbtree entry are trailing zero */
+ /* Both new entry and rbtree entry are trailing zero. */
cmp = tmem_tze_pfp_cmp(pgp->pfp,pfp_size,pcd->tze,pcd->size);
else
- /* new entry is trailing zero, rbtree entry is not */
+ /* New entry is trailing zero, rbtree entry is not. */
cmp = tmem_tze_pfp_cmp(pgp->pfp,pfp_size,pcd->pfp,PAGE_SIZE);
} else {
- /* both new entry and rbtree entry are full physical pages */
+ /* Both new entry and rbtree entry are full physical pages. */
ASSERT(pgp->pfp != NULL);
ASSERT(pcd->pfp != NULL);
cmp = tmem_page_cmp(pgp->pfp,pcd->pfp);
}
- /* walk tree or match depending on cmp */
+ /* Walk tree or match depending on cmp. */
if ( cmp < 0 )
new = &((*new)->rb_left);
else if ( cmp > 0 )
new = &((*new)->rb_right);
else
{
- /* match! if not compressed, free the no-longer-needed page */
- /* but if compressed, data is assumed static so don't free! */
+ /*
+ * Match! if not compressed, free the no-longer-needed page
+ * but if compressed, data is assumed static so don't free!
+ */
if ( cdata == NULL )
tmem_free_page(pgp->us.obj->pool,pgp->pfp);
deduped_puts++;
@@ -534,7 +538,7 @@ static int pcd_associate(struct tmem_page_descriptor *pgp, char *cdata, pagesize
}
}
- /* exited while loop with no match, so alloc a pcd and put it in the tree */
+ /* Exited while loop with no match, so alloc a pcd and put it in the tree. */
if ( (pcd = tmem_malloc(sizeof(struct tmem_page_content_descriptor), NULL)) == NULL )
{
ret = -ENOMEM;
@@ -548,8 +552,8 @@ static int pcd_associate(struct tmem_page_descriptor *pgp, char *cdata, pagesize
}
}
atomic_inc_and_max(global_pcd_count);
- RB_CLEAR_NODE(&pcd->pcd_rb_tree_node); /* is this necessary */
- INIT_LIST_HEAD(&pcd->pgp_list); /* is this necessary */
+ RB_CLEAR_NODE(&pcd->pcd_rb_tree_node); /* Is this necessary? */
+ INIT_LIST_HEAD(&pcd->pgp_list); /* Is this necessary? */
pcd->pgp_ref_count = 0;
if ( cdata != NULL )
{
@@ -591,7 +595,7 @@ unlock:
/************ PAGE DESCRIPTOR MANIPULATION ROUTINES *******************/
-/* allocate a struct tmem_page_descriptor and associate it with an object */
+/* Allocate a struct tmem_page_descriptor and associate it with an object. */
static struct tmem_page_descriptor *pgp_alloc(struct tmem_object_root *obj)
{
struct tmem_page_descriptor *pgp;
@@ -635,7 +639,7 @@ static void pgp_free_data(struct tmem_page_descriptor *pgp, struct tmem_pool *po
if ( pgp->pfp == NULL )
return;
if ( tmem_dedup_enabled() && pgp->firstbyte != NOT_SHAREABLE )
- pcd_disassociate(pgp,pool,0); /* pgp->size lost */
+ pcd_disassociate(pgp,pool,0); /* pgp->size lost. */
else if ( pgp_size )
tmem_free(pgp->cdata, pool);
else
@@ -683,7 +687,7 @@ static void pgp_free(struct tmem_page_descriptor *pgp)
__pgp_free(pgp, pool);
}
-/* remove pgp from global/pool/client lists and free it */
+/* Remove pgp from global/pool/client lists and free it. */
static void pgp_delist_free(struct tmem_page_descriptor *pgp)
{
struct client *client;
@@ -695,7 +699,7 @@ static void pgp_delist_free(struct tmem_page_descriptor *pgp)
client = pgp->us.obj->pool->client;
ASSERT(client != NULL);
- /* Delist pgp */
+ /* Delist pgp. */
if ( !is_persistent(pgp->us.obj->pool) )
{
spin_lock(&eph_lists_spinlock);
@@ -730,11 +734,11 @@ static void pgp_delist_free(struct tmem_page_descriptor *pgp)
life = get_cycles() - pgp->timestamp;
pgp->us.obj->pool->sum_life_cycles += life;
- /* free pgp */
+ /* Free pgp. */
pgp_free(pgp);
}
-/* called only indirectly by radix_tree_destroy */
+/* Called only indirectly by radix_tree_destroy. */
static void pgp_destroy(void *v)
{
struct tmem_page_descriptor *pgp = (struct tmem_page_descriptor *)v;
@@ -771,7 +775,7 @@ static struct tmem_page_descriptor *pgp_delete_from_obj(struct tmem_object_root
/************ RADIX TREE NODE MANIPULATION ROUTINES *******************/
-/* called only indirectly from radix_tree_insert */
+/* Called only indirectly from radix_tree_insert. */
static struct radix_tree_node *rtn_alloc(void *arg)
{
struct tmem_object_node *objnode;
@@ -790,7 +794,7 @@ static struct radix_tree_node *rtn_alloc(void *arg)
return &objnode->rtn;
}
-/* called only indirectly from radix_tree_delete/destroy */
+/* Called only indirectly from radix_tree_delete/destroy. */
static void rtn_free(struct radix_tree_node *rtn, void *arg)
{
struct tmem_pool *pool;
@@ -847,7 +851,7 @@ static unsigned oid_hash(struct tmem_oid *oidp)
BITS_PER_LONG) & OBJ_HASH_BUCKETS_MASK);
}
-/* searches for object==oid in pool, returns locked object if found */
+/* Searches for object==oid in pool, returns locked object if found. */
static struct tmem_object_root * obj_find(struct tmem_pool *pool,
struct tmem_oid *oidp)
{
@@ -862,7 +866,7 @@ restart_find:
obj = container_of(node, struct tmem_object_root, rb_tree_node);
switch ( oid_compare(&obj->oid, oidp) )
{
- case 0: /* equal */
+ case 0: /* Equal. */
if ( !spin_trylock(&obj->obj_spinlock) )
{
read_unlock(&pool->pool_rwlock);
@@ -881,7 +885,7 @@ restart_find:
return NULL;
}
-/* free an object that has no more pgps in it */
+/* Free an object that has no more pgps in it. */
static void obj_free(struct tmem_object_root *obj)
{
struct tmem_pool *pool;
@@ -894,7 +898,7 @@ static void obj_free(struct tmem_object_root *obj)
ASSERT(pool != NULL);
ASSERT(pool->client != NULL);
ASSERT_WRITELOCK(&pool->pool_rwlock);
- if ( obj->tree_root.rnode != NULL ) /* may be a "stump" with no leaves */
+ if ( obj->tree_root.rnode != NULL ) /* May be a "stump" with no leaves. */
radix_tree_destroy(&obj->tree_root, pgp_destroy);
ASSERT((long)obj->objnode_count == 0);
ASSERT(obj->tree_root.rnode == NULL);
@@ -941,8 +945,8 @@ static int obj_rb_insert(struct rb_root *root, struct tmem_object_root *obj)
}
/*
- * allocate, initialize, and insert an tmem_object_root
- * (should be called only if find failed)
+ * Allocate, initialize, and insert an tmem_object_root
+ * (should be called only if find failed).
*/
static struct tmem_object_root * obj_alloc(struct tmem_pool *pool,
struct tmem_oid *oidp)
@@ -967,7 +971,7 @@ static struct tmem_object_root * obj_alloc(struct tmem_pool *pool,
return obj;
}
-/* free an object after destroying any pgps in it */
+/* Free an object after destroying any pgps in it. */
static void obj_destroy(struct tmem_object_root *obj)
{
ASSERT_WRITELOCK(&obj->pool->pool_rwlock);
@@ -975,7 +979,7 @@ static void obj_destroy(struct tmem_object_root *obj)
obj_free(obj);
}
-/* destroys all objs in a pool, or only if obj->last_client matches cli_id */
+/* Destroys all objs in a pool, or only if obj->last_client matches cli_id. */
static void pool_destroy_objs(struct tmem_pool *pool, domid_t cli_id)
{
struct rb_node *node;
@@ -1047,7 +1051,7 @@ static int shared_pool_join(struct tmem_pool *pool, struct client *new_client)
return 0;
}
-/* reassign "ownership" of the pool to another client that shares this pool */
+/* Reassign "ownership" of the pool to another client that shares this pool. */
static void shared_pool_reassign(struct tmem_pool *pool)
{
struct share_list *sl;
@@ -1080,8 +1084,10 @@ static void shared_pool_reassign(struct tmem_pool *pool)
pool->pool_id = poolid;
}
-/* destroy all objects with last_client same as passed cli_id,
- remove pool's cli_id from list of sharers of this pool */
+/*
+ * Destroy all objects with last_client same as passed cli_id,
+ * remove pool's cli_id from list of sharers of this pool.
+ */
static int shared_pool_quit(struct tmem_pool *pool, domid_t cli_id)
{
struct share_list *sl;
@@ -1116,7 +1122,7 @@ static int shared_pool_quit(struct tmem_pool *pool, domid_t cli_id)
return -1;
}
-/* flush all data (owned by cli_id) from a pool and, optionally, free it */
+/* Flush all data (owned by cli_id) from a pool and, optionally, free it. */
static void pool_flush(struct tmem_pool *pool, domid_t cli_id)
{
ASSERT(pool != NULL);
@@ -1205,7 +1211,7 @@ static void client_free(struct client *client)
xfree(client);
}
-/* flush all data from a client and, optionally, free it */
+/* Flush all data from a client and, optionally, free it. */
static void client_flush(struct client *client)
{
int i;
@@ -1307,12 +1313,12 @@ static int tmem_evict(void)
goto found;
}
}
- /* global_ephemeral_page_list is empty, so we bail out. */
+ /* Global_ephemeral_page_list is empty, so we bail out. */
spin_unlock(&eph_lists_spinlock);
goto out;
found:
- /* Delist */
+ /* Delist. */
list_del_init(&pgp->us.client_eph_pages);
client->eph_count--;
list_del_init(&pgp->global_eph_pages);
@@ -1336,7 +1342,7 @@ found:
pcd_disassociate(pgp,pool,1);
}
- /* pgp already delist, so call pgp_free directly */
+ /* pgp already delist, so call pgp_free directly. */
pgp_free(pgp);
if ( obj->pgp_count == 0 )
{
@@ -1464,8 +1470,8 @@ static int do_tmem_dup_put(struct tmem_page_descriptor *pgp, xen_pfn_t cmfn,
ASSERT(pool != NULL);
client = pool->client;
if ( client->live_migrating )
- goto failed_dup; /* no dups allowed when migrating */
- /* can we successfully manipulate pgp to change out the data? */
+ goto failed_dup; /* No dups allowed when migrating. */
+ /* Can we successfully manipulate pgp to change out the data? */
if ( client->compress && pgp->size != 0 )
{
ret = do_tmem_put_compress(pgp, cmfn, clibuf);
@@ -1495,7 +1501,7 @@ copy_uncompressed:
}
done:
- /* successfully replaced data, clean up and return success */
+ /* Successfully replaced data, clean up and return success. */
if ( is_shared(pool) )
obj->last_client = client->cli_id;
spin_unlock(&obj->obj_spinlock);
@@ -1510,8 +1516,10 @@ bad_copy:
goto cleanup;
failed_dup:
- /* couldn't change out the data, flush the old data and return
- * -ENOSPC instead of -ENOMEM to differentiate failed _dup_ put */
+ /*
+ * Couldn't change out the data, flush the old data and return
+ * -ENOSPC instead of -ENOMEM to differentiate failed _dup_ put.
+ */
ret = -ENOSPC;
cleanup:
pgpfound = pgp_delete_from_obj(obj, pgp->index);
@@ -1545,7 +1553,7 @@ static int do_tmem_put(struct tmem_pool *pool,
pool->puts++;
refind:
- /* does page already exist (dup)? if so, handle specially */
+ /* Does page already exist (dup)? if so, handle specially. */
if ( (obj = obj_find(pool, oidp)) != NULL )
{
if ((pgp = pgp_lookup_in_obj(obj, index)) != NULL)
@@ -1554,14 +1562,14 @@ refind:
}
else
{
- /* no puts allowed into a frozen pool (except dup puts) */
+ /* No puts allowed into a frozen pool (except dup puts). */
if ( client->frozen )
goto unlock_obj;
}
}
else
{
- /* no puts allowed into a frozen pool (except dup puts) */
+ /* No puts allowed into a frozen pool (except dup puts). */
if ( client->frozen )
return ret;
if ( (obj = obj_alloc(pool, oidp)) == NULL )
@@ -1584,14 +1592,14 @@ refind:
write_unlock(&pool->pool_rwlock);
}
- /* When arrive here, we have a spinlocked obj for use */
+ /* When arrive here, we have a spinlocked obj for use. */
ASSERT_SPINLOCK(&obj->obj_spinlock);
if ( (pgp = pgp_alloc(obj)) == NULL )
goto unlock_obj;
ret = pgp_add_to_obj(obj, index, pgp);
if ( ret == -ENOMEM )
- /* warning, may result in partially built radix tree ("stump") */
+ /* Warning: may result in partially built radix tree ("stump"). */
goto free_pgp;
pgp->index = index;
@@ -1651,7 +1659,7 @@ insert_page:
spin_unlock(&eph_lists_spinlock);
}
else
- { /* is_persistent */
+ { /* is_persistent. */
spin_lock(&pers_lists_spinlock);
list_add_tail(&pgp->us.pool_pers_pages,
&pool->persistent_page_list);
@@ -1661,7 +1669,7 @@ insert_page:
if ( is_shared(pool) )
obj->last_client = client->cli_id;
- /* free the obj spinlock */
+ /* Free the obj spinlock. */
spin_unlock(&obj->obj_spinlock);
pool->good_puts++;
@@ -1954,7 +1962,7 @@ static int do_tmem_new_pool(domid_t this_cli_id,
/*
* Authorize okay, match a global shared pool or use the newly allocated
- * one
+ * one.
*/
first_unused_s_poolid = MAX_GLOBAL_SHARED_POOLS;
for ( i = 0; i < MAX_GLOBAL_SHARED_POOLS; i++ )
@@ -1963,7 +1971,7 @@ static int do_tmem_new_pool(domid_t this_cli_id,
{
if ( shpool->uuid[0] == uuid_lo && shpool->uuid[1] == uuid_hi )
{
- /* Succ to match a global shared pool */
+ /* Succ to match a global shared pool. */
tmem_client_info("(matches shared pool uuid=%"PRIx64".%"PRIx64") pool_id=%d\n",
uuid_hi, uuid_lo, d_poolid);
client->pools[d_poolid] = shpool;
@@ -1983,13 +1991,13 @@ static int do_tmem_new_pool(domid_t this_cli_id,
}
}
- /* Failed to find a global shard pool slot */
+ /* Failed to find a global shared pool slot. */
if ( first_unused_s_poolid == MAX_GLOBAL_SHARED_POOLS )
{
tmem_client_warn("tmem: failed... no global shared pool slots available\n");
goto fail;
}
- /* Add pool to global shard pool */
+ /* Add pool to global shared pool. */
else
{
INIT_LIST_HEAD(&pool->share_list);
@@ -2011,7 +2019,7 @@ fail:
/************ TMEM CONTROL OPERATIONS ************************************/
-/* freeze/thaw all pools belonging to client cli_id (all domains if -1) */
+/* Freeze/thaw all pools belonging to client cli_id (all domains if -1). */
static int tmemc_freeze_pools(domid_t cli_id, int arg)
{
struct client *client;
@@ -2047,7 +2055,7 @@ static int tmemc_flush_mem(domid_t cli_id, uint32_t kb)
tmem_client_str);
return -1;
}
- /* convert kb to pages, rounding up if necessary */
+ /* Convert kb to pages, rounding up if necessary. */
npages = (kb + ((1 << (PAGE_SHIFT-10))-1)) >> (PAGE_SHIFT-10);
flushed_pages = tmem_flush_npages(npages);
flushed_kb = flushed_pages << (PAGE_SHIFT-10);
@@ -2164,7 +2172,7 @@ static int tmemc_list_global_perf(tmem_cli_va_param_t buf, int off,
int n = 0, sum = 0;
n = scnprintf(info+n,BSIZE-n,"T=");
- n--; /* overwrite trailing comma */
+ n--; /* Overwrite trailing comma. */
n += scnprintf(info+n,BSIZE-n,"\n");
if ( sum + n >= len )
return sum;
@@ -2450,16 +2458,16 @@ static int tmemc_save_get_next_page(int cli_id, uint32_t pool_id,
ret = -1;
goto out;
}
- /* note: pool->cur_pgp is the pgp last returned by get_next_page */
+ /* Note: pool->cur_pgp is the pgp last returned by get_next_page. */
if ( pool->cur_pgp == NULL )
{
- /* process the first one */
+ /* Process the first one. */
pool->cur_pgp = pgp = list_entry((&pool->persistent_page_list)->next,
struct tmem_page_descriptor,us.pool_pers_pages);
} else if ( list_is_last(&pool->cur_pgp->us.pool_pers_pages,
&pool->persistent_page_list) )
{
- /* already processed the last one in the list */
+ /* Already processed the last one in the list. */
ret = -1;
goto out;
}
@@ -2665,7 +2673,7 @@ long do_tmem_op(tmem_cli_op_t uops)
return -EFAULT;
}
- /* Acquire wirte lock for all command at first */
+ /* Acquire write lock for all commands at first. */
write_lock(&tmem_rwlock);
if ( op.cmd == TMEM_CONTROL )
@@ -2715,7 +2723,7 @@ long do_tmem_op(tmem_cli_op_t uops)
rc = -ENODEV;
goto out;
}
- /* Commands only need read lock */
+ /* Commands that only need read lock. */
write_unlock(&tmem_rwlock);
read_lock(&tmem_rwlock);
@@ -2767,7 +2775,7 @@ out:
return rc;
}
-/* this should be called when the host is destroying a client */
+/* This should be called when the host is destroying a client (domain). */
void tmem_destroy(void *v)
{
struct client *client = (struct client *)v;
@@ -2790,7 +2798,7 @@ void tmem_destroy(void *v)
write_unlock(&tmem_rwlock);
}
-#define MAX_EVICTS 10 /* should be variable or set via XEN_SYSCTL_TMEM_OP_ ?? */
+#define MAX_EVICTS 10 /* Should be variable or set via XEN_SYSCTL_TMEM_OP_ ?? */
void *tmem_relinquish_pages(unsigned int order, unsigned int memflags)
{
struct page_info *pfp;
@@ -2832,7 +2840,7 @@ unsigned long tmem_freeable_pages(void)
return tmem_page_list_pages + _atomic_read(freeable_page_count);
}
-/* called at hypervisor startup */
+/* Called at hypervisor startup. */
static int __init init_tmem(void)
{
int i;
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread