[PATCH v1 for 4.7] Tmem cleanups.

[PATCH v1 for 4.7] Tmem cleanups.

[Konrad Rzeszutek Wilk]

Hey!

Since RFC [http://www.gossamer-threads.com/lists/xen/devel/431812]
 - Added Reviewed-by from Doug
 - Dropped the RFC


These four little cleanups move the bulk of tmem control ops
from tmem.c to tmem_control.c.

Last release I moved the control tmem ops from being part of tmem
hypercall to be part of the syscall subops - and this is the next
step in this cleanup. (See
http://lists.xen.org/archives/html/xen-devel/2015-10/msg03313.html)
which will allow me to follow up on the other steps:
b) Fix the toolstack (cleanup)
c) tmem tze, dedup, and zlib code drop

Anyhow sorry for this being so tardy - xSplice had more attention :-)

Regression tests show no problems.

The patches themselves have no functionality changes thought I was itching
to remove most of the counters. I will do that going forward, but need
to figure out which ones make sense or if some of them can be coalesced. 


 xen/common/Makefile            |   2 +-
 xen/common/tmem.c              | 618 +++++------------------------------------
 xen/common/tmem_control.c      | 443 +++++++++++++++++++++++++++++
 xen/include/xen/tmem_control.h |  33 +++
 xen/include/xen/tmem_xen.h     | 128 +++++++++
 5 files changed, 672 insertions(+), 552 deletions(-)

Konrad Rzeszutek Wilk (4):
      tmem: Move global stats in a tmem_statistics structure
      tmem: Wrap atomic_t in struct tmem_statistics as well.
      tmem: Move global_ individual variables in a global structure.
      tmem: Move bulk of tmem control functions in its own file.

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

[PATCH v1 1/4] tmem: Move global stats in a tmem_statistics structure

[Konrad Rzeszutek Wilk]

And adjust the macro: atomic_inc_and_max to update the structure.

Sadly one entry: pool->pgp_count cannot use this macro anymore
so unroll the macro for this instance.

No functional change. The name has the 'tmem_stats' as it will
be eventually non-local.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

---
Cc:  Andrew Cooper <andrew.cooper3@citrix.com>
Cc:  Jan Beulich <jbeulich@suse.com>
Cc:  Wei Liu <wei.liu2@citrix.com>
Cc:  Doug Goldstein <cardoe@cardoe.com>

v1: Posting as non-RFC
---
 xen/common/tmem.c | 135 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 73 insertions(+), 62 deletions(-)

diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index 16e249a..d362eae 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -28,25 +28,32 @@
 #define TMEM_SPEC_VERSION 1
 
 /* 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;
-static unsigned long alloc_failed = 0, alloc_page_failed = 0;
-static unsigned long evicted_pgs = 0, evict_attempts = 0;
-static unsigned long relinq_pgs = 0, relinq_attempts = 0;
-static unsigned long max_evicts_per_relinq = 0;
-static unsigned long low_on_memory = 0;
-static unsigned long deduped_puts = 0;
-static unsigned long tot_good_eph_puts = 0;
-static int global_obj_count_max = 0;
-static int global_pgp_count_max = 0;
-static int global_pcd_count_max = 0;
-static int global_page_count_max = 0;
-static int global_rtree_node_count_max = 0;
-static long global_eph_count_max = 0;
-static unsigned long failed_copies;
-static unsigned long pcd_tot_tze_size = 0;
-static unsigned long pcd_tot_csize = 0;
+struct tmem_statistics {
+    unsigned long total_tmem_ops;
+    unsigned long errored_tmem_ops;
+    unsigned long total_flush_pool;
+    unsigned long alloc_failed;
+    unsigned long alloc_page_failed;
+    unsigned long evicted_pgs;
+    unsigned long evict_attempts;
+    unsigned long relinq_pgs;
+    unsigned long relinq_attempts;
+    unsigned long max_evicts_per_relinq;
+    unsigned long low_on_memory;
+    unsigned long deduped_puts;
+    unsigned long tot_good_eph_puts;
+    int global_obj_count_max;
+    int global_pgp_count_max;
+    int global_pcd_count_max;
+    int global_page_count_max;
+    int global_rtree_node_count_max;
+    long global_eph_count_max;
+    unsigned long failed_copies;
+    unsigned long pcd_tot_tze_size;
+    unsigned long pcd_tot_csize;
+};
+
+static struct tmem_statistics tmem_stats;
 
 /************ CORE DATA STRUCTURES ************************************/
 
@@ -225,8 +232,8 @@ static atomic_t global_rtree_node_count = ATOMIC_INIT(0);
 
 #define atomic_inc_and_max(_c) do { \
     atomic_inc(&_c); \
-    if ( _atomic_read(_c) > _c##_max ) \
-        _c##_max = _atomic_read(_c); \
+    if ( _atomic_read(_c) > tmem_stats._c##_max ) \
+        tmem_stats._c##_max = _atomic_read(_c); \
 } while (0)
 
 #define atomic_dec_and_assert(_c) do { \
@@ -273,7 +280,7 @@ static void *tmem_malloc(size_t size, struct tmem_pool *pool)
         v = xmem_pool_alloc(size, tmem_mempool);
     }
     if ( v == NULL )
-        alloc_failed++;
+        tmem_stats.alloc_failed++;
     return v;
 }
 
@@ -300,7 +307,7 @@ static struct page_info *tmem_alloc_page(struct tmem_pool *pool)
     else
         pfp = __tmem_alloc_page();
     if ( pfp == NULL )
-        alloc_page_failed++;
+        tmem_stats.alloc_page_failed++;
     else
         atomic_inc_and_max(global_page_count);
     return pfp;
@@ -437,20 +444,20 @@ static void pcd_disassociate(struct tmem_page_descriptor *pgp, struct tmem_pool
     {
         /* Compressed data. */
         tmem_free(pcd_cdata, pool);
-        pcd_tot_csize -= pcd_csize;
+        tmem_stats.pcd_tot_csize -= pcd_csize;
     }
     else if ( pcd_size != PAGE_SIZE )
     {
         /* Trailing zero data. */
-        pcd_tot_tze_size -= pcd_size;
+        tmem_stats.pcd_tot_tze_size -= pcd_size;
         if ( pcd_size )
             tmem_free(pcd_tze, pool);
     } else {
         /* Real physical page. */
         if ( tmem_tze_enabled() )
-            pcd_tot_tze_size -= PAGE_SIZE;
+            tmem_stats.pcd_tot_tze_size -= PAGE_SIZE;
         if ( tmem_compression_enabled() )
-            pcd_tot_csize -= PAGE_SIZE;
+            tmem_stats.pcd_tot_csize -= PAGE_SIZE;
         tmem_free_page(pool,pfp);
     }
     write_unlock(&pcd_tree_rwlocks[firstbyte]);
@@ -533,7 +540,7 @@ static int pcd_associate(struct tmem_page_descriptor *pgp, char *cdata, pagesize
              */
             if ( cdata == NULL )
                 tmem_free_page(pgp->us.obj->pool,pgp->pfp);
-            deduped_puts++;
+            tmem_stats.deduped_puts++;
             goto match;
         }
     }
@@ -559,7 +566,7 @@ static int pcd_associate(struct tmem_page_descriptor *pgp, char *cdata, pagesize
     {
         memcpy(pcd->cdata,cdata,csize);
         pcd->size = csize;
-        pcd_tot_csize += csize;
+        tmem_stats.pcd_tot_csize += csize;
     } else if ( pfp_size == 0 ) {
         ASSERT(tmem_tze_enabled());
         pcd->size = 0;
@@ -568,15 +575,15 @@ static int pcd_associate(struct tmem_page_descriptor *pgp, char *cdata, pagesize
          ((pcd->tze = tmem_malloc(pfp_size,pgp->us.obj->pool)) != NULL) ) {
         tmem_tze_copy_from_pfp(pcd->tze,pgp->pfp,pfp_size);
         pcd->size = pfp_size;
-        pcd_tot_tze_size += pfp_size;
+        tmem_stats.pcd_tot_tze_size += pfp_size;
         tmem_free_page(pgp->us.obj->pool,pgp->pfp);
     } else {
         pcd->pfp = pgp->pfp;
         pcd->size = PAGE_SIZE;
         if ( tmem_tze_enabled() )
-            pcd_tot_tze_size += PAGE_SIZE;
+            tmem_stats.pcd_tot_tze_size += PAGE_SIZE;
         if ( tmem_compression_enabled() )
-            pcd_tot_csize += PAGE_SIZE;
+            tmem_stats.pcd_tot_csize += PAGE_SIZE;
     }
     rb_link_node(&pcd->pcd_rb_tree_node, parent, new);
     rb_insert_color(&pcd->pcd_rb_tree_node, root);
@@ -620,7 +627,9 @@ static struct tmem_page_descriptor *pgp_alloc(struct tmem_object_root *obj)
     pgp->index = -1;
     pgp->timestamp = get_cycles();
     atomic_inc_and_max(global_pgp_count);
-    atomic_inc_and_max(pool->pgp_count);
+    atomic_inc(&pool->pgp_count);
+    if ( _atomic_read(pool->pgp_count) > pool->pgp_count_max )
+        pool->pgp_count_max = _atomic_read(pool->pgp_count);
     return pgp;
 }
 
@@ -1295,7 +1304,7 @@ static int tmem_evict(void)
     int ret = 0;
     bool_t hold_pool_rwlock = 0;
 
-    evict_attempts++;
+    tmem_stats.evict_attempts++;
     spin_lock(&eph_lists_spinlock);
     if ( (client != NULL) && client_over_quota(client) &&
          !list_empty(&client->ephemeral_page_list) )
@@ -1353,7 +1362,7 @@ found:
         spin_unlock(&obj->obj_spinlock);
     if ( hold_pool_rwlock )
         write_unlock(&pool->pool_rwlock);
-    evicted_pgs++;
+    tmem_stats.evicted_pgs++;
     ret = 1;
 out:
     return ret;
@@ -1512,7 +1521,7 @@ done:
     return 1;
 
 bad_copy:
-    failed_copies++;
+    tmem_stats.failed_copies++;
     goto cleanup;
 
 failed_dup:
@@ -1650,8 +1659,8 @@ insert_page:
         spin_lock(&eph_lists_spinlock);
         list_add_tail(&pgp->global_eph_pages,
             &global_ephemeral_page_list);
-        if (++global_eph_count > global_eph_count_max)
-            global_eph_count_max = global_eph_count;
+        if (++global_eph_count > tmem_stats.global_eph_count_max)
+            tmem_stats.global_eph_count_max = global_eph_count;
         list_add_tail(&pgp->us.client_eph_pages,
             &client->ephemeral_page_list);
         if (++client->eph_count > client->eph_count_max)
@@ -1676,11 +1685,11 @@ insert_page:
     if ( is_persistent(pool) )
         client->succ_pers_puts++;
     else
-        tot_good_eph_puts++;
+        tmem_stats.tot_good_eph_puts++;
     return 1;
 
 bad_copy:
-    failed_copies++;
+    tmem_stats.failed_copies++;
 
 del_pgp_from_obj:
     ASSERT((obj != NULL) && (pgp != NULL) && (pgp->index != -1));
@@ -1778,7 +1787,7 @@ static int do_tmem_get(struct tmem_pool *pool,
 
 bad_copy:
     spin_unlock(&obj->obj_spinlock);
-    failed_copies++;
+    tmem_stats.failed_copies++;
     return rc;
 }
 
@@ -2190,22 +2199,24 @@ static int tmemc_list_global(tmem_cli_va_param_t buf, int off, uint32_t len,
     n += scnprintf(info,BSIZE,"G="
       "Tt:%lu,Te:%lu,Cf:%lu,Af:%lu,Pf:%lu,Ta:%lu,"
       "Lm:%lu,Et:%lu,Ea:%lu,Rt:%lu,Ra:%lu,Rx:%lu,Fp:%lu%c",
-      total_tmem_ops, errored_tmem_ops, failed_copies,
-      alloc_failed, alloc_page_failed, tmem_page_list_pages,
-      low_on_memory, evicted_pgs,
-      evict_attempts, relinq_pgs, relinq_attempts, max_evicts_per_relinq,
-      total_flush_pool, use_long ? ',' : '\n');
+      tmem_stats.total_tmem_ops, tmem_stats.errored_tmem_ops, tmem_stats.failed_copies,
+      tmem_stats.alloc_failed, tmem_stats.alloc_page_failed, tmem_page_list_pages,
+      tmem_stats.low_on_memory, tmem_stats.evicted_pgs,
+      tmem_stats.evict_attempts, tmem_stats.relinq_pgs, tmem_stats.relinq_attempts,
+      tmem_stats.max_evicts_per_relinq,
+      tmem_stats.total_flush_pool, use_long ? ',' : '\n');
     if (use_long)
         n += scnprintf(info+n,BSIZE-n,
           "Ec:%ld,Em:%ld,Oc:%d,Om:%d,Nc:%d,Nm:%d,Pc:%d,Pm:%d,"
           "Fc:%d,Fm:%d,Sc:%d,Sm:%d,Ep:%lu,Gd:%lu,Zt:%lu,Gz:%lu\n",
-          global_eph_count, global_eph_count_max,
-          _atomic_read(global_obj_count), global_obj_count_max,
-          _atomic_read(global_rtree_node_count), global_rtree_node_count_max,
-          _atomic_read(global_pgp_count), global_pgp_count_max,
-          _atomic_read(global_page_count), global_page_count_max,
-          _atomic_read(global_pcd_count), global_pcd_count_max,
-         tot_good_eph_puts,deduped_puts,pcd_tot_tze_size,pcd_tot_csize);
+          global_eph_count, tmem_stats.global_eph_count_max,
+          _atomic_read(global_obj_count), tmem_stats.global_obj_count_max,
+          _atomic_read(global_rtree_node_count), tmem_stats.global_rtree_node_count_max,
+          _atomic_read(global_pgp_count), tmem_stats.global_pgp_count_max,
+          _atomic_read(global_page_count), tmem_stats.global_page_count_max,
+          _atomic_read(global_pcd_count), tmem_stats.global_pcd_count_max,
+         tmem_stats.tot_good_eph_puts,tmem_stats.deduped_puts,tmem_stats.pcd_tot_tze_size,
+         tmem_stats.pcd_tot_csize);
     if ( sum + n >= len )
         return sum;
     if ( !copy_to_guest_offset(buf, off + sum, info, n + 1) )
@@ -2658,18 +2669,18 @@ long do_tmem_op(tmem_cli_op_t uops)
     if ( xsm_tmem_op(XSM_HOOK) )
         return -EPERM;
 
-    total_tmem_ops++;
+    tmem_stats.total_tmem_ops++;
 
     if ( client != NULL && client->domain->is_dying )
     {
-        errored_tmem_ops++;
+        tmem_stats.errored_tmem_ops++;
         return -ENODEV;
     }
 
     if ( unlikely(tmem_get_tmemop_from_client(&op, uops) != 0) )
     {
         tmem_client_err("tmem: can't get tmem struct from %s\n", tmem_client_str);
-        errored_tmem_ops++;
+        tmem_stats.errored_tmem_ops++;
         return -EFAULT;
     }
 
@@ -2764,14 +2775,14 @@ long do_tmem_op(tmem_cli_op_t uops)
             }
             read_unlock(&tmem_rwlock);
             if ( rc < 0 )
-                errored_tmem_ops++;
+                tmem_stats.errored_tmem_ops++;
             return rc;
         }
     }
 out:
     write_unlock(&tmem_rwlock);
     if ( rc < 0 )
-        errored_tmem_ops++;
+        tmem_stats.errored_tmem_ops++;
     return rc;
 }
 
@@ -2808,7 +2819,7 @@ void *tmem_relinquish_pages(unsigned int order, unsigned int memflags)
     if (!tmem_enabled() || !tmem_freeable_pages())
         return NULL;
 
-    relinq_attempts++;
+    tmem_stats.relinq_attempts++;
     if ( order > 0 )
     {
 #ifndef NDEBUG
@@ -2823,13 +2834,13 @@ void *tmem_relinquish_pages(unsigned int order, unsigned int memflags)
             break;
         evicts_per_relinq++;
     }
-    if ( evicts_per_relinq > max_evicts_per_relinq )
-        max_evicts_per_relinq = evicts_per_relinq;
+    if ( evicts_per_relinq > tmem_stats.max_evicts_per_relinq )
+        tmem_stats.max_evicts_per_relinq = evicts_per_relinq;
     if ( pfp != NULL )
     {
         if ( !(memflags & MEMF_tmem) )
             scrub_one_page(pfp);
-        relinq_pgs++;
+        tmem_stats.relinq_pgs++;
     }
 
     return pfp;
-- 
2.5.5


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

[PATCH v1 2/4] tmem: Wrap atomic_t in struct tmem_statistics as well.

[Konrad Rzeszutek Wilk]

The macros: atomic_inc_and_max and atomic_dec_and_assert
use also the 'stats' to access them. Had to open-code
access to pool->pgp_count as it would not work anymore.

No functional change.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

---
Cc:  Andrew Cooper <andrew.cooper3@citrix.com>
Cc:  Jan Beulich <jbeulich@suse.com>
Cc:  Wei Liu <wei.liu2@citrix.com>
Cc:  Doug Goldstein <cardoe@cardoe.com>

v1: First posting as non-RFC
---
 xen/common/tmem.c | 57 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index d362eae..b58ab4d 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -51,9 +51,32 @@ struct tmem_statistics {
     unsigned long failed_copies;
     unsigned long pcd_tot_tze_size;
     unsigned long pcd_tot_csize;
+    /* Global counters (should use long_atomic_t access). */
+    atomic_t global_obj_count;
+    atomic_t global_pgp_count;
+    atomic_t global_pcd_count;
+    atomic_t global_page_count;
+    atomic_t global_rtree_node_count;
 };
 
-static struct tmem_statistics tmem_stats;
+#define atomic_inc_and_max(_c) do { \
+    atomic_inc(&tmem_stats._c); \
+    if ( _atomic_read(tmem_stats._c) > tmem_stats._c##_max ) \
+        tmem_stats._c##_max = _atomic_read(tmem_stats._c); \
+} while (0)
+
+#define atomic_dec_and_assert(_c) do { \
+    atomic_dec(&tmem_stats._c); \
+    ASSERT(_atomic_read(tmem_stats._c) >= 0); \
+} while (0)
+
+static struct tmem_statistics tmem_stats = {
+    .global_obj_count = ATOMIC_INIT(0),
+    .global_pgp_count = ATOMIC_INIT(0),
+    .global_pcd_count = ATOMIC_INIT(0),
+    .global_page_count = ATOMIC_INIT(0),
+    .global_rtree_node_count = ATOMIC_INIT(0),
+};
 
 /************ CORE DATA STRUCTURES ************************************/
 
@@ -222,24 +245,7 @@ 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. */
-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);
-static atomic_t global_page_count = ATOMIC_INIT(0);
-static atomic_t global_rtree_node_count = ATOMIC_INIT(0);
-
-#define atomic_inc_and_max(_c) do { \
-    atomic_inc(&_c); \
-    if ( _atomic_read(_c) > tmem_stats._c##_max ) \
-        tmem_stats._c##_max = _atomic_read(_c); \
-} while (0)
-
-#define atomic_dec_and_assert(_c) do { \
-    atomic_dec(&_c); \
-    ASSERT(_atomic_read(_c) >= 0); \
-} while (0)
+static long global_eph_count; /* Atomicity depends on eph_lists_spinlock. */
 
 
 /*
@@ -685,7 +691,8 @@ static void pgp_free(struct tmem_page_descriptor *pgp)
     }
     pgp_free_data(pgp, pool);
     atomic_dec_and_assert(global_pgp_count);
-    atomic_dec_and_assert(pool->pgp_count);
+    atomic_dec(&pool->pgp_count);
+    ASSERT(_atomic_read(pool->pgp_count) >= 0);
     pgp->size = -1;
     if ( is_persistent(pool) && pool->client->live_migrating )
     {
@@ -2210,11 +2217,11 @@ static int tmemc_list_global(tmem_cli_va_param_t buf, int off, uint32_t len,
           "Ec:%ld,Em:%ld,Oc:%d,Om:%d,Nc:%d,Nm:%d,Pc:%d,Pm:%d,"
           "Fc:%d,Fm:%d,Sc:%d,Sm:%d,Ep:%lu,Gd:%lu,Zt:%lu,Gz:%lu\n",
           global_eph_count, tmem_stats.global_eph_count_max,
-          _atomic_read(global_obj_count), tmem_stats.global_obj_count_max,
-          _atomic_read(global_rtree_node_count), tmem_stats.global_rtree_node_count_max,
-          _atomic_read(global_pgp_count), tmem_stats.global_pgp_count_max,
-          _atomic_read(global_page_count), tmem_stats.global_page_count_max,
-          _atomic_read(global_pcd_count), tmem_stats.global_pcd_count_max,
+          _atomic_read(tmem_stats.global_obj_count), tmem_stats.global_obj_count_max,
+          _atomic_read(tmem_stats.global_rtree_node_count), tmem_stats.global_rtree_node_count_max,
+          _atomic_read(tmem_stats.global_pgp_count), tmem_stats.global_pgp_count_max,
+          _atomic_read(tmem_stats.global_page_count), tmem_stats.global_page_count_max,
+          _atomic_read(tmem_stats.global_pcd_count), tmem_stats.global_pcd_count_max,
          tmem_stats.tot_good_eph_puts,tmem_stats.deduped_puts,tmem_stats.pcd_tot_tze_size,
          tmem_stats.pcd_tot_csize);
     if ( sum + n >= len )
-- 
2.5.5


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

Re: [PATCH v1 2/4] tmem: Wrap atomic_t in struct tmem_statistics as well.

[Andrew Cooper]

On 02/06/16 16:04, Konrad Rzeszutek Wilk wrote:
> diff --git a/xen/common/tmem.c b/xen/common/tmem.c
> index d362eae..b58ab4d 100644
> --- a/xen/common/tmem.c
> +++ b/xen/common/tmem.c
> @@ -51,9 +51,32 @@ struct tmem_statistics {
>      unsigned long failed_copies;
>      unsigned long pcd_tot_tze_size;
>      unsigned long pcd_tot_csize;
> +    /* Global counters (should use long_atomic_t access). */
> +    atomic_t global_obj_count;
> +    atomic_t global_pgp_count;
> +    atomic_t global_pcd_count;
> +    atomic_t global_page_count;
> +    atomic_t global_rtree_node_count;
>  };
>  
> -static struct tmem_statistics tmem_stats;
> +#define atomic_inc_and_max(_c) do { \
> +    atomic_inc(&tmem_stats._c); \
> +    if ( _atomic_read(tmem_stats._c) > tmem_stats._c##_max ) \
> +        tmem_stats._c##_max = _atomic_read(tmem_stats._c); \
> +} while (0)
> +
> +#define atomic_dec_and_assert(_c) do { \
> +    atomic_dec(&tmem_stats._c); \
> +    ASSERT(_atomic_read(tmem_stats._c) >= 0); \
> +} while (0)

I know you are just modifying existing code (thus this complain doesn't
count against the patch), but these constructs are racy and broken.

You need to use atomic_{inc,dec}_and_test() for these to function as
intended.

Up to you whether you want to fix that in this patch, or a subsequent
non-cleanup patch.

~Andrew

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

Re: [PATCH v1 2/4] tmem: Wrap atomic_t in struct tmem_statistics as well.

[Konrad Rzeszutek Wilk]

On Thu, Jun 02, 2016 at 04:16:51PM +0100, Andrew Cooper wrote:
> On 02/06/16 16:04, Konrad Rzeszutek Wilk wrote:
> > diff --git a/xen/common/tmem.c b/xen/common/tmem.c
> > index d362eae..b58ab4d 100644
> > --- a/xen/common/tmem.c
> > +++ b/xen/common/tmem.c
> > @@ -51,9 +51,32 @@ struct tmem_statistics {
> >      unsigned long failed_copies;
> >      unsigned long pcd_tot_tze_size;
> >      unsigned long pcd_tot_csize;
> > +    /* Global counters (should use long_atomic_t access). */
> > +    atomic_t global_obj_count;
> > +    atomic_t global_pgp_count;
> > +    atomic_t global_pcd_count;
> > +    atomic_t global_page_count;
> > +    atomic_t global_rtree_node_count;
> >  };
> >  
> > -static struct tmem_statistics tmem_stats;
> > +#define atomic_inc_and_max(_c) do { \
> > +    atomic_inc(&tmem_stats._c); \
> > +    if ( _atomic_read(tmem_stats._c) > tmem_stats._c##_max ) \
> > +        tmem_stats._c##_max = _atomic_read(tmem_stats._c); \
> > +} while (0)
> > +
> > +#define atomic_dec_and_assert(_c) do { \
> > +    atomic_dec(&tmem_stats._c); \
> > +    ASSERT(_atomic_read(tmem_stats._c) >= 0); \
> > +} while (0)
> 
> I know you are just modifying existing code (thus this complain doesn't
> count against the patch), but these constructs are racy and broken.

Yes.
> 
> You need to use atomic_{inc,dec}_and_test() for these to function as
> intended.
> 
> Up to you whether you want to fix that in this patch, or a subsequent
> non-cleanup patch.

Subsequent. There were sooo many things I itched to do - but for right
now just want to do this simple cleanup.
> 
> ~Andrew

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

[PATCH v1 3/4] tmem: Move global_ individual variables in a global structure.

[Konrad Rzeszutek Wilk]

Put them all in one structure to make it easier to
figure out what can be removed. The structure is called
'tmem_global' as it will be eventually non-static.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

---
Cc:  Andrew Cooper <andrew.cooper3@citrix.com>
Cc:  Jan Beulich <jbeulich@suse.com>
Cc:  Wei Liu <wei.liu2@citrix.com>
Cc:  Doug Goldstein <cardoe@cardoe.com>

v1: First posting as non-RFC
---
 xen/common/tmem.c | 76 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 40 insertions(+), 36 deletions(-)

diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index b58ab4d..70cb61e 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -222,13 +222,6 @@ struct tmem_page_content_descriptor {
 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_client_list);
-
-static struct tmem_pool *global_shared_pools[MAX_GLOBAL_SHARED_POOLS] = { 0 };
-static bool_t global_shared_auth = 0;
-static atomic_t client_weight_total = ATOMIC_INIT(0);
 static int tmem_initialized = 0;
 
 struct xmem_pool *tmem_mempool = 0;
@@ -245,8 +238,20 @@ 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))
 
-static long global_eph_count; /* Atomicity depends on eph_lists_spinlock. */
+struct tmem_global {
+    struct list_head ephemeral_page_list;  /* All pages in ephemeral pools. */
+    struct list_head client_list;
+    struct tmem_pool *shared_pools[MAX_GLOBAL_SHARED_POOLS];
+    bool_t shared_auth;
+    atomic_t client_weight_total;
+    long eph_count;  /* Atomicity depends on eph_lists_spinlock. */
+};
 
+struct tmem_global tmem_global = {
+    .ephemeral_page_list = LIST_HEAD_INIT(tmem_global.ephemeral_page_list),
+    .client_list = LIST_HEAD_INIT(tmem_global.client_list),
+    .client_weight_total = ATOMIC_INIT(0),
+};
 
 /*
  * There two types of memory allocation interfaces in tmem.
@@ -724,8 +729,8 @@ static void pgp_delist_free(struct tmem_page_descriptor *pgp)
         ASSERT(client->eph_count >= 0);
         list_del_init(&pgp->us.client_eph_pages);
         if ( !list_empty(&pgp->global_eph_pages) )
-            global_eph_count--;
-        ASSERT(global_eph_count >= 0);
+            tmem_global.eph_count--;
+        ASSERT(tmem_global.eph_count >= 0);
         list_del_init(&pgp->global_eph_pages);
         spin_unlock(&eph_lists_spinlock);
     }
@@ -1126,9 +1131,9 @@ static int shared_pool_quit(struct tmem_pool *pool, domid_t cli_id)
         if (pool->shared_count)
             return pool->shared_count;
         for (s_poolid = 0; s_poolid < MAX_GLOBAL_SHARED_POOLS; s_poolid++)
-            if ( (global_shared_pools[s_poolid]) == pool )
+            if ( (tmem_global.shared_pools[s_poolid]) == pool )
             {
-                global_shared_pools[s_poolid] = NULL;
+                tmem_global.shared_pools[s_poolid] = NULL;
                 break;
             }
         return 0;
@@ -1209,7 +1214,7 @@ static struct client *client_create(domid_t cli_id)
     for ( i = 0; i < MAX_GLOBAL_SHARED_POOLS; i++)
         client->shared_auth_uuid[i][0] =
             client->shared_auth_uuid[i][1] = -1L;
-    list_add_tail(&client->client_list, &global_client_list);
+    list_add_tail(&client->client_list, &tmem_global.client_list);
     INIT_LIST_HEAD(&client->ephemeral_page_list);
     INIT_LIST_HEAD(&client->persistent_invalidated_list);
     tmem_client_info("ok\n");
@@ -1245,13 +1250,13 @@ static void client_flush(struct client *client)
 
 static bool_t client_over_quota(struct client *client)
 {
-    int total = _atomic_read(client_weight_total);
+    int total = _atomic_read(tmem_global.client_weight_total);
 
     ASSERT(client != NULL);
     if ( (total == 0) || (client->weight == 0) ||
           (client->eph_count == 0) )
         return 0;
-    return ( ((global_eph_count*100L) / client->eph_count ) >
+    return ( ((tmem_global.eph_count*100L) / client->eph_count ) >
              ((total*100L) / client->weight) );
 }
 
@@ -1280,7 +1285,7 @@ static bool_t tmem_try_to_evict_pgp(struct tmem_page_descriptor *pgp, bool_t *ho
             {
                 pgp->eviction_attempted++;
                 list_del(&pgp->global_eph_pages);
-                list_add_tail(&pgp->global_eph_pages,&global_ephemeral_page_list);
+                list_add_tail(&pgp->global_eph_pages,&tmem_global.ephemeral_page_list);
                 list_del(&pgp->us.client_eph_pages);
                 list_add_tail(&pgp->us.client_eph_pages,&client->ephemeral_page_list);
                 goto pcd_unlock;
@@ -1320,9 +1325,9 @@ static int tmem_evict(void)
             if ( tmem_try_to_evict_pgp(pgp, &hold_pool_rwlock) )
                 goto found;
     }
-    else if ( !list_empty(&global_ephemeral_page_list) )
+    else if ( !list_empty(&tmem_global.ephemeral_page_list) )
     {
-        list_for_each_entry(pgp, &global_ephemeral_page_list, global_eph_pages)
+        list_for_each_entry(pgp, &tmem_global.ephemeral_page_list, global_eph_pages)
             if ( tmem_try_to_evict_pgp(pgp, &hold_pool_rwlock) )
             {
                 client = pgp->us.obj->pool->client;
@@ -1338,8 +1343,8 @@ found:
     list_del_init(&pgp->us.client_eph_pages);
     client->eph_count--;
     list_del_init(&pgp->global_eph_pages);
-    global_eph_count--;
-    ASSERT(global_eph_count >= 0);
+    tmem_global.eph_count--;
+    ASSERT(tmem_global.eph_count >= 0);
     ASSERT(client->eph_count >= 0);
     spin_unlock(&eph_lists_spinlock);
 
@@ -1664,10 +1669,9 @@ insert_page:
     if ( !is_persistent(pool) )
     {
         spin_lock(&eph_lists_spinlock);
-        list_add_tail(&pgp->global_eph_pages,
-            &global_ephemeral_page_list);
-        if (++global_eph_count > tmem_stats.global_eph_count_max)
-            tmem_stats.global_eph_count_max = global_eph_count;
+        list_add_tail(&pgp->global_eph_pages, &tmem_global.ephemeral_page_list);
+        if (++tmem_global.eph_count > tmem_stats.global_eph_count_max)
+            tmem_stats.global_eph_count_max = tmem_global.eph_count;
         list_add_tail(&pgp->us.client_eph_pages,
             &client->ephemeral_page_list);
         if (++client->eph_count > client->eph_count_max)
@@ -1774,7 +1778,7 @@ static int do_tmem_get(struct tmem_pool *pool,
         } else {
             spin_lock(&eph_lists_spinlock);
             list_del(&pgp->global_eph_pages);
-            list_add_tail(&pgp->global_eph_pages,&global_ephemeral_page_list);
+            list_add_tail(&pgp->global_eph_pages,&tmem_global.ephemeral_page_list);
             list_del(&pgp->us.client_eph_pages);
             list_add_tail(&pgp->us.client_eph_pages,&client->ephemeral_page_list);
             spin_unlock(&eph_lists_spinlock);
@@ -1962,7 +1966,7 @@ static int do_tmem_new_pool(domid_t this_cli_id,
             pool->shared = 0;
             goto out;
         }
-        if ( client->shared_auth_required && !global_shared_auth )
+        if ( client->shared_auth_required && !tmem_global.shared_auth )
         {
             for ( i = 0; i < MAX_GLOBAL_SHARED_POOLS; i++)
                 if ( (client->shared_auth_uuid[i][0] == uuid_lo) &&
@@ -1983,7 +1987,7 @@ static int do_tmem_new_pool(domid_t this_cli_id,
         first_unused_s_poolid = MAX_GLOBAL_SHARED_POOLS;
         for ( i = 0; i < MAX_GLOBAL_SHARED_POOLS; i++ )
         {
-            if ( (shpool = global_shared_pools[i]) != NULL )
+            if ( (shpool = tmem_global.shared_pools[i]) != NULL )
             {
                 if ( shpool->uuid[0] == uuid_lo && shpool->uuid[1] == uuid_hi )
                 {
@@ -2020,7 +2024,7 @@ static int do_tmem_new_pool(domid_t this_cli_id,
             pool->shared_count = 0;
             if ( shared_pool_join(pool, client) )
                 goto fail;
-            global_shared_pools[first_unused_s_poolid] = pool;
+            tmem_global.shared_pools[first_unused_s_poolid] = pool;
         }
     }
 
@@ -2046,7 +2050,7 @@ static int tmemc_freeze_pools(domid_t cli_id, int arg)
     s = destroy ? "destroyed" : ( freeze ? "frozen" : "thawed" );
     if ( cli_id == TMEM_CLI_ID_NULL )
     {
-        list_for_each_entry(client,&global_client_list,client_list)
+        list_for_each_entry(client,&tmem_global.client_list,client_list)
             client->frozen = freeze;
         tmem_client_info("tmem: all pools %s for all %ss\n", s, tmem_client_str);
     }
@@ -2152,7 +2156,7 @@ static int tmemc_list_shared(tmem_cli_va_param_t buf, int off, uint32_t len,
 
     for ( i = 0; i < MAX_GLOBAL_SHARED_POOLS; i++ )
     {
-        if ( (p = global_shared_pools[i]) == NULL )
+        if ( (p = tmem_global.shared_pools[i]) == NULL )
             continue;
         n = scnprintf(info+n,BSIZE-n,"S=SI:%d,PT:%c%c,U0:%"PRIx64",U1:%"PRIx64,
                       i, is_persistent(p) ? 'P' : 'E',
@@ -2216,7 +2220,7 @@ static int tmemc_list_global(tmem_cli_va_param_t buf, int off, uint32_t len,
         n += scnprintf(info+n,BSIZE-n,
           "Ec:%ld,Em:%ld,Oc:%d,Om:%d,Nc:%d,Nm:%d,Pc:%d,Pm:%d,"
           "Fc:%d,Fm:%d,Sc:%d,Sm:%d,Ep:%lu,Gd:%lu,Zt:%lu,Gz:%lu\n",
-          global_eph_count, tmem_stats.global_eph_count_max,
+          tmem_global.eph_count, tmem_stats.global_eph_count_max,
           _atomic_read(tmem_stats.global_obj_count), tmem_stats.global_obj_count_max,
           _atomic_read(tmem_stats.global_rtree_node_count), tmem_stats.global_rtree_node_count_max,
           _atomic_read(tmem_stats.global_pgp_count), tmem_stats.global_pgp_count_max,
@@ -2240,7 +2244,7 @@ static int tmemc_list(domid_t cli_id, tmem_cli_va_param_t buf, uint32_t len,
     if ( cli_id == TMEM_CLI_ID_NULL ) {
         off = tmemc_list_global(buf,0,len,use_long);
         off += tmemc_list_shared(buf,off,len-off,use_long);
-        list_for_each_entry(client,&global_client_list,client_list)
+        list_for_each_entry(client,&tmem_global.client_list,client_list)
             off += tmemc_list_client(client, buf, off, len-off, use_long);
         off += tmemc_list_global_perf(buf,off,len-off,use_long);
     }
@@ -2264,8 +2268,8 @@ static int __tmemc_set_var(struct client *client, uint32_t subop, uint32_t arg1)
         client->weight = arg1;
         tmem_client_info("tmem: weight set to %d for %s=%d\n",
                         arg1, tmem_cli_id_str, cli_id);
-        atomic_sub(old_weight,&client_weight_total);
-        atomic_add(client->weight,&client_weight_total);
+        atomic_sub(old_weight,&tmem_global.client_weight_total);
+        atomic_add(client->weight,&tmem_global.client_weight_total);
         break;
     case XEN_SYSCTL_TMEM_OP_SET_CAP:
         client->cap = arg1;
@@ -2298,7 +2302,7 @@ static int tmemc_set_var(domid_t cli_id, uint32_t subop, uint32_t arg1)
 
     if ( cli_id == TMEM_CLI_ID_NULL )
     {
-        list_for_each_entry(client,&global_client_list,client_list)
+        list_for_each_entry(client,&tmem_global.client_list,client_list)
         {
             ret =  __tmemc_set_var(client, subop, arg1);
             if (ret)
@@ -2322,7 +2326,7 @@ static int tmemc_shared_pool_auth(domid_t cli_id, uint64_t uuid_lo,
 
     if ( cli_id == TMEM_CLI_ID_NULL )
     {
-        global_shared_auth = auth;
+        tmem_global.shared_auth = auth;
         return 1;
     }
     client = tmem_client_from_cli_id(cli_id);
-- 
2.5.5


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

[PATCH v1 4/4] tmem: Move bulk of tmem control functions in its own file.

[Konrad Rzeszutek Wilk]

The functionality that is related to migration is left inside
tmem.c. The list of control operations that are in tmem_control
with XEN_SYSCTL_TMEM_OP prefix are:

DESTROY, FLUSH, FREEZE, THAW, LIST, QUERY_FREEABLE_MB
SAVE_GET_CLIENT_CAP, SAVE_GET_CLIENT_FLAGS,
SAVE_GET_CLIENT_WEIGHT, SAVE_GET_MAXPOOLS,
SAVE_GET_POOL_FLAGS, SAVE_GET_POOL_NPAGES
SAVE_GET_POOL_UUID, SAVE_GET_VERSION
SET_CAP, SET_COMPRESS, SET_WEIGHT

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

---
Cc:  Andrew Cooper <andrew.cooper3@citrix.com>
Cc:  Jan Beulich <jbeulich@suse.com>
Cc:  Wei Liu <wei.liu2@citrix.com>
Cc:  Doug Goldstein <cardoe@cardoe.com>

v1: First posting as non-RFC
---
 xen/common/Makefile            |   2 +-
 xen/common/tmem.c              | 514 +----------------------------------------
 xen/common/tmem_control.c      | 443 +++++++++++++++++++++++++++++++++++
 xen/include/xen/tmem_control.h |  33 +++
 xen/include/xen/tmem_xen.h     | 128 ++++++++++
 5 files changed, 609 insertions(+), 511 deletions(-)
 create mode 100644 xen/common/tmem_control.c
 create mode 100644 xen/include/xen/tmem_control.h

diff --git a/xen/common/Makefile b/xen/common/Makefile
index afd84b6..537bdfd 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -68,7 +68,7 @@ obj-$(crash_debug) += gdbstub.o
 
 obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o multicall.o xlat.o)
 
-tmem-y := tmem.o tmem_xen.o
+tmem-y := tmem.o tmem_xen.o tmem_control.o
 tmem-$(CONFIG_COMPAT) += compat/tmem_xen.o
 obj-$(CONFIG_TMEM) += $(tmem-y)
 
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index 70cb61e..efaafc4 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -27,50 +27,7 @@
 
 #define TMEM_SPEC_VERSION 1
 
-/* Global statistics (none need to be locked). */
-struct tmem_statistics {
-    unsigned long total_tmem_ops;
-    unsigned long errored_tmem_ops;
-    unsigned long total_flush_pool;
-    unsigned long alloc_failed;
-    unsigned long alloc_page_failed;
-    unsigned long evicted_pgs;
-    unsigned long evict_attempts;
-    unsigned long relinq_pgs;
-    unsigned long relinq_attempts;
-    unsigned long max_evicts_per_relinq;
-    unsigned long low_on_memory;
-    unsigned long deduped_puts;
-    unsigned long tot_good_eph_puts;
-    int global_obj_count_max;
-    int global_pgp_count_max;
-    int global_pcd_count_max;
-    int global_page_count_max;
-    int global_rtree_node_count_max;
-    long global_eph_count_max;
-    unsigned long failed_copies;
-    unsigned long pcd_tot_tze_size;
-    unsigned long pcd_tot_csize;
-    /* Global counters (should use long_atomic_t access). */
-    atomic_t global_obj_count;
-    atomic_t global_pgp_count;
-    atomic_t global_pcd_count;
-    atomic_t global_page_count;
-    atomic_t global_rtree_node_count;
-};
-
-#define atomic_inc_and_max(_c) do { \
-    atomic_inc(&tmem_stats._c); \
-    if ( _atomic_read(tmem_stats._c) > tmem_stats._c##_max ) \
-        tmem_stats._c##_max = _atomic_read(tmem_stats._c); \
-} while (0)
-
-#define atomic_dec_and_assert(_c) do { \
-    atomic_dec(&tmem_stats._c); \
-    ASSERT(_atomic_read(tmem_stats._c) >= 0); \
-} while (0)
-
-static struct tmem_statistics tmem_stats = {
+struct tmem_statistics tmem_stats = {
     .global_obj_count = ATOMIC_INIT(0),
     .global_pgp_count = ATOMIC_INIT(0),
     .global_pcd_count = ATOMIC_INIT(0),
@@ -80,81 +37,6 @@ static struct tmem_statistics tmem_stats = {
 
 /************ CORE DATA STRUCTURES ************************************/
 
-#define MAX_POOLS_PER_DOMAIN 16
-#define MAX_GLOBAL_SHARED_POOLS  16
-
-struct tmem_pool;
-struct tmem_page_descriptor;
-struct tmem_page_content_descriptor;
-struct client {
-    struct list_head client_list;
-    struct tmem_pool *pools[MAX_POOLS_PER_DOMAIN];
-    struct domain *domain;
-    struct xmem_pool *persistent_pool;
-    struct list_head ephemeral_page_list;
-    long eph_count, eph_count_max;
-    domid_t cli_id;
-    uint32_t weight;
-    uint32_t cap;
-    bool_t compress;
-    bool_t frozen;
-    bool_t shared_auth_required;
-    /* 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. */
-    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. */
-    uint64_t shared_auth_uuid[MAX_GLOBAL_SHARED_POOLS][2];
-};
-
-struct share_list {
-    struct list_head share_list;
-    struct client *client;
-};
-
-#define POOL_PAGESHIFT (PAGE_SHIFT - 12)
-#define OBJ_HASH_BUCKETS 256 /* Must be power of two. */
-#define OBJ_HASH_BUCKETS_MASK (OBJ_HASH_BUCKETS-1)
-
-struct tmem_pool {
-    bool_t shared;
-    bool_t persistent;
-    bool_t is_dying;
-    struct client *client;
-    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 list_head persistent_page_list;
-    struct tmem_page_descriptor *cur_pgp;
-    /* Statistics collection. */
-    atomic_t pgp_count;
-    int pgp_count_max;
-    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;
-    uint64_t sum_evicted_cycles;
-    unsigned long puts, good_puts, no_mem_puts;
-    unsigned long dup_puts_flushed, dup_puts_replaced;
-    unsigned long gets, found_gets;
-    unsigned long flushs, flushs_found;
-    unsigned long flush_objs, flush_objs_found;
-};
-
-#define is_persistent(_p)  (_p->persistent)
-#define is_shared(_p)      (_p->shared)
-
 struct tmem_object_root {
     struct xen_tmem_oid oid;
     struct rb_node rb_tree_node; /* Protected by pool->pool_rwlock. */
@@ -238,14 +120,7 @@ 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))
 
-struct tmem_global {
-    struct list_head ephemeral_page_list;  /* All pages in ephemeral pools. */
-    struct list_head client_list;
-    struct tmem_pool *shared_pools[MAX_GLOBAL_SHARED_POOLS];
-    bool_t shared_auth;
     atomic_t client_weight_total;
-    long eph_count;  /* Atomicity depends on eph_lists_spinlock. */
-};
 
 struct tmem_global tmem_global = {
     .ephemeral_page_list = LIST_HEAD_INIT(tmem_global.ephemeral_page_list),
@@ -1307,7 +1182,7 @@ obj_unlock:
     return 0;
 }
 
-static int tmem_evict(void)
+int tmem_evict(void)
 {
     struct client *client = current->domain->tmem_client;
     struct tmem_page_descriptor *pgp = NULL, *pgp_del;
@@ -1380,31 +1255,6 @@ out:
     return ret;
 }
 
-static unsigned long tmem_flush_npages(unsigned long n)
-{
-    unsigned long avail_pages = 0;
-
-    while ( (avail_pages = tmem_page_list_pages) < n )
-    {
-        if (  !tmem_evict() )
-            break;
-    }
-    if ( avail_pages )
-    {
-        spin_lock(&tmem_page_list_lock);
-        while ( !page_list_empty(&tmem_page_list) )
-        {
-            struct page_info *pg = page_list_remove_head(&tmem_page_list);
-            scrub_one_page(pg);
-            tmem_page_list_pages--;
-            free_domheap_page(pg);
-        }
-        ASSERT(tmem_page_list_pages == 0);
-        INIT_PAGE_LIST_HEAD(&tmem_page_list);
-        spin_unlock(&tmem_page_list_lock);
-    }
-    return avail_pages;
-}
 
 /*
  * Under certain conditions (e.g. if each client is putting pages for exactly
@@ -2039,285 +1889,6 @@ fail:
 
 /************ TMEM CONTROL OPERATIONS ************************************/
 
-/* 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;
-    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" );
-    if ( cli_id == TMEM_CLI_ID_NULL )
-    {
-        list_for_each_entry(client,&tmem_global.client_list,client_list)
-            client->frozen = freeze;
-        tmem_client_info("tmem: all pools %s for all %ss\n", s, tmem_client_str);
-    }
-    else
-    {
-        if ( (client = tmem_client_from_cli_id(cli_id)) == NULL)
-            return -1;
-        client->frozen = freeze;
-        tmem_client_info("tmem: all pools %s for %s=%d\n",
-                         s, tmem_cli_id_str, cli_id);
-    }
-    return 0;
-}
-
-static int tmemc_flush_mem(domid_t cli_id, uint32_t kb)
-{
-    uint32_t npages, flushed_pages, flushed_kb;
-
-    if ( cli_id != TMEM_CLI_ID_NULL )
-    {
-        tmem_client_warn("tmem: %s-specific flush not supported yet, use --all\n",
-           tmem_client_str);
-        return -1;
-    }
-    /* 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);
-    return flushed_kb;
-}
-
-/*
- * These tmemc_list* routines output lots of stats in a format that is
- *  intended to be program-parseable, not human-readable. Further, by
- *  tying each group of stats to a line format indicator (e.g. G= for
- *  global stats) and each individual stat to a two-letter specifier
- *  (e.g. Ec:nnnnn in the G= line says there are nnnnn pages in the
- *  global ephemeral pool), it should allow the stats reported to be
- *  forward and backwards compatible as tmem evolves.
- */
-#define BSIZE 1024
-
-static int tmemc_list_client(struct client *c, tmem_cli_va_param_t buf,
-                             int off, uint32_t len, bool_t use_long)
-{
-    char info[BSIZE];
-    int i, n = 0, sum = 0;
-    struct tmem_pool *p;
-    bool_t s;
-
-    n = scnprintf(info,BSIZE,"C=CI:%d,ww:%d,ca:%d,co:%d,fr:%d,"
-        "Tc:%"PRIu64",Ge:%ld,Pp:%ld,Gp:%ld%c",
-        c->cli_id, c->weight, c->cap, c->compress, c->frozen,
-        c->total_cycles, c->succ_eph_gets, c->succ_pers_puts, c->succ_pers_gets,
-        use_long ? ',' : '\n');
-    if (use_long)
-        n += scnprintf(info+n,BSIZE-n,
-             "Ec:%ld,Em:%ld,cp:%ld,cb:%"PRId64",cn:%ld,cm:%ld\n",
-             c->eph_count, c->eph_count_max,
-             c->compressed_pages, c->compressed_sum_size,
-             c->compress_poor, c->compress_nomem);
-    if ( !copy_to_guest_offset(buf, off + sum, info, n + 1) )
-        sum += n;
-    for ( i = 0; i < MAX_POOLS_PER_DOMAIN; i++ )
-    {
-        if ( (p = c->pools[i]) == NULL )
-            continue;
-        s = is_shared(p);
-        n = scnprintf(info,BSIZE,"P=CI:%d,PI:%d,"
-                      "PT:%c%c,U0:%"PRIx64",U1:%"PRIx64"%c",
-                      c->cli_id, p->pool_id,
-                      is_persistent(p) ? 'P' : 'E', s ? 'S' : 'P',
-                      (uint64_t)(s ? p->uuid[0] : 0),
-                      (uint64_t)(s ? p->uuid[1] : 0LL),
-                      use_long ? ',' : '\n');
-        if (use_long)
-            n += scnprintf(info+n,BSIZE-n,
-             "Pc:%d,Pm:%d,Oc:%ld,Om:%ld,Nc:%lu,Nm:%lu,"
-             "ps:%lu,pt:%lu,pd:%lu,pr:%lu,px:%lu,gs:%lu,gt:%lu,"
-             "fs:%lu,ft:%lu,os:%lu,ot:%lu\n",
-             _atomic_read(p->pgp_count), p->pgp_count_max,
-             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->found_gets, p->gets,
-             p->flushs_found, p->flushs, p->flush_objs_found, p->flush_objs);
-        if ( sum + n >= len )
-            return sum;
-        if ( !copy_to_guest_offset(buf, off + sum, info, n + 1) )
-            sum += n;
-    }
-    return sum;
-}
-
-static int tmemc_list_shared(tmem_cli_va_param_t buf, int off, uint32_t len,
-                              bool_t use_long)
-{
-    char info[BSIZE];
-    int i, n = 0, sum = 0;
-    struct tmem_pool *p;
-    struct share_list *sl;
-
-    for ( i = 0; i < MAX_GLOBAL_SHARED_POOLS; i++ )
-    {
-        if ( (p = tmem_global.shared_pools[i]) == NULL )
-            continue;
-        n = scnprintf(info+n,BSIZE-n,"S=SI:%d,PT:%c%c,U0:%"PRIx64",U1:%"PRIx64,
-                      i, is_persistent(p) ? 'P' : 'E',
-                      is_shared(p) ? 'S' : 'P',
-                      p->uuid[0], p->uuid[1]);
-        list_for_each_entry(sl,&p->share_list, share_list)
-            n += scnprintf(info+n,BSIZE-n,",SC:%d",sl->client->cli_id);
-        n += scnprintf(info+n,BSIZE-n,"%c", use_long ? ',' : '\n');
-        if (use_long)
-            n += scnprintf(info+n,BSIZE-n,
-             "Pc:%d,Pm:%d,Oc:%ld,Om:%ld,Nc:%lu,Nm:%lu,"
-             "ps:%lu,pt:%lu,pd:%lu,pr:%lu,px:%lu,gs:%lu,gt:%lu,"
-             "fs:%lu,ft:%lu,os:%lu,ot:%lu\n",
-             _atomic_read(p->pgp_count), p->pgp_count_max,
-             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->found_gets, p->gets,
-             p->flushs_found, p->flushs, p->flush_objs_found, p->flush_objs);
-        if ( sum + n >= len )
-            return sum;
-        if ( !copy_to_guest_offset(buf, off + sum, info, n + 1) )
-            sum += n;
-    }
-    return sum;
-}
-
-static int tmemc_list_global_perf(tmem_cli_va_param_t buf, int off,
-                                  uint32_t len, bool_t use_long)
-{
-    char info[BSIZE];
-    int n = 0, sum = 0;
-
-    n = scnprintf(info+n,BSIZE-n,"T=");
-    n--; /* Overwrite trailing comma. */
-    n += scnprintf(info+n,BSIZE-n,"\n");
-    if ( sum + n >= len )
-        return sum;
-    if ( !copy_to_guest_offset(buf, off + sum, info, n + 1) )
-        sum += n;
-    return sum;
-}
-
-static int tmemc_list_global(tmem_cli_va_param_t buf, int off, uint32_t len,
-                              bool_t use_long)
-{
-    char info[BSIZE];
-    int n = 0, sum = off;
-
-    n += scnprintf(info,BSIZE,"G="
-      "Tt:%lu,Te:%lu,Cf:%lu,Af:%lu,Pf:%lu,Ta:%lu,"
-      "Lm:%lu,Et:%lu,Ea:%lu,Rt:%lu,Ra:%lu,Rx:%lu,Fp:%lu%c",
-      tmem_stats.total_tmem_ops, tmem_stats.errored_tmem_ops, tmem_stats.failed_copies,
-      tmem_stats.alloc_failed, tmem_stats.alloc_page_failed, tmem_page_list_pages,
-      tmem_stats.low_on_memory, tmem_stats.evicted_pgs,
-      tmem_stats.evict_attempts, tmem_stats.relinq_pgs, tmem_stats.relinq_attempts,
-      tmem_stats.max_evicts_per_relinq,
-      tmem_stats.total_flush_pool, use_long ? ',' : '\n');
-    if (use_long)
-        n += scnprintf(info+n,BSIZE-n,
-          "Ec:%ld,Em:%ld,Oc:%d,Om:%d,Nc:%d,Nm:%d,Pc:%d,Pm:%d,"
-          "Fc:%d,Fm:%d,Sc:%d,Sm:%d,Ep:%lu,Gd:%lu,Zt:%lu,Gz:%lu\n",
-          tmem_global.eph_count, tmem_stats.global_eph_count_max,
-          _atomic_read(tmem_stats.global_obj_count), tmem_stats.global_obj_count_max,
-          _atomic_read(tmem_stats.global_rtree_node_count), tmem_stats.global_rtree_node_count_max,
-          _atomic_read(tmem_stats.global_pgp_count), tmem_stats.global_pgp_count_max,
-          _atomic_read(tmem_stats.global_page_count), tmem_stats.global_page_count_max,
-          _atomic_read(tmem_stats.global_pcd_count), tmem_stats.global_pcd_count_max,
-         tmem_stats.tot_good_eph_puts,tmem_stats.deduped_puts,tmem_stats.pcd_tot_tze_size,
-         tmem_stats.pcd_tot_csize);
-    if ( sum + n >= len )
-        return sum;
-    if ( !copy_to_guest_offset(buf, off + sum, info, n + 1) )
-        sum += n;
-    return sum;
-}
-
-static int tmemc_list(domid_t cli_id, tmem_cli_va_param_t buf, uint32_t len,
-                               bool_t use_long)
-{
-    struct client *client;
-    int off = 0;
-
-    if ( cli_id == TMEM_CLI_ID_NULL ) {
-        off = tmemc_list_global(buf,0,len,use_long);
-        off += tmemc_list_shared(buf,off,len-off,use_long);
-        list_for_each_entry(client,&tmem_global.client_list,client_list)
-            off += tmemc_list_client(client, buf, off, len-off, use_long);
-        off += tmemc_list_global_perf(buf,off,len-off,use_long);
-    }
-    else if ( (client = tmem_client_from_cli_id(cli_id)) == NULL)
-        return -1;
-    else
-        off = tmemc_list_client(client, buf, 0, len, use_long);
-
-    return 0;
-}
-
-static int __tmemc_set_var(struct client *client, uint32_t subop, uint32_t arg1)
-{
-    domid_t cli_id = client->cli_id;
-    uint32_t old_weight;
-
-    switch (subop)
-    {
-    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",
-                        arg1, tmem_cli_id_str, cli_id);
-        atomic_sub(old_weight,&tmem_global.client_weight_total);
-        atomic_add(client->weight,&tmem_global.client_weight_total);
-        break;
-    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 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",
-                            tmem_compression_enabled() ? "enabled" : "disabled",
-                            tmem_client_str);
-            return -1;
-        }
-        client->compress = arg1 ? 1 : 0;
-        tmem_client_info("tmem: compression %s for %s=%d\n",
-            arg1 ? "enabled" : "disabled",tmem_cli_id_str,cli_id);
-        break;
-    default:
-        tmem_client_warn("tmem: unknown subop %d for tmemc_set_var\n", subop);
-        return -1;
-    }
-    return 0;
-}
-
-static int tmemc_set_var(domid_t cli_id, uint32_t subop, uint32_t arg1)
-{
-    struct client *client;
-    int ret = -1;
-
-    if ( cli_id == TMEM_CLI_ID_NULL )
-    {
-        list_for_each_entry(client,&tmem_global.client_list,client_list)
-        {
-            ret =  __tmemc_set_var(client, subop, arg1);
-            if (ret)
-                break;
-        }
-    }
-    else
-    {
-        client = tmem_client_from_cli_id(cli_id);
-        if ( client )
-            ret = __tmemc_set_var(client, subop, arg1);
-    }
-    return ret;
-}
-
 static int tmemc_shared_pool_auth(domid_t cli_id, uint64_t uuid_lo,
                                   uint64_t uuid_hi, bool_t auth)
 {
@@ -2371,8 +1942,6 @@ static int tmemc_save_subop(int cli_id, uint32_t pool_id,
                         uint32_t subop, tmem_cli_va_param_t buf, uint32_t arg1)
 {
     struct client *client = tmem_client_from_cli_id(cli_id);
-    struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
-                   ? NULL : client->pools[pool_id];
     uint32_t p;
     struct tmem_page_descriptor *pgp, *pgp2;
     int rc = -1;
@@ -2400,48 +1969,6 @@ static int tmemc_save_subop(int cli_id, uint32_t pool_id,
         if ( client == NULL && (client = client_create(cli_id)) != NULL )
             return 1;
         break;
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_VERSION:
-        rc = TMEM_SPEC_VERSION;
-        break;
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_MAXPOOLS:
-        rc = MAX_POOLS_PER_DOMAIN;
-        break;
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_WEIGHT:
-        if ( client == NULL )
-            break;
-        rc = client->weight == -1 ? -2 : client->weight;
-        break;
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_CAP:
-        if ( client == NULL )
-            break;
-        rc = client->cap == -1 ? -2 : client->cap;
-        break;
-    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 XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_FLAGS:
-         if ( pool == NULL )
-             break;
-         rc = (pool->persistent ? TMEM_POOL_PERSIST : 0) |
-              (pool->shared ? TMEM_POOL_SHARED : 0) |
-              (POOL_PAGESHIFT << TMEM_POOL_PAGESIZE_SHIFT) |
-              (TMEM_SPEC_VERSION << TMEM_POOL_VERSION_SHIFT);
-        break;
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_NPAGES:
-         if ( pool == NULL )
-             break;
-        rc = _atomic_read(pool->pgp_count);
-        break;
-    case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_UUID:
-         if ( pool == NULL )
-             break;
-        rc = 0;
-        if ( copy_to_guest(guest_handle_cast(buf, void), pool->uuid, 2) )
-            rc = -EFAULT;
-        break;
     case XEN_SYSCTL_TMEM_OP_SAVE_END:
         if ( client == NULL )
             break;
@@ -2589,50 +2116,19 @@ static int tmemc_restore_flush_page(int cli_id, uint32_t pool_id,
     return do_tmem_flush_page(pool,oidp,index);
 }
 
-int tmem_control(struct xen_sysctl_tmem_op *op)
+int do_tmem_control(struct xen_sysctl_tmem_op *op)
 {
     int ret;
     uint32_t pool_id = op->pool_id;
     uint32_t cmd = op->cmd;
     struct xen_tmem_oid *oidp = &op->oid;
 
-    if ( op->pad != 0 )
-        return -EINVAL;
-
-    write_lock(&tmem_rwlock);
+    ASSERT(rw_is_write_locked(&tmem_rwlock));
 
     switch (cmd)
     {
-    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 XEN_SYSCTL_TMEM_OP_FLUSH:
-        ret = tmemc_flush_mem(op->cli_id,op->arg1);
-        break;
-    case XEN_SYSCTL_TMEM_OP_LIST:
-        ret = tmemc_list(op->cli_id,
-                         guest_handle_cast(op->buf, char), op->arg1, op->arg2);
-        break;
-    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 XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB:
-        ret = tmem_freeable_pages() >> (20 - PAGE_SHIFT);
-        break;
     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);
@@ -2656,8 +2152,6 @@ int tmem_control(struct xen_sysctl_tmem_op *op)
         ret = -1;
     }
 
-    write_unlock(&tmem_rwlock);
-
     return ret;
 }
 
diff --git a/xen/common/tmem_control.c b/xen/common/tmem_control.c
new file mode 100644
index 0000000..81a2414
--- /dev/null
+++ b/xen/common/tmem_control.c
@@ -0,0 +1,443 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include <xen/init.h>
+#include <xen/list.h>
+#include <xen/radix-tree.h>
+#include <xen/rbtree.h>
+#include <xen/rwlock.h>
+#include <xen/tmem_control.h>
+#include <xen/tmem.h>
+#include <xen/tmem_xen.h>
+#include <public/sysctl.h>
+
+/************ TMEM CONTROL OPERATIONS ************************************/
+
+/* 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;
+    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" );
+    if ( cli_id == TMEM_CLI_ID_NULL )
+    {
+        list_for_each_entry(client,&tmem_global.client_list,client_list)
+            client->frozen = freeze;
+        tmem_client_info("tmem: all pools %s for all %ss\n", s, tmem_client_str);
+    }
+    else
+    {
+        if ( (client = tmem_client_from_cli_id(cli_id)) == NULL)
+            return -1;
+        client->frozen = freeze;
+        tmem_client_info("tmem: all pools %s for %s=%d\n",
+                         s, tmem_cli_id_str, cli_id);
+    }
+    return 0;
+}
+
+static unsigned long tmem_flush_npages(unsigned long n)
+{
+    unsigned long avail_pages = 0;
+
+    while ( (avail_pages = tmem_page_list_pages) < n )
+    {
+        if (  !tmem_evict() )
+            break;
+    }
+    if ( avail_pages )
+    {
+        spin_lock(&tmem_page_list_lock);
+        while ( !page_list_empty(&tmem_page_list) )
+        {
+            struct page_info *pg = page_list_remove_head(&tmem_page_list);
+            scrub_one_page(pg);
+            tmem_page_list_pages--;
+            free_domheap_page(pg);
+        }
+        ASSERT(tmem_page_list_pages == 0);
+        INIT_PAGE_LIST_HEAD(&tmem_page_list);
+        spin_unlock(&tmem_page_list_lock);
+    }
+    return avail_pages;
+}
+
+static int tmemc_flush_mem(domid_t cli_id, uint32_t kb)
+{
+    uint32_t npages, flushed_pages, flushed_kb;
+
+    if ( cli_id != TMEM_CLI_ID_NULL )
+    {
+        tmem_client_warn("tmem: %s-specific flush not supported yet, use --all\n",
+           tmem_client_str);
+        return -1;
+    }
+    /* 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);
+    return flushed_kb;
+}
+
+/*
+ * These tmemc_list* routines output lots of stats in a format that is
+ *  intended to be program-parseable, not human-readable. Further, by
+ *  tying each group of stats to a line format indicator (e.g. G= for
+ *  global stats) and each individual stat to a two-letter specifier
+ *  (e.g. Ec:nnnnn in the G= line says there are nnnnn pages in the
+ *  global ephemeral pool), it should allow the stats reported to be
+ *  forward and backwards compatible as tmem evolves.
+ */
+#define BSIZE 1024
+
+static int tmemc_list_client(struct client *c, tmem_cli_va_param_t buf,
+                             int off, uint32_t len, bool_t use_long)
+{
+    char info[BSIZE];
+    int i, n = 0, sum = 0;
+    struct tmem_pool *p;
+    bool_t s;
+
+    n = scnprintf(info,BSIZE,"C=CI:%d,ww:%d,ca:%d,co:%d,fr:%d,"
+        "Tc:%"PRIu64",Ge:%ld,Pp:%ld,Gp:%ld%c",
+        c->cli_id, c->weight, c->cap, c->compress, c->frozen,
+        c->total_cycles, c->succ_eph_gets, c->succ_pers_puts, c->succ_pers_gets,
+        use_long ? ',' : '\n');
+    if (use_long)
+        n += scnprintf(info+n,BSIZE-n,
+             "Ec:%ld,Em:%ld,cp:%ld,cb:%"PRId64",cn:%ld,cm:%ld\n",
+             c->eph_count, c->eph_count_max,
+             c->compressed_pages, c->compressed_sum_size,
+             c->compress_poor, c->compress_nomem);
+    if ( !copy_to_guest_offset(buf, off + sum, info, n + 1) )
+        sum += n;
+    for ( i = 0; i < MAX_POOLS_PER_DOMAIN; i++ )
+    {
+        if ( (p = c->pools[i]) == NULL )
+            continue;
+        s = is_shared(p);
+        n = scnprintf(info,BSIZE,"P=CI:%d,PI:%d,"
+                      "PT:%c%c,U0:%"PRIx64",U1:%"PRIx64"%c",
+                      c->cli_id, p->pool_id,
+                      is_persistent(p) ? 'P' : 'E', s ? 'S' : 'P',
+                      (uint64_t)(s ? p->uuid[0] : 0),
+                      (uint64_t)(s ? p->uuid[1] : 0LL),
+                      use_long ? ',' : '\n');
+        if (use_long)
+            n += scnprintf(info+n,BSIZE-n,
+             "Pc:%d,Pm:%d,Oc:%ld,Om:%ld,Nc:%lu,Nm:%lu,"
+             "ps:%lu,pt:%lu,pd:%lu,pr:%lu,px:%lu,gs:%lu,gt:%lu,"
+             "fs:%lu,ft:%lu,os:%lu,ot:%lu\n",
+             _atomic_read(p->pgp_count), p->pgp_count_max,
+             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->found_gets, p->gets,
+             p->flushs_found, p->flushs, p->flush_objs_found, p->flush_objs);
+        if ( sum + n >= len )
+            return sum;
+        if ( !copy_to_guest_offset(buf, off + sum, info, n + 1) )
+            sum += n;
+    }
+    return sum;
+}
+
+static int tmemc_list_shared(tmem_cli_va_param_t buf, int off, uint32_t len,
+                              bool_t use_long)
+{
+    char info[BSIZE];
+    int i, n = 0, sum = 0;
+    struct tmem_pool *p;
+    struct share_list *sl;
+
+    for ( i = 0; i < MAX_GLOBAL_SHARED_POOLS; i++ )
+    {
+        if ( (p = tmem_global.shared_pools[i]) == NULL )
+            continue;
+        n = scnprintf(info+n,BSIZE-n,"S=SI:%d,PT:%c%c,U0:%"PRIx64",U1:%"PRIx64,
+                      i, is_persistent(p) ? 'P' : 'E',
+                      is_shared(p) ? 'S' : 'P',
+                      p->uuid[0], p->uuid[1]);
+        list_for_each_entry(sl,&p->share_list, share_list)
+            n += scnprintf(info+n,BSIZE-n,",SC:%d",sl->client->cli_id);
+        n += scnprintf(info+n,BSIZE-n,"%c", use_long ? ',' : '\n');
+        if (use_long)
+            n += scnprintf(info+n,BSIZE-n,
+             "Pc:%d,Pm:%d,Oc:%ld,Om:%ld,Nc:%lu,Nm:%lu,"
+             "ps:%lu,pt:%lu,pd:%lu,pr:%lu,px:%lu,gs:%lu,gt:%lu,"
+             "fs:%lu,ft:%lu,os:%lu,ot:%lu\n",
+             _atomic_read(p->pgp_count), p->pgp_count_max,
+             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->found_gets, p->gets,
+             p->flushs_found, p->flushs, p->flush_objs_found, p->flush_objs);
+        if ( sum + n >= len )
+            return sum;
+        if ( !copy_to_guest_offset(buf, off + sum, info, n + 1) )
+            sum += n;
+    }
+    return sum;
+}
+
+static int tmemc_list_global_perf(tmem_cli_va_param_t buf, int off,
+                                  uint32_t len, bool_t use_long)
+{
+    char info[BSIZE];
+    int n = 0, sum = 0;
+
+    n = scnprintf(info+n,BSIZE-n,"T=");
+    n--; /* Overwrite trailing comma. */
+    n += scnprintf(info+n,BSIZE-n,"\n");
+    if ( sum + n >= len )
+        return sum;
+    if ( !copy_to_guest_offset(buf, off + sum, info, n + 1) )
+        sum += n;
+    return sum;
+}
+
+static int tmemc_list_global(tmem_cli_va_param_t buf, int off, uint32_t len,
+                              bool_t use_long)
+{
+    char info[BSIZE];
+    int n = 0, sum = off;
+
+    n += scnprintf(info,BSIZE,"G="
+      "Tt:%lu,Te:%lu,Cf:%lu,Af:%lu,Pf:%lu,Ta:%lu,"
+      "Lm:%lu,Et:%lu,Ea:%lu,Rt:%lu,Ra:%lu,Rx:%lu,Fp:%lu%c",
+      tmem_stats.total_tmem_ops, tmem_stats.errored_tmem_ops, tmem_stats.failed_copies,
+      tmem_stats.alloc_failed, tmem_stats.alloc_page_failed, tmem_page_list_pages,
+      tmem_stats.low_on_memory, tmem_stats.evicted_pgs,
+      tmem_stats.evict_attempts, tmem_stats.relinq_pgs, tmem_stats.relinq_attempts,
+      tmem_stats.max_evicts_per_relinq,
+      tmem_stats.total_flush_pool, use_long ? ',' : '\n');
+    if (use_long)
+        n += scnprintf(info+n,BSIZE-n,
+          "Ec:%ld,Em:%ld,Oc:%d,Om:%d,Nc:%d,Nm:%d,Pc:%d,Pm:%d,"
+          "Fc:%d,Fm:%d,Sc:%d,Sm:%d,Ep:%lu,Gd:%lu,Zt:%lu,Gz:%lu\n",
+          tmem_global.eph_count, tmem_stats.global_eph_count_max,
+          _atomic_read(tmem_stats.global_obj_count), tmem_stats.global_obj_count_max,
+          _atomic_read(tmem_stats.global_rtree_node_count), tmem_stats.global_rtree_node_count_max,
+          _atomic_read(tmem_stats.global_pgp_count), tmem_stats.global_pgp_count_max,
+          _atomic_read(tmem_stats.global_page_count), tmem_stats.global_page_count_max,
+          _atomic_read(tmem_stats.global_pcd_count), tmem_stats.global_pcd_count_max,
+         tmem_stats.tot_good_eph_puts,tmem_stats.deduped_puts,tmem_stats.pcd_tot_tze_size,
+         tmem_stats.pcd_tot_csize);
+    if ( sum + n >= len )
+        return sum;
+    if ( !copy_to_guest_offset(buf, off + sum, info, n + 1) )
+        sum += n;
+    return sum;
+}
+
+static int tmemc_list(domid_t cli_id, tmem_cli_va_param_t buf, uint32_t len,
+                               bool_t use_long)
+{
+    struct client *client;
+    int off = 0;
+
+    if ( cli_id == TMEM_CLI_ID_NULL ) {
+        off = tmemc_list_global(buf,0,len,use_long);
+        off += tmemc_list_shared(buf,off,len-off,use_long);
+        list_for_each_entry(client,&tmem_global.client_list,client_list)
+            off += tmemc_list_client(client, buf, off, len-off, use_long);
+        off += tmemc_list_global_perf(buf,off,len-off,use_long);
+    }
+    else if ( (client = tmem_client_from_cli_id(cli_id)) == NULL)
+        return -1;
+    else
+        off = tmemc_list_client(client, buf, 0, len, use_long);
+
+    return 0;
+}
+
+static int __tmemc_set_var(struct client *client, uint32_t subop, uint32_t arg1)
+{
+    domid_t cli_id = client->cli_id;
+    uint32_t old_weight;
+
+    switch (subop)
+    {
+    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",
+                        arg1, tmem_cli_id_str, cli_id);
+        atomic_sub(old_weight,&tmem_global.client_weight_total);
+        atomic_add(client->weight,&tmem_global.client_weight_total);
+        break;
+    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 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",
+                            tmem_compression_enabled() ? "enabled" : "disabled",
+                            tmem_client_str);
+            return -1;
+        }
+        client->compress = arg1 ? 1 : 0;
+        tmem_client_info("tmem: compression %s for %s=%d\n",
+            arg1 ? "enabled" : "disabled",tmem_cli_id_str,cli_id);
+        break;
+    default:
+        tmem_client_warn("tmem: unknown subop %d for tmemc_set_var\n", subop);
+        return -1;
+    }
+    return 0;
+}
+
+static int tmemc_set_var(domid_t cli_id, uint32_t subop, uint32_t arg1)
+{
+    struct client *client;
+    int ret = -1;
+
+    if ( cli_id == TMEM_CLI_ID_NULL )
+    {
+        list_for_each_entry(client,&tmem_global.client_list,client_list)
+        {
+            ret =  __tmemc_set_var(client, subop, arg1);
+            if (ret)
+                break;
+        }
+    }
+    else
+    {
+        client = tmem_client_from_cli_id(cli_id);
+        if ( client )
+            ret = __tmemc_set_var(client, subop, arg1);
+    }
+    return ret;
+}
+
+static int tmemc_save_subop(int cli_id, uint32_t pool_id,
+                        uint32_t subop, tmem_cli_va_param_t buf, uint32_t arg1)
+{
+    struct client *client = tmem_client_from_cli_id(cli_id);
+    struct tmem_pool *pool = (client == NULL || pool_id >= MAX_POOLS_PER_DOMAIN)
+                   ? NULL : client->pools[pool_id];
+    int rc = -1;
+
+    switch(subop)
+    {
+    case XEN_SYSCTL_TMEM_OP_SAVE_GET_VERSION:
+        rc = TMEM_SPEC_VERSION;
+        break;
+    case XEN_SYSCTL_TMEM_OP_SAVE_GET_MAXPOOLS:
+        rc = MAX_POOLS_PER_DOMAIN;
+        break;
+    case XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_WEIGHT:
+        if ( client == NULL )
+            break;
+        rc = client->weight == -1 ? -2 : client->weight;
+        break;
+    case XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_CAP:
+        if ( client == NULL )
+            break;
+        rc = client->cap == -1 ? -2 : client->cap;
+        break;
+    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 XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_FLAGS:
+         if ( pool == NULL )
+             break;
+         rc = (pool->persistent ? TMEM_POOL_PERSIST : 0) |
+              (pool->shared ? TMEM_POOL_SHARED : 0) |
+              (POOL_PAGESHIFT << TMEM_POOL_PAGESIZE_SHIFT) |
+              (TMEM_SPEC_VERSION << TMEM_POOL_VERSION_SHIFT);
+        break;
+    case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_NPAGES:
+         if ( pool == NULL )
+             break;
+        rc = _atomic_read(pool->pgp_count);
+        break;
+    case XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_UUID:
+         if ( pool == NULL )
+             break;
+        rc = 0;
+        if ( copy_to_guest(guest_handle_cast(buf, void), pool->uuid, 2) )
+            rc = -EFAULT;
+        break;
+    default:
+        rc = -1;
+    }
+    return rc;
+}
+
+int tmem_control(struct xen_sysctl_tmem_op *op)
+{
+    int ret;
+    uint32_t pool_id = op->pool_id;
+    uint32_t cmd = op->cmd;
+
+    if ( op->pad != 0 )
+        return -EINVAL;
+
+    write_lock(&tmem_rwlock);
+
+    switch (cmd)
+    {
+    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 XEN_SYSCTL_TMEM_OP_FLUSH:
+        ret = tmemc_flush_mem(op->cli_id,op->arg1);
+        break;
+    case XEN_SYSCTL_TMEM_OP_LIST:
+        ret = tmemc_list(op->cli_id,
+                         guest_handle_cast(op->buf, char), op->arg1, op->arg2);
+        break;
+    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 XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB:
+        ret = tmem_freeable_pages() >> (20 - PAGE_SHIFT);
+        break;
+    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:
+        ret = tmemc_save_subop(op->cli_id, pool_id, cmd,
+                               guest_handle_cast(op->buf, char), op->arg1);
+        break;
+    default:
+        ret = do_tmem_control(op);
+        break;
+    }
+
+    write_unlock(&tmem_rwlock);
+
+    return ret;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/tmem_control.h b/xen/include/xen/tmem_control.h
new file mode 100644
index 0000000..44bc07f
--- /dev/null
+++ b/xen/include/xen/tmem_control.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#ifndef __XEN_TMEM_CONTROL_H__
+#define __XEN_TMEM_CONTROL_H__
+
+#ifdef CONFIG_TMEM
+#include <public/sysctl.h>
+/* Variables and functions that tmem_control.c needs from tmem.c */
+
+extern struct tmem_statistics tmem_stats;
+extern struct tmem_global tmem_global;
+
+extern rwlock_t tmem_rwlock;
+
+int tmem_evict(void);
+int do_tmem_control(struct xen_sysctl_tmem_op *op);
+
+#endif /* CONFIG_TMEM */
+
+#endif /* __XEN_TMEM_CONTROL_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/tmem_xen.h b/xen/include/xen/tmem_xen.h
index 19ed835..582951a 100644
--- a/xen/include/xen/tmem_xen.h
+++ b/xen/include/xen/tmem_xen.h
@@ -16,6 +16,7 @@
 #include <xen/guest_access.h> /* copy_from_guest */
 #include <xen/hash.h> /* hash_long */
 #include <xen/domain_page.h> /* __map_domain_page */
+#include <xen/rbtree.h> /* struct rb_root */
 #include <xsm/xsm.h> /* xsm_tmem_control */
 #include <public/tmem.h>
 #ifdef CONFIG_COMPAT
@@ -341,4 +342,131 @@ extern int tmem_copy_tze_to_client(xen_pfn_t cmfn, void *tmem_va, pagesize_t len
 #define tmem_client_warn(fmt, args...) printk(XENLOG_G_WARNING fmt, ##args)
 #define tmem_client_info(fmt, args...) printk(XENLOG_G_INFO fmt, ##args)
 
+/* Global statistics (none need to be locked). */
+struct tmem_statistics {
+    unsigned long total_tmem_ops;
+    unsigned long errored_tmem_ops;
+    unsigned long total_flush_pool;
+    unsigned long alloc_failed;
+    unsigned long alloc_page_failed;
+    unsigned long evicted_pgs;
+    unsigned long evict_attempts;
+    unsigned long relinq_pgs;
+    unsigned long relinq_attempts;
+    unsigned long max_evicts_per_relinq;
+    unsigned long low_on_memory;
+    unsigned long deduped_puts;
+    unsigned long tot_good_eph_puts;
+    int global_obj_count_max;
+    int global_pgp_count_max;
+    int global_pcd_count_max;
+    int global_page_count_max;
+    int global_rtree_node_count_max;
+    long global_eph_count_max;
+    unsigned long failed_copies;
+    unsigned long pcd_tot_tze_size;
+    unsigned long pcd_tot_csize;
+    /* Global counters (should use long_atomic_t access). */
+    atomic_t global_obj_count;
+    atomic_t global_pgp_count;
+    atomic_t global_pcd_count;
+    atomic_t global_page_count;
+    atomic_t global_rtree_node_count;
+};
+
+#define atomic_inc_and_max(_c) do { \
+    atomic_inc(&tmem_stats._c); \
+    if ( _atomic_read(tmem_stats._c) > tmem_stats._c##_max ) \
+        tmem_stats._c##_max = _atomic_read(tmem_stats._c); \
+} while (0)
+
+#define atomic_dec_and_assert(_c) do { \
+    atomic_dec(&tmem_stats._c); \
+    ASSERT(_atomic_read(tmem_stats._c) >= 0); \
+} while (0)
+
+#define MAX_GLOBAL_SHARED_POOLS  16
+struct tmem_global {
+    struct list_head ephemeral_page_list;  /* All pages in ephemeral pools. */
+    struct list_head client_list;
+    struct tmem_pool *shared_pools[MAX_GLOBAL_SHARED_POOLS];
+    bool_t shared_auth;
+    long eph_count;  /* Atomicity depends on eph_lists_spinlock. */
+    atomic_t client_weight_total;
+};
+
+#define MAX_POOLS_PER_DOMAIN 16
+
+struct tmem_pool;
+struct tmem_page_descriptor;
+struct tmem_page_content_descriptor;
+struct client {
+    struct list_head client_list;
+    struct tmem_pool *pools[MAX_POOLS_PER_DOMAIN];
+    struct domain *domain;
+    struct xmem_pool *persistent_pool;
+    struct list_head ephemeral_page_list;
+    long eph_count, eph_count_max;
+    domid_t cli_id;
+    uint32_t weight;
+    uint32_t cap;
+    bool_t compress;
+    bool_t frozen;
+    bool_t shared_auth_required;
+    /* 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. */
+    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. */
+    uint64_t shared_auth_uuid[MAX_GLOBAL_SHARED_POOLS][2];
+};
+
+#define POOL_PAGESHIFT (PAGE_SHIFT - 12)
+#define OBJ_HASH_BUCKETS 256 /* Must be power of two. */
+#define OBJ_HASH_BUCKETS_MASK (OBJ_HASH_BUCKETS-1)
+
+#define is_persistent(_p)  (_p->persistent)
+#define is_shared(_p)      (_p->shared)
+
+struct tmem_pool {
+    bool_t shared;
+    bool_t persistent;
+    bool_t is_dying;
+    struct client *client;
+    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 list_head persistent_page_list;
+    struct tmem_page_descriptor *cur_pgp;
+    /* Statistics collection. */
+    atomic_t pgp_count;
+    int pgp_count_max;
+    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;
+    uint64_t sum_evicted_cycles;
+    unsigned long puts, good_puts, no_mem_puts;
+    unsigned long dup_puts_flushed, dup_puts_replaced;
+    unsigned long gets, found_gets;
+    unsigned long flushs, flushs_found;
+    unsigned long flush_objs, flush_objs_found;
+};
+
+struct share_list {
+    struct list_head share_list;
+    struct client *client;
+};
+
 #endif /* __XEN_TMEM_XEN_H__ */
-- 
2.5.5


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

Re: [PATCH v1 for 4.7] Tmem cleanups.

[Jan Beulich]

>>> On 02.06.16 at 17:04, <konrad.wilk@oracle.com> wrote:
> Konrad Rzeszutek Wilk (4):
>       tmem: Move global stats in a tmem_statistics structure
>       tmem: Wrap atomic_t in struct tmem_statistics as well.
>       tmem: Move global_ individual variables in a global structure.
>       tmem: Move bulk of tmem control functions in its own file.

It's not really clear what the policy for components with just a
single maintainer is, so if you feel like you want/need an ack,
you can have mine.

Jan


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

Re: [PATCH v1 for 4.7] Tmem cleanups.

[Konrad Rzeszutek Wilk]

On Thu, Jun 02, 2016 at 09:17:42AM -0600, Jan Beulich wrote:
> >>> On 02.06.16 at 17:04, <konrad.wilk@oracle.com> wrote:
> > Konrad Rzeszutek Wilk (4):
> >       tmem: Move global stats in a tmem_statistics structure
> >       tmem: Wrap atomic_t in struct tmem_statistics as well.
> >       tmem: Move global_ individual variables in a global structure.
> >       tmem: Move bulk of tmem control functions in its own file.
> 
> It's not really clear what the policy for components with just a
> single maintainer is, so if you feel like you want/need an ack,
> you can have mine.

Thank you!

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

Re: [PATCH v1 for 4.7] Tmem cleanups.

[Wei Liu]

On Thu, Jun 02, 2016 at 11:04:16AM -0400, Konrad Rzeszutek Wilk wrote:
> Hey!
> 
> Since RFC [http://www.gossamer-threads.com/lists/xen/devel/431812]
>  - Added Reviewed-by from Doug
>  - Dropped the RFC
> 
> 
> These four little cleanups move the bulk of tmem control ops
> from tmem.c to tmem_control.c.
> 
> Last release I moved the control tmem ops from being part of tmem
> hypercall to be part of the syscall subops - and this is the next
> step in this cleanup. (See
> http://lists.xen.org/archives/html/xen-devel/2015-10/msg03313.html)
> which will allow me to follow up on the other steps:
> b) Fix the toolstack (cleanup)
> c) tmem tze, dedup, and zlib code drop
> 
> Anyhow sorry for this being so tardy - xSplice had more attention :-)
> 

What do you mean here? There is no such thing called xSplice! :-P

> Regression tests show no problems.
> 
> The patches themselves have no functionality changes thought I was itching
> to remove most of the counters. I will do that going forward, but need
> to figure out which ones make sense or if some of them can be coalesced. 
> 
> 
>  xen/common/Makefile            |   2 +-
>  xen/common/tmem.c              | 618 +++++------------------------------------
>  xen/common/tmem_control.c      | 443 +++++++++++++++++++++++++++++
>  xen/include/xen/tmem_control.h |  33 +++
>  xen/include/xen/tmem_xen.h     | 128 +++++++++
>  5 files changed, 672 insertions(+), 552 deletions(-)
> 

Since this is only tmem code, the only risk is it doesn't build in
osstest, but I think the probability is quite low.

If you confirm that it builds on Debian Jessie, that would be good
enough for me:

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

> Konrad Rzeszutek Wilk (4):
>       tmem: Move global stats in a tmem_statistics structure
>       tmem: Wrap atomic_t in struct tmem_statistics as well.
>       tmem: Move global_ individual variables in a global structure.
>       tmem: Move bulk of tmem control functions in its own file.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

Re: [PATCH v1 for 4.7] Tmem cleanups.

[Konrad Rzeszutek Wilk]

On Thu, Jun 02, 2016 at 04:42:12PM +0100, Wei Liu wrote:
> On Thu, Jun 02, 2016 at 11:04:16AM -0400, Konrad Rzeszutek Wilk wrote:
> > Hey!
> > 
> > Since RFC [http://www.gossamer-threads.com/lists/xen/devel/431812]
> >  - Added Reviewed-by from Doug
> >  - Dropped the RFC
> > 
> > 
> > These four little cleanups move the bulk of tmem control ops
> > from tmem.c to tmem_control.c.
> > 
> > Last release I moved the control tmem ops from being part of tmem
> > hypercall to be part of the syscall subops - and this is the next
> > step in this cleanup. (See
> > http://lists.xen.org/archives/html/xen-devel/2015-10/msg03313.html)
> > which will allow me to follow up on the other steps:
> > b) Fix the toolstack (cleanup)
> > c) tmem tze, dedup, and zlib code drop
> > 
> > Anyhow sorry for this being so tardy - xSplice had more attention :-)
> > 
> 
> What do you mean here? There is no such thing called xSplice! :-P

Hehehe.
> 
> > Regression tests show no problems.
> > 
> > The patches themselves have no functionality changes thought I was itching
> > to remove most of the counters. I will do that going forward, but need
> > to figure out which ones make sense or if some of them can be coalesced. 
> > 
> > 
> >  xen/common/Makefile            |   2 +-
> >  xen/common/tmem.c              | 618 +++++------------------------------------
> >  xen/common/tmem_control.c      | 443 +++++++++++++++++++++++++++++
> >  xen/include/xen/tmem_control.h |  33 +++
> >  xen/include/xen/tmem_xen.h     | 128 +++++++++
> >  5 files changed, 672 insertions(+), 552 deletions(-)
> > 
> 
> Since this is only tmem code, the only risk is it doesn't build in
> osstest, but I think the probability is quite low.
> 
> If you confirm that it builds on Debian Jessie, that would be good
> enough for me:

Sure thing. Will make sure and if there are no issues will
push it in staging.

Thanks!
> 
> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> > Konrad Rzeszutek Wilk (4):
> >       tmem: Move global stats in a tmem_statistics structure
> >       tmem: Wrap atomic_t in struct tmem_statistics as well.
> >       tmem: Move global_ individual variables in a global structure.
> >       tmem: Move bulk of tmem control functions in its own file.
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel

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

Re: [PATCH v1 for 4.7] Tmem cleanups.

[Konrad Rzeszutek Wilk]

> > If you confirm that it builds on Debian Jessie, that would be good
> > enough for me:
> 
> Sure thing. Will make sure and if there are no issues will
> push it in staging.

Confirmed. No issues building on Jessie.
> 
> Thanks!
> > 
> > Release-acked-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > > Konrad Rzeszutek Wilk (4):
> > >       tmem: Move global stats in a tmem_statistics structure
> > >       tmem: Wrap atomic_t in struct tmem_statistics as well.
> > >       tmem: Move global_ individual variables in a global structure.
> > >       tmem: Move bulk of tmem control functions in its own file.
> > > 
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@lists.xen.org
> > > http://lists.xen.org/xen-devel

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