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.

BUG: 843781
Change-Id: Iac9b75d79cd5e036cd3eafc1e106074e2c6b5c47
Signed-off-by: Pranith Kumar K <pranithk@gluster.com>
Reviewed-on: http://review.gluster.com/3752
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

4 files changed, 52 insertions, 74 deletions

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
@@ -88,9 +88,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);
 
 void pl_trace_in (xlator_t *this, call_frame_t *frame, fd_t *fd, loc_t *loc,
diff --git a/xlators/features/locks/src/entrylk.c b/xlators/features/locks/src/entrylk.c
index 2a47657dbbb..b32c484b75a 100644
--- a/xlators/features/locks/src/entrylk.c
+++ b/xlators/features/locks/src/entrylk.c
@@ -782,7 +782,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;
@@ -791,24 +791,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 a2e65d55fc3..37d42dc0830 100644
--- a/xlators/features/locks/src/inodelk.c
+++ b/xlators/features/locks/src/inodelk.c
@@ -695,7 +695,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;
@@ -704,34 +704,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 2c0f583e6f7..8060349ee03 100644
--- a/xlators/features/locks/src/posix.c
+++ b/xlators/features/locks/src/posix.c
@@ -1463,7 +1463,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;
@@ -1471,16 +1471,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++;
         }
 
@@ -1959,9 +1949,6 @@ __dump_posixlks (pl_inode_t *pl_inode)
 
               count++;
         }
-
-
-
 }
 
 void
@@ -1983,24 +1970,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 */
@@ -2010,28 +2009,42 @@ 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:
         if (pathname)
                 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;
 }