diff options
Diffstat (limited to '0448-Posix-Use-simple-approach-to-close-fd.patch')
| -rw-r--r-- | 0448-Posix-Use-simple-approach-to-close-fd.patch | 341 |
1 files changed, 341 insertions, 0 deletions
diff --git a/0448-Posix-Use-simple-approach-to-close-fd.patch b/0448-Posix-Use-simple-approach-to-close-fd.patch new file mode 100644 index 0000000..f030358 --- /dev/null +++ b/0448-Posix-Use-simple-approach-to-close-fd.patch @@ -0,0 +1,341 @@ +From 175c99dccc47d2b4267a8819404e5cbeb8cfba11 Mon Sep 17 00:00:00 2001 +From: Mohit Agrawal <moagrawal@redhat.com> +Date: Thu, 12 Mar 2020 21:12:13 +0530 +Subject: [PATCH 448/449] Posix: Use simple approach to close fd + +Problem: posix_release(dir) functions add the fd's into a ctx->janitor_fds + and janitor thread closes the fd's.In brick_mux environment it is + difficult to handle race condition in janitor threads because brick + spawns a single janitor thread for all bricks. + +Solution: Use synctask to execute posix_release(dir) functions instead of + using background a thread to close fds. + +> Credits: Pranith Karampuri <pkarampu@redhat.com> +> Change-Id: Iffb031f0695a7da83d5a2f6bac8863dad225317e +> Fixes: bz#1811631 +> Signed-off-by: Mohit Agrawal <moagrawal@redhat.com> +> (Cherry pick from commit fb20713b380e1df8d7f9e9df96563be2f9144fd6) +> (Reviewed on upstream link https://review.gluster.org/#/c/glusterfs/+/24221/) + +BUG: 1790336 +Change-Id: Iffb031f0695a7da83d5a2f6bac8863dad225317e +Signed-off-by: Mohit Agrawal <moagrawal@redhat.com> +Reviewed-on: https://code.engineering.redhat.com/gerrit/202791 +Tested-by: Mohit Agrawal <moagrawa@redhat.com> +Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com> +--- + libglusterfs/src/glusterfs/glusterfs.h | 6 +- + libglusterfs/src/glusterfs/syncop.h | 7 +- + rpc/rpc-lib/src/rpcsvc.c | 6 ++ + run-tests.sh | 2 +- + tests/features/ssl-authz.t | 7 +- + xlators/storage/posix/src/posix-common.c | 4 -- + xlators/storage/posix/src/posix-helpers.c | 98 -------------------------- + xlators/storage/posix/src/posix-inode-fd-ops.c | 28 ++------ + xlators/storage/posix/src/posix.h | 3 - + 9 files changed, 20 insertions(+), 141 deletions(-) + +diff --git a/libglusterfs/src/glusterfs/glusterfs.h b/libglusterfs/src/glusterfs/glusterfs.h +index 584846e..495a4d7 100644 +--- a/libglusterfs/src/glusterfs/glusterfs.h ++++ b/libglusterfs/src/glusterfs/glusterfs.h +@@ -734,11 +734,7 @@ struct _glusterfs_ctx { + + struct list_head volfile_list; + +- /* Add members to manage janitor threads for cleanup fd */ +- struct list_head janitor_fds; +- pthread_cond_t janitor_cond; +- pthread_mutex_t janitor_lock; +- pthread_t janitor; ++ char volume_id[GF_UUID_BUF_SIZE]; /* Used only in protocol/client */ + }; + typedef struct _glusterfs_ctx glusterfs_ctx_t; + +diff --git a/libglusterfs/src/glusterfs/syncop.h b/libglusterfs/src/glusterfs/syncop.h +index 3011b4c..1e4c73b 100644 +--- a/libglusterfs/src/glusterfs/syncop.h ++++ b/libglusterfs/src/glusterfs/syncop.h +@@ -254,7 +254,7 @@ struct syncopctx { + task = synctask_get(); \ + stb->task = task; \ + if (task) \ +- frame = task->opframe; \ ++ frame = copy_frame(task->opframe); \ + else \ + frame = syncop_create_frame(THIS); \ + \ +@@ -269,10 +269,7 @@ struct syncopctx { + STACK_WIND_COOKIE(frame, cbk, (void *)stb, subvol, fn_op, params); \ + \ + __yield(stb); \ +- if (task) \ +- STACK_RESET(frame->root); \ +- else \ +- STACK_DESTROY(frame->root); \ ++ STACK_DESTROY(frame->root); \ + } while (0) + + /* +diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c +index 3f184bf..23ca1fd 100644 +--- a/rpc/rpc-lib/src/rpcsvc.c ++++ b/rpc/rpc-lib/src/rpcsvc.c +@@ -375,6 +375,12 @@ 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/run-tests.sh b/run-tests.sh +index 5683b21..c835d93 100755 +--- a/run-tests.sh ++++ b/run-tests.sh +@@ -356,7 +356,7 @@ function run_tests() + selected_tests=$((selected_tests+1)) + echo + echo $section_separator$section_separator +- if [[ $(get_test_status $t) == "BAD_TEST" ]] && \ ++ if [[ $(get_test_status $t) =~ "BAD_TEST" ]] && \ + [[ $skip_bad_tests == "yes" ]] + then + skipped_bad_tests=$((skipped_bad_tests+1)) +diff --git a/tests/features/ssl-authz.t b/tests/features/ssl-authz.t +index 132b598..497083e 100755 +--- a/tests/features/ssl-authz.t ++++ b/tests/features/ssl-authz.t +@@ -67,13 +67,14 @@ echo "Memory consumption for glusterfsd process" + for i in $(seq 1 100); do + gluster v heal $V0 info >/dev/null + done +- ++#Wait to cleanup memory ++sleep 10 + end=`pmap -x $glusterfsd_pid | grep total | awk -F " " '{print $4}'` + diff=$((end-start)) + +-# If memory consumption is more than 5M some leak in SSL code path ++# If memory consumption is more than 15M some leak in SSL code path + +-TEST [ $diff -lt 5000 ] ++TEST [ $diff -lt 15000 ] + + + # Set ssl-allow to a wildcard that includes our identity. +diff --git a/xlators/storage/posix/src/posix-common.c b/xlators/storage/posix/src/posix-common.c +index 2cb58ba..ac53796 100644 +--- a/xlators/storage/posix/src/posix-common.c ++++ b/xlators/storage/posix/src/posix-common.c +@@ -1041,10 +1041,6 @@ posix_init(xlator_t *this) + pthread_mutex_init(&_private->janitor_mutex, NULL); + pthread_cond_init(&_private->janitor_cond, NULL); + INIT_LIST_HEAD(&_private->fsyncs); +- ret = posix_spawn_ctx_janitor_thread(this); +- if (ret) +- goto out; +- + ret = gf_thread_create(&_private->fsyncer, NULL, posix_fsyncer, this, + "posixfsy"); + if (ret) { +diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c +index 2336add..39dbcce 100644 +--- a/xlators/storage/posix/src/posix-helpers.c ++++ b/xlators/storage/posix/src/posix-helpers.c +@@ -1582,104 +1582,6 @@ unlock: + return; + } + +-static struct posix_fd * +-janitor_get_next_fd(glusterfs_ctx_t *ctx, int32_t janitor_sleep) +-{ +- struct posix_fd *pfd = NULL; +- +- struct timespec timeout; +- +- pthread_mutex_lock(&ctx->janitor_lock); +- { +- if (list_empty(&ctx->janitor_fds)) { +- time(&timeout.tv_sec); +- timeout.tv_sec += janitor_sleep; +- timeout.tv_nsec = 0; +- +- pthread_cond_timedwait(&ctx->janitor_cond, &ctx->janitor_lock, +- &timeout); +- goto unlock; +- } +- +- pfd = list_entry(ctx->janitor_fds.next, struct posix_fd, list); +- +- list_del(ctx->janitor_fds.next); +- } +-unlock: +- pthread_mutex_unlock(&ctx->janitor_lock); +- +- return pfd; +-} +- +-static void * +-posix_ctx_janitor_thread_proc(void *data) +-{ +- xlator_t *this = NULL; +- struct posix_fd *pfd; +- glusterfs_ctx_t *ctx = NULL; +- struct posix_private *priv = NULL; +- int32_t sleep_duration = 0; +- +- this = data; +- ctx = THIS->ctx; +- THIS = this; +- +- priv = this->private; +- sleep_duration = priv->janitor_sleep_duration; +- while (1) { +- pfd = janitor_get_next_fd(ctx, sleep_duration); +- if (pfd) { +- if (pfd->dir == NULL) { +- gf_msg_trace(this->name, 0, "janitor: closing file fd=%d", +- pfd->fd); +- sys_close(pfd->fd); +- } else { +- gf_msg_debug(this->name, 0, "janitor: closing dir fd=%p", +- pfd->dir); +- sys_closedir(pfd->dir); +- } +- +- GF_FREE(pfd); +- } +- } +- +- return NULL; +-} +- +-int +-posix_spawn_ctx_janitor_thread(xlator_t *this) +-{ +- struct posix_private *priv = NULL; +- int ret = 0; +- glusterfs_ctx_t *ctx = NULL; +- +- priv = this->private; +- ctx = THIS->ctx; +- +- LOCK(&priv->lock); +- { +- if (!ctx->janitor) { +- pthread_mutex_init(&ctx->janitor_lock, NULL); +- pthread_cond_init(&ctx->janitor_cond, NULL); +- INIT_LIST_HEAD(&ctx->janitor_fds); +- +- ret = gf_thread_create(&ctx->janitor, NULL, +- posix_ctx_janitor_thread_proc, this, +- "posixctxjan"); +- +- if (ret) { +- gf_msg(this->name, GF_LOG_ERROR, errno, P_MSG_THREAD_FAILED, +- "spawning janitor " +- "thread failed"); +- goto unlock; +- } +- } +- } +-unlock: +- UNLOCK(&priv->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 5748b9f..d135d8b 100644 +--- a/xlators/storage/posix/src/posix-inode-fd-ops.c ++++ b/xlators/storage/posix/src/posix-inode-fd-ops.c +@@ -1358,7 +1358,6 @@ posix_releasedir(xlator_t *this, fd_t *fd) + struct posix_fd *pfd = NULL; + uint64_t tmp_pfd = 0; + int ret = 0; +- glusterfs_ctx_t *ctx = NULL; + + VALIDATE_OR_GOTO(this, out); + VALIDATE_OR_GOTO(fd, out); +@@ -1376,21 +1375,11 @@ posix_releasedir(xlator_t *this, fd_t *fd) + goto out; + } + +- ctx = THIS->ctx; +- +- pthread_mutex_lock(&ctx->janitor_lock); +- { +- INIT_LIST_HEAD(&pfd->list); +- list_add_tail(&pfd->list, &ctx->janitor_fds); +- pthread_cond_signal(&ctx->janitor_cond); +- } +- pthread_mutex_unlock(&ctx->janitor_lock); +- +- /*gf_msg_debug(this->name, 0, "janitor: closing dir fd=%p", pfd->dir); ++ gf_msg_debug(this->name, 0, "janitor: closing dir fd=%p", pfd->dir); + + sys_closedir(pfd->dir); + GF_FREE(pfd); +- */ ++ + out: + return 0; + } +@@ -2510,13 +2499,11 @@ posix_release(xlator_t *this, fd_t *fd) + struct posix_fd *pfd = NULL; + int ret = -1; + uint64_t tmp_pfd = 0; +- glusterfs_ctx_t *ctx = NULL; + + VALIDATE_OR_GOTO(this, out); + VALIDATE_OR_GOTO(fd, out); + + priv = this->private; +- ctx = THIS->ctx; + + ret = fd_ctx_del(fd, this, &tmp_pfd); + if (ret < 0) { +@@ -2531,13 +2518,10 @@ posix_release(xlator_t *this, fd_t *fd) + "pfd->dir is %p (not NULL) for file fd=%p", pfd->dir, fd); + } + +- pthread_mutex_lock(&ctx->janitor_lock); +- { +- INIT_LIST_HEAD(&pfd->list); +- list_add_tail(&pfd->list, &ctx->janitor_fds); +- pthread_cond_signal(&ctx->janitor_cond); +- } +- pthread_mutex_unlock(&ctx->janitor_lock); ++ gf_msg_debug(this->name, 0, "janitor: closing dir fd=%p", pfd->dir); ++ ++ sys_close(pfd->fd); ++ GF_FREE(pfd); + + if (!priv) + goto out; +diff --git a/xlators/storage/posix/src/posix.h b/xlators/storage/posix/src/posix.h +index ac9d83c..61495a7 100644 +--- a/xlators/storage/posix/src/posix.h ++++ b/xlators/storage/posix/src/posix.h +@@ -666,9 +666,6 @@ 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 + |