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: 1211749
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>

1 files changed, 19 insertions, 11 deletions

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;