From 8f811b1ef400035eaf1adf38db24cc33a8726aab Mon Sep 17 00:00:00 2001 From: Raghavendra G 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 Reviewed-on: http://review.gluster.org/11418 Reviewed-by: Shyamsundar Ranganathan Tested-by: Gluster Build System Reviewed-by: N Balachandran Tested-by: NetBSD Build System Reviewed-on: https://code.engineering.redhat.com/gerrit/52107 Reviewed-by: Nithya Balachandran --- 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