core: use reference counting for mem_acct structures

When freeing memory, our memory-accounting code expects to be able to
dereference from the (previously) allocated block to its owning
translator.  However, as we have already found once in option
validation and twice in logging, that translator might itself have
been freed and the dereference attempt causes on of our daemons to
crash with SIGSEGV.  This patch attempts to fix that as follows:

 * We no longer embed a struct mem_acct directly in a struct xlator,
   but instead allocate it separately.

 * Allocated memory blocks now contain a pointer to the mem_acct
   instead of the xlator.

 * The mem_acct structure contains a reference count, manipulated in
   both the normal and translator allocate/free code using atomic
   increments and decrements.

 * Because it's now a separate structure, we can defer freeing the
   mem_acct until its reference count reaches zero (either way).

 * Some unit tests were disabled, because they embedded their own
   copies of the implementation for what they were supposedly testing.
   Life's too short to spend time fixing tests that seem designed to
   impede progress by requiring a certain implementation as well as
   behavior.

Change-Id: Id929b11387927136f78626901729296b6c0d0fd7
BUG: 1219026
Signed-off-by: Jeff Darcy <jdarcy@redhat.com>
Reviewed-on: http://review.gluster.org/10417
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Krishnan Parthasarathi <kparthas@redhat.com>
Reviewed-by: Niels de Vos <ndevos@redhat.com>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/10723
Tested-by: NetBSD Build System

13 files changed, 198 insertions, 187 deletions

diff --git a/api/src/glfs.c b/api/src/glfs.c
index d6a6fe9c850..897d3eab809 100644
--- a/api/src/glfs.c
+++ b/api/src/glfs.c
@@ -965,7 +965,7 @@ glusterfs_ctx_destroy (glusterfs_ctx_t *ctx)
                 return 0;
 
         /* For all the graphs, crawl through the xlator_t structs and free
-         * all its members except for the mem_acct.rec member,
+         * all its members except for the mem_acct member,
          * as GF_FREE will be referencing it.
          */
         list_for_each_entry_safe (trav_graph, tmp, &ctx->graphs, list) {
diff --git a/libglusterfs/src/Makefile.am b/libglusterfs/src/Makefile.am
index f9accdf42f1..6f37e37a878 100644
--- a/libglusterfs/src/Makefile.am
+++ b/libglusterfs/src/Makefile.am
@@ -77,15 +77,4 @@ if UNITTEST
 CLEANFILES += *.gcda *.gcno *_xunit.xml
 noinst_PROGRAMS =
 TESTS =
-
-mem_pool_unittest_CPPFLAGS = $(libglusterfs_la_CPPFLAGS)
-mem_pool_unittest_SOURCES = mem-pool.c \
-                            mem-pool.h \
-                            unittest/mem_pool_unittest.c \
-                            unittest/log_mock.c \
-                            unittest/global_mock.c
-mem_pool_unittest_CFLAGS = $(UNITTEST_CFLAGS)
-mem_pool_unittest_LDFLAGS = $(UNITTEST_LDFLAGS)
-noinst_PROGRAMS += mem_pool_unittest
-TESTS += mem_pool_unittest
 endif
diff --git a/libglusterfs/src/client_t.c b/libglusterfs/src/client_t.c
index 5b0fd87a9c1..84257e66b09 100644
--- a/libglusterfs/src/client_t.c
+++ b/libglusterfs/src/client_t.c
@@ -155,23 +155,6 @@ gf_client_clienttable_destroy (clienttable_t *clienttable)
 
 
 /*
- * a more comprehensive feature test is shown at
- * http://lists.iptel.org/pipermail/semsdev/2010-October/005075.html
- * this is sufficient for RHEL5 i386 builds
- */
-#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 1)) && !defined(__i386__)
-# define INCREMENT_ATOMIC(lk,op) __sync_add_and_fetch(&op, 1)
-# define DECREMENT_ATOMIC(lk,op) __sync_sub_and_fetch(&op, 1)
-#else
-/* These are only here for old gcc, e.g. on RHEL5 i386.
- * We're not ever going to use this in an if stmt,
- * but let's be pedantically correct for style points */
-# define INCREMENT_ATOMIC(lk,op) do { LOCK (&lk); ++op; UNLOCK (&lk); } while (0)
-/* this is a gcc 'statement expression', it works with llvm/clang too */
-# define DECREMENT_ATOMIC(lk,op) ({ LOCK (&lk); --op; UNLOCK (&lk); op; })
-#endif
-
-/*
  * Increments ref.bind if the client is already present or creates a new
  * client with ref.bind = 1,ref.count = 1 it signifies that
  * as long as ref.bind is > 0 client should be alive.
diff --git a/libglusterfs/src/client_t.h b/libglusterfs/src/client_t.h
index 2accf0c6fc2..f6d161e930d 100644
--- a/libglusterfs/src/client_t.h
+++ b/libglusterfs/src/client_t.h
@@ -78,6 +78,23 @@ typedef struct clienttable clienttable_t;
 
 struct rpcsvc_auth_data;
 
+/*
+ * a more comprehensive feature test is shown at
+ * http://lists.iptel.org/pipermail/semsdev/2010-October/005075.html
+ * this is sufficient for RHEL5 i386 builds
+ */
+#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 1)) && !defined(__i386__)
+# define INCREMENT_ATOMIC(lk, op) __sync_add_and_fetch(&op, 1)
+# define DECREMENT_ATOMIC(lk, op) __sync_sub_and_fetch(&op, 1)
+#else
+/* These are only here for old gcc, e.g. on RHEL5 i386.
+ * We're not ever going to use this in an if stmt,
+ * but let's be pedantically correct for style points */
+# define INCREMENT_ATOMIC(lk, op) do { LOCK (&lk); ++op; UNLOCK (&lk); } while (0)
+/* this is a gcc 'statement expression', it works with llvm/clang too */
+# define DECREMENT_ATOMIC(lk, op) ({ LOCK (&lk); --op; UNLOCK (&lk); op; })
+#endif
+
 client_t *
 gf_client_get (xlator_t *this, struct rpcsvc_auth_data *cred, char *client_uid);
 
diff --git a/libglusterfs/src/mem-pool.c b/libglusterfs/src/mem-pool.c
index 0f9d792c0f1..7d95364c4b2 100644
--- a/libglusterfs/src/mem-pool.c
+++ b/libglusterfs/src/mem-pool.c
@@ -59,30 +59,32 @@ gf_mem_set_acct_info (xlator_t *xl, char **alloc_ptr, size_t size,
 
         GF_ASSERT (xl != NULL);
 
-        GF_ASSERT (xl->mem_acct.rec != NULL);
+        GF_ASSERT (xl->mem_acct != NULL);
 
-        GF_ASSERT (type <= xl->mem_acct.num_types);
+        GF_ASSERT (type <= xl->mem_acct->num_types);
 
-        LOCK(&xl->mem_acct.rec[type].lock);
+        LOCK(&xl->mem_acct->rec[type].lock);
         {
-		if (!xl->mem_acct.rec[type].typestr)
-			xl->mem_acct.rec[type].typestr = typestr;
-                xl->mem_acct.rec[type].size += size;
-                xl->mem_acct.rec[type].num_allocs++;
-                xl->mem_acct.rec[type].total_allocs++;
-                xl->mem_acct.rec[type].max_size =
-                        max (xl->mem_acct.rec[type].max_size,
-                             xl->mem_acct.rec[type].size);
-                xl->mem_acct.rec[type].max_num_allocs =
-                        max (xl->mem_acct.rec[type].max_num_allocs,
-                             xl->mem_acct.rec[type].num_allocs);
+		if (!xl->mem_acct->rec[type].typestr)
+			xl->mem_acct->rec[type].typestr = typestr;
+                xl->mem_acct->rec[type].size += size;
+                xl->mem_acct->rec[type].num_allocs++;
+                xl->mem_acct->rec[type].total_allocs++;
+                xl->mem_acct->rec[type].max_size =
+                        max (xl->mem_acct->rec[type].max_size,
+                             xl->mem_acct->rec[type].size);
+                xl->mem_acct->rec[type].max_num_allocs =
+                        max (xl->mem_acct->rec[type].max_num_allocs,
+                             xl->mem_acct->rec[type].num_allocs);
         }
-        UNLOCK(&xl->mem_acct.rec[type].lock);
+        UNLOCK(&xl->mem_acct->rec[type].lock);
+
+        INCREMENT_ATOMIC (xl->mem_acct->lock, xl->mem_acct->refcnt);
 
         header = (struct mem_header *) ptr;
         header->type = type;
         header->size = size;
-        header->xlator = xl;
+        header->mem_acct = xl->mem_acct;
         header->magic = GF_MEM_HEADER_MAGIC;
 
         ptr += sizeof (struct mem_header);
@@ -149,27 +151,23 @@ __gf_malloc (size_t size, uint32_t type, const char *typestr)
 void *
 __gf_realloc (void *ptr, size_t size)
 {
-        uint32_t           type = 0;
         size_t             tot_size = 0;
-        xlator_t          *xl = NULL;
         char              *new_ptr;
-        struct mem_header *header = NULL;
+        struct mem_header *old_header = NULL;
+        struct mem_header *new_header = NULL;
+        struct mem_header  tmp_header;
 
         if (!THIS->ctx->mem_acct_enable)
                 return REALLOC (ptr, size);
 
         REQUIRE(NULL != ptr);
 
-        tot_size = size + GF_MEM_HEADER_SIZE + GF_MEM_TRAILER_SIZE;
-
-        header = (struct mem_header *) (ptr - GF_MEM_HEADER_SIZE);
-
-        GF_ASSERT (header->magic == GF_MEM_HEADER_MAGIC);
+        old_header = (struct mem_header *) (ptr - GF_MEM_HEADER_SIZE);
+        GF_ASSERT (old_header->magic == GF_MEM_HEADER_MAGIC);
+        tmp_header = *old_header;
 
-        xl = (xlator_t *) header->xlator;
-        type = header->type;
-
-        new_ptr = realloc (header, tot_size);
+        tot_size = size + GF_MEM_HEADER_SIZE + GF_MEM_TRAILER_SIZE;
+        new_ptr = realloc (old_header, tot_size);
         if (!new_ptr) {
                 gf_msg_nomem ("", GF_LOG_ALERT, tot_size);
                 return NULL;
@@ -181,8 +179,25 @@ __gf_realloc (void *ptr, size_t size)
          * in ptr, but the compiler warnings complained
          * about the casting to and forth from void ** to
          * char **.
-         */
+         * TBD: it would be nice to adjust the memory accounting info here,
+         * but calling gf_mem_set_acct_info here is wrong because it bumps
+         * up counts as though this is a new allocation - which it's not.
+         * The consequence of doing nothing here is only that the sizes will be
+         * wrong, but at least the counts won't be.
+        uint32_t           type = 0;
+        xlator_t          *xl = NULL;
+        type = header->type;
+        xl = (xlator_t *) header->xlator;
         gf_mem_set_acct_info (xl, &new_ptr, size, type, NULL);
+         */
+
+        new_header = (struct mem_header *) new_ptr;
+        *new_header = tmp_header;
+        new_header->size = size;
+
+        new_ptr += sizeof (struct mem_header);
+        /* data follows in this gap of 'size' bytes */
+        *(uint32_t *) (new_ptr + size) = GF_MEM_TRAILER_MAGIC;
 
         return (void *)new_ptr;
 }
@@ -235,7 +250,7 @@ __gf_mem_invalidate (void *ptr)
 
         struct mem_invalid inval = {
                 .magic = GF_MEM_INVALID_MAGIC,
-                .xlator = header->xlator,
+                .mem_acct = header->mem_acct,
                 .type = header->type,
                 .size = header->size,
                 .baseaddr = ptr + GF_MEM_HEADER_SIZE,
@@ -271,7 +286,7 @@ void
 __gf_free (void *free_ptr)
 {
         void              *ptr = NULL;
-        xlator_t          *xl = NULL;
+        struct mem_acct   *mem_acct;
         struct mem_header *header = NULL;
 
         if (!THIS->ctx->mem_acct_enable) {
@@ -288,11 +303,8 @@ __gf_free (void *free_ptr)
         //Possible corruption, assert here
         GF_ASSERT (GF_MEM_HEADER_MAGIC == header->magic);
 
-        //gf_free expects xl to be available
-        GF_ASSERT (header->xlator != NULL);
-        xl = header->xlator;
-
-        if (!xl->mem_acct.rec) {
+        mem_acct = header->mem_acct;
+        if (!mem_acct) {
                 goto free;
         }
 
@@ -300,16 +312,21 @@ __gf_free (void *free_ptr)
         GF_ASSERT (GF_MEM_TRAILER_MAGIC ==
                 *(uint32_t *)((char *)free_ptr + header->size));
 
-        LOCK (&xl->mem_acct.rec[header->type].lock);
+        LOCK (&mem_acct->rec[header->type].lock);
         {
-                xl->mem_acct.rec[header->type].size -= header->size;
-                xl->mem_acct.rec[header->type].num_allocs--;
+                mem_acct->rec[header->type].size -= header->size;
+                mem_acct->rec[header->type].num_allocs--;
                 /* If all the instaces are freed up then ensure typestr is
                  * set to NULL */
-                if (!xl->mem_acct.rec[header->type].num_allocs)
-                        xl->mem_acct.rec[header->type].typestr = NULL;
+                if (!mem_acct->rec[header->type].num_allocs)
+                        mem_acct->rec[header->type].typestr = NULL;
         }
-        UNLOCK (&xl->mem_acct.rec[header->type].lock);
+        UNLOCK (&mem_acct->rec[header->type].lock);
+
+        if (DECREMENT_ATOMIC (mem_acct->lock, mem_acct->refcnt) == 0) {
+                FREE (mem_acct);
+        }
+
 free:
 #ifdef DEBUG
         __gf_mem_invalidate (ptr);
diff --git a/libglusterfs/src/mem-pool.h b/libglusterfs/src/mem-pool.h
index 81fb579a0ab..5115cef9f93 100644
--- a/libglusterfs/src/mem-pool.h
+++ b/libglusterfs/src/mem-pool.h
@@ -36,11 +36,6 @@
 #define GF_MEM_TRAILER_MAGIC 0xBAADF00D
 #define GF_MEM_INVALID_MAGIC 0xDEADC0DE
 
-struct mem_acct {
-        uint32_t            num_types;
-        struct mem_acct_rec     *rec;
-};
-
 struct mem_acct_rec {
 	const char     *typestr;
         size_t          size;
@@ -51,12 +46,25 @@ struct mem_acct_rec {
         gf_lock_t       lock;
 };
 
+struct mem_acct {
+        uint32_t            num_types;
+        /*
+         * The lock is only used on ancient platforms (e.g. RHEL5) to keep
+         * refcnt increment/decrement atomic.  We could even make its existence
+         * conditional on the right set of version/feature checks, but it's so
+         * lightweight that it's not worth the obfuscation.
+         */
+        gf_lock_t           lock;
+        unsigned int        refcnt;
+        struct mem_acct_rec rec[0];
+};
+
 struct mem_header {
-        uint32_t  type;
-        size_t    size;
-        void     *xlator;
-        uint32_t  magic;
-        int       padding[8];
+        uint32_t        type;
+        size_t          size;
+        struct mem_acct *mem_acct;
+        uint32_t        magic;
+        int             padding[8];
 };
 
 #define GF_MEM_HEADER_SIZE  (sizeof (struct mem_header))
@@ -64,7 +72,7 @@ struct mem_header {
 #ifdef DEBUG
 struct mem_invalid {
         uint32_t  magic;
-        void     *xlator;
+        void     *mem_acct;
         uint32_t  type;
         size_t    size;
         void     *baseaddr;
diff --git a/libglusterfs/src/options.c b/libglusterfs/src/options.c
index 44f74a498c5..75dce0ea3b6 100644
--- a/libglusterfs/src/options.c
+++ b/libglusterfs/src/options.c
@@ -1019,7 +1019,7 @@ xlator_validate_rec (xlator_t *xlator, char **op_errstr)
         THIS = xlator;
 
         /* Need this here, as this graph has not yet called init() */
-        if (!xlator->mem_acct.num_types) {
+        if (!xlator->mem_acct) {
                 if (!xlator->mem_acct_init)
                         xlator->mem_acct_init = default_mem_acct_init;
                 xlator->mem_acct_init (xlator);
diff --git a/libglusterfs/src/statedump.c b/libglusterfs/src/statedump.c
index a13f6a5f4ac..3eebd2106f9 100644
--- a/libglusterfs/src/statedump.c
+++ b/libglusterfs/src/statedump.c
@@ -225,29 +225,29 @@ gf_proc_dump_xlator_mem_info (xlator_t *xl)
         if (!xl)
                 return;
 
-        if (!xl->mem_acct.rec)
+        if (!xl->mem_acct)
                 return;
 
         gf_proc_dump_add_section ("%s.%s - Memory usage", xl->type, xl->name);
-        gf_proc_dump_write ("num_types", "%d", xl->mem_acct.num_types);
+        gf_proc_dump_write ("num_types", "%d", xl->mem_acct->num_types);
 
-        for (i = 0; i < xl->mem_acct.num_types; i++) {
-                if (!(memcmp (&xl->mem_acct.rec[i], &rec,
+        for (i = 0; i < xl->mem_acct->num_types; i++) {
+                if (!(memcmp (&xl->mem_acct->rec[i], &rec,
                               sizeof (struct mem_acct))))
                         continue;
 
                 gf_proc_dump_add_section ("%s.%s - usage-type %s memusage",
                                           xl->type, xl->name,
-                                          xl->mem_acct.rec[i].typestr);
-                gf_proc_dump_write ("size", "%u", xl->mem_acct.rec[i].size);
+                                          xl->mem_acct->rec[i].typestr);
+                gf_proc_dump_write ("size", "%u", xl->mem_acct->rec[i].size);
                 gf_proc_dump_write ("num_allocs", "%u",
-                                    xl->mem_acct.rec[i].num_allocs);
+                                    xl->mem_acct->rec[i].num_allocs);
                 gf_proc_dump_write ("max_size", "%u",
-                                    xl->mem_acct.rec[i].max_size);
+                                    xl->mem_acct->rec[i].max_size);
                 gf_proc_dump_write ("max_num_allocs", "%u",
-                                    xl->mem_acct.rec[i].max_num_allocs);
+                                    xl->mem_acct->rec[i].max_num_allocs);
                 gf_proc_dump_write ("total_allocs", "%u",
-                                    xl->mem_acct.rec[i].total_allocs);
+                                    xl->mem_acct->rec[i].total_allocs);
         }
 
         return;
@@ -261,29 +261,29 @@ gf_proc_dump_xlator_mem_info_only_in_use (xlator_t *xl)
         if (!xl)
                 return;
 
-        if (!xl->mem_acct.rec)
+        if (!xl->mem_acct->rec)
                 return;
 
         gf_proc_dump_add_section ("%s.%s - Memory usage", xl->type, xl->name);
-        gf_proc_dump_write ("num_types", "%d", xl->mem_acct.num_types);
+        gf_proc_dump_write ("num_types", "%d", xl->mem_acct->num_types);
 
-        for (i = 0; i < xl->mem_acct.num_types; i++) {
-                if (!xl->mem_acct.rec[i].size)
+        for (i = 0; i < xl->mem_acct->num_types; i++) {
+                if (!xl->mem_acct->rec[i].size)
                         continue;
 
                 gf_proc_dump_add_section ("%s.%s - usage-type %d", xl->type,
                                           xl->name,i);
 
                 gf_proc_dump_write ("size", "%u",
-                                    xl->mem_acct.rec[i].size);
+                                    xl->mem_acct->rec[i].size);
                 gf_proc_dump_write ("max_size", "%u",
-                                    xl->mem_acct.rec[i].max_size);
+                                    xl->mem_acct->rec[i].max_size);
                 gf_proc_dump_write ("num_allocs", "%u",
-                                    xl->mem_acct.rec[i].num_allocs);
+                                    xl->mem_acct->rec[i].num_allocs);
                 gf_proc_dump_write ("max_num_allocs", "%u",
-                                    xl->mem_acct.rec[i].max_num_allocs);
+                                    xl->mem_acct->rec[i].max_num_allocs);
                 gf_proc_dump_write ("total_allocs", "%u",
-                                    xl->mem_acct.rec[i].total_allocs);
+                                    xl->mem_acct->rec[i].total_allocs);
         }
 
         return;
diff --git a/libglusterfs/src/unittest/mem_pool_unittest.c b/libglusterfs/src/unittest/mem_pool_unittest.c
index 906ebb0a7f3..a94a15faed7 100644
--- a/libglusterfs/src/unittest/mem_pool_unittest.c
+++ b/libglusterfs/src/unittest/mem_pool_unittest.c
@@ -58,19 +58,20 @@ helper_xlator_init(uint32_t num_types)
 
     xl = test_calloc(1, sizeof(xlator_t));
     assert_non_null(xl);
-    xl->mem_acct.num_types = num_types;
-    xl->mem_acct.rec = test_calloc(num_types, sizeof(struct mem_acct_rec));
-    assert_non_null(xl->mem_acct.rec);
+    xl->mem_acct->num_types = num_types;
+    xl->mem_acct = test_calloc (sizeof(struct mem_acct)
+                                + sizeof(struct mem_acct_rec) * num_types);
+    assert_non_null(xl->mem_acct);
 
     xl->ctx = test_calloc(1, sizeof(glusterfs_ctx_t));
     assert_non_null(xl->ctx);
 
     for (i = 0; i < num_types; i++) {
-            ret = LOCK_INIT(&(xl->mem_acct.rec[i].lock));
+            ret = LOCK_INIT(&(xl->mem_acct->rec[i].lock));
             assert_int_equal(ret, 0);
     }
 
-    ENSURE(num_types == xl->mem_acct.num_types);
+    ENSURE(num_types == xl->mem_acct->num_types);
     ENSURE(NULL != xl);
 
     return xl;
@@ -81,12 +82,12 @@ helper_xlator_destroy(xlator_t *xl)
 {
     int i, ret;
 
-    for (i = 0; i < xl->mem_acct.num_types; i++) {
-            ret = LOCK_DESTROY(&(xl->mem_acct.rec[i].lock));
+    for (i = 0; i < xl->mem_acct->num_types; i++) {
+            ret = LOCK_DESTROY(&(xl->mem_acct->rec[i].lock));
             assert_int_equal(ret, 0);
     }
 
-    free(xl->mem_acct.rec);
+    free(xl->mem_acct->rec);
     free(xl->ctx);
     free(xl);
     return 0;
@@ -143,23 +144,23 @@ test_gf_mem_set_acct_info_asserts(void **state)
     type = 0;
 
 
-    // Check xl is NULL
+    /* Check xl is NULL */
     expect_assert_failure(gf_mem_set_acct_info(NULL, &alloc_ptr, size, type, ""));
-    // Check xl->mem_acct.rec = NULL
+    /* Check xl->mem_acct = NULL */
     expect_assert_failure(gf_mem_set_acct_info(&xltest, &alloc_ptr, 0, type, ""));
-    // Check type <= xl->mem_acct.num_types
+    /* Check type <= xl->mem_acct->num_types */
     type = 100;
     expect_assert_failure(gf_mem_set_acct_info(&xltest, &alloc_ptr, 0, type, ""));
-    // Check alloc is NULL
+    /* Check alloc is NULL */
     assert_int_equal(-1, gf_mem_set_acct_info(&xltest, NULL, size, type, ""));
 
-    // Initialize xl
+    /* Initialize xl */
     xl = helper_xlator_init(10);
 
-    // Test number of types
+    /* Test number of types */
     type = 100;
-    assert_true(NULL != xl->mem_acct.rec);
-    assert_true(type > xl->mem_acct.num_types);
+    assert_true(NULL != xl->mem_acct);
+    assert_true(type > xl->mem_acct->num_types);
     expect_assert_failure(gf_mem_set_acct_info(xl, &alloc_ptr, size, type, ""));
 
     helper_xlator_destroy(xl);
@@ -180,7 +181,7 @@ test_gf_mem_set_acct_info_memory(void **state)
 
     // Initialize xl
     xl = helper_xlator_init(10);
-    assert_null(xl->mem_acct.rec[type].typestr);
+    assert_null(xl->mem_acct->rec[type].typestr);
 
     // Test allocation
     temp_ptr = test_calloc(1, size + GF_MEM_HEADER_SIZE + GF_MEM_TRAILER_SIZE);
@@ -189,12 +190,12 @@ test_gf_mem_set_acct_info_memory(void **state)
     gf_mem_set_acct_info(xl, &alloc_ptr, size, type, typestr);
 
     //Check values
-    assert_ptr_equal(typestr, xl->mem_acct.rec[type].typestr);
-    assert_int_equal(xl->mem_acct.rec[type].size, size);
-    assert_int_equal(xl->mem_acct.rec[type].num_allocs, 1);
-    assert_int_equal(xl->mem_acct.rec[type].total_allocs, 1);
-    assert_int_equal(xl->mem_acct.rec[type].max_size, size);
-    assert_int_equal(xl->mem_acct.rec[type].max_num_allocs, 1);
+    assert_ptr_equal(typestr, xl->mem_acct->rec[type].typestr);
+    assert_int_equal(xl->mem_acct->rec[type].size, size);
+    assert_int_equal(xl->mem_acct->rec[type].num_allocs, 1);
+    assert_int_equal(xl->mem_acct->rec[type].total_allocs, 1);
+    assert_int_equal(xl->mem_acct->rec[type].max_size, size);
+    assert_int_equal(xl->mem_acct->rec[type].max_num_allocs, 1);
 
     // Check memory
     helper_check_memory_headers(temp_ptr, xl, size, type);
@@ -235,11 +236,11 @@ test_gf_calloc_default_calloc(void **state)
     memset(mem, 0x5A, size);
 
     // Check xl did not change
-    assert_int_equal(xl->mem_acct.rec[type].size, 0);
-    assert_int_equal(xl->mem_acct.rec[type].num_allocs, 0);
-    assert_int_equal(xl->mem_acct.rec[type].total_allocs, 0);
-    assert_int_equal(xl->mem_acct.rec[type].max_size, 0);
-    assert_int_equal(xl->mem_acct.rec[type].max_num_allocs, 0);
+    assert_int_equal(xl->mem_acct->rec[type].size, 0);
+    assert_int_equal(xl->mem_acct->rec[type].num_allocs, 0);
+    assert_int_equal(xl->mem_acct->rec[type].total_allocs, 0);
+    assert_int_equal(xl->mem_acct->rec[type].max_size, 0);
+    assert_int_equal(xl->mem_acct->rec[type].max_num_allocs, 0);
 
     free(mem);
     helper_xlator_destroy(xl);
@@ -269,11 +270,11 @@ test_gf_calloc_mem_acct_enabled(void **state)
     memset(mem, 0x5A, size);
 
     // Check xl values
-    assert_int_equal(xl->mem_acct.rec[type].size, size);
-    assert_int_equal(xl->mem_acct.rec[type].num_allocs, 1);
-    assert_int_equal(xl->mem_acct.rec[type].total_allocs, 1);
-    assert_int_equal(xl->mem_acct.rec[type].max_size, size);
-    assert_int_equal(xl->mem_acct.rec[type].max_num_allocs, 1);
+    assert_int_equal(xl->mem_acct->rec[type].size, size);
+    assert_int_equal(xl->mem_acct->rec[type].num_allocs, 1);
+    assert_int_equal(xl->mem_acct->rec[type].total_allocs, 1);
+    assert_int_equal(xl->mem_acct->rec[type].max_size, size);
+    assert_int_equal(xl->mem_acct->rec[type].max_num_allocs, 1);
 
     // Check memory
     helper_check_memory_headers(mem - sizeof(mem_header_t), xl, size, type);
@@ -302,11 +303,11 @@ test_gf_malloc_default_malloc(void **state)
     memset(mem, 0x5A, size);
 
     // Check xl did not change
-    assert_int_equal(xl->mem_acct.rec[type].size, 0);
-    assert_int_equal(xl->mem_acct.rec[type].num_allocs, 0);
-    assert_int_equal(xl->mem_acct.rec[type].total_allocs, 0);
-    assert_int_equal(xl->mem_acct.rec[type].max_size, 0);
-    assert_int_equal(xl->mem_acct.rec[type].max_num_allocs, 0);
+    assert_int_equal(xl->mem_acct->rec[type].size, 0);
+    assert_int_equal(xl->mem_acct->rec[type].num_allocs, 0);
+    assert_int_equal(xl->mem_acct->rec[type].total_allocs, 0);
+    assert_int_equal(xl->mem_acct->rec[type].max_size, 0);
+    assert_int_equal(xl->mem_acct->rec[type].max_num_allocs, 0);
 
     free(mem);
     helper_xlator_destroy(xl);
@@ -336,11 +337,11 @@ test_gf_malloc_mem_acct_enabled(void **state)
     memset(mem, 0x5A, size);
 
     // Check xl values
-    assert_int_equal(xl->mem_acct.rec[type].size, size);
-    assert_int_equal(xl->mem_acct.rec[type].num_allocs, 1);
-    assert_int_equal(xl->mem_acct.rec[type].total_allocs, 1);
-    assert_int_equal(xl->mem_acct.rec[type].max_size, size);
-    assert_int_equal(xl->mem_acct.rec[type].max_num_allocs, 1);
+    assert_int_equal(xl->mem_acct->rec[type].size, size);
+    assert_int_equal(xl->mem_acct->rec[type].num_allocs, 1);
+    assert_int_equal(xl->mem_acct->rec[type].total_allocs, 1);
+    assert_int_equal(xl->mem_acct->rec[type].max_size, size);
+    assert_int_equal(xl->mem_acct->rec[type].max_num_allocs, 1);
 
     // Check memory
     helper_check_memory_headers(mem - sizeof(mem_header_t), xl, size, type);
@@ -374,11 +375,11 @@ test_gf_realloc_default_realloc(void **state)
     memset(mem, 0x5A, size);
 
     // Check xl did not change
-    assert_int_equal(xl->mem_acct.rec[type].size, 0);
-    assert_int_equal(xl->mem_acct.rec[type].num_allocs, 0);
-    assert_int_equal(xl->mem_acct.rec[type].total_allocs, 0);
-    assert_int_equal(xl->mem_acct.rec[type].max_size, 0);
-    assert_int_equal(xl->mem_acct.rec[type].max_num_allocs, 0);
+    assert_int_equal(xl->mem_acct->rec[type].size, 0);
+    assert_int_equal(xl->mem_acct->rec[type].num_allocs, 0);
+    assert_int_equal(xl->mem_acct->rec[type].total_allocs, 0);
+    assert_int_equal(xl->mem_acct->rec[type].max_size, 0);
+    assert_int_equal(xl->mem_acct->rec[type].max_num_allocs, 0);
 
     free(mem);
     helper_xlator_destroy(xl);
@@ -419,11 +420,11 @@ test_gf_realloc_mem_acct_enabled(void **state)
     // not to the realloc + the malloc.
     // Is this a bug?
     //
-    assert_int_equal(xl->mem_acct.rec[type].size, size+1024);
-    assert_int_equal(xl->mem_acct.rec[type].num_allocs, 2);
-    assert_int_equal(xl->mem_acct.rec[type].total_allocs, 2);
-    assert_int_equal(xl->mem_acct.rec[type].max_size, size+1024);
-    assert_int_equal(xl->mem_acct.rec[type].max_num_allocs, 2);
+    assert_int_equal(xl->mem_acct->rec[type].size, size+1024);
+    assert_int_equal(xl->mem_acct->rec[type].num_allocs, 2);
+    assert_int_equal(xl->mem_acct->rec[type].total_allocs, 2);
+    assert_int_equal(xl->mem_acct->rec[type].max_size, size+1024);
+    assert_int_equal(xl->mem_acct->rec[type].max_num_allocs, 2);
 
     // Check memory
     helper_check_memory_headers(mem - sizeof(mem_header_t), xl, size, type);
diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c
index c670ee97aee..628f3499956 100644
--- a/libglusterfs/src/xlator.c
+++ b/libglusterfs/src/xlator.c
@@ -509,16 +509,22 @@ xlator_mem_acct_init (xlator_t *xl, int num_types)
         if (!xl->ctx->mem_acct_enable)
                 return 0;
 
-        xl->mem_acct.num_types = num_types;
 
-        xl->mem_acct.rec = CALLOC(num_types, sizeof(struct mem_acct_rec));
+        xl->mem_acct = MALLOC (sizeof(struct mem_acct)
+                               + sizeof(struct mem_acct_rec) * num_types);
 
-        if (!xl->mem_acct.rec) {
+        if (!xl->mem_acct) {
                 return -1;
         }
+        memset (xl->mem_acct, 0, sizeof(struct mem_acct));
+
+        xl->mem_acct->num_types = num_types;
+        LOCK_INIT (&xl->mem_acct->lock);
+        xl->mem_acct->refcnt = 1;
 
         for (i = 0; i < num_types; i++) {
-                ret = LOCK_INIT(&(xl->mem_acct.rec[i].lock));
+                memset (&xl->mem_acct->rec[i], 0, sizeof(struct mem_acct_rec));
+                ret = LOCK_INIT(&(xl->mem_acct->rec[i].lock));
                 if (ret) {
                         fprintf(stderr, "Unable to lock..errno : %d",errno);
                 }
@@ -559,17 +565,22 @@ xlator_list_destroy (xlator_list_t *list)
 static int
 xlator_memrec_free (xlator_t *xl)
 {
-        uint32_t i = 0;
+        uint32_t        i               = 0;
+        struct mem_acct *mem_acct       = NULL;
 
-        if (!xl)
+        if (!xl) {
                 return 0;
+        }
+        mem_acct = xl->mem_acct;
 
-        if (xl->mem_acct.rec) {
-                for (i = 0; i < xl->mem_acct.num_types; i++) {
-                        LOCK_DESTROY (&(xl->mem_acct.rec[i].lock));
+        if (mem_acct) {
+                for (i = 0; i < mem_acct->num_types; i++) {
+                        LOCK_DESTROY (&(mem_acct->rec[i].lock));
+                }
+                if (DECREMENT_ATOMIC (mem_acct->lock, mem_acct->refcnt) == 0) {
+                        FREE (mem_acct);
+                        xl->mem_acct = NULL;
                 }
-                FREE (xl->mem_acct.rec);
-                xl->mem_acct.rec = NULL;
         }
 
         return 0;
@@ -653,7 +664,6 @@ xlator_tree_free_memacct (xlator_t *tree)
 {
         xlator_t *trav = tree;
         xlator_t *prev = tree;
-        int          i = 0;
 
         if (!tree) {
                 gf_log ("parser", GF_LOG_ERROR, "Translator tree not found");
@@ -662,13 +672,7 @@ xlator_tree_free_memacct (xlator_t *tree)
 
         while (prev) {
                 trav = prev->next;
-                if (prev->mem_acct.rec) {
-                        for (i = 0; i < prev->mem_acct.num_types; i++) {
-                                LOCK_DESTROY (&(prev->mem_acct.rec[i].lock));
-                        }
-                        FREE (prev->mem_acct.rec);
-                }
-                GF_FREE (prev);
+                xlator_memrec_free (prev);
                 prev = trav;
         }
 
diff --git a/libglusterfs/src/xlator.h b/libglusterfs/src/xlator.h
index a238913d03b..ca30f99650e 100644
--- a/libglusterfs/src/xlator.h
+++ b/libglusterfs/src/xlator.h
@@ -889,7 +889,7 @@ struct _xlator {
         inode_table_t      *itable;
         char                init_succeeded;
         void               *private;
-        struct mem_acct     mem_acct;
+        struct mem_acct    *mem_acct;
         uint64_t            winds;
         char                switched;
 
diff --git a/xlators/cluster/dht/src/Makefile.am b/xlators/cluster/dht/src/Makefile.am
index a8e1ec0d286..f6c9ef2607a 100644
--- a/xlators/cluster/dht/src/Makefile.am
+++ b/xlators/cluster/dht/src/Makefile.am
@@ -53,13 +53,4 @@ if UNITTEST
 CLEANFILES += *.gcda *.gcno *_xunit.xml
 noinst_PROGRAMS =
 TESTS =
-
-dht_layout_unittest_CPPFLAGS = $(AM_CPPFLAGS)
-dht_layout_unittest_SOURCES = unittest/dht_layout_unittest.c \
-                              unittest/dht_layout_mock.c \
-                              dht-layout.c
-dht_layout_unittest_CFLAGS = $(AM_CFLAGS) $(UNITTEST_CFLAGS)
-dht_layout_unittest_LDFLAGS = $(UNITTEST_LDFLAGS)
-noinst_PROGRAMS += dht_layout_unittest
-TESTS += dht_layout_unittest
 endif
diff --git a/xlators/cluster/dht/src/unittest/dht_layout_unittest.c b/xlators/cluster/dht/src/unittest/dht_layout_unittest.c
index 3702af366f9..84a89160e38 100644
--- a/xlators/cluster/dht/src/unittest/dht_layout_unittest.c
+++ b/xlators/cluster/dht/src/unittest/dht_layout_unittest.c
@@ -33,9 +33,10 @@ helper_xlator_init(uint32_t num_types)
 
     xl = test_calloc(1, sizeof(xlator_t));
     assert_non_null(xl);
-    xl->mem_acct.num_types = num_types;
-    xl->mem_acct.rec = test_calloc(num_types, sizeof(struct mem_acct_rec));
-    assert_non_null(xl->mem_acct.rec);
+    xl->mem_acct->num_types = num_types;
+    xl->mem_acct = test_calloc (sizeof(struct mem_acct)
+                                + sizeof(struct mem_acct_rec) + num_types);
+    assert_non_null(xl->mem_acct);
 
     xl->ctx = test_calloc(1, sizeof(glusterfs_ctx_t));
     assert_non_null(xl->ctx);