path: root/0465-posix-Implement-a-janitor-thread-to-close-fd.patch

From 143b93b230b429cc712353243ed794b68494c040 Mon Sep 17 00:00:00 2001
From: Mohit Agrawal <moagrawa@redhat.com>
Date: Mon, 27 Jul 2020 18:08:00 +0530
Subject: [PATCH 465/465] posix: Implement a janitor thread to close fd

Problem: In the commit fb20713b380e1df8d7f9e9df96563be2f9144fd6 we use
         syntask to close fd but we have found the patch is reducing the
         performance

Solution: Use janitor thread to close fd's and save the pfd ctx into
          ctx janitor list and also save the posix_xlator into pfd object to
          avoid the race condition during cleanup in brick_mux environment

> Change-Id: Ifb3d18a854b267333a3a9e39845bfefb83fbc092
> Fixes: #1396
> Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
> (Reviewed on upstream link https://review.gluster.org/#/c/glusterfs/+/24755/)
> (Cherry pick from commit 41b9616435cbdf671805856e487e373060c9455b

Change-Id: Ifb3d18a854b267333a3a9e39845bfefb83fbc092
BUG: 1851989
Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/209448
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
---
 glusterfsd/src/glusterfsd.c                    |  4 ++
 libglusterfs/src/glusterfs/glusterfs.h         |  7 ++
 rpc/rpc-lib/src/rpcsvc.c                       |  6 --
 xlators/storage/posix/src/posix-common.c       | 34 +++++++++-
 xlators/storage/posix/src/posix-helpers.c      | 93 ++++++++++++++++++++++++++
 xlators/storage/posix/src/posix-inode-fd-ops.c | 33 ++++-----
 xlators/storage/posix/src/posix.h              |  7 ++
 7 files changed, 161 insertions(+), 23 deletions(-)

diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c
index 9821180..955bf1d 100644
--- a/glusterfsd/src/glusterfsd.c
+++ b/glusterfsd/src/glusterfsd.c
@@ -1839,6 +1839,10 @@ glusterfs_ctx_defaults_init(glusterfs_ctx_t *ctx)
 
     INIT_LIST_HEAD(&cmd_args->xlator_options);
     INIT_LIST_HEAD(&cmd_args->volfile_servers);
+    ctx->pxl_count = 0;
+    pthread_mutex_init(&ctx->fd_lock, NULL);
+    pthread_cond_init(&ctx->fd_cond, NULL);
+    INIT_LIST_HEAD(&ctx->janitor_fds);
 
     lim.rlim_cur = RLIM_INFINITY;
     lim.rlim_max = RLIM_INFINITY;
diff --git a/libglusterfs/src/glusterfs/glusterfs.h b/libglusterfs/src/glusterfs/glusterfs.h
index 495a4d7..bf6a987 100644
--- a/libglusterfs/src/glusterfs/glusterfs.h
+++ b/libglusterfs/src/glusterfs/glusterfs.h
@@ -733,6 +733,13 @@ struct _glusterfs_ctx {
     } stats;
 
     struct list_head volfile_list;
+    /* Add members to manage janitor threads for cleanup fd */
+    struct list_head janitor_fds;
+    pthread_cond_t fd_cond;
+    pthread_mutex_t fd_lock;
+    pthread_t janitor;
+    /* The variable is use to save total posix xlator count */
+    uint32_t pxl_count;
 
     char volume_id[GF_UUID_BUF_SIZE]; /* Used only in protocol/client */
 };
diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c
index 23ca1fd..3f184bf 100644
--- a/rpc/rpc-lib/src/rpcsvc.c
+++ b/rpc/rpc-lib/src/rpcsvc.c
@@ -375,12 +375,6 @@ rpcsvc_program_actor(rpcsvc_request_t *req)
 
     req->ownthread = program->ownthread;
     req->synctask = program->synctask;
-    if (((req->procnum == GFS3_OP_RELEASE) ||
-         (req->procnum == GFS3_OP_RELEASEDIR)) &&
-        (program->prognum == GLUSTER_FOP_PROGRAM)) {
-        req->ownthread = _gf_false;
-        req->synctask = _gf_true;
-    }
 
     err = SUCCESS;
     gf_log(GF_RPCSVC, GF_LOG_TRACE, "Actor found: %s - %s for %s",
diff --git a/xlators/storage/posix/src/posix-common.c b/xlators/storage/posix/src/posix-common.c
index b317627..c5a43a1 100644
--- a/xlators/storage/posix/src/posix-common.c
+++ b/xlators/storage/posix/src/posix-common.c
@@ -150,6 +150,7 @@ posix_notify(xlator_t *this, int32_t event, void *data, ...)
     struct timespec sleep_till = {
         0,
     };
+    glusterfs_ctx_t *ctx = this->ctx;
 
     switch (event) {
         case GF_EVENT_PARENT_UP: {
@@ -160,8 +161,6 @@ posix_notify(xlator_t *this, int32_t event, void *data, ...)
         case GF_EVENT_PARENT_DOWN: {
             if (!victim->cleanup_starting)
                 break;
-            gf_log(this->name, GF_LOG_INFO, "Sending CHILD_DOWN for brick %s",
-                   victim->name);
 
             if (priv->janitor) {
                 pthread_mutex_lock(&priv->janitor_mutex);
@@ -187,6 +186,16 @@ posix_notify(xlator_t *this, int32_t event, void *data, ...)
                 GF_FREE(priv->janitor);
             }
             priv->janitor = NULL;
+            pthread_mutex_lock(&ctx->fd_lock);
+            {
+                while (priv->rel_fdcount > 0) {
+                    pthread_cond_wait(&priv->fd_cond, &ctx->fd_lock);
+                }
+            }
+            pthread_mutex_unlock(&ctx->fd_lock);
+
+            gf_log(this->name, GF_LOG_INFO, "Sending CHILD_DOWN for brick %s",
+                   victim->name);
             default_notify(this->parents->xlator, GF_EVENT_CHILD_DOWN, data);
         } break;
         default:
@@ -1038,7 +1047,13 @@ posix_init(xlator_t *this)
     pthread_cond_init(&_private->fsync_cond, NULL);
     pthread_mutex_init(&_private->janitor_mutex, NULL);
     pthread_cond_init(&_private->janitor_cond, NULL);
+    pthread_cond_init(&_private->fd_cond, NULL);
     INIT_LIST_HEAD(&_private->fsyncs);
+    _private->rel_fdcount = 0;
+    ret = posix_spawn_ctx_janitor_thread(this);
+    if (ret)
+        goto out;
+
     ret = gf_thread_create(&_private->fsyncer, NULL, posix_fsyncer, this,
                            "posixfsy");
     if (ret) {
@@ -1133,6 +1148,8 @@ posix_fini(xlator_t *this)
 {
     struct posix_private *priv = this->private;
     gf_boolean_t health_check = _gf_false;
+    glusterfs_ctx_t *ctx = this->ctx;
+    uint32_t count;
     int ret = 0;
 
     if (!priv)
@@ -1166,6 +1183,19 @@ posix_fini(xlator_t *this)
         priv->janitor = NULL;
     }
 
+    pthread_mutex_lock(&ctx->fd_lock);
+    {
+        count = --ctx->pxl_count;
+        if (count == 0) {
+            pthread_cond_signal(&ctx->fd_cond);
+        }
+    }
+    pthread_mutex_unlock(&ctx->fd_lock);
+
+    if (count == 0) {
+        pthread_join(ctx->janitor, NULL);
+    }
+
     if (priv->fsyncer) {
         (void)gf_thread_cleanup_xint(priv->fsyncer);
         priv->fsyncer = 0;
diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c
index 39dbcce..73a44be 100644
--- a/xlators/storage/posix/src/posix-helpers.c
+++ b/xlators/storage/posix/src/posix-helpers.c
@@ -1582,6 +1582,99 @@ unlock:
     return;
 }
 
+static struct posix_fd *
+janitor_get_next_fd(glusterfs_ctx_t *ctx)
+{
+    struct posix_fd *pfd = NULL;
+
+    while (list_empty(&ctx->janitor_fds)) {
+        if (ctx->pxl_count == 0) {
+            return NULL;
+        }
+
+        pthread_cond_wait(&ctx->fd_cond, &ctx->fd_lock);
+    }
+
+    pfd = list_first_entry(&ctx->janitor_fds, struct posix_fd, list);
+    list_del_init(&pfd->list);
+
+    return pfd;
+}
+
+static void
+posix_close_pfd(xlator_t *xl, struct posix_fd *pfd)
+{
+    THIS = xl;
+
+    if (pfd->dir == NULL) {
+        gf_msg_trace(xl->name, 0, "janitor: closing file fd=%d", pfd->fd);
+        sys_close(pfd->fd);
+    } else {
+        gf_msg_debug(xl->name, 0, "janitor: closing dir fd=%p", pfd->dir);
+        sys_closedir(pfd->dir);
+    }
+
+    GF_FREE(pfd);
+}
+
+static void *
+posix_ctx_janitor_thread_proc(void *data)
+{
+    xlator_t *xl;
+    struct posix_fd *pfd;
+    glusterfs_ctx_t *ctx = NULL;
+    struct posix_private *priv_fd;
+
+    ctx = data;
+
+    pthread_mutex_lock(&ctx->fd_lock);
+
+    while ((pfd = janitor_get_next_fd(ctx)) != NULL) {
+        pthread_mutex_unlock(&ctx->fd_lock);
+
+        xl = pfd->xl;
+        posix_close_pfd(xl, pfd);
+
+        pthread_mutex_lock(&ctx->fd_lock);
+
+        priv_fd = xl->private;
+        priv_fd->rel_fdcount--;
+        if (!priv_fd->rel_fdcount)
+            pthread_cond_signal(&priv_fd->fd_cond);
+    }
+
+    pthread_mutex_unlock(&ctx->fd_lock);
+
+    return NULL;
+}
+
+int
+posix_spawn_ctx_janitor_thread(xlator_t *this)
+{
+    int ret = 0;
+    glusterfs_ctx_t *ctx = NULL;
+
+    ctx = this->ctx;
+
+    pthread_mutex_lock(&ctx->fd_lock);
+    {
+        if (ctx->pxl_count++ == 0) {
+            ret = gf_thread_create(&ctx->janitor, NULL,
+                                   posix_ctx_janitor_thread_proc, ctx,
+                                   "posixctxjan");
+
+            if (ret) {
+                gf_msg(this->name, GF_LOG_ERROR, errno, P_MSG_THREAD_FAILED,
+                       "spawning janitor thread failed");
+                ctx->pxl_count--;
+            }
+        }
+    }
+    pthread_mutex_unlock(&ctx->fd_lock);
+
+    return ret;
+}
+
 static int
 is_fresh_file(int64_t ctime_sec)
 {
diff --git a/xlators/storage/posix/src/posix-inode-fd-ops.c b/xlators/storage/posix/src/posix-inode-fd-ops.c
index 81f4a6b..21119ea 100644
--- a/xlators/storage/posix/src/posix-inode-fd-ops.c
+++ b/xlators/storage/posix/src/posix-inode-fd-ops.c
@@ -1352,6 +1352,22 @@ out:
     return 0;
 }
 
+static void
+posix_add_fd_to_cleanup(xlator_t *this, struct posix_fd *pfd)
+{
+    glusterfs_ctx_t *ctx = this->ctx;
+    struct posix_private *priv = this->private;
+
+    pfd->xl = this;
+    pthread_mutex_lock(&ctx->fd_lock);
+    {
+        list_add_tail(&pfd->list, &ctx->janitor_fds);
+        priv->rel_fdcount++;
+        pthread_cond_signal(&ctx->fd_cond);
+    }
+    pthread_mutex_unlock(&ctx->fd_lock);
+}
+
 int32_t
 posix_releasedir(xlator_t *this, fd_t *fd)
 {
@@ -1374,11 +1390,7 @@ posix_releasedir(xlator_t *this, fd_t *fd)
                "pfd->dir is NULL for fd=%p", fd);
         goto out;
     }
-
-    gf_msg_debug(this->name, 0, "janitor: closing dir fd=%p", pfd->dir);
-
-    sys_closedir(pfd->dir);
-    GF_FREE(pfd);
+    posix_add_fd_to_cleanup(this, pfd);
 
 out:
     return 0;
@@ -2494,7 +2506,6 @@ out:
 int32_t
 posix_release(xlator_t *this, fd_t *fd)
 {
-    struct posix_private *priv = NULL;
     struct posix_fd *pfd = NULL;
     int ret = -1;
     uint64_t tmp_pfd = 0;
@@ -2502,8 +2513,6 @@ posix_release(xlator_t *this, fd_t *fd)
     VALIDATE_OR_GOTO(this, out);
     VALIDATE_OR_GOTO(fd, out);
 
-    priv = this->private;
-
     ret = fd_ctx_del(fd, this, &tmp_pfd);
     if (ret < 0) {
         gf_msg(this->name, GF_LOG_WARNING, 0, P_MSG_PFD_NULL,
@@ -2517,13 +2526,7 @@ posix_release(xlator_t *this, fd_t *fd)
                "pfd->dir is %p (not NULL) for file fd=%p", pfd->dir, fd);
     }
 
-    gf_msg_debug(this->name, 0, "janitor: closing dir fd=%p", pfd->dir);
-
-    sys_close(pfd->fd);
-    GF_FREE(pfd);
-
-    if (!priv)
-        goto out;
+    posix_add_fd_to_cleanup(this, pfd);
 
 out:
     return 0;
diff --git a/xlators/storage/posix/src/posix.h b/xlators/storage/posix/src/posix.h
index 124dbb4..07f367b 100644
--- a/xlators/storage/posix/src/posix.h
+++ b/xlators/storage/posix/src/posix.h
@@ -134,6 +134,8 @@ struct posix_fd {
     off_t dir_eof; /* offset at dir EOF */
     int odirect;
     struct list_head list; /* to add to the janitor list */
+    xlator_t *xl;
+    char _pad[4]; /* manual padding */
 };
 
 struct posix_private {
@@ -204,6 +206,7 @@ struct posix_private {
     pthread_cond_t fsync_cond;
     pthread_mutex_t janitor_mutex;
     pthread_cond_t janitor_cond;
+    pthread_cond_t fd_cond;
     int fsync_queue_count;
 
     enum {
@@ -259,6 +262,7 @@ struct posix_private {
     gf_boolean_t fips_mode_rchecksum;
     gf_boolean_t ctime;
     gf_boolean_t janitor_task_stop;
+    uint32_t rel_fdcount;
 };
 
 typedef struct {
@@ -665,6 +669,9 @@ posix_cs_maintenance(xlator_t *this, fd_t *fd, loc_t *loc, int *pfd,
 int
 posix_check_dev_file(xlator_t *this, inode_t *inode, char *fop, int *op_errno);
 
+int
+posix_spawn_ctx_janitor_thread(xlator_t *this);
+
 void
 posix_update_iatt_buf(struct iatt *buf, int fd, char *loc, dict_t *xdata);
 
-- 
1.8.3.1