Multiple files: reduce work while under lock.

Mostly, unlock before logging.
In some cases, moved different code that was not needed
to be under lock (for example, taking time, or malloc'ing)
to be executed before taking the lock.

Note: logging might be slightly less accurate in order, since it may
not be done now under the lock, so order of logs is racy. I think
it's a reasonable compromise.

Compile-tested only!
updates: bz#1193929
Signed-off-by: Yaniv Kaul <ykaul@redhat.com>

Change-Id: I2438710016afc9f4f62a176ef1a0d3ed793b4f89

19 files changed, 155 insertions, 152 deletions

diff --git a/glusterfsd/src/glusterfsd-mgmt.c b/glusterfsd/src/glusterfsd-mgmt.c
index 6dec11b2bc2..a78acb602a0 100644
--- a/glusterfsd/src/glusterfsd-mgmt.c
+++ b/glusterfsd/src/glusterfsd-mgmt.c
@@ -2008,10 +2008,11 @@ volfile:
             if (!strcmp(volfile_id, volfile_obj->vol_id)) {
                 if (!memcmp(sha256_hash, volfile_obj->volfile_checksum,
                             sizeof(volfile_obj->volfile_checksum))) {
+                    UNLOCK(&ctx->volfile_lock);
                     gf_log(frame->this->name, GF_LOG_INFO,
                            "No change in volfile,"
                            "continuing");
-                    goto out;
+                    goto post_unlock;
                 }
                 volfile_tmp = volfile_obj;
                 break;
@@ -2021,10 +2022,11 @@ volfile:
         /* coverity[secure_temp] mkstemp uses 0600 as the mode */
         tmp_fd = mkstemp(template);
         if (-1 == tmp_fd) {
+            UNLOCK(&ctx->volfile_lock);
             gf_msg(frame->this->name, GF_LOG_ERROR, 0, glusterfsd_msg_39,
                    "Unable to create temporary file: %s", template);
             ret = -1;
-            goto out;
+            goto post_unlock;
         }
 
         /* Calling unlink so that when the file is closed or program
@@ -2065,11 +2067,11 @@ volfile:
                    "No need to re-load volfile, reconfigure done");
             if (!volfile_tmp) {
                 ret = -1;
+                UNLOCK(&ctx->volfile_lock);
                 gf_log("mgmt", GF_LOG_ERROR,
-                       "Graph "
-                       "reconfigure succeeded with out having "
+                       "Graph reconfigure succeeded with out having "
                        "checksum.");
-                goto out;
+                goto post_unlock;
             }
             memcpy(volfile_tmp->volfile_checksum, sha256_hash,
                    sizeof(volfile_tmp->volfile_checksum));
@@ -2077,8 +2079,9 @@ volfile:
         }
 
         if (ret < 0) {
+            UNLOCK(&ctx->volfile_lock);
             gf_log("glusterfsd-mgmt", GF_LOG_DEBUG, "Reconfigure failed !!");
-            goto out;
+            goto post_unlock;
         }
 
         ret = glusterfs_process_volfp(ctx, tmpfp);
@@ -2118,7 +2121,7 @@ out:
 
     if (locked)
         UNLOCK(&ctx->volfile_lock);
-
+post_unlock:
     GF_FREE(frame->local);
     frame->local = NULL;
     STACK_DESTROY(frame->root);
diff --git a/libglusterfs/src/event-epoll.c b/libglusterfs/src/event-epoll.c
index dcaf9804529..be8b204b7a7 100644
--- a/libglusterfs/src/event-epoll.c
+++ b/libglusterfs/src/event-epoll.c
@@ -1018,7 +1018,7 @@ event_handled_epoll(struct event_pool *event_pool, int fd, int idx, int gen)
                          " from gen=%d to slot->gen=%d, fd=%d, "
                          "slot->fd=%d",
                          idx, gen, slot->gen, fd, slot->fd);
-            goto post_unlock;
+            goto unlock;
         }
 
         /* This call also picks up the changes made by another
@@ -1033,7 +1033,7 @@ event_handled_epoll(struct event_pool *event_pool, int fd, int idx, int gen)
             ret = epoll_ctl(event_pool->fd, EPOLL_CTL_MOD, fd, &epoll_event);
         }
     }
-post_unlock:
+unlock:
     UNLOCK(&slot->lock);
 
     event_slot_unref(event_pool, slot, idx);
diff --git a/libglusterfs/src/fd.c b/libglusterfs/src/fd.c
index 44a9ee69d67..b8aac726093 100644
--- a/libglusterfs/src/fd.c
+++ b/libglusterfs/src/fd.c
@@ -766,13 +766,13 @@ fd_anonymous_with_flags(inode_t *inode, int32_t flags)
 {
     fd_t *fd = NULL;
 
+    if (flags & O_DIRECT)
+        flags = GF_ANON_FD_FLAGS | O_DIRECT;
+    else
+        flags = GF_ANON_FD_FLAGS;
+
     LOCK(&inode->lock);
     {
-        if (flags & O_DIRECT)
-            flags = GF_ANON_FD_FLAGS | O_DIRECT;
-        else
-            flags = GF_ANON_FD_FLAGS;
-
         fd = __fd_anonymous(inode, flags);
     }
     UNLOCK(&inode->lock);
diff --git a/libglusterfs/src/gidcache.c b/libglusterfs/src/gidcache.c
index 87147163e9e..40fcffbb35e 100644
--- a/libglusterfs/src/gidcache.c
+++ b/libglusterfs/src/gidcache.c
@@ -64,8 +64,8 @@ gid_cache_lookup(gid_cache_t *cache, uint64_t id, uint64_t uid, uint64_t gid)
     time_t now;
     const gid_list_t *agl;
 
-    LOCK(&cache->gc_lock);
     now = time(NULL);
+    LOCK(&cache->gc_lock);
     bucket = id % cache->gc_nbuckets;
     agl = BUCKET_START(cache->gc_cache, bucket);
     for (i = 0; i < AUX_GID_CACHE_ASSOC; i++, agl++) {
@@ -132,8 +132,8 @@ gid_cache_add(gid_cache_t *cache, gid_list_t *gl)
     if (!cache->gc_max_age)
         return 0;
 
-    LOCK(&cache->gc_lock);
     now = time(NULL);
+    LOCK(&cache->gc_lock);
 
     /*
      * Scan for the first free entry or one that matches this id. The id
diff --git a/libglusterfs/src/rbthash.c b/libglusterfs/src/rbthash.c
index 1bdd45f9fae..ae2e158d61e 100644
--- a/libglusterfs/src/rbthash.c
+++ b/libglusterfs/src/rbthash.c
@@ -252,10 +252,11 @@ rbthash_insert_entry(rbthash_table_t *tbl, rbthash_entry_t *entry)
     LOCK(&bucket->bucketlock);
     {
         if (!rb_probe(bucket->bucket, (void *)entry)) {
+            UNLOCK(&bucket->bucketlock);
             gf_msg(GF_RBTHASH, GF_LOG_ERROR, 0, LG_MSG_RBTHASH_INSERT_FAILED,
-                   "Failed to insert"
-                   " entry");
+                   "Failed to insert entry");
             ret = -1;
+            goto err;
         }
     }
     UNLOCK(&bucket->bucketlock);
diff --git a/rpc/rpc-lib/src/rpc-drc.c b/rpc/rpc-lib/src/rpc-drc.c
index e2a448fa640..bd8695c5ccf 100644
--- a/rpc/rpc-lib/src/rpc-drc.c
+++ b/rpc/rpc-lib/src/rpc-drc.c
@@ -699,47 +699,42 @@ rpcsvc_drc_init(rpcsvc_t *svc, dict_t *options)
     LOCK_INIT(&drc->lock);
     svc->drc = drc;
 
-    LOCK(&drc->lock);
-
     /* Specify type of DRC to be used */
     ret = dict_get_uint32(options, "nfs.drc-type", &drc_type);
     if (ret) {
         gf_log(GF_RPCSVC, GF_LOG_DEBUG,
-               "drc type not set."
-               " Continuing with default");
+               "drc type not set. Continuing with default");
         drc_type = DRC_DEFAULT_TYPE;
     }
 
-    drc->type = drc_type;
-
     /* Set the global cache size (no. of ops to cache) */
     ret = dict_get_uint32(options, "nfs.drc-size", &drc_size);
     if (ret) {
         gf_log(GF_RPCSVC, GF_LOG_DEBUG,
-               "drc size not set."
-               " Continuing with default size");
+               "drc size not set. Continuing with default size");
         drc_size = DRC_DEFAULT_CACHE_SIZE;
     }
 
+    LOCK(&drc->lock);
+
+    drc->type = drc_type;
     drc->global_cache_size = drc_size;
 
     /* Mempool for cached ops */
     drc->mempool = mem_pool_new(drc_cached_op_t, drc->global_cache_size);
     if (!drc->mempool) {
+        UNLOCK(&drc->lock);
         gf_log(GF_RPCSVC, GF_LOG_ERROR,
-               "Failed to get mempool for"
-               " DRC, drc-size: %d",
-               drc->global_cache_size);
+               "Failed to get mempool for DRC, drc-size: %d", drc_size);
         ret = -1;
-        goto out;
+        goto post_unlock;
     }
 
     /* What percent of cache to be evicted whenever it fills up */
     ret = dict_get_uint32(options, "nfs.drc-lru-factor", &drc_factor);
     if (ret) {
         gf_log(GF_RPCSVC, GF_LOG_DEBUG,
-               "drc lru factor not set."
-               " Continuing with policy default");
+               "drc lru factor not set. Continuing with policy default");
         drc_factor = DRC_DEFAULT_LRU_FACTOR;
     }
 
@@ -750,15 +745,16 @@ rpcsvc_drc_init(rpcsvc_t *svc, dict_t *options)
 
     ret = rpcsvc_register_notify(svc, rpcsvc_drc_notify, THIS);
     if (ret) {
+        UNLOCK(&drc->lock);
         gf_log(GF_RPCSVC, GF_LOG_ERROR,
                "registration of drc_notify function failed");
-        goto out;
+        goto post_unlock;
     }
 
-    gf_log(GF_RPCSVC, GF_LOG_DEBUG, "drc init successful");
     drc->status = DRC_INITIATED;
-out:
     UNLOCK(&drc->lock);
+    gf_log(GF_RPCSVC, GF_LOG_DEBUG, "drc init successful");
+post_unlock:
     if (ret == -1) {
         if (drc->mempool) {
             mem_pool_destroy(drc->mempool);
diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index dceab865fab..d9e4796cc5a 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -263,11 +263,7 @@ __afr_set_in_flight_sb_status(xlator_t *this, afr_local_t *local,
 
     count = gf_bits_count(tmp_map);
 
-    if (count == 1)
-        index = gf_bits_index(tmp_map);
-
     for (i = 0; i < priv->child_count; i++) {
-        mask = 0;
         if (!local->transaction.failed_subvols[i])
             continue;
 
@@ -281,6 +277,7 @@ __afr_set_in_flight_sb_status(xlator_t *this, afr_local_t *local,
     switch (txn_type) {
         case AFR_METADATA_TRANSACTION:
             if ((metadatamap_old != 0) && (metadatamap == 0) && (count == 1)) {
+                index = gf_bits_index(tmp_map);
                 local->transaction.in_flight_sb_errno = local->replies[index]
                                                             .op_errno;
                 local->transaction.in_flight_sb = _gf_true;
@@ -293,6 +290,7 @@ __afr_set_in_flight_sb_status(xlator_t *this, afr_local_t *local,
 
         case AFR_DATA_TRANSACTION:
             if ((datamap_old != 0) && (datamap == 0) && (count == 1)) {
+                index = gf_bits_index(tmp_map);
                 local->transaction.in_flight_sb_errno = local->replies[index]
                                                             .op_errno;
                 local->transaction.in_flight_sb = _gf_true;
@@ -776,6 +774,7 @@ afr_spb_choice_timeout_cancel(xlator_t *this, inode_t *inode)
     {
         ret = __afr_inode_ctx_get(this, inode, &ctx);
         if (ret < 0 || !ctx) {
+            UNLOCK(&inode->lock);
             gf_msg(this->name, GF_LOG_WARNING, 0,
                    AFR_MSG_SPLIT_BRAIN_CHOICE_ERROR,
                    "Failed to cancel split-brain choice timer.");
@@ -788,8 +787,8 @@ afr_spb_choice_timeout_cancel(xlator_t *this, inode_t *inode)
         }
         ret = 0;
     }
-out:
     UNLOCK(&inode->lock);
+out:
     return ret;
 }
 
@@ -865,10 +864,11 @@ afr_set_split_brain_choice(int ret, call_frame_t *frame, void *opaque)
     {
         ret = __afr_inode_ctx_get(this, inode, &ctx);
         if (ret) {
+            UNLOCK(&inode->lock);
             gf_msg(this->name, GF_LOG_ERROR, 0,
                    AFR_MSG_SPLIT_BRAIN_CHOICE_ERROR,
                    "Failed to get inode_ctx for %s", loc->name);
-            goto unlock;
+            goto post_unlock;
         }
 
         old_spb_choice = ctx->spb_choice;
@@ -936,6 +936,7 @@ afr_set_split_brain_choice(int ret, call_frame_t *frame, void *opaque)
     }
 unlock:
     UNLOCK(&inode->lock);
+post_unlock:
     if (!timer_set)
         inode_unref(inode);
     if (timer_cancelled)
@@ -3264,7 +3265,6 @@ afr_lookup_do(call_frame_t *frame, xlator_t *this, int err)
 
     if (err < 0) {
         local->op_errno = err;
-        ret = -1;
         goto out;
     }
 
@@ -3275,7 +3275,6 @@ afr_lookup_do(call_frame_t *frame, xlator_t *this, int err)
                                        &local->loc);
     if (ret) {
         local->op_errno = -ret;
-        ret = -1;
         goto out;
     }
 
diff --git a/xlators/cluster/afr/src/afr-inode-read.c b/xlators/cluster/afr/src/afr-inode-read.c
index 1dcef5c44d2..523a5b48880 100644
--- a/xlators/cluster/afr/src/afr-inode-read.c
+++ b/xlators/cluster/afr/src/afr-inode-read.c
@@ -1158,16 +1158,17 @@ afr_fgetxattr_pathinfo_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
 
         ret = dict_set_dynstrn(local->dict, xattr_cky, xattr_cky_len, xattr);
         if (ret) {
+            UNLOCK(&frame->lock);
             gf_msg(this->name, GF_LOG_ERROR, -ret, AFR_MSG_DICT_SET_FAILED,
                    "Cannot set xattr cookie key");
-            goto unlock;
+            goto post_unlock;
         }
 
         local->cont.getxattr.xattr_len += strlen(xattr) + 1;
     }
 unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     if (!callcnt) {
         if (!local->cont.getxattr.xattr_len)
             goto unwind;
@@ -1281,16 +1282,17 @@ afr_getxattr_pathinfo_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
 
         ret = dict_set_dynstrn(local->dict, xattr_cky, xattr_cky_len, xattr);
         if (ret) {
+            UNLOCK(&frame->lock);
             gf_msg(this->name, GF_LOG_ERROR, -ret, AFR_MSG_DICT_SET_FAILED,
                    "Cannot set xattr cookie key");
-            goto unlock;
+            goto post_unlock;
         }
 
         local->cont.getxattr.xattr_len += strlen(xattr) + 1;
     }
 unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     if (!callcnt) {
         if (!local->cont.getxattr.xattr_len)
             goto unwind;
diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c
index 912e3c6e104..5ce1864007b 100644
--- a/xlators/cluster/dht/src/dht-common.c
+++ b/xlators/cluster/dht/src/dht-common.c
@@ -3569,18 +3569,16 @@ dht_unlink_linkfile_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
         if ((op_ret == -1) &&
             !((op_errno == ENOENT) || (op_errno == ENOTCONN))) {
             local->op_errno = op_errno;
+            UNLOCK(&frame->lock);
             gf_msg_debug(this->name, op_errno,
-                         "Unlink link: subvolume %s"
-                         " returned -1",
-                         prev->name);
-            goto unlock;
+                         "Unlink link: subvolume %s returned -1", prev->name);
+            goto post_unlock;
         }
 
         local->op_ret = 0;
     }
-unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     dht_set_fixed_dir_stat(&local->preparent);
     dht_set_fixed_dir_stat(&local->postparent);
     DHT_STACK_UNWIND(unlink, frame, local->op_ret, local->op_errno,
@@ -3610,9 +3608,10 @@ dht_unlink_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
             } else {
                 local->op_ret = 0;
             }
+            UNLOCK(&frame->lock);
             gf_msg_debug(this->name, op_errno,
                          "Unlink: subvolume %s returned -1", prev->name);
-            goto unlock;
+            goto post_unlock;
         }
 
         local->op_ret = 0;
@@ -3627,9 +3626,8 @@ dht_unlink_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
                                       &local->postparent, 1);
         }
     }
-unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     if (!local->op_ret) {
         hashed_subvol = dht_subvol_get_hashed(this, &local->loc);
         if (hashed_subvol && hashed_subvol != local->cached_subvol) {
@@ -3695,16 +3693,16 @@ dht_err_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
     {
         if (op_ret == -1) {
             local->op_errno = op_errno;
+            UNLOCK(&frame->lock);
             gf_msg_debug(this->name, op_errno, "subvolume %s returned -1",
                          prev->name);
-            goto unlock;
+            goto post_unlock;
         }
 
         local->op_ret = 0;
     }
-unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     this_call_cnt = dht_frame_return(frame);
     if (is_last_call(this_call_cnt)) {
         if ((local->fop == GF_FOP_SETXATTR) ||
@@ -3816,11 +3814,14 @@ dht_setxattr_non_mds_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
         if (op_ret && !local->op_ret) {
             local->op_ret = op_ret;
             local->op_errno = op_errno;
+            UNLOCK(&frame->lock);
             gf_msg_debug(this->name, op_errno, "subvolume %s returned -1",
                          prev->this->name);
+            goto post_unlock;
         }
     }
     UNLOCK(&frame->lock);
+post_unlock:
     this_call_cnt = dht_frame_return(frame);
 
     if (is_last_call(this_call_cnt)) {
@@ -4249,11 +4250,12 @@ dht_find_local_subvol_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
     {
         this_call_cnt = --local->call_cnt;
         if (op_ret < 0) {
-            gf_msg(this->name, GF_LOG_ERROR, op_errno, DHT_MSG_GET_XATTR_FAILED,
-                   "getxattr err for dir");
             local->op_ret = -1;
             local->op_errno = op_errno;
-            goto unlock;
+            UNLOCK(&frame->lock);
+            gf_msg(this->name, GF_LOG_ERROR, op_errno, DHT_MSG_GET_XATTR_FAILED,
+                   "getxattr err for dir");
+            goto post_unlock;
         }
 
         ret = dict_get_str(xattr, local->xsel, &uuid_list);
@@ -4279,13 +4281,12 @@ dht_find_local_subvol_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
              uuid_str = next_uuid_str) {
             next_uuid_str = strtok_r(NULL, " ", &saveptr);
             if (gf_uuid_parse(uuid_str, node_uuid)) {
-                gf_msg(this->name, GF_LOG_ERROR, 0, DHT_MSG_UUID_PARSE_ERROR,
-                       "Failed to parse uuid"
-                       " for %s",
-                       prev->name);
                 local->op_ret = -1;
                 local->op_errno = EINVAL;
-                goto unlock;
+                UNLOCK(&frame->lock);
+                gf_msg(this->name, GF_LOG_ERROR, 0, DHT_MSG_UUID_PARSE_ERROR,
+                       "Failed to parse uuid for %s", prev->name);
+                goto post_unlock;
             }
 
             count++;
@@ -4342,7 +4343,7 @@ dht_find_local_subvol_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
     local->op_ret = 0;
 unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     if (!is_last_call(this_call_cnt))
         goto out;
 
@@ -4383,23 +4384,28 @@ dht_vgetxattr_dir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
         this_call_cnt = --local->call_cnt;
         if (op_ret < 0) {
             if (op_errno != ENOTCONN) {
-                gf_msg(this->name, GF_LOG_ERROR, op_errno,
-                       DHT_MSG_GET_XATTR_FAILED, "getxattr err for dir");
                 local->op_ret = -1;
                 local->op_errno = op_errno;
+                UNLOCK(&frame->lock);
+                gf_msg(this->name, GF_LOG_ERROR, op_errno,
+                       DHT_MSG_GET_XATTR_FAILED, "getxattr err for dir");
+                goto post_unlock;
             }
 
             goto unlock;
         }
 
         ret = dht_vgetxattr_alloc_and_fill(local, xattr, this, op_errno);
-        if (ret)
+        if (ret) {
+            UNLOCK(&frame->lock);
             gf_msg(this->name, GF_LOG_ERROR, op_errno, DHT_MSG_DICT_SET_FAILED,
                    "alloc or fill failure");
+            goto post_unlock;
+        }
     }
 unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     if (!is_last_call(this_call_cnt))
         goto out;
 
@@ -4511,9 +4517,7 @@ dht_mds_getxattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
         local->op_ret = op_ret;
         goto out;
     }
-    if (dict_get(xattr, conf->xattr_name)) {
-        dict_del(xattr, conf->xattr_name);
-    }
+    dict_del(xattr, conf->xattr_name);
     local->op_ret = 0;
 
     if (!local->xattr) {
@@ -4551,13 +4555,8 @@ dht_getxattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
             goto unlock;
         }
 
-        if (dict_get(xattr, conf->xattr_name)) {
-            dict_del(xattr, conf->xattr_name);
-        }
-
-        if (dict_get(xattr, conf->mds_xattr_key)) {
-            dict_del(xattr, conf->mds_xattr_key);
-        }
+        dict_del(xattr, conf->xattr_name);
+        dict_del(xattr, conf->mds_xattr_key);
 
         /* filter out following two xattrs that need not
          * be visible on the mount point for geo-rep -
@@ -4565,9 +4564,7 @@ dht_getxattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
          * trusted.tier.tier-dht.commithash
          */
 
-        if (dict_get(xattr, conf->commithash_xattr_name)) {
-            dict_del(xattr, conf->commithash_xattr_name);
-        }
+        dict_del(xattr, conf->commithash_xattr_name);
 
         if (frame->root->pid >= 0 && dht_is_tier_xlator(this)) {
             dict_del(xattr, GF_XATTR_TIER_LAYOUT_FIXED_KEY);
@@ -4660,13 +4657,14 @@ dht_getxattr_get_real_filename_cbk(call_frame_t *frame, void *cookie,
 
                 local->op_ret = op_ret;
                 local->op_errno = op_errno;
+                UNLOCK(&frame->lock);
                 gf_msg(this->name, GF_LOG_WARNING, op_errno,
                        DHT_MSG_UPGRADE_BRICKS,
                        "At least "
                        "one of the bricks does not support "
                        "this operation. Please upgrade all "
                        "bricks.");
-                goto unlock;
+                goto post_unlock;
             }
 
             if (op_errno == ENOENT) {
@@ -4681,9 +4679,10 @@ dht_getxattr_get_real_filename_cbk(call_frame_t *frame, void *cookie,
              * down subvol and return a good result(if any)
              * from other subvol.
              */
+            UNLOCK(&frame->lock);
             gf_msg(this->name, GF_LOG_WARNING, op_errno,
                    DHT_MSG_GET_XATTR_FAILED, "Failed to get real filename.");
-            goto unlock;
+            goto post_unlock;
         }
 
         /* This subvol has the required file.
@@ -4704,13 +4703,13 @@ dht_getxattr_get_real_filename_cbk(call_frame_t *frame, void *cookie,
 
         local->op_ret = op_ret;
         local->op_errno = 0;
-        gf_msg_debug(this->name, 0,
-                     "Found a matching "
-                     "file.");
+        UNLOCK(&frame->lock);
+        gf_msg_debug(this->name, 0, "Found a matching file.");
+        goto post_unlock;
     }
 unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     this_call_cnt = dht_frame_return(frame);
     if (is_last_call(this_call_cnt)) {
         DHT_STACK_UNWIND(getxattr, frame, local->op_ret, local->op_errno,
@@ -6061,16 +6060,16 @@ dht_removexattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
     {
         if (op_ret == -1) {
             local->op_errno = op_errno;
+            UNLOCK(&frame->lock);
             gf_msg_debug(this->name, op_errno, "subvolume %s returned -1",
                          prev->name);
-            goto unlock;
+            goto post_unlock;
         }
 
         local->op_ret = 0;
     }
-unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     this_call_cnt = dht_frame_return(frame);
     if (is_last_call(this_call_cnt)) {
         DHT_STACK_UNWIND(removexattr, frame, local->op_ret, local->op_errno,
@@ -6257,16 +6256,16 @@ dht_fd_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
     {
         if (op_ret == -1) {
             local->op_errno = op_errno;
+            UNLOCK(&frame->lock);
             gf_msg_debug(this->name, op_errno, "subvolume %s returned -1",
                          prev->name);
-            goto unlock;
+            goto post_unlock;
         }
 
         local->op_ret = 0;
     }
-unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     this_call_cnt = dht_frame_return(frame);
     if (is_last_call(this_call_cnt))
         DHT_STACK_UNWIND(open, frame, local->op_ret, local->op_errno, local->fd,
@@ -7146,12 +7145,10 @@ dht_fsyncdir_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
     {
         if (op_ret == -1)
             local->op_errno = op_errno;
-
-        if (op_ret == 0)
+        else if (op_ret == 0)
             local->op_ret = 0;
     }
     UNLOCK(&frame->lock);
-
     this_call_cnt = dht_frame_return(frame);
     if (is_last_call(this_call_cnt))
         DHT_STACK_UNWIND(fsyncdir, frame, local->op_ret, local->op_errno,
diff --git a/xlators/cluster/dht/src/dht-hashfn.c b/xlators/cluster/dht/src/dht-hashfn.c
index 16ee6d2d405..3def6b17666 100644
--- a/xlators/cluster/dht/src/dht-hashfn.c
+++ b/xlators/cluster/dht/src/dht-hashfn.c
@@ -74,29 +74,30 @@ dht_hash_compute(xlator_t *this, int type, const char *name, uint32_t *hash_p)
 
     priv = this->private;
 
+    len = strlen(name) + 1;
+    rsync_friendly_name = alloca(len);
+
     LOCK(&priv->lock);
     {
         if (priv->extra_regex_valid) {
-            len = strlen(name) + 1;
-            rsync_friendly_name = alloca(len);
             munged = dht_munge_name(name, rsync_friendly_name, len,
                                     &priv->extra_regex);
         }
 
         if (!munged && priv->rsync_regex_valid) {
-            len = strlen(name) + 1;
-            rsync_friendly_name = alloca(len);
             gf_msg_trace(this->name, 0, "trying regex for %s", name);
             munged = dht_munge_name(name, rsync_friendly_name, len,
                                     &priv->rsync_regex);
             if (munged) {
+                UNLOCK(&priv->lock);
                 gf_msg_debug(this->name, 0, "munged down to %s",
                              rsync_friendly_name);
+                goto post_unlock;
             }
         }
     }
     UNLOCK(&priv->lock);
-
+post_unlock:
     if (!munged) {
         rsync_friendly_name = (char *)name;
     }
diff --git a/xlators/cluster/dht/src/dht-helper.c b/xlators/cluster/dht/src/dht-helper.c
index 12e7a4fd2c2..1a5ba256ffd 100644
--- a/xlators/cluster/dht/src/dht-helper.c
+++ b/xlators/cluster/dht/src/dht-helper.c
@@ -94,12 +94,13 @@ dht_fd_ctx_set(xlator_t *this, fd_t *fd, xlator_t *dst)
                 goto unlock;
             } else {
                 /* This would be a big problem*/
+                /* Overwrite and hope for the best*/
+                fd_ctx->opened_on_dst = (uint64_t)(uintptr_t)dst;
+                UNLOCK(&fd->lock);
                 gf_msg(this->name, GF_LOG_WARNING, 0, DHT_MSG_INVALID_VALUE,
                        "Different dst found in the fd ctx");
 
-                /* Overwrite and hope for the best*/
-                fd_ctx->opened_on_dst = (uint64_t)(uintptr_t)dst;
-                goto unlock;
+                goto out;
             }
         }
         ret = __dht_fd_ctx_set(this, fd, dst);
@@ -124,13 +125,13 @@ dht_fd_ctx_get(xlator_t *this, fd_t *fd)
     {
         ret = __fd_ctx_get(fd, this, &tmp_val);
         if ((ret < 0) || (tmp_val == 0)) {
-            UNLOCK(&fd->lock);
-            goto out;
+            goto unlock;
         }
 
         fd_ctx = (dht_fd_ctx_t *)(uintptr_t)tmp_val;
         GF_REF_GET(fd_ctx);
     }
+unlock:
     UNLOCK(&fd->lock);
 
 out:
@@ -2134,16 +2135,15 @@ dht_get_lock_subvolume(xlator_t *this, struct gf_flock *lock,
         ret = __dht_lock_subvol_set(inode, this, cached_subvol);
         if (ret) {
             gf_uuid_unparse(inode->gfid, gfid);
+            UNLOCK(&inode->lock);
             gf_msg(this->name, GF_LOG_WARNING, 0, DHT_MSG_SET_INODE_CTX_FAILED,
-                   "Failed to set lock_subvol in "
-                   "inode ctx for gfid %s",
-                   gfid);
-            goto unlock;
+                   "Failed to set lock_subvol in inode ctx for gfid %s", gfid);
+            goto post_unlock;
         }
         subvol = cached_subvol;
     }
-unlock:
     UNLOCK(&inode->lock);
+post_unlock:
     if (!subvol && inode && lock->l_type != F_UNLCK) {
         inode_unref(inode);
     }
diff --git a/xlators/cluster/dht/src/dht-inode-read.c b/xlators/cluster/dht/src/dht-inode-read.c
index f46370a9208..cacfe353272 100644
--- a/xlators/cluster/dht/src/dht-inode-read.c
+++ b/xlators/cluster/dht/src/dht-inode-read.c
@@ -272,19 +272,19 @@ dht_attr_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
     {
         if (op_ret == -1) {
             local->op_errno = op_errno;
+            UNLOCK(&frame->lock);
             gf_msg_debug(this->name, op_errno, "subvolume %s returned -1",
                          prev->name);
 
-            goto unlock;
+            goto post_unlock;
         }
 
         dht_iatt_merge(this, &local->stbuf, stbuf);
 
         local->op_ret = 0;
     }
-unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     this_call_cnt = dht_frame_return(frame);
     if (is_last_call(this_call_cnt)) {
         DHT_STACK_UNWIND(stat, frame, local->op_ret, local->op_errno,
diff --git a/xlators/cluster/dht/src/dht-inode-write.c b/xlators/cluster/dht/src/dht-inode-write.c
index d0d12fd7658..b26b7058d3e 100644
--- a/xlators/cluster/dht/src/dht-inode-write.c
+++ b/xlators/cluster/dht/src/dht-inode-write.c
@@ -1113,9 +1113,10 @@ dht_setattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
     {
         if (op_ret == -1) {
             local->op_errno = op_errno;
+            UNLOCK(&frame->lock);
             gf_msg_debug(this->name, op_errno, "subvolume %s returned -1",
                          prev->name);
-            goto unlock;
+            goto post_unlock;
         }
 
         dht_iatt_merge(this, &local->prebuf, statpre);
@@ -1124,9 +1125,8 @@ dht_setattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
         local->op_ret = 0;
         local->op_errno = 0;
     }
-unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     this_call_cnt = dht_frame_return(frame);
     if (is_last_call(this_call_cnt)) {
         if (local->op_ret == 0)
@@ -1151,24 +1151,22 @@ dht_non_mds_setattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
     local = frame->local;
     prev = cookie;
 
+    if (op_ret == -1) {
+        gf_msg(this->name, op_errno, 0, 0, "subvolume %s returned -1",
+               prev->name);
+        goto post_unlock;
+    }
+
     LOCK(&frame->lock);
     {
-        if (op_ret == -1) {
-            gf_msg(this->name, op_errno, 0, 0, "subvolume %s returned -1",
-                   prev->name);
-
-            goto unlock;
-        }
-
         dht_iatt_merge(this, &local->prebuf, statpre);
         dht_iatt_merge(this, &local->stbuf, statpost);
 
         local->op_ret = 0;
         local->op_errno = 0;
     }
-unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     this_call_cnt = dht_frame_return(frame);
     if (is_last_call(this_call_cnt)) {
         dht_inode_ctx_time_set(local->loc.inode, this, &local->stbuf);
diff --git a/xlators/features/barrier/src/barrier.c b/xlators/features/barrier/src/barrier.c
index 4f8fa211d0b..a601c7fa04f 100644
--- a/xlators/features/barrier/src/barrier.c
+++ b/xlators/features/barrier/src/barrier.c
@@ -461,7 +461,7 @@ out:
 int
 notify(xlator_t *this, int event, void *data, ...)
 {
-    barrier_priv_t *priv = NULL;
+    barrier_priv_t *priv = this->private;
     dict_t *dict = NULL;
     int ret = -1;
     int barrier_enabled = _gf_false;
@@ -469,7 +469,6 @@ notify(xlator_t *this, int event, void *data, ...)
         0,
     };
 
-    priv = this->private;
     GF_ASSERT(priv);
     INIT_LIST_HEAD(&queue);
 
@@ -491,19 +490,23 @@ notify(xlator_t *this, int event, void *data, ...)
                     if (barrier_enabled) {
                         ret = __barrier_enable(this, priv);
                     } else {
+                        UNLOCK(&priv->lock);
                         gf_log(this->name, GF_LOG_ERROR, "Already disabled.");
+                        goto post_unlock;
                     }
                 } else {
                     if (!barrier_enabled) {
                         __barrier_disable(this, &queue);
                         ret = 0;
                     } else {
+                        UNLOCK(&priv->lock);
                         gf_log(this->name, GF_LOG_ERROR, "Already enabled");
+                        goto post_unlock;
                     }
                 }
             }
             UNLOCK(&priv->lock);
-
+        post_unlock:
             if (!list_empty(&queue))
                 barrier_dequeue_all(this, &queue);
 
diff --git a/xlators/features/bit-rot/src/bitd/bit-rot-scrub.c b/xlators/features/bit-rot/src/bitd/bit-rot-scrub.c
index 35318dcfa4e..a6beb2edb92 100644
--- a/xlators/features/bit-rot/src/bitd/bit-rot-scrub.c
+++ b/xlators/features/bit-rot/src/bitd/bit-rot-scrub.c
@@ -720,8 +720,10 @@ br_scrubber_exit_control(xlator_t *this)
         if (scrub_monitor->state == BR_SCRUB_STATE_ACTIVE) {
             (void)br_fsscan_activate(this);
         } else {
+            UNLOCK(&scrub_monitor->lock);
             gf_msg(this->name, GF_LOG_INFO, 0, BRB_MSG_SCRUB_INFO,
                    "Volume waiting to get rescheduled..");
+            return;
         }
     }
     UNLOCK(&scrub_monitor->lock);
diff --git a/xlators/features/locks/src/posix.c b/xlators/features/locks/src/posix.c
index 0feb11e3b78..3f1c7a733ff 100644
--- a/xlators/features/locks/src/posix.c
+++ b/xlators/features/locks/src/posix.c
@@ -504,7 +504,9 @@ pl_check_n_create_fdctx(xlator_t *this, fd_t *fd)
         if (ret != 0) {
             GF_FREE(fdctx);
             fdctx = NULL;
+            UNLOCK(&fd->lock);
             gf_log(this->name, GF_LOG_DEBUG, "failed to set fd ctx");
+            goto out;
         }
     }
 unlock:
diff --git a/xlators/features/quota/src/quota.c b/xlators/features/quota/src/quota.c
index 8812a301924..a0c236d4cf6 100644
--- a/xlators/features/quota/src/quota.c
+++ b/xlators/features/quota/src/quota.c
@@ -3292,12 +3292,11 @@ quota_stat_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
         goto out;
     }
 
-    LOCK(&ctx->lock);
-    {
-        if (buf)
-            ctx->buf = *buf;
+    if (buf) {
+        LOCK(&ctx->lock);
+        ctx->buf = *buf;
+        UNLOCK(&ctx->lock);
     }
-    UNLOCK(&ctx->lock);
 
 out:
     QUOTA_STACK_UNWIND(stat, frame, op_ret, op_errno, buf, xdata);
@@ -3371,12 +3370,11 @@ quota_fstat_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
         goto out;
     }
 
-    LOCK(&ctx->lock);
-    {
-        if (buf)
-            ctx->buf = *buf;
+    if (buf) {
+        LOCK(&ctx->lock);
+        ctx->buf = *buf;
+        UNLOCK(&ctx->lock);
     }
-    UNLOCK(&ctx->lock);
 
 out:
     QUOTA_STACK_UNWIND(fstat, frame, op_ret, op_errno, buf, xdata);
@@ -3666,12 +3664,11 @@ quota_setattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
         goto out;
     }
 
-    LOCK(&ctx->lock);
-    {
-        if (statpost)
-            ctx->buf = *statpost;
+    if (statpost) {
+        LOCK(&ctx->lock);
+        ctx->buf = *statpost;
+        UNLOCK(&ctx->lock);
     }
-    UNLOCK(&ctx->lock);
 
 out:
     QUOTA_STACK_UNWIND(setattr, frame, op_ret, op_errno, statpre, statpost,
diff --git a/xlators/lib/src/libxlator.c b/xlators/lib/src/libxlator.c
index e1a22b60340..075229dae76 100644
--- a/xlators/lib/src/libxlator.c
+++ b/xlators/lib/src/libxlator.c
@@ -198,10 +198,11 @@ cluster_markerxtime_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
         }
 
         if (dict_get_ptr(dict, marker_xattr, (void **)&net_timebuf)) {
+            local->count[MCNT_NOTFOUND]++;
+            UNLOCK(&frame->lock);
             gf_log(this->name, GF_LOG_WARNING,
                    "Unable to get <uuid>.xtime attr");
-            local->count[MCNT_NOTFOUND]++;
-            goto unlock;
+            goto post_unlock;
         }
 
         if (local->count[MCNT_FOUND]) {
@@ -221,7 +222,7 @@ cluster_markerxtime_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
     }
 unlock:
     UNLOCK(&frame->lock);
-
+post_unlock:
     if (callcnt == 0)
         cluster_marker_unwind(frame, marker_xattr, local->net_timebuf, 8, dict);
 
diff --git a/xlators/meta/src/frames-file.c b/xlators/meta/src/frames-file.c
index 995d7680e9b..9a13db9a934 100644
--- a/xlators/meta/src/frames-file.c
+++ b/xlators/meta/src/frames-file.c
@@ -30,9 +30,10 @@ frames_file_fill(xlator_t *this, inode_t *file, strfd_t *strfd)
 
     pool = this->ctx->pool;
 
+    strprintf(strfd, "{ \n\t\"Stack\": [\n");
+
     LOCK(&pool->lock);
     {
-        strprintf(strfd, "{ \n\t\"Stack\": [\n");
         list_for_each_entry(stack, &pool->all_frames, all_frames)
         {
             strprintf(strfd, "\t   {\n");