cluster/afr: Remove backward compatibility for locks with v1

When we have cascading locks with same lk-owner there is a possibility for
a deadlock to happen. One example is as follows:

self-heal takes a lock in data-domain for big name with 256 chars of "aaaa...a"
and starts heal in a 3-way replication when brick-0 is offline and healing from
brick-1 to brick-2 is in progress. So this lock is active on brick-1 and
brick-2. Now brick-0 comes online and an operation wants to take full lock and
the lock is granted at brick-0 and it is waiting for lock on brick-1. As part
of entry healing it takes full locks on all the available bricks and then
proceeds with healing the entry. Now this lock will start waiting on brick-0
because some other operation already has a granted lock on it. This leads to a
deadlock. Operation is waiting for unlock on "aaaa..." by heal where as heal is
waiting for the operation to unlock on brick-0. Initially I thought this is
happening because healing is trying to take a lock on all the available bricks
instead of just the bricks that are participating in heal. But later realized
that same kind of deadlock can happen if a brick goes down after the heal
starts but comes back before it completes. So the essential problem is the
cascading locks with same lk-owner which were added for backward compatibility
with afr-v1 which can be safely removed now that versions with afr-v1 are
already EOL. This patch removes the compatibility with v1 which requires
cascading locks with same lk-owner.

In the next version we can make locking-scheme option a dummy and switch
completely to v2.

BUG: 1401404
Change-Id: Ic9afab8260f5ff4dff5329eb0429811bcb879079
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/16024
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Ravishankar N <ravishankar@redhat.com>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>

2 files changed, 13 insertions, 69 deletions

diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index 80aea0c7d62..fcbfe9b0f2d 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5278,7 +5278,6 @@ afr_selfheal_locked_data_inspect (call_frame_t *frame, xlator_t *this,
                                   gf_boolean_t *pflag)
 {
         int ret = -1;
-        unsigned char *locked_on = NULL;
         unsigned char *data_lock = NULL;
         unsigned char *sources = NULL;
         unsigned char *sinks = NULL;
@@ -5287,12 +5286,8 @@ afr_selfheal_locked_data_inspect (call_frame_t *frame, xlator_t *this,
         afr_private_t   *priv = NULL;
         fd_t          *fd = NULL;
         struct afr_reply *locked_replies = NULL;
-        gf_boolean_t granular_locks = _gf_false;
 
         priv = this->private;
-	if (strcmp ("granular", priv->locking_scheme) == 0)
-	        granular_locks = _gf_true;
-        locked_on = alloca0 (priv->child_count);
         data_lock = alloca0 (priv->child_count);
         sources = alloca0 (priv->child_count);
         sinks = alloca0 (priv->child_count);
@@ -5313,42 +5308,23 @@ afr_selfheal_locked_data_inspect (call_frame_t *frame, xlator_t *this,
 
         locked_replies = alloca0 (sizeof (*locked_replies) * priv->child_count);
 
-        if (!granular_locks) {
-                ret = afr_selfheal_tryinodelk (frame, this, inode,
-                                              priv->sh_domain, 0, 0, locked_on);
-        }
+        ret = afr_selfheal_inodelk (frame, this, inode, this->name,
+                                    0, 0, data_lock);
         {
-                if (!granular_locks && (ret == 0)) {
+                if (ret == 0) {
                         ret = -afr_final_errno (frame->local, priv);
                         if (ret == 0)
-                                ret = -ENOTCONN;/* all invalid responses */
+                                ret = -ENOTCONN; /* all invalid responses */
                         goto out;
                 }
-                ret = afr_selfheal_inodelk (frame, this, inode, this->name,
-                                            0, 0, data_lock);
-                {
-                        if (ret == 0) {
-                                ret = -afr_final_errno (frame->local, priv);
-                                if (ret == 0)
-                                        ret = -ENOTCONN;
-                                /* all invalid responses */
-                                goto unlock;
-                        }
-                        ret = __afr_selfheal_data_prepare (frame, this, inode,
-                                                           data_lock, sources,
-                                                           sinks, healed_sinks,
-                                                           undid_pending,
-                                                           locked_replies,
-                                                           pflag);
-                        *dsh = afr_decide_heal_info (priv, sources, ret);
-                }
-                afr_selfheal_uninodelk (frame, this, inode, this->name, 0, 0,
-                                        data_lock);
-        }
-unlock:
-        if (!granular_locks)
-                afr_selfheal_uninodelk (frame, this, inode, priv->sh_domain, 0,
-                                        0, locked_on);
+                ret = __afr_selfheal_data_prepare (frame, this, inode,
+                                                   data_lock, sources, sinks,
+                                                   healed_sinks, undid_pending,
+                                                   locked_replies, pflag);
+                *dsh = afr_decide_heal_info (priv, sources, ret);
+        }
+        afr_selfheal_uninodelk (frame, this, inode, this->name, 0, 0,
+                                data_lock);
 out:
         if (locked_replies)
                 afr_replies_wipe (locked_replies, priv->child_count);
diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c
index d8fe5422372..08a45787027 100644
--- a/xlators/cluster/afr/src/afr-self-heal-entry.c
+++ b/xlators/cluster/afr/src/afr-self-heal-entry.c
@@ -21,11 +21,6 @@
  * this name can ever come, entry-lock with this name is going to prevent
  * self-heals from older versions while the granular entry-self-heal is going
  * on in newer version.*/
-#define LONG_FILENAME "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"\
-                      "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"\
-                      "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"\
-                      "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"\
-                      "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
 
 static int
 afr_selfheal_entry_delete (xlator_t *this, inode_t *dir, const char *name,
@@ -1050,21 +1045,16 @@ afr_selfheal_entry (call_frame_t *frame, xlator_t *this, inode_t *inode)
 {
 	afr_private_t *priv = NULL;
 	unsigned char *locked_on = NULL;
-        unsigned char *long_name_locked = NULL;
 	fd_t *fd = NULL;
 	int ret = 0;
-	gf_boolean_t granular_locks = _gf_false;
 
 	priv = this->private;
-	if (strcmp ("granular", priv->locking_scheme) == 0)
-	        granular_locks = _gf_true;
 
 	fd = afr_selfheal_data_opendir (this, inode);
 	if (!fd)
 		return -EIO;
 
 	locked_on = alloca0 (priv->child_count);
-	long_name_locked = alloca0 (priv->child_count);
 
 	ret = afr_selfheal_tie_breaker_entrylk (frame, this, inode,
 	                                        priv->sh_domain, NULL,
@@ -1084,29 +1074,7 @@ afr_selfheal_entry (call_frame_t *frame, xlator_t *this, inode_t *inode)
 			goto unlock;
 		}
 
-                if (!granular_locks) {
-                        ret = afr_selfheal_tryentrylk (frame, this, inode,
-                                                      this->name, LONG_FILENAME,
-                                                      long_name_locked);
-                }
-                {
-                        if (!granular_locks && ret < 1) {
-                                gf_msg_debug (this->name, 0, "%s: Skipping"
-                                              " entry self-heal as only %d "
-                                              "sub-volumes could be "
-                                              "locked in special-filename "
-                                              "domain",
-                                              uuid_utoa (fd->inode->gfid),
-                                              ret);
-                                ret = -ENOTCONN;
-                                goto unlock;
-                        }
-                        ret = __afr_selfheal_entry (frame, this, fd, locked_on);
-                }
-                if (!granular_locks)
-                        afr_selfheal_unentrylk (frame, this, inode, this->name,
-                                               LONG_FILENAME, long_name_locked,
-                                               NULL);
+                ret = __afr_selfheal_entry (frame, this, fd, locked_on);
 	}
 unlock:
 	afr_selfheal_unentrylk (frame, this, inode, priv->sh_domain, NULL,