From aa3b01533efcd85fc1e654ac14a03ab8e1d5bbab Mon Sep 17 00:00:00 2001 From: Anand Avati Date: Tue, 3 Dec 2013 16:30:45 -0800 Subject: locks: various fixes - implement ref/unref of entry locks (and fix bad pointer deref crashes) - code cleanup and deleted various data types - fix improper read/write lock conflict detection in entrylk - fix indefinite hang of blocked locks on disconnect - register locks in client_t synchronously, fix crashes in disconnect path Change-Id: Id273690c9111b8052139d1847060d1fb5a711924 BUG: 849630 Signed-off-by: Anand Avati Reviewed-on: http://review.gluster.org/6638 Tested-by: Gluster Build System Reviewed-by: Kaleb KEITHLEY Reviewed-by: Vijay Bellur --- xlators/features/locks/src/inodelk.c | 208 ++++++++++++++++------------------- 1 file changed, 95 insertions(+), 113 deletions(-) (limited to 'xlators/features/locks/src/inodelk.c') diff --git a/xlators/features/locks/src/inodelk.c b/xlators/features/locks/src/inodelk.c index 508523e1106..969b67a61db 100644 --- a/xlators/features/locks/src/inodelk.c +++ b/xlators/features/locks/src/inodelk.c @@ -35,7 +35,7 @@ __pl_inodelk_ref (pl_inode_lock_t *lock) lock->ref++; } -inline void +void __pl_inodelk_unref (pl_inode_lock_t *lock) { lock->ref--; @@ -204,7 +204,7 @@ __lock_inodelk (xlator_t *this, pl_inode_t *pl_inode, pl_inode_lock_t *lock, int ret = -EINVAL; conf = __inodelk_grantable (dom, lock); - if (conf){ + if (conf) { ret = -EAGAIN; if (can_block == 0) goto out; @@ -232,7 +232,7 @@ __lock_inodelk (xlator_t *this, pl_inode_t *pl_inode, pl_inode_lock_t *lock, gettimeofday (&lock->blkd_time, NULL); list_add_tail (&lock->blocked_locks, &dom->blocked_inodelks); - gf_log (this->name, GF_LOG_TRACE, + gf_log (this->name, GF_LOG_DEBUG, "Lock is grantable, but blocking to prevent starvation"); gf_log (this->name, GF_LOG_TRACE, "%s (pid=%d) (lk-owner=%s) %"PRId64" - %"PRId64" => Blocked", @@ -307,6 +307,8 @@ __inode_unlock_lock (xlator_t *this, pl_inode_lock_t *lock, pl_dom_list_t *dom) out: return conf; } + + static void __grant_blocked_inode_locks (xlator_t *this, pl_inode_t *pl_inode, struct list_head *granted, pl_dom_list_t *dom) @@ -363,6 +365,7 @@ grant_blocked_inode_locks (xlator_t *this, pl_inode_t *pl_inode, &lock->user_flock, 0, 0, lock->volume); STACK_UNWIND_STRICT (inodelk, lock->frame, 0, 0, NULL); + lock->frame = NULL; } pthread_mutex_lock (&pl_inode->mutex); @@ -375,103 +378,101 @@ grant_blocked_inode_locks (xlator_t *this, pl_inode_t *pl_inode, pthread_mutex_unlock (&pl_inode->mutex); } -/* Release all inodelks from this client */ -static int -release_inode_locks_of_client (xlator_t *this, pl_dom_list_t *dom, - inode_t *inode, client_t *client) + +static void +pl_inodelk_log_cleanup (pl_inode_lock_t *lock) { - pl_inode_lock_t *tmp = NULL; - pl_inode_lock_t *l = NULL; + pl_inode_t *pl_inode = NULL; + char *path = NULL; + char *file = NULL; - pl_inode_t * pinode = NULL; + pl_inode = lock->pl_inode; - struct list_head released; + inode_path (pl_inode->refkeeper, NULL, &path); - char *path = NULL; - char *file = NULL; + if (path) + file = path; + else + file = uuid_utoa (pl_inode->refkeeper->gfid); - INIT_LIST_HEAD (&released); + 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); +} - pinode = pl_inode_get (this, inode); - pthread_mutex_lock (&pinode->mutex); - { +/* Release all entrylks from this client */ +int +pl_inodelk_client_cleanup (xlator_t *this, pl_ctx_t *ctx) +{ + pl_inode_lock_t *tmp = NULL; + pl_inode_lock_t *l = NULL; + pl_dom_list_t *dom = NULL; + pl_inode_t *pl_inode = NULL; + + struct list_head released; - list_for_each_entry_safe (l, tmp, &dom->blocked_inodelks, blocked_locks) { - if (l->client != client) - continue; + INIT_LIST_HEAD (&released); - list_del_init (&l->blocked_locks); + pthread_mutex_lock (&ctx->lock); + { + list_for_each_entry_safe (l, tmp, &ctx->inodelk_lockers, + client_list) { + list_del_init (&l->client_list); + list_add_tail (&l->client_list, &released); - inode_path (inode, NULL, &path); - if (path) - file = path; - else - file = uuid_utoa (inode->gfid); + pl_inodelk_log_cleanup (l); - gf_log (this->name, GF_LOG_DEBUG, - "releasing blocking lock on %s held by " - "{client=%p, pid=%"PRId64" lk-owner=%s}", - file, client, (uint64_t) l->client_pid, - lkowner_utoa (&l->owner)); + pl_inode = l->pl_inode; - list_add (&l->blocked_locks, &released); - if (path) { - GF_FREE (path); - path = NULL; + pthread_mutex_lock (&pl_inode->mutex); + { + __delete_inode_lock (l); } + pthread_mutex_unlock (&pl_inode->mutex); } + } + pthread_mutex_unlock (&ctx->lock); - list_for_each_entry_safe (l, tmp, &dom->inodelk_list, list) { - if (l->client != client) - continue; - - inode_path (inode, NULL, &path); - if (path) - file = path; - else - file = uuid_utoa (inode->gfid); - - gf_log (this->name, GF_LOG_DEBUG, - "releasing granted lock on %s held by " - "{client=%p, pid=%"PRId64" lk-owner=%s}", - file, client, (uint64_t) l->client_pid, - lkowner_utoa (&l->owner)); - - if (path) { - GF_FREE (path); - path = NULL; - } + list_for_each_entry_safe (l, tmp, &released, client_list) { + list_del_init (&l->client_list); - __delete_inode_lock (l); - __pl_inodelk_unref (l); - } - } - GF_FREE (path); + if (l->frame) + STACK_UNWIND_STRICT (inodelk, l->frame, -1, EAGAIN, + NULL); + + pl_inode = l->pl_inode; - pthread_mutex_unlock (&pinode->mutex); + dom = get_domain (pl_inode, l->volume); - list_for_each_entry_safe (l, tmp, &released, blocked_locks) { - list_del_init (&l->blocked_locks); + grant_blocked_inode_locks (this, pl_inode, dom); - STACK_UNWIND_STRICT (inodelk, l->frame, -1, EAGAIN, NULL); - //No need to take lock as the locks are only in one list - __pl_inodelk_unref (l); + pthread_mutex_lock (&pl_inode->mutex); + { + __pl_inodelk_unref (l); + } + pthread_mutex_unlock (&pl_inode->mutex); } - grant_blocked_inode_locks (this, pinode, dom); return 0; } static int -pl_inode_setlk (xlator_t *this, pl_inode_t *pl_inode, pl_inode_lock_t *lock, - int can_block, pl_dom_list_t *dom) +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) { int ret = -EINVAL; pl_inode_lock_t *retlock = NULL; gf_boolean_t unref = _gf_true; + lock->pl_inode = pl_inode; + + if (ctx) + pthread_mutex_lock (&ctx->lock); pthread_mutex_lock (&pl_inode->mutex); { if (lock->fl_type != F_UNLCK) { @@ -495,6 +496,10 @@ pl_inode_setlk (xlator_t *this, pl_inode_t *pl_inode, pl_inode_lock_t *lock, if (can_block) unref = _gf_false; } + + if (ctx && (!ret || can_block)) + list_add_tail (&lock->client_list, + &ctx->inodelk_lockers); } else { retlock = __inode_unlock_lock (this, lock, dom); if (!retlock) { @@ -503,16 +508,21 @@ pl_inode_setlk (xlator_t *this, pl_inode_t *pl_inode, pl_inode_lock_t *lock, ret = -EINVAL; goto out; } - __pl_inodelk_unref (retlock); + list_del_init (&retlock->client_list); + __pl_inodelk_unref (retlock); ret = 0; } - } out: - if (unref) - __pl_inodelk_unref (lock); + if (unref) + __pl_inodelk_unref (lock); + } pthread_mutex_unlock (&pl_inode->mutex); + if (ctx) + pthread_mutex_unlock (&ctx->lock); + grant_blocked_inode_locks (this, pl_inode, dom); + return ret; } @@ -552,6 +562,7 @@ new_inode_lock (struct gf_flock *flock, client_t *client, pid_t client_pid, INIT_LIST_HEAD (&lock->list); INIT_LIST_HEAD (&lock->blocked_locks); + INIT_LIST_HEAD (&lock->client_list); __pl_inodelk_ref (lock); return lock; @@ -627,6 +638,15 @@ pl_common_inodelk (call_frame_t *frame, xlator_t *this, pl_trace_in (this, frame, fd, loc, cmd, flock, volume); + if (frame->root->client) { + ctx = pl_ctx_get (frame->root->client, this); + if (!ctx) { + op_errno = ENOMEM; + gf_log (this->name, GF_LOG_INFO, "pl_ctx_get() failed"); + goto unwind; + } + } + pinode = pl_inode_get (this, inode); if (!pinode) { op_errno = ENOMEM; @@ -639,27 +659,6 @@ pl_common_inodelk (call_frame_t *frame, xlator_t *this, goto unwind; } - if (frame->root->lk_owner.len == 0) { - /* - special case: this means release all locks - from this client - */ - gf_log (this->name, GF_LOG_TRACE, - "Releasing all locks from client %p", frame->root->client); - - release_inode_locks_of_client (this, dom, inode, frame->root->client); - _pl_convert_volume (volume, &res1); - if (res1) { - dom = get_domain (pinode, res1); - if (dom) - release_inode_locks_of_client (this, dom, - inode, frame->root->client); - } - - op_ret = 0; - goto unwind; - } - reqlock = new_inode_lock (flock, frame->root->client, frame->root->pid, frame, this, volume, conn_id); @@ -678,8 +677,8 @@ 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, pinode, reqlock, - can_block, dom); + ret = pl_inode_setlk (this, ctx, pinode, reqlock, can_block, + dom); if (ret < 0) { if ((can_block) && (F_UNLCK != flock->l_type)) { @@ -704,23 +703,6 @@ pl_common_inodelk (call_frame_t *frame, xlator_t *this, op_ret = 0; - ctx = pl_ctx_get (frame->root->client, this); - - if (ctx == NULL) { - gf_log (this->name, GF_LOG_INFO, "pl_ctx_get() failed"); - goto unwind; - } - - if (flock->l_type == F_UNLCK) - pl_del_locker (ctx->ltable, volume, loc, fd, - &frame->root->lk_owner, - GF_FOP_INODELK); - else - pl_add_locker (ctx->ltable, volume, loc, fd, - frame->root->pid, - &frame->root->lk_owner, - GF_FOP_INODELK); - unwind: if ((inode != NULL) && (flock !=NULL)) { pl_update_refkeeper (this, inode); -- cgit