14f8ab
From 52dc121c412de9c1cc3058d782b949dc7b25dc3e Mon Sep 17 00:00:00 2001
14f8ab
From: Soumya Koduri <skoduri@redhat.com>
14f8ab
Date: Thu, 25 Jul 2019 12:56:12 +0530
14f8ab
Subject: [PATCH 264/265] gfapi: Fix deadlock while processing upcall
14f8ab
14f8ab
As mentioned in bug1733166, there could be potential deadlock
14f8ab
while processing upcalls depending on how each xlator choose
14f8ab
to act on it. The right way of fixing such issues
14f8ab
is to change rpc callback communication process.
14f8ab
- https://github.com/gluster/glusterfs/issues/697
14f8ab
14f8ab
Till then, making changes in gfapi layer to avoid any I/O
14f8ab
processing.
14f8ab
14f8ab
This is backport of below mainline patch
14f8ab
> https://review.gluster.org/#/c/glusterfs/+/23108/
14f8ab
> bz#1733166
14f8ab
> https://review.gluster.org/#/c/glusterfs/+/23107/ (release-6)
14f8ab
14f8ab
Change-Id: I2079e95339e5d761d5060707f4555cfacab95c83
14f8ab
fixes: bz#1733520
14f8ab
Signed-off-by: Soumya Koduri <skoduri@redhat.com>
14f8ab
Reviewed-on: https://code.engineering.redhat.com/gerrit/177675
14f8ab
Tested-by: RHGS Build Bot <nigelb@redhat.com>
14f8ab
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
14f8ab
---
14f8ab
 api/src/glfs-fops.c | 164 +++++++++++++++++++++++++++++++++++++++++-----------
14f8ab
 1 file changed, 131 insertions(+), 33 deletions(-)
14f8ab
14f8ab
diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c
14f8ab
index 396f18c..e6adea5 100644
14f8ab
--- a/api/src/glfs-fops.c
14f8ab
+++ b/api/src/glfs-fops.c
14f8ab
@@ -34,7 +34,7 @@
14f8ab
 
14f8ab
 struct upcall_syncop_args {
14f8ab
     struct glfs *fs;
14f8ab
-    struct glfs_upcall *up_arg;
14f8ab
+    struct gf_upcall upcall_data;
14f8ab
 };
14f8ab
 
14f8ab
 #define READDIRBUF_SIZE (sizeof(struct dirent) + GF_NAME_MAX + 1)
14f8ab
@@ -5716,8 +5716,28 @@ out:
14f8ab
 static int
14f8ab
 upcall_syncop_args_free(struct upcall_syncop_args *args)
14f8ab
 {
14f8ab
-    if (args && args->up_arg)
14f8ab
-        GLFS_FREE(args->up_arg);
14f8ab
+    dict_t *dict = NULL;
14f8ab
+    struct gf_upcall *upcall_data = NULL;
14f8ab
+
14f8ab
+    if (args) {
14f8ab
+        upcall_data = &args->upcall_data;
14f8ab
+        switch (upcall_data->event_type) {
14f8ab
+            case GF_UPCALL_CACHE_INVALIDATION:
14f8ab
+                dict = ((struct gf_upcall_cache_invalidation *)(upcall_data
14f8ab
+                                                                    ->data))
14f8ab
+                           ->dict;
14f8ab
+                break;
14f8ab
+            case GF_UPCALL_RECALL_LEASE:
14f8ab
+                dict = ((struct gf_upcall_recall_lease *)(upcall_data->data))
14f8ab
+                           ->dict;
14f8ab
+                break;
14f8ab
+        }
14f8ab
+        if (dict)
14f8ab
+            dict_unref(dict);
14f8ab
+
14f8ab
+        GF_FREE(upcall_data->client_uid);
14f8ab
+        GF_FREE(upcall_data->data);
14f8ab
+    }
14f8ab
     GF_FREE(args);
14f8ab
     return 0;
14f8ab
 }
14f8ab
@@ -5727,14 +5747,7 @@ glfs_upcall_syncop_cbk(int ret, call_frame_t *frame, void *opaque)
14f8ab
 {
14f8ab
     struct upcall_syncop_args *args = opaque;
14f8ab
 
14f8ab
-    /* Here we not using upcall_syncop_args_free as application
14f8ab
-     * will be cleaning up the args->up_arg using glfs_free
14f8ab
-     * post processing upcall.
14f8ab
-     */
14f8ab
-    if (ret) {
14f8ab
-        upcall_syncop_args_free(args);
14f8ab
-    } else
14f8ab
-        GF_FREE(args);
14f8ab
+    (void)upcall_syncop_args_free(args);
14f8ab
 
14f8ab
     return 0;
14f8ab
 }
14f8ab
@@ -5743,29 +5756,17 @@ static int
14f8ab
 glfs_cbk_upcall_syncop(void *opaque)
14f8ab
 {
14f8ab
     struct upcall_syncop_args *args = opaque;
14f8ab
+    struct gf_upcall *upcall_data = NULL;
14f8ab
     struct glfs_upcall *up_arg = NULL;
14f8ab
     struct glfs *fs;
14f8ab
+    int ret = -1;
14f8ab
 
14f8ab
     fs = args->fs;
14f8ab
-    up_arg = args->up_arg;
14f8ab
-
14f8ab
-    if (fs->up_cbk && up_arg) {
14f8ab
-        (fs->up_cbk)(up_arg, fs->up_data);
14f8ab
-        return 0;
14f8ab
-    }
14f8ab
-
14f8ab
-    return -1;
14f8ab
-}
14f8ab
+    upcall_data = &args->upcall_data;
14f8ab
 
14f8ab
-static struct upcall_syncop_args *
14f8ab
-upcall_syncop_args_init(struct glfs *fs, struct gf_upcall *upcall_data)
14f8ab
-{
14f8ab
-    struct upcall_syncop_args *args = NULL;
14f8ab
-    int ret = -1;
14f8ab
-    struct glfs_upcall *up_arg = NULL;
14f8ab
-
14f8ab
-    if (!fs || !upcall_data)
14f8ab
+    if (!upcall_data) {
14f8ab
         goto out;
14f8ab
+    }
14f8ab
 
14f8ab
     up_arg = GLFS_CALLOC(1, sizeof(struct gf_upcall), glfs_release_upcall,
14f8ab
                          glfs_mt_upcall_entry_t);
14f8ab
@@ -5795,6 +5796,8 @@ upcall_syncop_args_init(struct glfs *fs, struct gf_upcall *upcall_data)
14f8ab
     if (up_arg->reason == GLFS_UPCALL_EVENT_NULL) {
14f8ab
         gf_msg(THIS->name, GF_LOG_DEBUG, errno, API_MSG_INVALID_ENTRY,
14f8ab
                "Upcall_EVENT_NULL received. Skipping it.");
14f8ab
+        ret = 0;
14f8ab
+        GLFS_FREE(up_arg);
14f8ab
         goto out;
14f8ab
     } else if (ret) {
14f8ab
         gf_msg(THIS->name, GF_LOG_ERROR, errno, API_MSG_INVALID_ENTRY,
14f8ab
@@ -5802,6 +5805,85 @@ upcall_syncop_args_init(struct glfs *fs, struct gf_upcall *upcall_data)
14f8ab
         goto out;
14f8ab
     }
14f8ab
 
14f8ab
+    if (fs->up_cbk && up_arg)
14f8ab
+        (fs->up_cbk)(up_arg, fs->up_data);
14f8ab
+
14f8ab
+    /* application takes care of calling glfs_free on up_arg post
14f8ab
+     * their processing */
14f8ab
+
14f8ab
+out:
14f8ab
+    return ret;
14f8ab
+}
14f8ab
+
14f8ab
+static struct gf_upcall_cache_invalidation *
14f8ab
+gf_copy_cache_invalidation(struct gf_upcall_cache_invalidation *src)
14f8ab
+{
14f8ab
+    struct gf_upcall_cache_invalidation *dst = NULL;
14f8ab
+
14f8ab
+    if (!src)
14f8ab
+        goto out;
14f8ab
+
14f8ab
+    dst = GF_CALLOC(1, sizeof(struct gf_upcall_cache_invalidation),
14f8ab
+                    glfs_mt_upcall_entry_t);
14f8ab
+
14f8ab
+    if (!dst) {
14f8ab
+        gf_msg(THIS->name, GF_LOG_ERROR, ENOMEM, API_MSG_ALLOC_FAILED,
14f8ab
+               "Upcall entry allocation failed.");
14f8ab
+        goto out;
14f8ab
+    }
14f8ab
+
14f8ab
+    dst->flags = src->flags;
14f8ab
+    dst->expire_time_attr = src->expire_time_attr;
14f8ab
+    dst->stat = src->stat;
14f8ab
+    dst->p_stat = src->p_stat;
14f8ab
+    dst->oldp_stat = src->oldp_stat;
14f8ab
+
14f8ab
+    if (src->dict)
14f8ab
+        dst->dict = dict_copy_with_ref(src->dict, NULL);
14f8ab
+
14f8ab
+    return dst;
14f8ab
+out:
14f8ab
+    return NULL;
14f8ab
+}
14f8ab
+
14f8ab
+static struct gf_upcall_recall_lease *
14f8ab
+gf_copy_recall_lease(struct gf_upcall_recall_lease *src)
14f8ab
+{
14f8ab
+    struct gf_upcall_recall_lease *dst = NULL;
14f8ab
+
14f8ab
+    if (!src)
14f8ab
+        goto out;
14f8ab
+
14f8ab
+    dst = GF_CALLOC(1, sizeof(struct gf_upcall_recall_lease),
14f8ab
+                    glfs_mt_upcall_entry_t);
14f8ab
+
14f8ab
+    if (!dst) {
14f8ab
+        gf_msg(THIS->name, GF_LOG_ERROR, ENOMEM, API_MSG_ALLOC_FAILED,
14f8ab
+               "Upcall entry allocation failed.");
14f8ab
+        goto out;
14f8ab
+    }
14f8ab
+
14f8ab
+    dst->lease_type = src->lease_type;
14f8ab
+    memcpy(dst->tid, src->tid, 16);
14f8ab
+
14f8ab
+    if (src->dict)
14f8ab
+        dst->dict = dict_copy_with_ref(src->dict, NULL);
14f8ab
+
14f8ab
+    return dst;
14f8ab
+out:
14f8ab
+    return NULL;
14f8ab
+}
14f8ab
+
14f8ab
+static struct upcall_syncop_args *
14f8ab
+upcall_syncop_args_init(struct glfs *fs, struct gf_upcall *upcall_data)
14f8ab
+{
14f8ab
+    struct upcall_syncop_args *args = NULL;
14f8ab
+    int ret = -1;
14f8ab
+    struct gf_upcall *t_data = NULL;
14f8ab
+
14f8ab
+    if (!fs || !upcall_data)
14f8ab
+        goto out;
14f8ab
+
14f8ab
     args = GF_CALLOC(1, sizeof(struct upcall_syncop_args),
14f8ab
                      glfs_mt_upcall_entry_t);
14f8ab
     if (!args) {
14f8ab
@@ -5819,15 +5901,31 @@ upcall_syncop_args_init(struct glfs *fs, struct gf_upcall *upcall_data)
14f8ab
      * notification without taking any lock/ref.
14f8ab
      */
14f8ab
     args->fs = fs;
14f8ab
-    args->up_arg = up_arg;
14f8ab
+    t_data = &(args->upcall_data);
14f8ab
+    t_data->client_uid = gf_strdup(upcall_data->client_uid);
14f8ab
 
14f8ab
-    /* application takes care of calling glfs_free on up_arg post
14f8ab
-     * their processing */
14f8ab
+    gf_uuid_copy(t_data->gfid, upcall_data->gfid);
14f8ab
+    t_data->event_type = upcall_data->event_type;
14f8ab
+
14f8ab
+    switch (t_data->event_type) {
14f8ab
+        case GF_UPCALL_CACHE_INVALIDATION:
14f8ab
+            t_data->data = gf_copy_cache_invalidation(
14f8ab
+                (struct gf_upcall_cache_invalidation *)upcall_data->data);
14f8ab
+            break;
14f8ab
+        case GF_UPCALL_RECALL_LEASE:
14f8ab
+            t_data->data = gf_copy_recall_lease(
14f8ab
+                (struct gf_upcall_recall_lease *)upcall_data->data);
14f8ab
+            break;
14f8ab
+    }
14f8ab
+
14f8ab
+    if (!t_data->data)
14f8ab
+        goto out;
14f8ab
 
14f8ab
     return args;
14f8ab
 out:
14f8ab
-    if (up_arg) {
14f8ab
-        GLFS_FREE(up_arg);
14f8ab
+    if (ret) {
14f8ab
+        GF_FREE(args->upcall_data.client_uid);
14f8ab
+        GF_FREE(args);
14f8ab
     }
14f8ab
 
14f8ab
     return NULL;
14f8ab
-- 
14f8ab
1.8.3.1
14f8ab