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