From fd441bca2a68ca93765f7a270cab1b7ffdf90799 Mon Sep 17 00:00:00 2001 From: Vikas Gorur Date: Mon, 12 Oct 2009 07:01:41 +0000 Subject: cluster/afr: Hold second lock after first lock has been granted for rename transactions. Hold the lock on the {higher_path} only after the lock on the {lower_path} has been granted successfully. Signed-off-by: Anand V. Avati BUG: 112 (parallel deletion of files mounted by different clients on the same back-end hangs and/or does not completely delete) URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=112 --- xlators/cluster/afr/src/afr-transaction.c | 114 ++++++++++++++++++++++-------- 1 file changed, 84 insertions(+), 30 deletions(-) diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c index f7bd5a44e..5f2696b64 100644 --- a/xlators/cluster/afr/src/afr-transaction.c +++ b/xlators/cluster/afr/src/afr-transaction.c @@ -894,18 +894,11 @@ afr_lock_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int done = 0; int child_index = (long) cookie; - int call_count = 0; - local = frame->local; priv = this->private; LOCK (&frame->lock); { - if (local->transaction.type == AFR_ENTRY_RENAME_TRANSACTION) { - /* wait for the other lock to return */ - call_count = --local->call_count; - } - if (op_ret == -1) { if (op_errno == ENOSYS) { /* return ENOTSUP */ @@ -922,16 +915,14 @@ afr_lock_cbk (call_frame_t *frame, void *cookie, xlator_t *this, } UNLOCK (&frame->lock); - if (call_count == 0) { - if ((local->op_ret == -1) && - (local->op_errno == ENOSYS)) { - afr_unlock (frame, this); - } else { - local->transaction.locked_nodes[child_index] = 1; - local->transaction.lock_count++; - afr_lock_rec (frame, this, child_index + 1); - } - } + if ((local->op_ret == -1) && + (local->op_errno == ENOSYS)) { + afr_unlock (frame, this); + } else { + local->transaction.locked_nodes[child_index] = 1; + local->transaction.lock_count++; + afr_lock_rec (frame, this, child_index + 1); + } return 0; } @@ -954,6 +945,81 @@ lower_path (loc_t *l1, const char *b1, loc_t *l2, const char *b2) } +int32_t +afr_lock_lower_cbk (call_frame_t *frame, void *cookie, xlator_t *this, + int32_t op_ret, int32_t op_errno) +{ + afr_private_t *priv = NULL; + afr_local_t *local = NULL; + + int child_index = (long) cookie; + + loc_t * lower = NULL; + loc_t * higher = NULL; + + const char *lower_name = NULL; + const char *higher_name = NULL; + + priv = this->private; + local = frame->local; + + LOCK (&frame->lock); + { + if (op_ret == -1) { + if (op_errno == ENOSYS) { + /* return ENOTSUP */ + + gf_log (this->name, GF_LOG_ERROR, + "subvolume does not support locking. " + "please load features/posix-locks xlator on server"); + + local->op_ret = op_ret; + } + + local->child_up[child_index] = 0; + local->op_errno = op_errno; + } + } + UNLOCK (&frame->lock); + + if (local->op_ret != 0) { + afr_unlock (frame, this); + goto out; + } else { + local->transaction.locked_nodes[child_index] = 1; + } + + /* The lower path has been locked. Now lock the higher path */ + + lower = lower_path (&local->transaction.parent_loc, + local->transaction.basename, + &local->transaction.new_parent_loc, + local->transaction.new_basename); + + lower_name = (lower == &local->transaction.parent_loc ? + local->transaction.basename : + local->transaction.new_basename); + + higher = (lower == &local->transaction.parent_loc ? + &local->transaction.new_parent_loc : + &local->transaction.parent_loc); + + higher_name = (higher == &local->transaction.parent_loc ? + local->transaction.basename : + local->transaction.new_basename); + + STACK_WIND_COOKIE (frame, afr_lock_cbk, + (void *) (long) child_index, + priv->children[child_index], + priv->children[child_index]->fops->entrylk, + this->name, higher, higher_name, + ENTRYLK_LOCK, ENTRYLK_WRLCK); + +out: + return 0; +} + + static int afr_lock_rec (call_frame_t *frame, xlator_t *this, int child_index) { @@ -1041,8 +1107,6 @@ int afr_lock_rec (call_frame_t *frame, xlator_t *this, int child_index) case AFR_ENTRY_RENAME_TRANSACTION: { - local->call_count = 2; - lower = lower_path (&local->transaction.parent_loc, local->transaction.basename, &local->transaction.new_parent_loc, @@ -1060,23 +1124,13 @@ int afr_lock_rec (call_frame_t *frame, xlator_t *this, int child_index) local->transaction.basename : local->transaction.new_basename); - - /* TODO: these locks should be blocking */ - - STACK_WIND_COOKIE (frame, afr_lock_cbk, + STACK_WIND_COOKIE (frame, afr_lock_lower_cbk, (void *) (long) child_index, priv->children[child_index], priv->children[child_index]->fops->entrylk, this->name, lower, lower_name, ENTRYLK_LOCK, ENTRYLK_WRLCK); - STACK_WIND_COOKIE (frame, afr_lock_cbk, - (void *) (long) child_index, - priv->children[child_index], - priv->children[child_index]->fops->entrylk, - this->name, higher, higher_name, - ENTRYLK_LOCK, ENTRYLK_WRLCK); - break; } -- cgit