From 143b93b230b429cc712353243ed794b68494c040 Mon Sep 17 00:00:00 2001 From: Mohit Agrawal 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 > (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 Reviewed-on: https://code.engineering.redhat.com/gerrit/209448 Tested-by: RHGS Build Bot Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- 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