From f9aaa26332ba7007265967bc29a1a2a99234a26d Mon Sep 17 00:00:00 2001 From: Krutika Dhananjay Date: Mon, 23 Jan 2017 17:40:40 +0530 Subject: storage/posix: Execute syscalls in xattrop under different locks Backport of: https://review.gluster.org/16462 and https://review.gluster.org/16792 ... 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: I2054a06701ecab11aed1c04e80ee57bbe2e52564 >BUG: 1421938 >Signed-off-by: Krutika Dhananjay >Reviewed-on: https://review.gluster.org/16462 >Smoke: Gluster Build System >NetBSD-regression: NetBSD Build System >Reviewed-by: Raghavendra G >CentOS-regression: Gluster Build System (cherry picked from commit b5c26a462caf97bfc5380c81092f5c331ccaf1ae) Change-Id: I2054a06701ecab11aed1c04e80ee57bbe2e52564 BUG: 1427390 Signed-off-by: Krutika Dhananjay Reviewed-on: https://review.gluster.org/16777 Smoke: Gluster Build System CentOS-regression: Gluster Build System NetBSD-regression: NetBSD Build System Reviewed-by: Raghavendra G Reviewed-by: Niels de Vos --- xlators/storage/posix/src/posix-handle.c | 10 ++-- 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, 113 insertions(+), 36 deletions(-) diff --git a/xlators/storage/posix/src/posix-handle.c b/xlators/storage/posix/src/posix-handle.c index ddafb0d9b04..d3f48f859bf 100644 --- a/xlators/storage/posix/src/posix-handle.c +++ b/xlators/storage/posix/src/posix-handle.c @@ -940,6 +940,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; @@ -961,12 +962,14 @@ 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) { + ret = -1; goto unlock; + } POSIX_GET_FILE_UNLINK_PATH (priv->base_path, gfid, unlink_path); @@ -985,7 +988,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 10e91370440..190298db235 100644 --- a/xlators/storage/posix/src/posix-helpers.c +++ b/xlators/storage/posix/src/posix-helpers.c @@ -2178,39 +2178,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 aa5a526423f..6582d609af4 100644 --- a/xlators/storage/posix/src/posix.c +++ b/xlators/storage/posix/src/posix.c @@ -107,19 +107,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) { @@ -133,6 +135,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; } @@ -5425,6 +5429,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; @@ -5447,8 +5452,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, @@ -5557,7 +5567,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 87f91e57747..e6304250d14 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); -- cgit