Skip to content

Commit 64d9c59

Browse files
fyin1wenlingz
fyin1
authored and
wenlingz
committed
dm: enhence the mevent API
There is one race issue between mevent callback (which is called in mevent_dispatch thread) and mevent_delete (which could be called in dev thread). And the callback is called after mevent_delete. libevent have the exactly same issue. The issue is decripted here: https://github.com/libevent/libevent/blob/master/whatsnew-2.1.txt The fixing is: We introduce a teardown callback to mevent and make sure there is no race issue between callback and teardown call. This patch updates the mevent API and the caller as well. Tracked-On: #1877 Signed-off-by: Yin Fengwei <fengwei.yin@intel.com> Acked-by: Yu Wang <yu1.wang@intel.com>
1 parent eec3a34 commit 64d9c59

File tree

10 files changed

+52
-35
lines changed

10 files changed

+52
-35
lines changed

‎devicemodel/arch/x86/pm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ smi_cmd_handler(struct vmctx *ctx, int vcpu, int in, int port, int bytes,
296296
pm1_control |= PM1_SCI_EN;
297297
if (power_button == NULL) {
298298
power_button = mevent_add(SIGTERM, EVF_SIGNAL,
299-
power_button_handler, ctx);
299+
power_button_handler, ctx, NULL, NULL);
300300
old_power_handler = signal(SIGTERM, SIG_IGN);
301301
}
302302
break;

‎devicemodel/core/mevent.c

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,13 @@ static int mevent_pipefd[2];
5656
static pthread_mutex_t mevent_lmutex = PTHREAD_MUTEX_INITIALIZER;
5757

5858
struct mevent {
59-
void (*me_func)(int, enum ev_type, void *);
59+
void (*run)(int, enum ev_type, void *);
60+
void *run_param;
61+
void (*teardown)(void *);
62+
void *teardown_param;
63+
6064
int me_fd;
6165
enum ev_type me_type;
62-
void *me_param;
6366
int me_cq;
6467
int me_state;
6568

@@ -114,7 +117,7 @@ mevent_notify(void)
114117
* If calling from outside the i/o thread, write a byte on the
115118
* pipe to force the i/o thread to exit the blocking epoll call.
116119
*/
117-
if (mevent_pipefd[1] != 0 && pthread_self() != mevent_tid)
120+
if (mevent_pipefd[1] != 0 && !is_dispatch_thread())
118121
if (write(mevent_pipefd[1], &c, 1) <= 0)
119122
return -1;
120123
return 0;
@@ -152,12 +155,15 @@ mevent_destroy(void)
152155
LIST_REMOVE(mevp, me_list);
153156
epoll_ctl(epoll_fd, EPOLL_CTL_DEL, mevp->me_fd, NULL);
154157

155-
if ((mevp->me_type == EVF_READ ||
156-
mevp->me_type == EVF_READ_ET ||
157-
mevp->me_type == EVF_WRITE ||
158-
mevp->me_type == EVF_WRITE_ET) &&
159-
mevp->me_fd != STDIN_FILENO)
160-
close(mevp->me_fd);
158+
if ((mevp->me_type == EVF_READ ||
159+
mevp->me_type == EVF_READ_ET ||
160+
mevp->me_type == EVF_WRITE ||
161+
mevp->me_type == EVF_WRITE_ET) &&
162+
mevp->me_fd != STDIN_FILENO)
163+
close(mevp->me_fd);
164+
165+
if (mevp->teardown)
166+
mevp->teardown(mevp->teardown_param);
161167

162168
free(mevp);
163169
}
@@ -168,12 +174,15 @@ mevent_destroy(void)
168174
list_foreach_safe(mevp, &del_head, me_list, tmpp) {
169175
LIST_REMOVE(mevp, me_list);
170176

171-
if ((mevp->me_type == EVF_READ ||
172-
mevp->me_type == EVF_READ_ET ||
173-
mevp->me_type == EVF_WRITE ||
174-
mevp->me_type == EVF_WRITE_ET) &&
175-
mevp->me_fd != STDIN_FILENO)
176-
close(mevp->me_fd);
177+
if ((mevp->me_type == EVF_READ ||
178+
mevp->me_type == EVF_READ_ET ||
179+
mevp->me_type == EVF_WRITE ||
180+
mevp->me_type == EVF_WRITE_ET) &&
181+
mevp->me_fd != STDIN_FILENO)
182+
close(mevp->me_fd);
183+
184+
if (mevp->teardown)
185+
mevp->teardown(mevp->teardown_param);
177186

178187
free(mevp);
179188
}
@@ -190,19 +199,20 @@ mevent_handle(struct epoll_event *kev, int numev)
190199
mevp = kev[i].data.ptr;
191200

192201
if (mevp->me_state)
193-
(*mevp->me_func)(mevp->me_fd, mevp->me_type, mevp->me_param);
202+
(*mevp->run)(mevp->me_fd, mevp->me_type, mevp->run_param);
194203
}
195204
}
196205

197206
struct mevent *
198207
mevent_add(int tfd, enum ev_type type,
199-
void (*func)(int, enum ev_type, void *), void *param)
208+
void (*run)(int, enum ev_type, void *), void *run_param,
209+
void (*teardown)(void *), void *teardown_param)
200210
{
201211
int ret;
202212
struct epoll_event ee;
203213
struct mevent *lp, *mevp;
204214

205-
if (tfd < 0 || func == NULL)
215+
if (tfd < 0 || run == NULL)
206216
return NULL;
207217

208218
if (type == EVF_TIMER)
@@ -227,10 +237,13 @@ mevent_add(int tfd, enum ev_type type,
227237

228238
mevp->me_fd = tfd;
229239
mevp->me_type = type;
230-
mevp->me_func = func;
231-
mevp->me_param = param;
232240
mevp->me_state = 1;
233241

242+
mevp->run = run;
243+
mevp->run_param = run_param;
244+
mevp->teardown = teardown;
245+
mevp->teardown_param = teardown_param;
246+
234247
ee.events = mevent_kq_filter(mevp);
235248
ee.data.ptr = mevp;
236249
ret = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, mevp->me_fd, &ee);
@@ -241,6 +254,8 @@ mevent_add(int tfd, enum ev_type type,
241254

242255
return mevp;
243256
} else {
257+
if (mevp->teardown)
258+
mevp->teardown(mevp->teardown_param);
244259
free(mevp);
245260
return NULL;
246261
}
@@ -308,6 +323,9 @@ mevent_drain_del_list(void)
308323
if (evp->closefd) {
309324
close(evp->me_fd);
310325
}
326+
327+
if (evp->teardown)
328+
evp->teardown(evp->teardown_param);
311329
free(evp);
312330
}
313331
mevent_qunlock();
@@ -323,7 +341,7 @@ mevent_delete_event(struct mevent *evp, int closefd)
323341
evp->closefd = closefd;
324342

325343
epoll_ctl(epoll_fd, EPOLL_CTL_DEL, evp->me_fd, NULL);
326-
if (!is_dispatch_thread()) {
344+
if (!is_dispatch_thread() && evp->teardown != NULL) {
327345
mevent_add_to_del_list(evp, closefd);
328346
} else {
329347
if (evp->closefd) {
@@ -398,7 +416,7 @@ mevent_dispatch(void)
398416
/*
399417
* Add internal event handler for the pipe write fd
400418
*/
401-
pipev = mevent_add(mevent_pipefd[0], EVF_READ, mevent_pipe_read, NULL);
419+
pipev = mevent_add(mevent_pipefd[0], EVF_READ, mevent_pipe_read, NULL, NULL, NULL);
402420
assert(pipev != NULL);
403421

404422
for (;;) {

‎devicemodel/core/timer.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ acrn_timer_init(struct acrn_timer *timer, void (*cb)(void *), void *param)
7171
return -1;
7272
}
7373

74-
timer->mevp = mevent_add(timer->fd, EVF_READ, timer_handler, timer);
74+
timer->mevp = mevent_add(timer->fd, EVF_READ, timer_handler, timer, NULL, NULL);
7575
if (timer->mevp == NULL) {
7676
close(timer->fd);
7777
perror("acrn_timer mevent add failed.\n");

‎devicemodel/hw/pci/virtio/vhost.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ vhost_vq_register_eventfd(struct vhost_dev *vdev,
283283
*/
284284
if (is_register) {
285285
vq->mevp = mevent_add(vq->call_fd, EVF_READ,
286-
vhost_vq_notify, vq);
286+
vhost_vq_notify, vq, NULL, NULL);
287287
if (!vq->mevp) {
288288
WPRINTF("mevent_add failed\n");
289289
rc = -1;

‎devicemodel/hw/pci/virtio/virtio_console.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ virtio_console_add_backend(struct virtio_console *console,
679679
if (virtio_console_backend_can_read(be_type)) {
680680
if (isatty(fd)) {
681681
be->evp = mevent_add(fd, EVF_READ,
682-
virtio_console_backend_read, be);
682+
virtio_console_backend_read, be, NULL, NULL);
683683
if (be->evp == NULL) {
684684
WPRINTF(("vtcon: mevent_add failed\n"));
685685
error = -1;

‎devicemodel/hw/pci/virtio/virtio_input.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,7 @@ virtio_input_init(struct vmctx *ctx, struct pci_vdev *dev, char *opts)
634634
goto fail;
635635
}
636636

637-
vi->mevp = mevent_add(vi->fd, EVF_READ, virtio_input_read_event, vi);
637+
vi->mevp = mevent_add(vi->fd, EVF_READ, virtio_input_read_event, vi, NULL, NULL);
638638
if (vi->mevp == NULL) {
639639
WPRINTF(("vtinput: could not register event\n"));
640640
goto fail;

‎devicemodel/hw/pci/virtio/virtio_mei.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,7 +1017,7 @@ vmei_host_client_native_connect(struct vmei_host_client *hclient)
10171017

10181018
/* add READ event into mevent */
10191019
hclient->rx_mevp = mevent_add(hclient->client_fd, EVF_READ,
1020-
vmei_rx_callback, hclient);
1020+
vmei_rx_callback, hclient, NULL, NULL);
10211021
if (!hclient->rx_mevp)
10221022
return MEI_HBM_REJECTED;
10231023

@@ -1951,7 +1951,7 @@ vmei_start(struct virtio_mei *vmei, bool do_rescan)
19511951

19521952
hclient->client_fd = pipefd[0];
19531953
hclient->rx_mevp = mevent_add(hclient->client_fd, EVF_READ,
1954-
vmei_rx_callback, hclient);
1954+
vmei_rx_callback, hclient, NULL, NULL);
19551955
vmei->hbm_fd = pipefd[1];
19561956

19571957
if (do_rescan) {
@@ -2051,7 +2051,7 @@ static int vmei_add_reset_event(struct virtio_mei *vmei)
20512051

20522052
vmei->dev_state_first = true;
20532053
vmei->reset_mevp = mevent_add(dev_state_fd, EVF_READ_ET,
2054-
vmei_reset_callback, vmei);
2054+
vmei_reset_callback, vmei, NULL, NULL);
20552055
if (!vmei->reset_mevp) {
20562056
close(dev_state_fd);
20572057
return -ENOMEM;

‎devicemodel/hw/pci/virtio/virtio_net.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -707,7 +707,7 @@ virtio_net_tap_setup(struct virtio_net *net, char *devname)
707707

708708
if (vhost_fd < 0) {
709709
net->mevp = mevent_add(net->tapfd, EVF_READ,
710-
virtio_net_rx_callback, net);
710+
virtio_net_rx_callback, net, NULL, NULL);
711711
if (net->mevp == NULL) {
712712
WPRINTF(("Could not register event\n"));
713713
close(net->tapfd);

‎devicemodel/hw/uart_core.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,8 +270,7 @@ uart_opentty(struct uart_vdev *uart)
270270
{
271271
ttyopen(&uart->tty);
272272
if (isatty(uart->tty.fd_in)) {
273-
uart->mev = mevent_add(uart->tty.fd_in, EVF_READ,
274-
uart_drain, uart);
273+
uart->mev = mevent_add(uart->tty.fd_in, EVF_READ, uart_drain, uart, NULL, NULL);
275274
assert(uart->mev != NULL);
276275
}
277276
}

‎devicemodel/include/mevent.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ enum ev_type {
4141
struct mevent;
4242

4343
struct mevent *mevent_add(int fd, enum ev_type type,
44-
void (*func)(int, enum ev_type, void *),
45-
void *param);
44+
void (*run)(int, enum ev_type, void *), void *param,
45+
void (*teardown)(void *), void *teardown_param);
4646
int mevent_enable(struct mevent *evp);
4747
int mevent_disable(struct mevent *evp);
4848
int mevent_delete(struct mevent *evp);

0 commit comments

Comments
 (0)