summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnand Avati <avati@redhat.com>2012-06-27 15:04:55 -0700
committerAnand Avati <avati@redhat.com>2012-07-04 00:17:18 -0700
commitd90596a15c03434f14258d754e37b84e3ec57310 (patch)
treeba572d2de97a17dd97bc5b9f7450b4eaad38b3a5
parent9781fea0dbde3faeeeb52451965de5d891e79bf2 (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.c22
-rw-r--r--xlators/cluster/afr/src/afr-transaction.c45
-rw-r--r--xlators/cluster/afr/src/afr.h2
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;