diff options
Diffstat (limited to 'xlators/features/locks/src/inodelk.c')
| -rw-r--r-- | xlators/features/locks/src/inodelk.c | 85 |
1 files changed, 59 insertions, 26 deletions
diff --git a/xlators/features/locks/src/inodelk.c b/xlators/features/locks/src/inodelk.c index 4bc5d80c866..8866cf759dc 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: |