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 --- libglusterfs/src/dict.c | 31 +++++----------------- libglusterfs/src/dict.h | 6 ++--- libglusterfs/src/event-epoll.c | 12 +++------ libglusterfs/src/fd-lk.c | 59 +++++------------------------------------- libglusterfs/src/fd-lk.h | 7 +---- libglusterfs/src/fd.c | 24 +++-------------- libglusterfs/src/fd.h | 2 +- libglusterfs/src/iobuf.c | 52 +++++-------------------------------- libglusterfs/src/iobuf.h | 4 +-- 9 files changed, 35 insertions(+), 162 deletions(-) (limited to 'libglusterfs') diff --git a/libglusterfs/src/dict.c b/libglusterfs/src/dict.c index a47c8529709..d0be267d2ee 100644 --- a/libglusterfs/src/dict.c +++ b/libglusterfs/src/dict.c @@ -579,8 +579,8 @@ dict_destroy (dict_t *this) data_pair_t *pair = this->members_list; data_pair_t *prev = this->members_list; glusterfs_ctx_t *ctx = NULL; - uint32_t total_pairs = 0; uint64_t current_max = 0; + uint32_t total_pairs = 0; LOCK_DESTROY (&this->lock); @@ -629,7 +629,7 @@ dict_destroy (dict_t *this) void dict_unref (dict_t *this) { - int32_t ref; + uint64_t ref = 0; if (!this) { gf_msg_callingfn ("dict", GF_LOG_WARNING, EINVAL, @@ -637,12 +637,7 @@ dict_unref (dict_t *this) return; } - LOCK (&this->lock); - - this->refcount--; - ref = this->refcount; - - UNLOCK (&this->lock); + ref = GF_ATOMIC_DEC (this->refcount); if (!ref) dict_destroy (this); @@ -657,12 +652,7 @@ dict_ref (dict_t *this) return NULL; } - LOCK (&this->lock); - - this->refcount++; - - UNLOCK (&this->lock); - + GF_ATOMIC_INC (this->refcount); return this; } @@ -678,12 +668,7 @@ data_unref (data_t *this) return; } - LOCK (&this->lock); - - this->refcount--; - ref = this->refcount; - - UNLOCK (&this->lock); + ref = GF_ATOMIC_DEC (this->refcount); if (!ref) data_destroy (this); @@ -698,11 +683,7 @@ data_ref (data_t *this) return NULL; } - LOCK (&this->lock); - - this->refcount++; - - UNLOCK (&this->lock); + GF_ATOMIC_INC (this->refcount); return this; } diff --git a/libglusterfs/src/dict.h b/libglusterfs/src/dict.h index 713da61d4ce..48c6f242723 100644 --- a/libglusterfs/src/dict.h +++ b/libglusterfs/src/dict.h @@ -67,7 +67,7 @@ struct _data { unsigned char is_const:1; int32_t len; char *data; - int32_t refcount; + gf_atomic_t refcount; gf_lock_t lock; }; @@ -84,7 +84,7 @@ struct _dict { unsigned char is_static:1; int32_t hash_size; int32_t count; - int32_t refcount; + gf_atomic_t refcount; data_pair_t **members; data_pair_t *members_list; char *extra_free; @@ -93,7 +93,7 @@ struct _dict { data_pair_t *members_internal; data_pair_t free_pair; gf_boolean_t free_pair_in_use; - uint32_t max_count; + uint64_t max_count; }; typedef gf_boolean_t (*dict_match_t) (dict_t *d, char *k, data_t *v, diff --git a/libglusterfs/src/event-epoll.c b/libglusterfs/src/event-epoll.c index 7fc53ffeaf6..45f724ab055 100644 --- a/libglusterfs/src/event-epoll.c +++ b/libglusterfs/src/event-epoll.c @@ -32,7 +32,7 @@ struct event_slot_epoll { int fd; int events; int gen; - int ref; + gf_atomic_t ref; int do_close; int in_handler; int handled_error; @@ -197,12 +197,7 @@ event_slot_get (struct event_pool *event_pool, int idx) return NULL; slot = &table[offset]; - - LOCK (&slot->lock); - { - slot->ref++; - } - UNLOCK (&slot->lock); + GF_ATOMIC_INC (slot->ref); return slot; } @@ -216,9 +211,10 @@ event_slot_unref (struct event_pool *event_pool, struct event_slot_epoll *slot, int fd = -1; int do_close = 0; + ref = GF_ATOMIC_DEC (slot->ref); + LOCK (&slot->lock); { - ref = --slot->ref; fd = slot->fd; do_close = slot->do_close; } diff --git a/libglusterfs/src/fd-lk.c b/libglusterfs/src/fd-lk.c index 358cf3b616a..e516f64bff6 100644 --- a/libglusterfs/src/fd-lk.c +++ b/libglusterfs/src/fd-lk.c @@ -65,15 +65,11 @@ fd_lk_ctx_unref (fd_lk_ctx_t *lk_ctx) GF_VALIDATE_OR_GOTO ("fd-lk", lk_ctx, err); - LOCK (&lk_ctx->lock); - { - ref = --lk_ctx->ref; - if (ref < 0) - GF_ASSERT (!ref); - if (ref == 0) - _fd_lk_destroy_lock_list (lk_ctx); - } - UNLOCK (&lk_ctx->lock); + ref = GF_ATOMIC_DEC (lk_ctx->ref); + if (ref < 0) + GF_ASSERT (!ref); + if (ref == 0) + _fd_lk_destroy_lock_list (lk_ctx); if (ref == 0) { LOCK_DESTROY (&lk_ctx->lock); @@ -85,59 +81,18 @@ err: return -1; } -fd_lk_ctx_t * -_fd_lk_ctx_ref (fd_lk_ctx_t *lk_ctx) -{ - if (!lk_ctx) { - gf_msg_callingfn ("fd-lk", GF_LOG_WARNING, EINVAL, - LG_MSG_INVALID_ARG, "invalid argument"); - return NULL; - } - - ++lk_ctx->ref; - - return lk_ctx; -} - fd_lk_ctx_t * fd_lk_ctx_ref (fd_lk_ctx_t *lk_ctx) { - fd_lk_ctx_t *new_lk_ctx = NULL; - if (!lk_ctx) { gf_msg_callingfn ("fd-lk", GF_LOG_WARNING, EINVAL, LG_MSG_INVALID_ARG, "invalid argument"); return NULL; } - LOCK (&lk_ctx->lock); - { - new_lk_ctx = _fd_lk_ctx_ref (lk_ctx); - } - UNLOCK (&lk_ctx->lock); - - return new_lk_ctx; -} - -fd_lk_ctx_t * -fd_lk_ctx_try_ref (fd_lk_ctx_t *lk_ctx) -{ - int ret = -1; - fd_lk_ctx_t *new_lk_ctx = NULL; - - if (!lk_ctx) { - goto out; - } - - ret = TRY_LOCK (&lk_ctx->lock); - if (ret) - goto out; + GF_ATOMIC_INC (lk_ctx->ref); - new_lk_ctx = _fd_lk_ctx_ref (lk_ctx); - UNLOCK (&lk_ctx->lock); - -out: - return new_lk_ctx; + return lk_ctx; } fd_lk_ctx_t * diff --git a/libglusterfs/src/fd-lk.h b/libglusterfs/src/fd-lk.h index 51f62991681..11d864dc207 100644 --- a/libglusterfs/src/fd-lk.h +++ b/libglusterfs/src/fd-lk.h @@ -30,7 +30,7 @@ struct _fd; struct fd_lk_ctx { struct list_head lk_list; - int ref; + gf_atomic_t ref; gf_lock_t lock; }; typedef struct fd_lk_ctx fd_lk_ctx_t; @@ -45,15 +45,10 @@ struct fd_lk_ctx_node { }; typedef struct fd_lk_ctx_node fd_lk_ctx_node_t; -fd_lk_ctx_t * -_fd_lk_ctx_ref (fd_lk_ctx_t *lk_ctx); fd_lk_ctx_t * fd_lk_ctx_ref (fd_lk_ctx_t *lk_ctx); -fd_lk_ctx_t * -fd_lk_ctx_try_ref (fd_lk_ctx_t *lk_ctx); - fd_lk_ctx_t * fd_lk_ctx_create (void); diff --git a/libglusterfs/src/fd.c b/libglusterfs/src/fd.c index a8a88b0f96f..c6b1a4de611 100644 --- a/libglusterfs/src/fd.c +++ b/libglusterfs/src/fd.c @@ -435,7 +435,7 @@ gf_fd_fdptr_get (fdtable_t *fdtable, int64_t fd) fd_t * __fd_ref (fd_t *fd) { - ++fd->refcount; + GF_ATOMIC_INC (fd->refcount); return fd; } @@ -444,28 +444,13 @@ __fd_ref (fd_t *fd) fd_t * fd_ref (fd_t *fd) { - fd_t *refed_fd = NULL; - if (!fd) { gf_msg_callingfn ("fd", GF_LOG_ERROR, EINVAL, LG_MSG_INVALID_ARG, "null fd"); return NULL; } - LOCK (&fd->inode->lock); - refed_fd = __fd_ref (fd); - UNLOCK (&fd->inode->lock); - - return refed_fd; -} - - -fd_t * -__fd_unref (fd_t *fd) -{ - GF_ASSERT (fd->refcount); - - --fd->refcount; + GF_ATOMIC_INC (fd->refcount); return fd; } @@ -551,8 +536,7 @@ fd_unref (fd_t *fd) LOCK (&fd->inode->lock); { - __fd_unref (fd); - refcount = fd->refcount; + refcount = GF_ATOMIC_DEC (fd->refcount); if (refcount == 0) { if (!list_empty (&fd->inode_list)) { list_del_init (&fd->inode_list); @@ -1186,7 +1170,7 @@ fdentry_dump_to_dict (fdentry_t *fdentry, char *prefix, dict_t *dict, memset (key, 0, sizeof (key)); snprintf (key, sizeof (key), "%s.refcount", prefix); - ret = dict_set_int32 (dict, key, fdentry->fd->refcount); + ret = dict_set_int32 (dict, key, GF_ATOMIC_GET (fdentry->fd->refcount)); if (ret) return; diff --git a/libglusterfs/src/fd.h b/libglusterfs/src/fd.h index 3b4acf762ae..3f68b1c4997 100644 --- a/libglusterfs/src/fd.h +++ b/libglusterfs/src/fd.h @@ -40,7 +40,7 @@ struct _fd_ctx { struct _fd { uint64_t pid; int32_t flags; - int32_t refcount; + gf_atomic_t refcount; struct list_head inode_list; struct _inode *inode; gf_lock_t lock; /* used ONLY for manipulating diff --git a/libglusterfs/src/iobuf.c b/libglusterfs/src/iobuf.c index 76584fc9cde..d4f0d89c338 100644 --- a/libglusterfs/src/iobuf.c +++ b/libglusterfs/src/iobuf.c @@ -126,7 +126,7 @@ __iobuf_arena_destroy_iobufs (struct iobuf_arena *iobuf_arena) iobuf = iobuf_arena->iobufs; for (i = 0; i < iobuf_cnt; i++) { - GF_ASSERT (iobuf->ref == 0); + GF_ASSERT (GF_ATOMIC_GET(iobuf->ref) == 0); LOCK_DESTROY (&iobuf->lock); list_del_init (&iobuf->list); @@ -528,23 +528,6 @@ out: } -struct iobuf * -__iobuf_ref (struct iobuf *iobuf) -{ - iobuf->ref++; - - return iobuf; -} - - -struct iobuf * -__iobuf_unref (struct iobuf *iobuf) -{ - iobuf->ref--; - - return iobuf; -} - struct iobuf * __iobuf_get (struct iobuf_arena *iobuf_arena, size_t page_size) { @@ -619,7 +602,7 @@ iobuf_get_from_stdalloc (struct iobuf_pool *iobuf_pool, size_t page_size) LOCK_INIT (&iobuf->lock); /* Hold a ref because you are allocating and using it */ - iobuf->ref = 1; + GF_ATOMIC_INIT (iobuf->ref, 1); ret = 0; out: @@ -835,12 +818,7 @@ iobuf_unref (struct iobuf *iobuf) GF_VALIDATE_OR_GOTO ("iobuf", iobuf, out); - LOCK (&iobuf->lock); - { - __iobuf_unref (iobuf); - ref = iobuf->ref; - } - UNLOCK (&iobuf->lock); + ref = GF_ATOMIC_DEC (iobuf->ref); if (!ref) iobuf_put (iobuf); @@ -854,12 +832,7 @@ struct iobuf * iobuf_ref (struct iobuf *iobuf) { GF_VALIDATE_OR_GOTO ("iobuf", iobuf, out); - - LOCK (&iobuf->lock); - { - __iobuf_ref (iobuf); - } - UNLOCK (&iobuf->lock); + GF_ATOMIC_INC (iobuf->ref); out: return iobuf; @@ -888,8 +861,7 @@ iobref_new () LOCK_INIT (&iobref->lock); - iobref->ref++; - + GF_ATOMIC_INIT (iobref->ref, 1); return iobref; } @@ -898,12 +870,7 @@ struct iobref * iobref_ref (struct iobref *iobref) { GF_VALIDATE_OR_GOTO ("iobuf", iobref, out); - - LOCK (&iobref->lock); - { - iobref->ref++; - } - UNLOCK (&iobref->lock); + GF_ATOMIC_INC (iobref->ref); out: return iobref; @@ -940,12 +907,7 @@ iobref_unref (struct iobref *iobref) int ref = 0; GF_VALIDATE_OR_GOTO ("iobuf", iobref, out); - - LOCK (&iobref->lock); - { - ref = (--iobref->ref); - } - UNLOCK (&iobref->lock); + ref = GF_ATOMIC_DEC (iobref->ref); if (!ref) iobref_destroy (iobref); diff --git a/libglusterfs/src/iobuf.h b/libglusterfs/src/iobuf.h index 03aa954bbdd..830904c2fe9 100644 --- a/libglusterfs/src/iobuf.h +++ b/libglusterfs/src/iobuf.h @@ -65,7 +65,7 @@ struct iobuf { struct iobuf_arena *iobuf_arena; gf_lock_t lock; /* for ->ptr and ->ref */ - int ref; /* 0 == passive, >0 == active */ + gf_atomic_t ref; /* 0 == passive, >0 == active */ void *ptr; /* usable memory region by the consumer */ @@ -150,7 +150,7 @@ void iobuf_to_iovec(struct iobuf *iob, struct iovec *iov); struct iobref { gf_lock_t lock; - int ref; + gf_atomic_t ref; struct iobuf **iobrefs; int alloced; int used; -- cgit