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>

1 files changed, 12 insertions, 36 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);