From 2449a1824c6f7b57889335caaeb09f4c5cb3efce Mon Sep 17 00:00:00 2001 From: Soumya Koduri Date: Thu, 28 Mar 2019 14:59:00 +0530 Subject: [PATCH 48/52] 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. Upstream references- mainline : https://review.gluster.org/22436 release-6.0 : https://review.gluster.org/22459 Change-Id: I4d8645e3588fab2c3ca534e0112773aaab68a5dd fixes: bz#1694565 Signed-off-by: Soumya Koduri Reviewed-on: https://code.engineering.redhat.com/gerrit/166586 Tested-by: RHGS Build Bot Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- 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 88cd32b..01ba60b 100644 --- a/api/src/glfs-fops.c +++ b/api/src/glfs-fops.c @@ -5714,6 +5714,16 @@ out: } 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) { struct upcall_syncop_args *args = opaque; @@ -5770,15 +5780,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) @@ -5789,16 +5797,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; + } + + /* 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, NULL, NULL, - &args); + 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: -- 1.8.3.1