From 478e3949c19c7ef60737f170d9f3ee60aa9f59c9 Mon Sep 17 00:00:00 2001

From: Krutika Dhananjay <kdhananj@redhat.com>

Date: Fri, 25 Nov 2016 15:54:30 +0530

Subject: [PATCH 193/206] cluster/afr: Fix deadlock due to compound fops

        Backport of: http://review.gluster.org/15929

When an afr data transaction is eligible for using

eager-lock, this information is represented in

local->transaction.eager_lock_on. However, if non-blocking

inodelk attempt (which is a full lock) fails, AFR falls back

to blocking locks which are range locks. At this point,

local->transaction.eager_lock[] per brick is reset but

local->transaction.eager_lock_on is still true.

When AFR decides to compound post-op and unlock, it is after

confirming that the transaction did not use eager lock (well,

except for a small bug where local->transaction.locks_acquired[]

is not considered).

But within afr_post_op_unlock_do(), afr again incorrectly sets

the lock range to full-lock based on local->transaction.eager_lock_on

value. This is a bug and can lead to deadlock since the locks acquired

were range locks and a full unlock is being sent leading to unlock failure

and thereby every other lock request (be it from SHD or other clients or

glfsheal) getting blocked forever and the user perceives a hang.

FIX:

Unconditionally rely on the range locks in inodelk object for unlocking

when using compounded post-op + unlock.

Big thanks to Pranith for helping with the debugging.

Change-Id: Ie5af0de1f1a9dc1821dfde3654bab0e54d753281

BUG: 1396166

Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/91332

Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

---

 xlators/cluster/afr/src/afr-transaction.c | 22 ++++++++--------------

 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c

index fe53f49..690062f 100644

--- a/xlators/cluster/afr/src/afr-transaction.c

+++ b/xlators/cluster/afr/src/afr-transaction.c

@@ -838,7 +838,7 @@ afr_changelog_post_op_now (call_frame_t *frame, xlator_t *this)

         int                     ret             = 0;

         int                     idx             = 0;

         int                     nothing_failed  = 1;

-        int                     piggyback       = 0;

+        gf_boolean_t            compounded_unlock = _gf_true;

         gf_boolean_t            need_undirty    = _gf_false;

         afr_handle_quorum (frame);

@@ -912,9 +912,11 @@ afr_changelog_post_op_now (call_frame_t *frame, xlator_t *this)

                                 if (local->transaction.pre_op[i] &&

                                     local->transaction.eager_lock[i]) {

                                         if (fd_ctx->lock_piggyback[i])

-                                                piggyback = 1;

+                                                compounded_unlock = _gf_false;

+                                        else if (fd_ctx->lock_acquired[i])

+                                                compounded_unlock = _gf_false;

                                 }

-                                if (piggyback == 1)

+                                if (compounded_unlock == _gf_false)

                                         break;

                         }

                 }

@@ -923,7 +925,7 @@ afr_changelog_post_op_now (call_frame_t *frame, xlator_t *this)

         /* Do not compound if any brick got piggybacked lock as

          * unlock should not be done for that. */

-        if (local->compound && !piggyback) {

+        if (local->compound && compounded_unlock) {

                 afr_post_op_unlock_do (frame, this, xattr,

                                        afr_changelog_post_op_done,

                                        AFR_TRANSACTION_POST_OP);

@@ -1434,11 +1436,9 @@ afr_post_op_unlock_do (call_frame_t *frame, xlator_t *this, dict_t *xattr,

         compound_args_t         *args           = NULL;

         afr_internal_lock_t     *int_lock       = NULL;

         afr_inodelk_t           *inodelk        = NULL;

-        struct gf_flock         *flock_use      = NULL;

 	int                     i               = 0;

 	int                     call_count      = 0;

         struct gf_flock         flock           = {0,};

-        struct gf_flock         full_flock      = {0,};

         int                     ret             = 0;

 	local = frame->local;

@@ -1451,8 +1451,6 @@ afr_post_op_unlock_do (call_frame_t *frame, xlator_t *this, dict_t *xattr,

                 flock.l_start = inodelk->flock.l_start;

                 flock.l_len   = inodelk->flock.l_len;

                 flock.l_type  = F_UNLCK;

-                full_flock.l_type = F_UNLCK;

-

         }

         ret = afr_changelog_prepare (this, frame, &call_count, changelog_resume,

@@ -1480,22 +1478,18 @@ afr_post_op_unlock_do (call_frame_t *frame, xlator_t *this, dict_t *xattr,

                             local->fd, GF_XATTROP_ADD_ARRAY,

                             xattr, xdata);

         i++;

-        if (!local->transaction.eager_lock_on)

-                flock_use = &flock;

-        else

-                flock_use = &full_flock;

         if (afr_is_inodelk_transaction(local)) {

                 if (local->fd) {

                         COMPOUND_PACK_ARGS (finodelk, GF_FOP_FINODELK,

                                             args, i,

                                             int_lock->domain, local->fd,

-                                            F_SETLK, flock_use, NULL);

+                                            F_SETLK, &flock, NULL);

                 } else {

                         COMPOUND_PACK_ARGS (inodelk, GF_FOP_INODELK,

                                             args, i,

                                             int_lock->domain, &local->loc,

-                                            F_SETLK, flock_use, NULL);

+                                            F_SETLK, &flock, NULL);

                 }

         }

-- 

2.9.3