1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
|
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
|
