cluster/afr: Fix inode_forget assert failure

Problem:
If two self-heals are triggered on same inode in
parallel then one inode will be linked and the other
inode will not be linked as an inode with that gfid
is already linked in inode table. Calling inode-forget
on that inode leads to assert failure.

Fix:
Always use linked inode for performing self-heal.

Added inode-forgets in other places as well even though
its not really a memory leak.

Change-Id: Ib84bf080c8cb6a4243f66541ece587db28f9a052
BUG: 1091597
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/7567
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Anand Avati <avati@redhat.com>

2 files changed, 47 insertions, 28 deletions

diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c
index 4dac8311340..6b39dcf9dbc 100644
--- a/xlators/cluster/afr/src/afr-self-heal-common.c
+++ b/xlators/cluster/afr/src/afr-self-heal-common.c
@@ -773,20 +773,16 @@ afr_is_entry_set (xlator_t *this, dict_t *xdata)
 }
 
 
-void
+inode_t*
 afr_inode_link (inode_t *inode, struct iatt *iatt)
 {
 	inode_t *linked_inode = NULL;
 
 	linked_inode = inode_link (inode, NULL, NULL, iatt);
 
-	uuid_copy (inode->gfid, iatt->ia_gfid);
-	inode->ia_type = iatt->ia_type;
-
-	if (linked_inode) {
+	if (linked_inode)
 		inode_lookup (linked_inode);
-		inode_unref (linked_inode);
-	}
+        return linked_inode;
 }
 
 
@@ -801,12 +797,13 @@ afr_inode_link (inode_t *inode, struct iatt *iatt)
 
 int
 afr_selfheal_unlocked_inspect (call_frame_t *frame, xlator_t *this,
-			       inode_t *inode, uuid_t gfid,
+			       uuid_t gfid, inode_t **link_inode,
 			       gf_boolean_t *data_selfheal,
 			       gf_boolean_t *metadata_selfheal,
 			       gf_boolean_t *entry_selfheal)
 {
 	afr_private_t *priv = NULL;
+        inode_t *inode = NULL;
 	int i = 0;
 	int valid_cnt = 0;
 	struct iatt first = {0, };
@@ -815,11 +812,15 @@ afr_selfheal_unlocked_inspect (call_frame_t *frame, xlator_t *this,
 
 	priv = this->private;
 
+        inode = afr_inode_find (this, gfid);
+        if (!inode)
+                goto out;
+
 	replies = alloca0 (sizeof (*replies) * priv->child_count);
 
 	ret = afr_selfheal_unlocked_discover (frame, inode, gfid, replies);
 	if (ret)
-		return ret;
+                goto out;
 
 	for (i = 0; i < priv->child_count; i++) {
 		if (!replies[i].valid)
@@ -849,7 +850,8 @@ afr_selfheal_unlocked_inspect (call_frame_t *frame, xlator_t *this,
 				(int) replies[i].poststat.ia_type,
 				priv->children[i]->name,
 				uuid_utoa (replies[i].poststat.ia_gfid));
-			return -EIO;
+                        ret = -EIO;
+                        goto out;
 		}
 
 		if (!IA_EQUAL (first, replies[i].poststat, uid)) {
@@ -898,13 +900,22 @@ afr_selfheal_unlocked_inspect (call_frame_t *frame, xlator_t *this,
 		}
 	}
 
-	if (valid_cnt > 0)
-		afr_inode_link (inode, &first);
-
-	if (valid_cnt < 2)
-		return -ENOTCONN;
-
-	return 0;
+	if (valid_cnt > 0) {
+		*link_inode = afr_inode_link (inode, &first);
+                if (!*link_inode) {
+                        ret = -EINVAL;
+                        goto out;
+                }
+        } else if (valid_cnt < 2) {
+		ret = -ENOTCONN;
+                goto out;
+        }
+
+        ret = 0;
+out:
+        if (inode)
+                inode_unref (inode);
+	return ret;
 }
 
 
@@ -967,22 +978,18 @@ afr_frame_create (xlator_t *this)
 int
 afr_selfheal (xlator_t *this, uuid_t gfid)
 {
-	inode_t *inode = NULL;
+        inode_t *inode = NULL;
 	call_frame_t *frame = NULL;
 	int ret = -1;
 	gf_boolean_t data_selfheal = _gf_false;
 	gf_boolean_t metadata_selfheal = _gf_false;
 	gf_boolean_t entry_selfheal = _gf_false;
 
-	inode = afr_inode_find (this, gfid);
-	if (!inode)
-		goto out;
-
 	frame = afr_frame_create (this);
 	if (!frame)
 		goto out;
 
-	ret = afr_selfheal_unlocked_inspect (frame, this, inode, gfid,
+	ret = afr_selfheal_unlocked_inspect (frame, this, gfid, &inode,
 					     &data_selfheal,
 					     &metadata_selfheal,
 					     &entry_selfheal);
@@ -999,9 +1006,8 @@ afr_selfheal (xlator_t *this, uuid_t gfid)
 		afr_selfheal_entry (frame, this, inode);
 
 	inode_forget (inode, 1);
+        inode_unref (inode);
 out:
-	if (inode)
-		inode_unref (inode);
 	if (frame)
 		AFR_STACK_DESTROY (frame);
 
diff --git a/xlators/cluster/afr/src/afr-self-heald.c b/xlators/cluster/afr/src/afr-self-heald.c
index 4bfe909bcb9..787cb4ff557 100644
--- a/xlators/cluster/afr/src/afr-self-heald.c
+++ b/xlators/cluster/afr/src/afr-self-heald.c
@@ -186,8 +186,10 @@ afr_shd_inode_find (xlator_t *this, xlator_t *subvol, uuid_t gfid)
 	struct iatt iatt = {0, };
 
 	inode = inode_find (this->itable, gfid);
-	if (inode)
+	if (inode) {
+                inode_lookup (inode);
 		goto out;
+        }
 
 	loc.inode = inode_new (this->itable);
 	if (!loc.inode)
@@ -245,6 +247,9 @@ afr_shd_index_opendir (xlator_t *this, int child)
 	fd = fd_anonymous (inode);
 out:
 	loc_wipe (&rootloc);
+        if (inode)
+                inode_unref (inode);
+
 	if (xattr)
 		dict_unref (xattr);
 	return fd;
@@ -440,8 +445,12 @@ afr_shd_index_sweep (struct subvol_healer *healer)
 			break;
 	}
 
-	if (fd)
+	if (fd) {
+                if (fd->inode)
+                        inode_forget (fd->inode, 1);
 		fd_unref (fd);
+        }
+
 	if (!ret)
 		ret = count;
 	return ret;
@@ -942,8 +951,12 @@ afr_shd_gather_index_entries (xlator_t *this, int child, dict_t *output)
 			break;
 	}
 
-	if (fd)
+	if (fd) {
+                if (fd->inode)
+                        inode_forget (fd->inode, 1);
 		fd_unref (fd);
+        }
+
 	if (!ret)
 		ret = count;
 	return ret;