summaryrefslogtreecommitdiffstats
path: root/xlators/features/locks/src/inodelk.c
diff options
context:
space:
mode:
Diffstat (limited to 'xlators/features/locks/src/inodelk.c')
-rw-r--r--xlators/features/locks/src/inodelk.c85
1 files changed, 59 insertions, 26 deletions
diff --git a/xlators/features/locks/src/inodelk.c b/xlators/features/locks/src/inodelk.c
index c76cb7f9199..ef06531cfde 100644
--- a/xlators/features/locks/src/inodelk.c
+++ b/xlators/features/locks/src/inodelk.c
@@ -383,24 +383,13 @@ static void
pl_inodelk_log_cleanup (pl_inode_lock_t *lock)
{
pl_inode_t *pl_inode = NULL;
- char *path = NULL;
- char *file = NULL;
pl_inode = lock->pl_inode;
- inode_path (pl_inode->refkeeper, NULL, &path);
-
- if (path)
- file = path;
- else
- file = uuid_utoa (pl_inode->refkeeper->gfid);
-
- gf_log (THIS->name, GF_LOG_WARNING,
- "releasing lock on %s held by "
- "{client=%p, pid=%"PRId64" lk-owner=%s}",
- file, lock->client, (uint64_t) lock->client_pid,
- lkowner_utoa (&lock->owner));
- GF_FREE (path);
+ gf_log (THIS->name, GF_LOG_WARNING, "releasing lock on %s held by "
+ "{client=%p, pid=%"PRId64" lk-owner=%s}",
+ uuid_utoa (pl_inode->gfid), lock->client,
+ (uint64_t) lock->client_pid, lkowner_utoa (&lock->owner));
}
@@ -501,6 +490,7 @@ pl_inodelk_client_cleanup (xlator_t *this, pl_ctx_t *ctx)
__pl_inodelk_unref (l);
}
pthread_mutex_unlock (&pl_inode->mutex);
+ inode_unref (pl_inode->inode);
}
return 0;
@@ -509,13 +499,36 @@ pl_inodelk_client_cleanup (xlator_t *this, pl_ctx_t *ctx)
static int
pl_inode_setlk (xlator_t *this, pl_ctx_t *ctx, pl_inode_t *pl_inode,
- pl_inode_lock_t *lock, int can_block, pl_dom_list_t *dom)
+ pl_inode_lock_t *lock, int can_block, pl_dom_list_t *dom,
+ inode_t *inode)
{
- int ret = -EINVAL;
- pl_inode_lock_t *retlock = NULL;
- gf_boolean_t unref = _gf_true;
+ int ret = -EINVAL;
+ pl_inode_lock_t *retlock = NULL;
+ gf_boolean_t unref = _gf_true;
+ gf_boolean_t need_inode_unref = _gf_false;
+ short fl_type;
lock->pl_inode = pl_inode;
+ fl_type = lock->fl_type;
+
+ /* Ideally, AFTER a successful lock (both blocking and non-blocking) or
+ * an unsuccessful blocking lock operation, the inode needs to be ref'd.
+ *
+ * But doing so might give room to a race where the lock-requesting
+ * client could send a DISCONNECT just before this thread refs the inode
+ * after the locking is done, and the epoll thread could unref the inode
+ * in cleanup which means the inode's refcount would come down to 0, and
+ * the call to pl_forget() at this point destroys @pl_inode. Now when
+ * the io-thread executing this function tries to access pl_inode,
+ * it could crash on account of illegal memory access.
+ *
+ * To get around this problem, the inode is ref'd once even before
+ * adding the lock into client_list as a precautionary measure.
+ * This way even if there are DISCONNECTs, there will always be 1 extra
+ * ref on the inode, so @pl_inode is still alive until after the
+ * current stack unwinds.
+ */
+ pl_inode->inode = inode_ref (inode);
if (ctx)
pthread_mutex_lock (&ctx->lock);
@@ -542,12 +555,24 @@ pl_inode_setlk (xlator_t *this, pl_ctx_t *ctx, pl_inode_t *pl_inode,
lock->user_flock.l_len);
if (can_block)
unref = _gf_false;
+ /* For all but the case where a non-blocking
+ * lock attempt fails, the extra ref taken at
+ * the start of this function must be negated.
+ */
+ else
+ need_inode_unref = _gf_true;
}
if (ctx && (!ret || can_block))
list_add_tail (&lock->client_list,
&ctx->inodelk_lockers);
} else {
+ /* Irrespective of whether unlock succeeds or not,
+ * the extra inode ref that was done at the start of
+ * this function must be negated. Towards this,
+ * @need_inode_unref flag is set unconditionally here.
+ */
+ need_inode_unref = _gf_true;
retlock = __inode_unlock_lock (this, lock, dom);
if (!retlock) {
gf_log (this->name, GF_LOG_DEBUG,
@@ -568,7 +593,16 @@ out:
if (ctx)
pthread_mutex_unlock (&ctx->lock);
- grant_blocked_inode_locks (this, pl_inode, dom);
+ if (need_inode_unref)
+ inode_unref (pl_inode->inode);
+
+ /* The following (extra) unref corresponds to the ref that
+ * was done at the time the lock was granted.
+ */
+ if ((fl_type == F_UNLCK) && (ret == 0)) {
+ inode_unref (pl_inode->inode);
+ grant_blocked_inode_locks (this, pl_inode, dom);
+ }
return ret;
}
@@ -707,7 +741,7 @@ pl_common_inodelk (call_frame_t *frame, xlator_t *this,
}
reqlock = new_inode_lock (flock, frame->root->client, frame->root->pid,
- frame, this, volume, conn_id);
+ frame, this, dom->domain, conn_id);
if (!reqlock) {
op_ret = -1;
@@ -725,7 +759,7 @@ pl_common_inodelk (call_frame_t *frame, xlator_t *this,
case F_SETLK:
memcpy (&reqlock->user_flock, flock, sizeof (struct gf_flock));
ret = pl_inode_setlk (this, ctx, pinode, reqlock, can_block,
- dom);
+ dom, inode);
if (ret < 0) {
if ((can_block) && (F_UNLCK != flock->l_type)) {
@@ -751,10 +785,9 @@ pl_common_inodelk (call_frame_t *frame, xlator_t *this,
op_ret = 0;
unwind:
- if ((inode != NULL) && (flock !=NULL)) {
- pl_update_refkeeper (this, inode);
- pl_trace_out (this, frame, fd, loc, cmd, flock, op_ret, op_errno, volume);
- }
+ if (flock != NULL)
+ pl_trace_out (this, frame, fd, loc, cmd, flock, op_ret,
+ op_errno, volume);
STACK_UNWIND_STRICT (inodelk, frame, op_ret, op_errno, NULL);
out: