From c9b96e26a3bab09a4146b2bec57a57c69b9aa8b7 Mon Sep 17 00:00:00 2001 From: Pranith Kumar K Date: Sat, 28 Jul 2012 12:18:50 +0530 Subject: features/locks: Fix statedump code RCA: Taking blocking mutex/spin locks lead to dead locks because of the locking order in statedumps. Also we were asked to remove gf_logs if possible to avoid extra cost in signal handlers. Fix: changed blocking mutes/spin locks to their non-blocking variants. Removed gf_logs in locks xlator statedump code-path. Tests: State-dump success cases are working fine. Triggered try-lock failures by putting statedumps in a while loop. In parallel did chown of the same file in a while loop. Change-Id: I81539a62f8216f267f57bb703ef132c85bfd557d BUG: 843781 Signed-off-by: Pranith Kumar K Reviewed-on: http://review.gluster.com/3747 Tested-by: Gluster Build System Reviewed-by: Anand Avati --- xlators/features/locks/src/common.h | 4 ++ xlators/features/locks/src/entrylk.c | 16 +------- xlators/features/locks/src/inodelk.c | 27 +----------- xlators/features/locks/src/posix.c | 79 +++++++++++++++++++++--------------- 4 files changed, 52 insertions(+), 74 deletions(-) (limited to 'xlators/features/locks') diff --git a/xlators/features/locks/src/common.h b/xlators/features/locks/src/common.h index 2cf9aa38093..97faa5b92be 100644 --- a/xlators/features/locks/src/common.h +++ b/xlators/features/locks/src/common.h @@ -87,9 +87,13 @@ grant_blocked_entry_locks (xlator_t *this, pl_inode_t *pl_inode, void pl_update_refkeeper (xlator_t *this, inode_t *inode); +int32_t +__get_inodelk_count (xlator_t *this, pl_inode_t *pl_inode); int32_t get_inodelk_count (xlator_t *this, inode_t *inode); +int32_t +__get_entrylk_count (xlator_t *this, pl_inode_t *pl_inode); int32_t get_entrylk_count (xlator_t *this, inode_t *inode); diff --git a/xlators/features/locks/src/entrylk.c b/xlators/features/locks/src/entrylk.c index 2c4904ff3d7..69a04b53d23 100644 --- a/xlators/features/locks/src/entrylk.c +++ b/xlators/features/locks/src/entrylk.c @@ -777,7 +777,7 @@ pl_fentrylk (call_frame_t *frame, xlator_t *this, } -static int32_t +int32_t __get_entrylk_count (xlator_t *this, pl_inode_t *pl_inode) { int32_t count = 0; @@ -786,24 +786,10 @@ __get_entrylk_count (xlator_t *this, pl_inode_t *pl_inode) list_for_each_entry (dom, &pl_inode->dom_list, inode_list) { list_for_each_entry (lock, &dom->entrylk_list, domain_list) { - - gf_log (this->name, GF_LOG_DEBUG, - " XATTR DEBUG" - " domain: %s %s on %s state = Active", - dom->domain, - lock->type == ENTRYLK_RDLCK ? "ENTRYLK_RDLCK" : - "ENTRYLK_WRLCK", lock->basename); count++; } list_for_each_entry (lock, &dom->blocked_entrylks, blocked_locks) { - - gf_log (this->name, GF_LOG_DEBUG, - " XATTR DEBUG" - " domain: %s %s on %s state = Blocked", - dom->domain, - lock->type == ENTRYLK_RDLCK ? "ENTRYLK_RDLCK" : - "ENTRYLK_WRLCK", lock->basename); count++; } diff --git a/xlators/features/locks/src/inodelk.c b/xlators/features/locks/src/inodelk.c index eba85795c20..8ac039fba55 100644 --- a/xlators/features/locks/src/inodelk.c +++ b/xlators/features/locks/src/inodelk.c @@ -694,7 +694,7 @@ pl_finodelk (call_frame_t *frame, xlator_t *this, } -static int32_t +int32_t __get_inodelk_count (xlator_t *this, pl_inode_t *pl_inode) { int32_t count = 0; @@ -703,34 +703,9 @@ __get_inodelk_count (xlator_t *this, pl_inode_t *pl_inode) list_for_each_entry (dom, &pl_inode->dom_list, inode_list) { list_for_each_entry (lock, &dom->inodelk_list, list) { - - gf_log (this->name, GF_LOG_DEBUG, - " XATTR DEBUG" - " domain: %s %s (pid=%d) (lk-owner=%s) %"PRId64" - %"PRId64" " - "state = Active", - dom->domain, - lock->fl_type == F_UNLCK ? "Unlock" : "Lock", - lock->client_pid, - lkowner_utoa (&lock->owner), - lock->user_flock.l_start, - lock->user_flock.l_len); - count++; } - list_for_each_entry (lock, &dom->blocked_inodelks, blocked_locks) { - - gf_log (this->name, GF_LOG_DEBUG, - " XATTR DEBUG" - " domain: %s %s (pid=%d) (lk-owner=%s) %"PRId64" - %"PRId64" " - "state = Blocked", - dom->domain, - lock->fl_type == F_UNLCK ? "Unlock" : "Lock", - lock->client_pid, - lkowner_utoa (&lock->owner), - lock->user_flock.l_start, - lock->user_flock.l_len); - count++; } diff --git a/xlators/features/locks/src/posix.c b/xlators/features/locks/src/posix.c index 09ce6377466..e48fa204e71 100644 --- a/xlators/features/locks/src/posix.c +++ b/xlators/features/locks/src/posix.c @@ -1459,7 +1459,7 @@ out: return ret; } -static int32_t +int32_t __get_posixlk_count (xlator_t *this, pl_inode_t *pl_inode) { posix_lock_t *lock = NULL; @@ -1467,16 +1467,6 @@ __get_posixlk_count (xlator_t *this, pl_inode_t *pl_inode) list_for_each_entry (lock, &pl_inode->ext_list, list) { - gf_log (this->name, GF_LOG_DEBUG, - " XATTR DEBUG" - "%s (pid=%d) (lk-owner=%s) %"PRId64" - %"PRId64" state: %s", - lock->fl_type == F_UNLCK ? "Unlock" : "Lock", - lock->client_pid, - lkowner_utoa (&lock->owner), - lock->user_flock.l_start, - lock->user_flock.l_len, - lock->blocked == 1 ? "Blocked" : "Active"); - count++; } @@ -1960,9 +1950,6 @@ __dump_posixlks (pl_inode_t *pl_inode) count++; } - - - } void @@ -1984,24 +1971,36 @@ pl_dump_inode_priv (xlator_t *this, inode_t *inode) uint64_t tmp_pl_inode = 0; pl_inode_t *pl_inode = NULL; char *pathname = NULL; + gf_boolean_t section_added = _gf_false; int count = 0; - GF_VALIDATE_OR_GOTO (this->name, inode, out); - - ret = inode_ctx_get (inode, this, &tmp_pl_inode); + if (!inode) { + errno = EINVAL; + goto out; + } - if (ret != 0) + ret = TRY_LOCK (&inode->lock); + if (ret) + goto out; + { + ret = __inode_ctx_get (inode, this, &tmp_pl_inode); + if (ret) + goto unlock; + } +unlock: + UNLOCK (&inode->lock); + if (ret) goto out; pl_inode = (pl_inode_t *)(long)tmp_pl_inode; - if (!pl_inode) { ret = -1; goto out; } gf_proc_dump_add_section("xlator.features.locks.%s.inode", this->name); + section_added = _gf_true; /*We are safe to call __inode_path since we have the * inode->table->lock */ @@ -2011,27 +2010,41 @@ pl_dump_inode_priv (xlator_t *this, inode_t *inode) gf_proc_dump_write("mandatory", "%d", pl_inode->mandatory); - count = get_entrylk_count (this, inode); - if (count) { - gf_proc_dump_write("entrylk-count", "%d", count); - dump_entrylks(pl_inode); - } + ret = pthread_mutex_trylock (&pl_inode->mutex); + if (ret) + goto out; + { + count = __get_entrylk_count (this, pl_inode); + if (count) { + gf_proc_dump_write("entrylk-count", "%d", count); + __dump_entrylks (pl_inode); + } - count = get_inodelk_count (this, inode); - if (count) { - gf_proc_dump_write("inodelk-count", "%d", count); - dump_inodelks(pl_inode); - } + count = __get_inodelk_count (this, pl_inode); + if (count) { + gf_proc_dump_write("inodelk-count", "%d", count); + __dump_inodelks (pl_inode); + } - count = get_posixlk_count (this, inode); - if (count) { - gf_proc_dump_write("posixlk-count", "%d", count); - dump_posixlks(pl_inode); + count = __get_posixlk_count (this, pl_inode); + if (count) { + gf_proc_dump_write("posixlk-count", "%d", count); + __dump_posixlks (pl_inode); + } } + pthread_mutex_unlock (&pl_inode->mutex); out: GF_FREE (pathname); + if (ret && inode) { + if (!section_added) + gf_proc_dump_add_section ("xlator.features.locks.%s." + "inode", this->name); + gf_proc_dump_write ("Unable to print lock state", "(Lock " + "acquisition failure) %s", + uuid_utoa (inode->gfid)); + } return ret; } -- cgit