diff options
author | Anand Avati <avati@redhat.com> | 2012-06-27 15:04:55 -0700 |
---|---|---|
committer | Anand Avati <avati@redhat.com> | 2012-07-04 00:17:18 -0700 |
commit | d90596a15c03434f14258d754e37b84e3ec57310 (patch) | |
tree | ba572d2de97a17dd97bc5b9f7450b4eaad38b3a5 | |
parent | 9781fea0dbde3faeeeb52451965de5d891e79bf2 (diff) |
cluster/afr: cleanup lk_owner and PID mess
Historically PID (frame->root->pid) was used by the locks translator
to identify a locker (and make decisions about which locks contend
or cooperate/merge). Since the introduction of lock_owner parameter
the usage of PID (for locks) was deprecated and is now unused. This
patch nukes the usage of PID in AFR
The usage of lk_owner has also ended up being a mess, because of the
differentiation required between ->lk() and ->inodelk(), (->lk() needs
to be identified by the process (roughly) and ->inodelk() needs to be
identified by the transaction) and also because of optimizations like
eager locking (locks are no more identified by the transaction as they
now get inherited by the next transaction).
The scheme (and technique) now is:
- All FOPs (the third phase of the transaction) happen with the lk_owner
which is set by the topmost layer (FUSE, NFS etc.)
- All entrylks are issued with lk_owner set to the frame->root address.
- Inodelks which will not be subject to eager locking are issued with
lk_owner set to frame->root.
- Inodelks which are subject to eager locking are issued with lk_owner
set to the address of fd_t (which are the only type of frames which
get subject to the eager locking optimization)
- At the start of the transaction, the transaction frame's lk_owner is
set to the either frame->root or fd_t (and never unmodified) depending
on the type of transaction.
- Just before the third phase (FOP phase) the set lk_owner is "saved"
away and overwritten by the lk_owner submitted by the top layer (FUSE
or NFS)
- Right after the third phase, the saved lk_owner is "restored" to resume
the transaction into the POST-OP and eventually UNLOCK using the same
lk_owner which was used during the LOCK phase.
Change-Id: I6ab8e4d6b65ae4185fa85ad3fded8e9188b2f929
BUG: 836033
Signed-off-by: Anand Avati <avati@redhat.com>
Reviewed-on: http://review.gluster.com/3620
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Pranith Kumar Karampuri <pranithk@gluster.com>
-rw-r--r-- | xlators/cluster/afr/src/afr-lk-common.c | 22 | ||||
-rw-r--r-- | xlators/cluster/afr/src/afr-transaction.c | 45 | ||||
-rw-r--r-- | xlators/cluster/afr/src/afr.h | 2 |
3 files changed, 28 insertions, 41 deletions
diff --git a/xlators/cluster/afr/src/afr-lk-common.c b/xlators/cluster/afr/src/afr-lk-common.c index 92356c82e4c..555a9b9faf6 100644 --- a/xlators/cluster/afr/src/afr-lk-common.c +++ b/xlators/cluster/afr/src/afr-lk-common.c @@ -589,7 +589,6 @@ afr_unlock_inodelk (call_frame_t *frame, xlator_t *this) int i = 0; int piggyback = 0; afr_fd_ctx_t *fd_ctx = NULL; - gf_boolean_t fd_lock_owner = _gf_false; local = frame->local; @@ -624,11 +623,6 @@ afr_unlock_inodelk (call_frame_t *frame, xlator_t *this) if (local->fd) { flock_use = &flock; if (!local->transaction.eager_lock[i]) { - if (fd_lock_owner) { - afr_set_lk_owner (frame, this, - frame->root); - fd_lock_owner = _gf_false; - } goto wind; } @@ -645,10 +639,6 @@ afr_unlock_inodelk (call_frame_t *frame, xlator_t *this) } UNLOCK (&local->fd->lock); - if (!fd_lock_owner) { - afr_set_lk_owner (frame, this, local->fd); - fd_lock_owner = _gf_true; - } if (piggyback) { afr_unlock_inodelk_cbk (frame, (void *) (long) i, this, 1, 0, NULL); @@ -1432,7 +1422,6 @@ afr_nonblocking_inodelk (call_frame_t *frame, xlator_t *this) struct gf_flock full_flock = {0,}; struct gf_flock *flock_use = NULL; int piggyback = 0; - gf_boolean_t fd_lock_owner = _gf_false; local = frame->local; int_lock = &local->internal_lock; @@ -1482,11 +1471,6 @@ afr_nonblocking_inodelk (call_frame_t *frame, xlator_t *this) flock_use = &flock; if (!priv->eager_lock) { - if (fd_lock_owner) { - afr_set_lk_owner (frame, this, - frame->root); - fd_lock_owner = _gf_false; - } goto wind; } @@ -1502,11 +1486,6 @@ afr_nonblocking_inodelk (call_frame_t *frame, xlator_t *this) } UNLOCK (&local->fd->lock); - if (!fd_lock_owner) { - afr_set_lk_owner (frame, this, local->fd); - fd_lock_owner = _gf_true; - } - if (piggyback) { /* (op_ret == 1) => indicate piggybacked lock */ afr_nonblocking_inodelk_cbk (frame, (void *) (long) i, @@ -1761,7 +1740,6 @@ afr_unlock (call_frame_t *frame, xlator_t *this) local = frame->local; if (transaction_lk_op (local)) { - afr_set_lk_owner (frame, this, frame->root); if (is_afr_lock_transaction (local)) afr_unlock_inodelk (frame, this); else diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c index 6634b3102a1..0e9956d7bed 100644 --- a/xlators/cluster/afr/src/afr-transaction.c +++ b/xlators/cluster/afr/src/afr-transaction.c @@ -72,24 +72,24 @@ afr_fd_ctx_get (fd_t *fd, xlator_t *this) static void -afr_pid_save (call_frame_t *frame) +afr_save_lk_owner (call_frame_t *frame) { afr_local_t * local = NULL; local = frame->local; - local->saved_pid = frame->root->pid; + local->saved_lk_owner = frame->root->lk_owner; } static void -afr_pid_restore (call_frame_t *frame) +afr_restore_lk_owner (call_frame_t *frame) { afr_local_t * local = NULL; local = frame->local; - frame->root->pid = local->saved_pid; + frame->root->lk_owner = local->saved_lk_owner; } @@ -780,7 +780,13 @@ afr_changelog_pre_op_cbk (call_frame_t *frame, void *cookie, xlator_t *this, __mark_all_success (local->pending, priv->child_count, local->transaction.type); - afr_pid_restore (frame); + /* Perform fops with the lk-owner from top xlator. + * Eg: lk-owner of posix-lk and flush should be same, + * flush cant clear the posix-lks without that lk-owner. + */ + afr_save_lk_owner (frame); + frame->root->lk_owner = + local->transaction.main_frame->root->lk_owner; local->transaction.fop (frame, this); } @@ -1035,7 +1041,6 @@ afr_post_nonblocking_inodelk_cbk (call_frame_t *frame, xlator_t *this) gf_log (this->name, GF_LOG_DEBUG, "Non blocking inodelks failed. Proceeding to blocking"); int_lock->lock_cbk = afr_post_blocking_inodelk_cbk; - afr_set_lk_owner (frame, this, frame->root); afr_blocking_lock (frame, this); } else { @@ -1199,12 +1204,6 @@ afr_lock_rec (call_frame_t *frame, xlator_t *this) int 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, frame->root); - afr_set_lock_number (frame, this); return afr_lock_rec (frame, this); @@ -1222,18 +1221,20 @@ afr_internal_lock_finish (call_frame_t *frame, xlator_t *this) priv = this->private; local = frame->local; - /* Perform fops with the lk-owner from top xlator. - * Eg: lk-owner of posix-lk and flush should be same, - * flush cant clear the posix-lks without that lk-owner. - */ - frame->root->lk_owner = local->transaction.main_frame->root->lk_owner; if (__fop_changelog_needed (frame, this)) { afr_changelog_pre_op (frame, this); } else { __mark_all_success (local->pending, priv->child_count, local->transaction.type); - afr_pid_restore (frame); + + /* Perform fops with the lk-owner from top xlator. + * Eg: lk-owner of posix-lk and flush should be same, + * flush cant clear the posix-lks without that lk-owner. + */ + afr_save_lk_owner (frame); + frame->root->lk_owner = + local->transaction.main_frame->root->lk_owner; local->transaction.fop (frame, this); } @@ -1253,6 +1254,8 @@ afr_transaction_resume (call_frame_t *frame, xlator_t *this) int_lock = &local->internal_lock; priv = this->private; + afr_restore_lk_owner (frame); + if (__fop_changelog_needed (frame, this)) { afr_changelog_post_op (frame, this); } else { @@ -1294,6 +1297,12 @@ afr_transaction (call_frame_t *frame, xlator_t *this, afr_transaction_type type) local = frame->local; priv = this->private; + if (local->fd && priv->eager_lock && + local->transaction.type == AFR_DATA_TRANSACTION) + afr_set_lk_owner (frame, this, local->fd); + else + afr_set_lk_owner (frame, this, frame->root); + afr_transaction_local_init (local, this); local->transaction.resume = afr_transaction_resume; diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h index c8e01fcb841..e9867cb826d 100644 --- a/xlators/cluster/afr/src/afr.h +++ b/xlators/cluster/afr/src/afr.h @@ -380,7 +380,7 @@ typedef struct _afr_local { unsigned char read_child_returned; unsigned int first_up_child; - pid_t saved_pid; + gf_lkowner_t saved_lk_owner; int32_t op_ret; int32_t op_errno; |