From c7d1aee76d5713d1f337ab1c831c0ed74e4676e1 Mon Sep 17 00:00:00 2001 From: Yaniv Kaul Date: Thu, 27 Dec 2018 18:56:16 +0200 Subject: [PATCH] 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 Change-Id: I2438710016afc9f4f62a176ef1a0d3ed793b4f89 --- glusterfsd/src/glusterfsd-mgmt.c | 17 +-- libglusterfs/src/event-epoll.c | 4 +- libglusterfs/src/fd.c | 10 +- libglusterfs/src/gidcache.c | 4 +- libglusterfs/src/rbthash.c | 5 +- rpc/rpc-lib/src/rpc-drc.c | 30 +++-- xlators/cluster/afr/src/afr-common.c | 15 ++- xlators/cluster/afr/src/afr-inode-read.c | 10 +- xlators/cluster/dht/src/dht-common.c | 103 +++++++++--------- xlators/cluster/dht/src/dht-hashfn.c | 11 +- xlators/cluster/dht/src/dht-helper.c | 20 ++-- xlators/cluster/dht/src/dht-inode-read.c | 6 +- xlators/cluster/dht/src/dht-inode-write.c | 22 ++-- xlators/features/barrier/src/barrier.c | 9 +- .../features/bit-rot/src/bitd/bit-rot-scrub.c | 2 + xlators/features/locks/src/posix.c | 2 + xlators/features/quota/src/quota.c | 27 ++--- xlators/lib/src/libxlator.c | 7 +- xlators/meta/src/frames-file.c | 3 +- 19 files changed, 155 insertions(+), 152 deletions(-) diff --git a/glusterfsd/src/glusterfsd-mgmt.c b/glusterfsd/src/glusterfsd-mgmt.c index 6dec11b2b..a78acb602 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 dcaf98045..be8b204b7 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 44a9ee69d..b8aac7260 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 87147163e..40fcffbb3 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 1bdd45f9f..ae2e158d6 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 e2a448fa6..bd8695c5c 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 dceab865f..d9e4796cc 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 1dcef5c44..523a5b488 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 912e3c6e1..5ce186400 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 16ee6d2d4..3def6b176 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 12e7a4fd2..1a5ba256f 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 f46370a92..cacfe3532 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 d0d12fd76..b26b7058d 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 4f8fa211d..a601c7fa0 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 35318dcfa..a6beb2edb 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 0feb11e3b..3f1c7a733 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 8812a3019..a0c236d4c 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 e1a22b603..075229dae 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 .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 995d7680e..9a13db9a9 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");