From 9564e09e5315846a38ed18e05630ef73be5b2adb Mon Sep 17 00:00:00 2001 From: Vijay Bellur Date: Thu, 22 Sep 2011 08:30:48 -0700 Subject: Revert "cluster/afr: eager locking of FD writes" This reverts commit 81456ec2dfb312ae60c5c4e6f960a3cbf8aaaa4c. Change-Id: Id03335117f5137f5d09781850bf4fba6eca0f73d Reviewed-on: http://review.gluster.com/492 Tested-by: Gluster Build System Reviewed-by: Vijay Bellur --- xlators/cluster/afr/src/afr-common.c | 23 ---- xlators/cluster/afr/src/afr-lk-common.c | 184 ++++++++---------------------- xlators/cluster/afr/src/afr-transaction.c | 10 +- xlators/cluster/afr/src/afr.c | 2 - xlators/cluster/afr/src/afr.h | 12 +- xlators/features/locks/src/inodelk.c | 3 +- 6 files changed, 61 insertions(+), 173 deletions(-) diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c index 94938d97f..a435a38b1 100644 --- a/xlators/cluster/afr/src/afr-common.c +++ b/xlators/cluster/afr/src/afr-common.c @@ -363,7 +363,6 @@ afr_local_transaction_cleanup (afr_local_t *local, xlator_t *this) GF_FREE (local->transaction.child_errno); GF_FREE (local->child_errno); - GF_FREE (local->transaction.eager_lock); GF_FREE (local->transaction.basename); GF_FREE (local->transaction.new_basename); @@ -1265,22 +1264,6 @@ afr_fd_ctx_set (xlator_t *this, fd_t *fd) goto unlock; } - fd_ctx->lock_piggyback = GF_CALLOC (sizeof (*fd_ctx->lock_piggyback), - priv->child_count, - gf_afr_mt_char); - if (!fd_ctx->lock_piggyback) { - ret = -ENOMEM; - goto unlock; - } - - fd_ctx->lock_acquired = GF_CALLOC (sizeof (*fd_ctx->lock_acquired), - priv->child_count, - gf_afr_mt_char); - if (!fd_ctx->lock_acquired) { - ret = -ENOMEM; - goto unlock; - } - fd_ctx->up_count = priv->up_count; fd_ctx->down_count = priv->down_count; @@ -1521,12 +1504,6 @@ afr_cleanup_fd_ctx (xlator_t *this, fd_t *fd) if (fd_ctx->pre_op_piggyback) GF_FREE (fd_ctx->pre_op_piggyback); - if (fd_ctx->lock_piggyback) - GF_FREE (fd_ctx->lock_piggyback); - - if (fd_ctx->lock_acquired) - GF_FREE (fd_ctx->lock_acquired); - GF_FREE (fd_ctx); } diff --git a/xlators/cluster/afr/src/afr-lk-common.c b/xlators/cluster/afr/src/afr-lk-common.c index 157d54d42..c059a6b9e 100644 --- a/xlators/cluster/afr/src/afr-lk-common.c +++ b/xlators/cluster/afr/src/afr-lk-common.c @@ -555,7 +555,6 @@ afr_unlock_inodelk_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int32_t op_ret, int32_t op_errno) { afr_local_t *local = NULL; - int child_index = (long) cookie; local = frame->local; @@ -569,8 +568,6 @@ afr_unlock_inodelk_cbk (call_frame_t *frame, void *cookie, xlator_t *this, local->loc.path, strerror (op_errno)); } - local->transaction.eager_lock[child_index] = 0; - afr_unlock_common_cbk (frame, cookie, this, op_ret, op_errno); return 0; @@ -584,13 +581,8 @@ afr_unlock_inodelk (call_frame_t *frame, xlator_t *this) afr_local_t *local = NULL; afr_private_t *priv = NULL; struct gf_flock flock = {0,}; - struct gf_flock full_flock = {0,}; - struct gf_flock *flock_use = &flock; int call_count = 0; int i = 0; - int piggyback = 0; - afr_fd_ctx_t *fd_ctx = NULL; - local = frame->local; int_lock = &local->internal_lock; @@ -600,8 +592,6 @@ afr_unlock_inodelk (call_frame_t *frame, xlator_t *this) flock.l_len = int_lock->lk_flock.l_len; flock.l_type = F_UNLCK; - full_flock.l_type = F_UNLCK; - call_count = afr_locked_nodes_count (int_lock->inode_locked_nodes, priv->child_count); @@ -614,70 +604,42 @@ afr_unlock_inodelk (call_frame_t *frame, xlator_t *this) goto out; } - if (local->fd) - fd_ctx = afr_fd_ctx_get (local->fd, this); - for (i = 0; i < priv->child_count; i++) { - if ((int_lock->inode_locked_nodes[i] & LOCKED_YES) - != LOCKED_YES) - continue; - - if (local->fd) { - if (!local->transaction.eager_lock[i]) { - goto wind; - } + if (int_lock->inode_locked_nodes[i] & LOCKED_YES) { + if (local->fd) { + afr_trace_inodelk_in (frame, AFR_INODELK_TRANSACTION, + AFR_UNLOCK_OP, &flock, F_SETLK, i); - piggyback = 0; - flock_use = &full_flock; - - LOCK (&local->fd->lock); - { - if (fd_ctx->lock_piggyback[i]) { - fd_ctx->lock_piggyback[i]--; - piggyback = 1; - } else { - fd_ctx->lock_acquired[i]--; - } - } - UNLOCK (&local->fd->lock); + STACK_WIND_COOKIE (frame, afr_unlock_inodelk_cbk, + (void *) (long)i, + priv->children[i], + priv->children[i]->fops->finodelk, + this->name, local->fd, + F_SETLK, &flock); - if (piggyback) { - afr_unlock_inodelk_cbk (frame, (void *) (long) i, - this, 1, 0); if (!--call_count) break; - continue; - } - wind: - afr_trace_inodelk_in (frame, AFR_INODELK_TRANSACTION, - AFR_UNLOCK_OP, flock_use, F_SETLK, i); - - STACK_WIND_COOKIE (frame, afr_unlock_inodelk_cbk, - (void *) (long)i, - priv->children[i], - priv->children[i]->fops->finodelk, - this->name, local->fd, - F_SETLK, flock_use); + } else { + afr_trace_inodelk_in (frame, AFR_INODELK_TRANSACTION, + AFR_UNLOCK_OP, &flock, F_SETLK, i); - if (!--call_count) - break; + STACK_WIND_COOKIE (frame, afr_unlock_inodelk_cbk, + (void *) (long)i, + priv->children[i], + priv->children[i]->fops->inodelk, + this->name, &local->loc, + F_SETLK, &flock); - } else { - afr_trace_inodelk_in (frame, AFR_INODELK_TRANSACTION, - AFR_UNLOCK_OP, &flock, F_SETLK, i); + if (!--call_count) + break; - STACK_WIND_COOKIE (frame, afr_unlock_inodelk_cbk, - (void *) (long)i, - priv->children[i], - priv->children[i]->fops->inodelk, - this->name, &local->loc, - F_SETLK, &flock); + } - if (!--call_count) - break; } + } + out: return 0; } @@ -1326,7 +1288,6 @@ afr_nonblocking_inodelk_cbk (call_frame_t *frame, void *cookie, xlator_t *this, afr_private_t *priv = NULL; int call_count = 0; int child_index = (long) cookie; - afr_fd_ctx_t *fd_ctx = NULL; local = frame->local; int_lock = &local->internal_lock; @@ -1342,7 +1303,7 @@ afr_nonblocking_inodelk_cbk (call_frame_t *frame, void *cookie, xlator_t *this, } UNLOCK (&frame->lock); - if (op_ret < 0) { + if (op_ret < 0 ) { if (op_errno == ENOSYS) { /* return ENOTSUP */ gf_log (this->name, GF_LOG_ERROR, @@ -1354,27 +1315,10 @@ afr_nonblocking_inodelk_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int_lock->lock_op_errno = op_errno; local->op_errno = op_errno; } - } else { + } else if (op_ret == 0) { int_lock->inode_locked_nodes[child_index] |= LOCKED_YES; int_lock->inodelk_lock_count++; - - if (priv->eager_lock && local->fd) { - fd_ctx = afr_fd_ctx_get (local->fd, this); - local->transaction.eager_lock[child_index] = 1; - /* piggybacked */ - - if (op_ret == 1) { - /* piggybacked */ - } else if (op_ret == 0) { - /* lock acquired from server */ - LOCK (&local->fd->lock); - { - fd_ctx->lock_acquired[child_index]++; - } - UNLOCK (&local->fd->lock); - } - } } if (call_count == 0) { @@ -1414,9 +1358,6 @@ afr_nonblocking_inodelk (call_frame_t *frame, xlator_t *this) int i = 0; int ret = 0; struct gf_flock flock = {0,}; - struct gf_flock full_flock = {0,}; - struct gf_flock *flock_use = &flock; - int piggyback = 0; local = frame->local; int_lock = &local->internal_lock; @@ -1426,8 +1367,6 @@ afr_nonblocking_inodelk (call_frame_t *frame, xlator_t *this) flock.l_len = int_lock->lk_flock.l_len; flock.l_type = int_lock->lk_flock.l_type; - full_flock.l_type = int_lock->lk_flock.l_type; - initialize_inodelk_variables (frame, this); if (local->fd) { @@ -1459,72 +1398,49 @@ afr_nonblocking_inodelk (call_frame_t *frame, xlator_t *this) goto out; } - frame->root->lk_owner = (long) (local->fd); - /* Send non-blocking inodelk calls only on up children and where the fd has been opened */ for (i = 0; i < priv->child_count; i++) { - if (!local->child_up[i] || !fd_ctx->opened_on[i]) - continue; - - if (!priv->eager_lock) - goto wind; - - flock_use = &full_flock; - piggyback = 0; + if (local->child_up[i] && fd_ctx->opened_on[i]) { + afr_trace_inodelk_in (frame, AFR_INODELK_NB_TRANSACTION, + AFR_LOCK_OP, &flock, F_SETLK, i); - LOCK (&local->fd->lock); - { - if (fd_ctx->lock_acquired[i]) { - fd_ctx->lock_piggyback[i]++; - piggyback = 1; - } - } - UNLOCK (&local->fd->lock); + STACK_WIND_COOKIE (frame, afr_nonblocking_inodelk_cbk, + (void *) (long) i, + priv->children[i], + priv->children[i]->fops->finodelk, + this->name, local->fd, + F_SETLK, &flock); - if (piggyback) { - /* (op_ret == 1) => indicate piggybacked lock */ - afr_nonblocking_inodelk_cbk (frame, (void *) (long) i, - this, 1, 0); if (!--call_count) break; - continue; - } - wind: - afr_trace_inodelk_in (frame, AFR_INODELK_NB_TRANSACTION, - AFR_LOCK_OP, flock_use, F_SETLK, i); - STACK_WIND_COOKIE (frame, afr_nonblocking_inodelk_cbk, - (void *) (long) i, - priv->children[i], - priv->children[i]->fops->finodelk, - this->name, local->fd, - F_SETLK, flock_use); + } - if (!--call_count) - break; } } else { call_count = internal_lock_count (frame, this, NULL); int_lock->lk_call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (!local->child_up[i]) - continue; - afr_trace_inodelk_in (frame, AFR_INODELK_NB_TRANSACTION, - AFR_LOCK_OP, &flock, F_SETLK, i); + if (local->child_up[i]) { + afr_trace_inodelk_in (frame, AFR_INODELK_NB_TRANSACTION, + AFR_LOCK_OP, &flock, F_SETLK, i); - STACK_WIND_COOKIE (frame, afr_nonblocking_inodelk_cbk, - (void *) (long) i, - priv->children[i], - priv->children[i]->fops->inodelk, - this->name, &local->loc, - F_SETLK, &flock); + STACK_WIND_COOKIE (frame, afr_nonblocking_inodelk_cbk, + (void *) (long) i, + priv->children[i], + priv->children[i]->fops->inodelk, + this->name, &local->loc, + F_SETLK, &flock); - if (!--call_count) - break; + if (!--call_count) + break; + + } } } + out: return ret; } diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c index c0c409bbc..ac329958d 100644 --- a/xlators/cluster/afr/src/afr-transaction.c +++ b/xlators/cluster/afr/src/afr-transaction.c @@ -377,6 +377,13 @@ afr_changelog_post_op_cbk (call_frame_t *frame, void *cookie, xlator_t *this, child_index = (long) cookie; + if (op_ret == 1) { + } + + if (op_ret == 0) { + __mark_pre_op_undone_on_fd (frame, this, child_index); + } + LOCK (&frame->lock); { call_count = --local->call_count; @@ -556,7 +563,6 @@ afr_changelog_post_op (call_frame_t *frame, xlator_t *this) afr_changelog_post_op_cbk (frame, (void *)(long)i, this, 1, 0, xattr[i]); } else { - __mark_pre_op_undone_on_fd (frame, this, i); STACK_WIND_COOKIE (frame, afr_changelog_post_op_cbk, (void *) (long) i, @@ -1128,6 +1134,8 @@ afr_lock (call_frame_t *frame, xlator_t *this) { afr_pid_save (frame); + frame->root->pid = (long) frame->root; + afr_set_lk_owner (frame, this); afr_set_lock_number (frame, this); diff --git a/xlators/cluster/afr/src/afr.c b/xlators/cluster/afr/src/afr.c index 47224ec8c..a662037e7 100644 --- a/xlators/cluster/afr/src/afr.c +++ b/xlators/cluster/afr/src/afr.c @@ -744,8 +744,6 @@ init (xlator_t *this) pthread_mutex_init (&priv->mutex, NULL); INIT_LIST_HEAD (&priv->saved_fds); - priv->eager_lock = _gf_true; - ret = 0; out: return ret; diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h index 6a9907e97..6f40ded12 100644 --- a/xlators/cluster/afr/src/afr.h +++ b/xlators/cluster/afr/src/afr.h @@ -92,7 +92,6 @@ typedef struct _afr_private { pthread_mutex_t mutex; struct list_head saved_fds; /* list of fds on which locks have succeeded */ gf_boolean_t optimistic_change_log; - gf_boolean_t eager_lock; char vol_uuid[UUID_SIZE + 1]; int32_t *last_event; @@ -598,8 +597,6 @@ typedef struct _afr_local { struct { off_t start, len; - int *eager_lock; - char *basename; char *new_basename; @@ -639,9 +636,6 @@ typedef struct { unsigned int *opened_on; /* which subvolumes the fd is open on */ unsigned int *pre_op_piggyback; - unsigned int *lock_piggyback; - unsigned int *lock_acquired; - int flags; int32_t wbflags; uint64_t up_count; /* number of CHILD_UPs this fd has seen */ @@ -945,10 +939,6 @@ afr_transaction_local_init (afr_local_t *local, afr_private_t *priv) priv->child_count, gf_afr_mt_int32_t); - local->transaction.eager_lock = GF_CALLOC (sizeof (*local->transaction.eager_lock), - priv->child_count, - gf_afr_mt_int32_t); - local->internal_lock.transaction_lk_type = AFR_TRANSACTION_LK; return 0; @@ -957,5 +947,5 @@ afr_transaction_local_init (afr_local_t *local, afr_private_t *priv) int32_t afr_marker_getxattr (call_frame_t *frame, xlator_t *this, loc_t *loc, const char *name,afr_local_t *local, afr_private_t *priv ); -afr_fd_ctx_t * afr_fd_ctx_get (fd_t *fd, xlator_t *this); + #endif /* __AFR_H__ */ diff --git a/xlators/features/locks/src/inodelk.c b/xlators/features/locks/src/inodelk.c index a90f244ec..6fa9f3997 100644 --- a/xlators/features/locks/src/inodelk.c +++ b/xlators/features/locks/src/inodelk.c @@ -131,8 +131,7 @@ static int inodelk_conflict (pl_inode_lock_t *l1, pl_inode_lock_t *l2) { return (inodelk_overlap (l1, l2) && - inodelk_type_conflict (l1, l2) && - !same_inodelk_owner (l1, l2)); + inodelk_type_conflict (l1, l2)); } /* Determine if lock is grantable or not */ -- cgit