From 430484c92ab5a6234958d1143e0bb14aeb0cd1c0 Mon Sep 17 00:00:00 2001 From: Mohit Agrawal Date: Fri, 20 Oct 2017 12:39:29 +0530 Subject: glusterfs: Use gcc builtin ATOMIC operator to increase/decreate refcount. Problem: In glusterfs code base we call mutex_lock/unlock to take reference/dereference for a object.Sometime it could be reason for lock contention also. Solution: There is no need to use mutex to increase/decrease ref counter, instead of using mutex use gcc builtin ATOMIC operation. Test: I have not observed yet how much performance gain after apply this patch specific to glusterfs but i have tested same with below small program(mutex and atomic both) and get good difference. static int numOuterLoops; static void * threadFunc(void *arg) { int j; for (j = 0; j < numOuterLoops; j++) { __atomic_add_fetch (&glob, 1,__ATOMIC_ACQ_REL); } return NULL; } int main(int argc, char *argv[]) { int opt, s, j; int numThreads; pthread_t *thread; int verbose; int64_t n = 0; if (argc < 2 ) { printf(" Please provide 2 args Num of threads && Outer Loop\n"); exit (-1); } numThreads = atoi(argv[1]); numOuterLoops = atoi (argv[2]); if (1) { printf("\tthreads: %d; outer loops: %d;\n", numThreads, numOuterLoops); } thread = calloc(numThreads, sizeof(pthread_t)); if (thread == NULL) { printf ("calloc error so exit\n"); exit (-1); } __atomic_store (&glob, &n, __ATOMIC_RELEASE); for (j = 0; j < numThreads; j++) { s = pthread_create(&thread[j], NULL, threadFunc, NULL); if (s != 0) { printf ("pthread_create failed so exit\n"); exit (-1); } } for (j = 0; j < numThreads; j++) { s = pthread_join(thread[j], NULL); if (s != 0) { printf ("pthread_join failed so exit\n"); exit (-1); } } printf("glob value is %ld\n",__atomic_load_n (&glob,__ATOMIC_RELAXED)); exit(0); } time ./thr_count 800 800000 threads: 800; outer loops: 800000; glob value is 640000000 real 1m10.288s user 0m57.269s sys 3m31.565s time ./thr_count_atomic 800 800000 threads: 800; outer loops: 800000; glob value is 640000000 real 0m20.313s user 1m20.558s sys 0m0.028 Change-Id: Ie5030a52ea264875e002e108dd4b207b15ab7cc7 Signed-off-by: Mohit Agrawal --- xlators/cluster/dht/src/dht-common.h | 4 +- xlators/cluster/dht/src/dht-layout.c | 43 +++++----------------- .../cluster/dht/src/unittest/dht_layout_unittest.c | 4 +- xlators/debug/io-stats/src/io-stats.c | 29 ++++++--------- .../features/changelog/src/changelog-ev-handle.h | 22 ++++------- xlators/features/changelog/src/changelog-rpc.c | 2 +- xlators/nfs/server/src/nfs3.c | 4 +- xlators/protocol/client/src/client-handshake.c | 16 ++------ xlators/protocol/client/src/client.c | 2 +- 9 files changed, 42 insertions(+), 84 deletions(-) (limited to 'xlators') diff --git a/xlators/cluster/dht/src/dht-common.h b/xlators/cluster/dht/src/dht-common.h index 3d6fbb1fd79..cc7d41b3bfa 100644 --- a/xlators/cluster/dht/src/dht-common.h +++ b/xlators/cluster/dht/src/dht-common.h @@ -87,8 +87,8 @@ struct dht_layout { */ int gen; int type; - int ref; /* use with dht_conf_t->layout_lock */ - uint32_t search_unhashed; + gf_atomic_t ref; /* use with dht_conf_t->layout_lock */ + gf_boolean_t search_unhashed; struct { int err; /* 0 = normal -1 = dir exists and no xattr diff --git a/xlators/cluster/dht/src/dht-layout.c b/xlators/cluster/dht/src/dht-layout.c index 419ce329a1d..97b98e01451 100644 --- a/xlators/cluster/dht/src/dht-layout.c +++ b/xlators/cluster/dht/src/dht-layout.c @@ -48,12 +48,12 @@ dht_layout_new (xlator_t *this, int cnt) layout->gen = conf->gen; } - layout->ref = 1; + GF_ATOMIC_INIT (layout->ref, 1); ENSURE(NULL != layout); ENSURE(layout->type == DHT_HASH_TYPE_DM); ENSURE(layout->cnt == cnt); - ENSURE(layout->ref == 1); + ENSURE(GF_ATOMIC_GET (layout->ref) == 1); out: return layout; } @@ -62,24 +62,13 @@ out: dht_layout_t * dht_layout_get (xlator_t *this, inode_t *inode) { - dht_conf_t *conf = NULL; dht_layout_t *layout = NULL; int ret = 0; - conf = this->private; - if (!conf) - goto out; - - LOCK (&conf->layout_lock); - { - ret = dht_inode_ctx_layout_get (inode, this, &layout); - if ((!ret) && layout) { - layout->ref++; - } + ret = dht_inode_ctx_layout_get (inode, this, &layout); + if ((!ret) && layout) { + GF_ATOMIC_INC (layout->ref); } - UNLOCK (&conf->layout_lock); - -out: return layout; } @@ -100,7 +89,7 @@ dht_layout_set (xlator_t *this, inode_t *inode, dht_layout_t *layout) { oldret = dht_inode_ctx_layout_get (inode, this, &old_layout); if (layout) - layout->ref++; + GF_ATOMIC_INC (layout->ref); ret = dht_inode_ctx_layout_set (inode, this, layout); } UNLOCK (&conf->layout_lock); @@ -108,6 +97,8 @@ dht_layout_set (xlator_t *this, inode_t *inode, dht_layout_t *layout) if (!oldret) { dht_layout_unref (this, old_layout); } + if (ret) + GF_ATOMIC_DEC (layout->ref); out: return ret; @@ -117,19 +108,12 @@ out: void dht_layout_unref (xlator_t *this, dht_layout_t *layout) { - dht_conf_t *conf = NULL; int ref = 0; if (!layout || layout->preset || !this->private) return; - conf = this->private; - - LOCK (&conf->layout_lock); - { - ref = --layout->ref; - } - UNLOCK (&conf->layout_lock); + ref = GF_ATOMIC_DEC (layout->ref); if (!ref) GF_FREE (layout); @@ -139,17 +123,10 @@ dht_layout_unref (xlator_t *this, dht_layout_t *layout) dht_layout_t * dht_layout_ref (xlator_t *this, dht_layout_t *layout) { - dht_conf_t *conf = NULL; - if (layout->preset || !this->private) return layout; - conf = this->private; - LOCK (&conf->layout_lock); - { - layout->ref++; - } - UNLOCK (&conf->layout_lock); + GF_ATOMIC_INC (layout->ref); return layout; } diff --git a/xlators/cluster/dht/src/unittest/dht_layout_unittest.c b/xlators/cluster/dht/src/unittest/dht_layout_unittest.c index 84a89160e38..fb6445ce1f3 100644 --- a/xlators/cluster/dht/src/unittest/dht_layout_unittest.c +++ b/xlators/cluster/dht/src/unittest/dht_layout_unittest.c @@ -90,7 +90,7 @@ test_dht_layout_new(void **state) assert_non_null(layout); assert_int_equal(layout->type, DHT_HASH_TYPE_DM); assert_int_equal(layout->cnt, cnt); - assert_int_equal(layout->ref, 1); + assert_int_equal(GF_ATOMIC_GET (layout->ref), 1); assert_int_equal(layout->gen, 0); assert_int_equal(layout->spread_cnt, 0); free(layout); @@ -107,7 +107,7 @@ test_dht_layout_new(void **state) assert_non_null(layout); assert_int_equal(layout->type, DHT_HASH_TYPE_DM); assert_int_equal(layout->cnt, cnt); - assert_int_equal(layout->ref, 1); + assert_int_equal(GF_ATOMIC_GET (layout->ref), 1); assert_int_equal(layout->gen, conf->gen); assert_int_equal(layout->spread_cnt, conf->dir_spread_cnt); free(layout); diff --git a/xlators/debug/io-stats/src/io-stats.c b/xlators/debug/io-stats/src/io-stats.c index c0cd0e2477b..4864c67a101 100644 --- a/xlators/debug/io-stats/src/io-stats.c +++ b/xlators/debug/io-stats/src/io-stats.c @@ -74,7 +74,7 @@ struct ios_stat { char *filename; gf_atomic_t counters[IOS_STATS_TYPE_MAX]; struct ios_stat_lat thru_counters [IOS_STATS_THRU_MAX]; - int refcnt; + gf_atomic_t refcnt; }; struct ios_stat_list { @@ -333,13 +333,10 @@ ios_fd_ctx_set (fd_t *fd, xlator_t *this, struct ios_fd *iosfd) static int ios_stat_ref (struct ios_stat *iosstat) { - LOCK (&iosstat->lock); - { - iosstat->refcnt++; - } - UNLOCK (&iosstat->lock); + uint64_t refcnt = 0; + refcnt = GF_ATOMIC_INC (iosstat->refcnt); - return iosstat->refcnt; + return refcnt; } @@ -347,18 +344,16 @@ static int ios_stat_unref (struct ios_stat *iosstat) { int cleanup = 0; - LOCK (&iosstat->lock); - { - iosstat->refcnt--; - if (iosstat->refcnt == 0) { - if (iosstat->filename) { - GF_FREE (iosstat->filename); - iosstat->filename = NULL; - } - cleanup = 1; + uint64_t refcnt = 0; + + refcnt = GF_ATOMIC_DEC (iosstat->refcnt); + if (refcnt == 0) { + if (iosstat->filename) { + GF_FREE (iosstat->filename); + iosstat->filename = NULL; } + cleanup = 1; } - UNLOCK (&iosstat->lock); if (cleanup) { LOCK_DESTROY (&iosstat->lock); diff --git a/xlators/features/changelog/src/changelog-ev-handle.h b/xlators/features/changelog/src/changelog-ev-handle.h index eef0492a9ee..e89af8793a8 100644 --- a/xlators/features/changelog/src/changelog-ev-handle.h +++ b/xlators/features/changelog/src/changelog-ev-handle.h @@ -24,7 +24,7 @@ typedef struct changelog_rpc_clnt { gf_lock_t lock; - unsigned long ref; + gf_atomic_t ref; gf_boolean_t disconnected; unsigned int filter; @@ -43,11 +43,7 @@ typedef struct changelog_rpc_clnt { static inline void changelog_rpc_clnt_ref (changelog_rpc_clnt_t *crpc) { - LOCK (&crpc->lock); - { - ++crpc->ref; - } - UNLOCK (&crpc->lock); + GF_ATOMIC_INC (crpc->ref); } static inline void @@ -66,16 +62,14 @@ static inline void changelog_rpc_clnt_unref (changelog_rpc_clnt_t *crpc) { gf_boolean_t gone = _gf_false; + uint64_t ref = 0; + + ref = GF_ATOMIC_DEC (crpc->ref); - LOCK (&crpc->lock); - { - if (!(--crpc->ref) - && changelog_rpc_clnt_is_disconnected (crpc)) { - list_del (&crpc->list); - gone = _gf_true; - } + if (!ref && changelog_rpc_clnt_is_disconnected (crpc)) { + list_del (&crpc->list); + gone = _gf_true; } - UNLOCK (&crpc->lock); if (gone) crpc->cleanup (crpc); diff --git a/xlators/features/changelog/src/changelog-rpc.c b/xlators/features/changelog/src/changelog-rpc.c index 5524e433cbb..72b5cdbc7cb 100644 --- a/xlators/features/changelog/src/changelog-rpc.c +++ b/xlators/features/changelog/src/changelog-rpc.c @@ -211,7 +211,7 @@ changelog_rpc_clnt_init (xlator_t *this, /* Take a ref, the last unref will be on RPC_CLNT_DESTROY * which comes as a result of last rpc_clnt_unref. */ - crpc->ref = 1; + GF_ATOMIC_INIT (crpc->ref, 1); changelog_set_disconnect_flag (crpc, _gf_false); crpc->filter = rpc_req->filter; diff --git a/xlators/nfs/server/src/nfs3.c b/xlators/nfs/server/src/nfs3.c index b053eb3250e..6428a77442e 100644 --- a/xlators/nfs/server/src/nfs3.c +++ b/xlators/nfs/server/src/nfs3.c @@ -539,8 +539,8 @@ static void __nfs3_call_state_wipe (nfs3_call_state_t *cs) { if (cs->fd) { - gf_msg_trace (GF_NFS3, 0, "fd 0x%lx ref: %d", - (long)cs->fd, cs->fd->refcount); + gf_msg_trace (GF_NFS3, 0, "fd 0x%lx ref: %"PRId64, + (long)cs->fd, GF_ATOMIC_GET (cs->fd->refcount)); fd_unref (cs->fd); } diff --git a/xlators/protocol/client/src/client-handshake.c b/xlators/protocol/client/src/client-handshake.c index ca3d140d08e..6429dc2e8ae 100644 --- a/xlators/protocol/client/src/client-handshake.c +++ b/xlators/protocol/client/src/client-handshake.c @@ -32,7 +32,7 @@ int client_set_lk_version_cbk (struct rpc_req *req, struct iovec *iov, int client_set_lk_version (xlator_t *this); typedef struct client_fd_lk_local { - int ref; + gf_atomic_t ref; gf_boolean_t error; gf_lock_t lock; clnt_fd_ctx_t *fdctx; @@ -285,11 +285,7 @@ clnt_fd_lk_local_ref (xlator_t *this, clnt_fd_lk_local_t *local) { GF_VALIDATE_OR_GOTO (this->name, local, out); - LOCK (&local->lock); - { - local->ref++; - } - UNLOCK (&local->lock); + GF_ATOMIC_INC (local->ref); out: return local; } @@ -301,11 +297,7 @@ clnt_fd_lk_local_unref (xlator_t *this, clnt_fd_lk_local_t *local) GF_VALIDATE_OR_GOTO (this->name, local, out); - LOCK (&local->lock); - { - ref = --local->ref; - } - UNLOCK (&local->lock); + ref = GF_ATOMIC_DEC (local->ref); if (ref == 0) { LOCK_DESTROY (&local->lock); @@ -325,7 +317,7 @@ clnt_fd_lk_local_create (clnt_fd_ctx_t *fdctx) if (!local) goto out; - local->ref = 1; + GF_ATOMIC_INIT (local->ref, 1); local->error = _gf_false; local->fdctx = fdctx; diff --git a/xlators/protocol/client/src/client.c b/xlators/protocol/client/src/client.c index 8df697ed883..8ec9eaeb8a9 100644 --- a/xlators/protocol/client/src/client.c +++ b/xlators/protocol/client/src/client.c @@ -2844,7 +2844,7 @@ client_fd_lk_ctx_dump (xlator_t *this, fd_lk_ctx_t *lk_ctx, int nth_fd) fd_lk_ctx_node_t *plock = NULL; char key[GF_DUMP_MAX_BUF_LEN] = {0,}; - lk_ctx_ref = fd_lk_ctx_try_ref (lk_ctx); + lk_ctx_ref = fd_lk_ctx_ref (lk_ctx); if (!lk_ctx_ref) return; -- cgit