Blob Blame History Raw
From 8f811b1ef400035eaf1adf38db24cc33a8726aab Mon Sep 17 00:00:00 2001
From: Raghavendra G <rgowdapp@redhat.com>
Date: Fri, 26 Jun 2015 11:53:11 +0530
Subject: [PATCH 197/200] cluster/dht: use refcount to manage memory used to store migration
 information.

Without refcounting, we might free up memory while other fops are
still accessing it.

BUG: 1238167
Change-Id: Ia4fa4a651cd6fe2394a0c20cef83c8d2cbc8750f
Signed-off-by: Raghavendra G <rgowdapp@redhat.com>
Reviewed-on: http://review.gluster.org/11418
Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: N Balachandran <nbalacha@redhat.com>
Tested-by: NetBSD Build System <jenkins@build.gluster.org>
Reviewed-on: https://code.engineering.redhat.com/gerrit/52107
Reviewed-by: Nithya Balachandran <nbalacha@redhat.com>
---
 xlators/cluster/dht/src/dht-common.h |    2 +
 xlators/cluster/dht/src/dht-helper.c |   65 +++++++++++++++++++++++-----------
 2 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/xlators/cluster/dht/src/dht-common.h b/xlators/cluster/dht/src/dht-common.h
index c04e85d..5e86b32 100644
--- a/xlators/cluster/dht/src/dht-common.h
+++ b/xlators/cluster/dht/src/dht-common.h
@@ -20,6 +20,7 @@
 #include "dht-messages.h"
 #include "libxlator.h"
 #include "syncop.h"
+#include "refcount.h"
 
 #ifndef _DHT_H
 #define _DHT_H
@@ -506,6 +507,7 @@ struct dir_dfmeta {
 typedef struct dht_migrate_info {
         xlator_t *src_subvol;
         xlator_t *dst_subvol;
+        GF_REF_DECL;
 } dht_migrate_info_t;
 
 #define ENTRY_MISSING(op_ret, op_errno) (op_ret == -1 && op_errno == ENOENT)
diff --git a/xlators/cluster/dht/src/dht-helper.c b/xlators/cluster/dht/src/dht-helper.c
index 6742f29..d8cc61d 100644
--- a/xlators/cluster/dht/src/dht-helper.c
+++ b/xlators/cluster/dht/src/dht-helper.c
@@ -19,6 +19,17 @@
 #include "dht-common.h"
 #include "dht-helper.h"
 
+void
+dht_free_mig_info (void *data)
+{
+        dht_migrate_info_t *miginfo = NULL;
+
+        miginfo = data;
+        GF_FREE (miginfo);
+
+        return;
+}
+
 static inline int
 dht_inode_ctx_set_mig_info (xlator_t *this, inode_t *inode,
                             xlator_t *src_subvol, xlator_t *dst_subvol)
@@ -33,12 +44,13 @@ dht_inode_ctx_set_mig_info (xlator_t *this, inode_t *inode,
 
         miginfo->src_subvol = src_subvol;
         miginfo->dst_subvol = dst_subvol;
+        GF_REF_INIT (miginfo, dht_free_mig_info);
 
         value = (uint64_t) miginfo;
 
         ret = inode_ctx_set1 (inode, this, &value);
         if (ret < 0) {
-                GF_FREE (miginfo);
+                GF_REF_PUT (miginfo);
         }
 
 out:
@@ -54,11 +66,18 @@ dht_inode_ctx_get_mig_info (xlator_t *this, inode_t *inode,
         uint64_t            tmp_miginfo = 0;
         dht_migrate_info_t *miginfo     = NULL;
 
-        ret =  inode_ctx_get1 (inode, this, &tmp_miginfo);
-        if ((ret < 0) || (tmp_miginfo == 0))
-                goto out;
+        LOCK (&inode->lock);
+        {
+                ret =  __inode_ctx_get1 (inode, this, &tmp_miginfo);
+                if ((ret < 0) || (tmp_miginfo == 0)) {
+                        UNLOCK (&inode->lock);
+                        goto out;
+                }
 
-        miginfo = (dht_migrate_info_t *)tmp_miginfo;
+                miginfo = (dht_migrate_info_t *)tmp_miginfo;
+                GF_REF_GET (miginfo);
+        }
+        UNLOCK (&inode->lock);
 
         if (src_subvol)
                 *src_subvol = miginfo->src_subvol;
@@ -66,6 +85,8 @@ dht_inode_ctx_get_mig_info (xlator_t *this, inode_t *inode,
         if (dst_subvol)
                 *dst_subvol = miginfo->dst_subvol;
 
+        GF_REF_PUT (miginfo);
+
 out:
         return ret;
 }
@@ -900,21 +921,22 @@ out:
 int
 dht_migration_complete_check_task (void *data)
 {
-        int           ret         = -1;
-        xlator_t     *src_node    = NULL;
-        xlator_t     *dst_node    = NULL, *linkto_target = NULL;
-        dht_local_t  *local       = NULL;
-        dict_t       *dict        = NULL;
-        struct iatt   stbuf       = {0,};
-        xlator_t     *this        = NULL;
-        call_frame_t *frame       = NULL;
-        loc_t         tmp_loc     = {0,};
-        char         *path        = NULL;
-        dht_conf_t   *conf        = NULL;
-        inode_t      *inode       = NULL;
-        fd_t         *iter_fd     = NULL;
-        uint64_t      tmp_miginfo = 0;
-        int           open_failed = 0;
+        int                 ret         = -1;
+        xlator_t           *src_node    = NULL;
+        xlator_t           *dst_node    = NULL, *linkto_target = NULL;
+        dht_local_t        *local       = NULL;
+        dict_t             *dict        = NULL;
+        struct iatt         stbuf       = {0,};
+        xlator_t           *this        = NULL;
+        call_frame_t       *frame       = NULL;
+        loc_t               tmp_loc     = {0,};
+        char               *path        = NULL;
+        dht_conf_t         *conf        = NULL;
+        inode_t            *inode       = NULL;
+        fd_t               *iter_fd     = NULL;
+        uint64_t            tmp_miginfo = 0;
+        dht_migrate_info_t *miginfo     = NULL;
+        int                 open_failed = 0;
 
         this  = THIS;
         frame = data;
@@ -1018,7 +1040,8 @@ dht_migration_complete_check_task (void *data)
            done on all the fd of inode */
         ret = inode_ctx_reset1 (inode, this, &tmp_miginfo);
         if (tmp_miginfo) {
-                GF_FREE ((void *)tmp_miginfo);
+                miginfo = (void *)tmp_miginfo;
+                GF_REF_PUT (miginfo);
                 goto out;
         }
 
-- 
1.7.1