From bdede85bb9ac8039171857913f6818f327557176 Mon Sep 17 00:00:00 2001 From: Krutika Dhananjay Date: Mon, 23 Jan 2017 17:40:40 +0530 Subject: [PATCH 468/473] storage/posix: Execute syscalls in xattrop under different locks Backport of: https://review.gluster.org/16462 ... and not inode->lock. This is to prevent the epoll thread from *potentially* being blocked on this lock in the worst case for extended period elsewhere in the brick stack, while the syscalls in xattrop are being performed under the same lock by a different thread. This could potentially lead to ping-timeout, if the only available epoll thread is busy waiting on the inode->lock, thereby preventing it from picking up the ping request from the client(s). Also removed some unused functions. Change-Id: Ia8acbba6d0afec75b3b49485f234c598cec6571b BUG: 1415178 Signed-off-by: Krutika Dhananjay Reviewed-on: https://code.engineering.redhat.com/gerrit/106796 Reviewed-by: Raghavendra Gowdappa --- xlators/storage/posix/src/posix-handle.c | 8 +-- xlators/storage/posix/src/posix-helpers.c | 85 ++++++++++++++++++++++++------- xlators/storage/posix/src/posix.c | 36 ++++++++----- xlators/storage/posix/src/posix.h | 18 ++++++- 4 files changed, 111 insertions(+), 36 deletions(-) diff --git a/xlators/storage/posix/src/posix-handle.c b/xlators/storage/posix/src/posix-handle.c index c8524a6..ef56d10 100644 --- a/xlators/storage/posix/src/posix-handle.c +++ b/xlators/storage/posix/src/posix-handle.c @@ -952,6 +952,7 @@ posix_create_link_if_gfid_exists (xlator_t *this, uuid_t gfid, char *real_path, inode_t *inode = NULL; struct stat stbuf = {0,}; struct posix_private *priv = NULL; + posix_inode_ctx_t *ctx = NULL; priv = this->private; @@ -973,11 +974,11 @@ posix_create_link_if_gfid_exists (xlator_t *this, uuid_t gfid, char *real_path, LOCK (&inode->lock); { - ret = __inode_ctx_get0 (inode, this, &ctx_int); + ret = __posix_inode_ctx_get_all (inode, this, &ctx); if (ret) goto unlock; - if (ctx_int != GF_UNLINK_TRUE) + if (ctx->unlink_flag != GF_UNLINK_TRUE) goto unlock; POSIX_GET_FILE_UNLINK_PATH (priv->base_path, gfid, @@ -997,7 +998,8 @@ posix_create_link_if_gfid_exists (xlator_t *this, uuid_t gfid, char *real_path, goto unlock; } ctx_int = GF_UNLINK_FALSE; - ret = __inode_ctx_set0 (inode, this, &ctx_int); + ret = __posix_inode_ctx_set_unlink_flag (inode, this, + ctx_int); } unlock: UNLOCK (&inode->lock); diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c index 72f76c0..c09cecf 100644 --- a/xlators/storage/posix/src/posix-helpers.c +++ b/xlators/storage/posix/src/posix-helpers.c @@ -2270,39 +2270,88 @@ posix_fdget_objectsignature (int fd, dict_t *xattr) return -EINVAL; } +posix_inode_ctx_t * +__posix_inode_ctx_get (inode_t *inode, xlator_t *this) +{ + int ret = -1; + uint64_t ctx_uint = 0; + posix_inode_ctx_t *ctx_p = NULL; + + ret = __inode_ctx_get (inode, this, &ctx_uint); + if (ret == 0) { + return (posix_inode_ctx_t *)ctx_uint; + } + + ctx_p = GF_CALLOC (1, sizeof (*ctx_p), gf_posix_mt_inode_ctx_t); + if (!ctx_p) + return NULL; + + pthread_mutex_init (&ctx_p->xattrop_lock, NULL); + + ret = __inode_ctx_set (inode, this, (uint64_t *)&ctx_p); + if (ret < 0) { + pthread_mutex_destroy (&ctx_p->xattrop_lock); + GF_FREE (ctx_p); + return NULL; + } + + return ctx_p; +} int -posix_inode_ctx_get (inode_t *inode, xlator_t *this, uint64_t *ctx) +__posix_inode_ctx_set_unlink_flag (inode_t *inode, xlator_t *this, uint64_t ctx) { - int ret = -1; - uint64_t ctx_int = 0; + posix_inode_ctx_t *ctx_p = NULL; - GF_VALIDATE_OR_GOTO (this->name, this, out); - GF_VALIDATE_OR_GOTO (this->name, inode, out); + ctx_p = __posix_inode_ctx_get (inode, this); + if (ctx_p == NULL) + return -1; - ret = inode_ctx_get (inode, this, &ctx_int); + ctx_p->unlink_flag = ctx; - if (ret) - return ret; + return 0; +} - if (ctx) - *ctx = ctx_int; +int +posix_inode_ctx_set_unlink_flag (inode_t *inode, xlator_t *this, uint64_t ctx) +{ + int ret = -1; + + LOCK(&inode->lock); + { + ret = __posix_inode_ctx_set_unlink_flag (inode, this, ctx); + } + UNLOCK(&inode->lock); -out: return ret; } +int +__posix_inode_ctx_get_all (inode_t *inode, xlator_t *this, + posix_inode_ctx_t **ctx) +{ + posix_inode_ctx_t *ctx_p = NULL; + + ctx_p = __posix_inode_ctx_get (inode, this); + if (ctx_p == NULL) + return -1; + + *ctx = ctx_p; + + return 0; +} int -posix_inode_ctx_set (inode_t *inode, xlator_t *this, uint64_t ctx) +posix_inode_ctx_get_all (inode_t *inode, xlator_t *this, + posix_inode_ctx_t **ctx) { - int ret = -1; + int ret = 0; - GF_VALIDATE_OR_GOTO (this->name, this, out); - GF_VALIDATE_OR_GOTO (this->name, inode, out); - GF_VALIDATE_OR_GOTO (this->name, ctx, out); + LOCK(&inode->lock); + { + ret = __posix_inode_ctx_get_all (inode, this, ctx); + } + UNLOCK(&inode->lock); - ret = inode_ctx_set (inode, this, &ctx); -out: return ret; } diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c index afd0ff8..7eaf98b 100644 --- a/xlators/storage/posix/src/posix.c +++ b/xlators/storage/posix/src/posix.c @@ -108,19 +108,21 @@ out: int posix_forget (xlator_t *this, inode_t *inode) { - uint64_t tmp_cache = 0; - int ret = 0; - char *unlink_path = NULL; - struct posix_private *priv_posix = NULL; + int ret = 0; + char *unlink_path = NULL; + uint64_t ctx_uint = 0; + posix_inode_ctx_t *ctx = NULL; + struct posix_private *priv_posix = NULL; priv_posix = (struct posix_private *) this->private; - ret = inode_ctx_del (inode, this, &tmp_cache); - if (ret < 0) { - ret = 0; - goto out; - } - if (tmp_cache == GF_UNLINK_TRUE) { + ret = inode_ctx_del (inode, this, &ctx_uint); + if (!ctx_uint) + return 0; + + ctx = (posix_inode_ctx_t *)ctx_uint; + + if (ctx->unlink_flag == GF_UNLINK_TRUE) { POSIX_GET_FILE_UNLINK_PATH(priv_posix->base_path, inode->gfid, unlink_path); if (!unlink_path) { @@ -134,6 +136,8 @@ posix_forget (xlator_t *this, inode_t *inode) ret = sys_unlink(unlink_path); } out: + pthread_mutex_destroy (&ctx->xattrop_lock); + GF_FREE (ctx); return ret; } @@ -1696,7 +1700,7 @@ posix_add_unlink_to_ctx (inode_t *inode, xlator_t *this, char *unlink_path) } ctx = GF_UNLINK_TRUE; - ret = posix_inode_ctx_set (inode, this, ctx); + ret = posix_inode_ctx_set_unlink_flag (inode, this, ctx); if (ret < 0) { goto out; } @@ -5426,6 +5430,7 @@ _posix_handle_xattr_keyvalue_pair (dict_t *d, char *k, data_t *v, inode_t *inode = NULL; xlator_t *this = NULL; posix_xattr_filler_t *filler = NULL; + posix_inode_ctx_t *ctx = NULL; filler = tmp; @@ -5448,8 +5453,13 @@ _posix_handle_xattr_keyvalue_pair (dict_t *d, char *k, data_t *v, } } #endif + op_ret = posix_inode_ctx_get_all (inode, this, &ctx); + if (op_ret < 0) { + op_errno = ENOMEM; + goto out; + } - LOCK (&inode->lock); + pthread_mutex_lock (&ctx->xattrop_lock); { if (filler->real_path) { size = sys_lgetxattr (filler->real_path, k, @@ -5558,7 +5568,7 @@ _posix_handle_xattr_keyvalue_pair (dict_t *d, char *k, data_t *v, op_errno = errno; } unlock: - UNLOCK (&inode->lock); + pthread_mutex_unlock (&ctx->xattrop_lock); if (op_ret == -1) goto out; diff --git a/xlators/storage/posix/src/posix.h b/xlators/storage/posix/src/posix.h index 6fd32c1..fb63004 100644 --- a/xlators/storage/posix/src/posix.h +++ b/xlators/storage/posix/src/posix.h @@ -190,6 +190,11 @@ typedef struct { int32_t op_errno; } posix_xattr_filler_t; +typedef struct { + uint64_t unlink_flag; + pthread_mutex_t xattrop_lock; +} posix_inode_ctx_t; + #define POSIX_BASE_PATH(this) (((struct posix_private *)this->private)->base_path) #define POSIX_BASE_PATH_LEN(this) (((struct posix_private *)this->private)->base_path_length) @@ -217,8 +222,17 @@ typedef struct { /* Helper functions */ -int posix_inode_ctx_get (inode_t *inode, xlator_t *this, uint64_t *ctx); -int posix_inode_ctx_set (inode_t *inode, xlator_t *this, uint64_t ctx); +int posix_inode_ctx_set_unlink_flag (inode_t *inode, xlator_t *this, + uint64_t ctx); + +int posix_inode_ctx_get_all (inode_t *inode, xlator_t *this, + posix_inode_ctx_t **ctx); + +int __posix_inode_ctx_set_unlink_flag (inode_t *inode, xlator_t *this, + uint64_t ctx); + +int __posix_inode_ctx_get_all (inode_t *inode, xlator_t *this, + posix_inode_ctx_t **ctx); int posix_gfid_set (xlator_t *this, const char *path, loc_t *loc, dict_t *xattr_req); -- 1.8.3.1