storage/posix: Execute syscalls in xattrop under different locks

... 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>

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 c8524a69fe5..ef56d105d7e 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 3fa6dace327..af3d01b469c 100644
--- a/xlators/storage/posix/src/posix-helpers.c
+++ b/xlators/storage/posix/src/posix-helpers.c
@@ -2177,39 +2177,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 aa7e7404099..123fa9eb4df 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;
 }
 
@@ -1714,7 +1718,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;
         }
@@ -5442,6 +5446,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;
 
@@ -5464,8 +5469,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,
@@ -5574,7 +5584,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);