summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKrutika Dhananjay <kdhananj@redhat.com>2016-11-25 15:54:30 +0530
committerPranith Kumar Karampuri <pkarampu@redhat.com>2016-11-26 03:10:31 -0800
commit2fe8ba52108e94268bc816ba79074a96c4538271 (patch)
treec8a1a1411177483504b283a0619943579350b619
parent3a5169907b44d79e207c35941b1973b1f60d2079 (diff)
cluster/afr: Fix deadlock due to compound fops
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: Idb4938f90397fb4bd90921f9ae6ea582042e5c67 BUG: 1398566 Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com> Reviewed-on: http://review.gluster.org/15929 Smoke: Gluster Build System <jenkins@build.gluster.org> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> CentOS-regression: Gluster Build System <jenkins@build.gluster.org> Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
-rw-r--r--xlators/cluster/afr/src/afr-transaction.c22
1 files changed, 8 insertions, 14 deletions
diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c
index a456c827f4b..f81932b0c8c 100644
--- a/xlators/cluster/afr/src/afr-transaction.c
+++ b/xlators/cluster/afr/src/afr-transaction.c
@@ -839,7 +839,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);
@@ -913,9 +913,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;
}
}
@@ -924,7 +926,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);
@@ -1439,11 +1441,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;
@@ -1456,8 +1456,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,
@@ -1485,22 +1483,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);
}
}