From bdede85bb9ac8039171857913f6818f327557176 Mon Sep 17 00:00:00 2001

From: Krutika Dhananjay <kdhananj@redhat.com>

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 <kdhananj@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/106796

Reviewed-by: Raghavendra Gowdappa <rgowdapp@redhat.com>

---

 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