From 3f9f00b2e5ec4ad845f3c8edef56fec0b4e4bbc1 Mon Sep 17 00:00:00 2001
From: Soumya Koduri <skoduri@redhat.com>
Date: Thu, 28 Mar 2019 14:59:00 +0530
Subject: gfapi: Unblock epoll thread for upcall processing

With commit#ad35193,we have made changes to offload
processing upcall notifications to synctask so as not
to block epoll threads. However seems like the issue wasnt
fully addressed.

In "glfs_cbk_upcall_data" -> "synctask_new1" after creating synctask
if there is no callback defined, the thread waits on synctask_join
till the syncfn is finished. So that way even with those changes,
epoll threads are blocked till the upcalls are processed.

Hence the right fix now is to define a callback function for that
synctask "glfs_cbk_upcall_syncop" so as to unblock epoll/notify threads
completely and the upcall processing can happen in parallel by synctask
threads.

Change-Id: I4d8645e3588fab2c3ca534e0112773aaab68a5dd
fixes: bz#1694563
Signed-off-by: Soumya Koduri <skoduri@redhat.com>
---
 api/src/glfs-fops.c | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c
index 4e349b3ddb5..266863b503e 100644
--- a/api/src/glfs-fops.c
+++ b/api/src/glfs-fops.c
@@ -4864,6 +4864,16 @@ out:
         return ret;
 }
 
+static int
+glfs_upcall_syncop_cbk(int ret, call_frame_t *frame, void *opaque)
+{
+    struct upcall_syncop_args *args = opaque;
+
+    GF_FREE(args->upcall_data);
+    GF_FREE(args);
+    return 0;
+}
+
 static int
 glfs_cbk_upcall_syncop(void *opaque)
 {
@@ -4924,15 +4934,13 @@ out:
                 GLFS_FREE (up_arg);
         }
 
-        return ret;
+        return 0;
 }
 
 static void
 glfs_cbk_upcall_data(struct glfs *fs, struct gf_upcall *upcall_data)
 {
-    struct upcall_syncop_args args = {
-        0,
-    };
+    struct upcall_syncop_args *args = NULL;
     int ret = -1;
 
     if (!fs || !upcall_data)
@@ -4943,16 +4951,34 @@ glfs_cbk_upcall_data(struct glfs *fs, struct gf_upcall *upcall_data)
         goto out;
     }
 
-    args.fs = fs;
-    args.upcall_data = upcall_data;
+    args = GF_CALLOC(1, sizeof(struct upcall_syncop_args),
+                     glfs_mt_upcall_entry_t);
+    if (!args) {
+        gf_msg(THIS->name, GF_LOG_ERROR, ENOMEM, API_MSG_ALLOC_FAILED,
+               "Upcall syncop args allocation failed.");
+        goto out;
+    }
 
-    ret = synctask_new(THIS->ctx->env, glfs_cbk_upcall_syncop, NULL, NULL,
-                       &args);
+    /* Note: we are not taking any ref on fs here.
+     * Ideally applications have to unregister for upcall events
+     * or stop polling for upcall events before performing
+     * glfs_fini. And as for outstanding synctasks created, we wait
+     * for all syncenv threads to finish tasks before cleaning up the
+     * fs->ctx. Hence it seems safe to process these callback
+     * notification without taking any lock/ref.
+     */
+    args->fs = fs;
+    args->upcall_data = gf_memdup(upcall_data, sizeof(*upcall_data));
+
+    ret = synctask_new(THIS->ctx->env, glfs_cbk_upcall_syncop,
+                       glfs_upcall_syncop_cbk, NULL, args);
     /* should we retry incase of failure? */
     if (ret) {
         gf_msg(THIS->name, GF_LOG_ERROR, errno, API_MSG_UPCALL_SYNCOP_FAILED,
                "Synctak for Upcall event_type(%d) and gfid(%s) failed",
                upcall_data->event_type, (char *)(upcall_data->gfid));
+        GF_FREE(args->upcall_data);
+        GF_FREE(args);
     }
 
 out:
-- 
cgit