protocol/client: reduce lock contention

Current use of a per-client mutex to protect fdctx introduces lock
contentions when there are dozens of file operations active.

Use finer grain spinlock to reduce contention, and put retrieving
fdctx out of lock.

Change-Id: Iea3e2eb481e76a5d73a582ba81529180c5b88248
BUG: 1519598
Signed-off-by: Zhang Huan <zhanghuan@open-fs.com>

6 files changed, 53 insertions, 58 deletions

diff --git a/xlators/protocol/client/src/client-handshake.c b/xlators/protocol/client/src/client-handshake.c
index 6429dc2e8ae..68ea20ffed7 100644
--- a/xlators/protocol/client/src/client-handshake.c
+++ b/xlators/protocol/client/src/client-handshake.c
@@ -164,12 +164,12 @@ clnt_fd_lk_reacquire_failed (xlator_t *this, clnt_fd_ctx_t *fdctx,
         GF_VALIDATE_OR_GOTO (this->name, conf, out);
         GF_VALIDATE_OR_GOTO (this->name, fdctx, out);
 
-        pthread_mutex_lock (&conf->lock);
+        pthread_spin_lock (&conf->fd_lock);
         {
                 fdctx->remote_fd     = -1;
                 fdctx->lk_heal_state = GF_LK_HEAL_DONE;
         }
-        pthread_mutex_unlock (&conf->lock);
+        pthread_spin_unlock (&conf->fd_lock);
 
         ret = 0;
 out:
@@ -488,11 +488,11 @@ client_reacquire_lock_cbk (struct rpc_req *req, struct iovec *iov,
 
         if (!clnt_fd_lk_local_error_status (this, local) &&
             clnt_fd_lk_local_unref (this, local) == 0) {
-                pthread_mutex_lock (&conf->lock);
+                pthread_spin_lock (&conf->fd_lock);
                 {
                         fdctx->lk_heal_state = GF_LK_HEAL_DONE;
                 }
-                pthread_mutex_unlock (&conf->lock);
+                pthread_spin_unlock (&conf->fd_lock);
 
                 fdctx->reopen_done (fdctx, fdctx->remote_fd, this);
         }
@@ -634,7 +634,7 @@ client_reopen_done (clnt_fd_ctx_t *fdctx, int64_t rfd, xlator_t *this)
 
         conf = this->private;
 
-        pthread_mutex_lock (&conf->lock);
+        pthread_spin_lock (&conf->fd_lock);
         {
                 fdctx->remote_fd = rfd;
                 fdctx->reopen_attempts = 0;
@@ -644,7 +644,7 @@ client_reopen_done (clnt_fd_ctx_t *fdctx, int64_t rfd, xlator_t *this)
                 else
                         destroy = _gf_true;
         }
-        pthread_mutex_unlock (&conf->lock);
+        pthread_spin_unlock (&conf->fd_lock);
 
         if (destroy)
                 client_fdctx_destroy (this, fdctx);
@@ -726,7 +726,7 @@ client3_3_reopen_cbk (struct rpc_req *req, struct iovec *iov, int count,
                 goto out;
         }
 
-        pthread_mutex_lock (&conf->lock);
+        pthread_spin_lock (&conf->fd_lock);
         {
                 if (!fdctx->released) {
                         if (conf->lk_heal &&
@@ -737,7 +737,7 @@ client3_3_reopen_cbk (struct rpc_req *req, struct iovec *iov, int count,
                         }
                 }
         }
-        pthread_mutex_unlock (&conf->lock);
+        pthread_spin_unlock (&conf->fd_lock);
 
         ret = 0;
 
@@ -975,11 +975,12 @@ client_attempt_reopen (fd_t *fd, xlator_t *this)
                 goto out;
 
         conf = this->private;
-        pthread_mutex_lock (&conf->lock);
+
+        fdctx = this_fd_get_ctx (fd, this);
+        if (!fdctx)
+                goto out;
+        pthread_spin_lock (&conf->fd_lock);
         {
-                fdctx = this_fd_get_ctx (fd, this);
-                if (!fdctx)
-                        goto unlock;
                 if (__is_fd_reopen_in_progress (fdctx))
                         goto unlock;
                 if (fdctx->remote_fd != -1)
@@ -994,7 +995,7 @@ client_attempt_reopen (fd_t *fd, xlator_t *this)
                 }
         }
 unlock:
-        pthread_mutex_unlock (&conf->lock);
+        pthread_spin_unlock (&conf->fd_lock);
         if (reopen)
                 protocol_client_reopen (fdctx, this);
 out:
@@ -1017,7 +1018,7 @@ client_post_handshake (call_frame_t *frame, xlator_t *this)
         conf = this->private;
         INIT_LIST_HEAD (&reopen_head);
 
-        pthread_mutex_lock (&conf->lock);
+        pthread_spin_lock (&conf->fd_lock);
         {
                 list_for_each_entry_safe (fdctx, tmp, &conf->saved_fds,
                                           sfd_pos) {
@@ -1030,7 +1031,7 @@ client_post_handshake (call_frame_t *frame, xlator_t *this)
                         count++;
                 }
         }
-        pthread_mutex_unlock (&conf->lock);
+        pthread_spin_unlock (&conf->fd_lock);
 
         /* Delay notifying CHILD_UP to parents
            until all locks are recovered */
diff --git a/xlators/protocol/client/src/client-helpers.c b/xlators/protocol/client/src/client-helpers.c
index f9d5c2c8b12..c078b88b84d 100644
--- a/xlators/protocol/client/src/client-helpers.c
+++ b/xlators/protocol/client/src/client-helpers.c
@@ -299,18 +299,20 @@ client_get_remote_fd (xlator_t *this, fd_t *fd, int flags, int64_t *remote_fd)
         GF_VALIDATE_OR_GOTO (this->name, fd, out);
         GF_VALIDATE_OR_GOTO (this->name, remote_fd, out);
 
-        conf = this->private;
-        pthread_mutex_lock (&conf->lock);
-        {
-                fdctx = this_fd_get_ctx (fd, this);
-                if (!fdctx)
-                        *remote_fd = GF_ANON_FD_NO;
-                else if (__is_fd_reopen_in_progress (fdctx))
-                        *remote_fd = -1;
-                else
-                        *remote_fd = fdctx->remote_fd;
+        fdctx = this_fd_get_ctx (fd, this);
+        if (!fdctx) {
+                *remote_fd = GF_ANON_FD_NO;
+        } else {
+                conf = this->private;
+                pthread_spin_lock (&conf->fd_lock);
+                {
+                        if (__is_fd_reopen_in_progress (fdctx))
+                                *remote_fd = -1;
+                        else
+                                *remote_fd = fdctx->remote_fd;
+                }
+                pthread_spin_unlock (&conf->fd_lock);
         }
-        pthread_mutex_unlock (&conf->lock);
 
         if ((flags & FALLBACK_TO_ANON_FD) && (*remote_fd == -1))
                 *remote_fd = GF_ANON_FD_NO;
diff --git a/xlators/protocol/client/src/client-lk.c b/xlators/protocol/client/src/client-lk.c
index 0cf2be3c562..6468fb4511e 100644
--- a/xlators/protocol/client/src/client-lk.c
+++ b/xlators/protocol/client/src/client-lk.c
@@ -60,7 +60,6 @@ int
 dump_client_locks (inode_t *inode)
 {
         fd_t             *fd    = NULL;
-        clnt_conf_t    *conf  = NULL;
         xlator_t         *this  = NULL;
         clnt_fd_ctx_t  *fdctx = NULL;
 
@@ -68,19 +67,13 @@ dump_client_locks (inode_t *inode)
         int locks_fd_count   = 0;
 
         this = THIS;
-        conf = this->private;
 
         LOCK (&inode->lock);
         {
                 list_for_each_entry (fd, &inode->fd_list, inode_list) {
                         locks_fd_count = 0;
 
-                        pthread_mutex_lock (&conf->lock);
-                        {
-                                fdctx = this_fd_get_ctx (fd, this);
-                        }
-                        pthread_mutex_unlock (&conf->lock);
-
+                        fdctx = this_fd_get_ctx (fd, this);
                         if (fdctx)
                                 locks_fd_count = dump_client_locks_fd (fdctx);
 
@@ -504,19 +497,12 @@ client_add_lock_for_recovery (fd_t *fd, struct gf_flock *flock,
         clnt_fd_ctx_t       *fdctx = NULL;
         xlator_t            *this  = NULL;
         client_posix_lock_t *lock  = NULL;
-        clnt_conf_t         *conf  = NULL;
 
         int ret = 0;
 
         this = THIS;
-        conf = this->private;
-
-        pthread_mutex_lock (&conf->lock);
-        {
-                fdctx = this_fd_get_ctx (fd, this);
-        }
-        pthread_mutex_unlock (&conf->lock);
 
+        fdctx = this_fd_get_ctx (fd, this);
         if (!fdctx) {
                 gf_msg (this->name, GF_LOG_WARNING, 0, PC_MSG_FD_GET_FAIL,
                         "failed to get fd context. sending EBADFD");
diff --git a/xlators/protocol/client/src/client-rpc-fops.c b/xlators/protocol/client/src/client-rpc-fops.c
index 30e0699d446..c8d1cc14f77 100644
--- a/xlators/protocol/client/src/client-rpc-fops.c
+++ b/xlators/protocol/client/src/client-rpc-fops.c
@@ -371,11 +371,11 @@ client_add_fd_to_saved_fds (xlator_t *this, fd_t *fd, loc_t *loc, int32_t flags,
 
         this_fd_set_ctx (fd, this, loc, fdctx);
 
-        pthread_mutex_lock (&conf->lock);
+        pthread_spin_lock (&conf->fd_lock);
         {
                 list_add_tail (&fdctx->sfd_pos, &conf->saved_fds);
         }
-        pthread_mutex_unlock (&conf->lock);
+        pthread_spin_unlock (&conf->fd_lock);
 out:
         return ret;
 }
@@ -3268,10 +3268,10 @@ client_fdctx_destroy (xlator_t *this, clnt_fd_ctx_t *fdctx)
         pthread_mutex_lock (&conf->lock);
         {
                 parent_down   = conf->parent_down;
-                lk_ctx        = fdctx->lk_ctx;
-                fdctx->lk_ctx = NULL;
         }
         pthread_mutex_unlock (&conf->lock);
+        lk_ctx        = fdctx->lk_ctx;
+        fdctx->lk_ctx = NULL;
 
         if (lk_ctx)
                 fd_lk_ctx_unref (lk_ctx);
@@ -3336,10 +3336,10 @@ client3_3_releasedir (call_frame_t *frame, xlator_t *this,
         args = data;
         conf = this->private;
 
-        pthread_mutex_lock (&conf->lock);
-        {
-                fdctx = this_fd_del_ctx (args->fd, this);
-                if (fdctx != NULL) {
+        fdctx = this_fd_del_ctx (args->fd, this);
+        if (fdctx != NULL) {
+                pthread_spin_lock (&conf->fd_lock);
+                {
                         remote_fd = fdctx->remote_fd;
 
                         /* fdctx->remote_fd == -1 indicates a reopen attempt
@@ -3354,8 +3354,8 @@ client3_3_releasedir (call_frame_t *frame, xlator_t *this,
                                 destroy = _gf_true;
                         }
                 }
+                pthread_spin_unlock (&conf->fd_lock);
         }
-        pthread_mutex_unlock (&conf->lock);
 
         if (destroy)
                 client_fdctx_destroy (this, fdctx);
@@ -3382,10 +3382,10 @@ client3_3_release (call_frame_t *frame, xlator_t *this,
         args = data;
         conf = this->private;
 
-        pthread_mutex_lock (&conf->lock);
-        {
-                fdctx = this_fd_del_ctx (args->fd, this);
-                if (fdctx != NULL) {
+        fdctx = this_fd_del_ctx (args->fd, this);
+        if (fdctx != NULL) {
+                pthread_spin_lock (&conf->fd_lock);
+                {
                         remote_fd     = fdctx->remote_fd;
                         lk_heal_state = fdctx->lk_heal_state;
 
@@ -3400,8 +3400,8 @@ client3_3_release (call_frame_t *frame, xlator_t *this,
                                 destroy = _gf_true;
                         }
                 }
+                pthread_spin_unlock (&conf->fd_lock);
         }
-        pthread_mutex_unlock (&conf->lock);
 
         if (destroy)
                 client_fdctx_destroy (this, fdctx);
diff --git a/xlators/protocol/client/src/client.c b/xlators/protocol/client/src/client.c
index 8ec9eaeb8a9..955db86e1c2 100644
--- a/xlators/protocol/client/src/client.c
+++ b/xlators/protocol/client/src/client.c
@@ -49,6 +49,7 @@ client_fini_complete (xlator_t *this)
 
         this->private = NULL;
 
+        pthread_spin_destroy (&conf->fd_lock);
         pthread_mutex_destroy (&conf->lock);
         GF_FREE (conf);
 
@@ -2243,14 +2244,14 @@ client_mark_fd_bad (xlator_t *this)
 
         conf = this->private;
 
-        pthread_mutex_lock (&conf->lock);
+        pthread_spin_lock (&conf->fd_lock);
         {
                 list_for_each_entry_safe (fdctx, tmp, &conf->saved_fds,
                                           sfd_pos) {
                         fdctx->remote_fd = -1;
                 }
         }
-        pthread_mutex_unlock (&conf->lock);
+        pthread_spin_unlock (&conf->fd_lock);
 
         return 0;
 }
@@ -2755,6 +2756,7 @@ init (xlator_t *this)
                 goto out;
 
         pthread_mutex_init (&conf->lock, NULL);
+        pthread_spin_init (&conf->fd_lock, 0);
         INIT_LIST_HEAD (&conf->saved_fds);
 
         conf->child_up = _gf_false;
@@ -2909,12 +2911,14 @@ client_priv_dump (xlator_t *this)
 
         gf_proc_dump_add_section(key_prefix);
 
+        pthread_spin_lock (&conf->fd_lock);
         list_for_each_entry(tmp, &conf->saved_fds, sfd_pos) {
                 sprintf (key, "fd.%d.remote_fd", i);
                 gf_proc_dump_write(key, "%d", tmp->remote_fd);
                 client_fd_lk_ctx_dump (this, tmp->lk_ctx, i);
                 i++;
         }
+        pthread_spin_unlock (&conf->fd_lock);
 
         gf_proc_dump_write("connecting", "%d", conf->connecting);
 
diff --git a/xlators/protocol/client/src/client.h b/xlators/protocol/client/src/client.h
index c025b9812b7..dd3ee4dd4f0 100644
--- a/xlators/protocol/client/src/client.h
+++ b/xlators/protocol/client/src/client.h
@@ -145,6 +145,8 @@ typedef struct clnt_conf {
         struct clnt_options    opt;
         struct rpc_clnt_config rpc_conf;
 	struct list_head       saved_fds;
+        pthread_spinlock_t     fd_lock; /* protects saved_fds list
+                                         * and all fdctx */
         pthread_mutex_t        lock;
         int                    connecting;
         int                    connected;