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 +- 3 files changed, 14 insertions(+), 37 deletions(-) (limited to 'xlators/cluster/dht/src') 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); -- cgit