summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKrutika Dhananjay <kdhananj@redhat.com>2017-01-23 17:40:40 +0530
committerNiels de Vos <ndevos@redhat.com>2017-03-11 12:22:35 -0500
commitf9aaa26332ba7007265967bc29a1a2a99234a26d (patch)
treecfab1d887c604dd70f797aead77e763382bc7e2c
parenta671a19d660dabad4292a2fbb190ee26a13a1532 (diff)
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 <kdhananj@redhat.com> >Reviewed-on: https://review.gluster.org/16462 >Smoke: Gluster Build System <jenkins@build.gluster.org> >NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> >Reviewed-by: Raghavendra G <rgowdapp@redhat.com> >CentOS-regression: Gluster Build System <jenkins@build.gluster.org> (cherry picked from commit b5c26a462caf97bfc5380c81092f5c331ccaf1ae) Change-Id: I2054a06701ecab11aed1c04e80ee57bbe2e52564 BUG: 1427390 Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com> Reviewed-on: https://review.gluster.org/16777 Smoke: Gluster Build System <jenkins@build.gluster.org> CentOS-regression: Gluster Build System <jenkins@build.gluster.org> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> Reviewed-by: Raghavendra G <rgowdapp@redhat.com> Reviewed-by: Niels de Vos <ndevos@redhat.com>
-rw-r--r--xlators/storage/posix/src/posix-handle.c10
-rw-r--r--xlators/storage/posix/src/posix-helpers.c85
-rw-r--r--xlators/storage/posix/src/posix.c36
-rw-r--r--xlators/storage/posix/src/posix.h18
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);