[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 2/4] chardev: make qemu_chr_fe_set_handlers() contex
From: |
Marc-André Lureau |
Subject: |
[Qemu-devel] [PATCH 2/4] chardev: make qemu_chr_fe_set_handlers() context switching safer |
Date: |
Wed, 20 Feb 2019 17:06:26 +0100 |
qemu_chr_fe_set_handlers() may switch the context of various
sources. In order to prevent dispatch races from different threads,
let's acquire or freeze the context, do all the source switches, and
then release/resume the contexts. This should help to make context
switching safer.
Signed-off-by: Marc-André Lureau <address@hidden>
---
include/chardev/char-fe.h | 23 +++++++++
chardev/char-fe.c | 103 +++++++++++++++++++++++++++++++++-----
chardev/char-mux.c | 14 +++---
3 files changed, 121 insertions(+), 19 deletions(-)
diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index aa1b864ccd..4051435a1c 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -84,6 +84,14 @@ bool qemu_chr_fe_backend_open(CharBackend *be);
* Set the front end char handlers. The front end takes the focus if
* any of the handler is non-NULL.
*
+ * A chardev may have multiple main loop sources. In order to prevent
+ * races when switching contexts, the function will temporarily block
+ * the contexts before the source switch to prevent them from
+ * dispatching in different threads concurrently.
+ *
+ * The current and the new @context must be acquirable or
+ * running & dispatched in a loop (the function will hang otherwise).
+ *
* Without associated Chardev, nothing is changed.
*/
void qemu_chr_fe_set_handlers_full(CharBackend *b,
@@ -110,6 +118,21 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
GMainContext *context,
bool set_open);
+/**
+ * qemu_chr_fe_set_handlers_internal:
+ *
+ * Same as qemu_chr_fe_set_handlers(), without context freezing.
+ */
+void qemu_chr_fe_set_handlers_internal(CharBackend *b,
+ IOCanReadHandler *fd_can_read,
+ IOReadHandler *fd_read,
+ IOEventHandler *fd_event,
+ BackendChangeHandler *be_change,
+ void *opaque,
+ GMainContext *context,
+ bool set_open,
+ bool sync_state);
+
/**
* qemu_chr_fe_take_focus:
*
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index f3530a90e6..90cd7db007 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -246,15 +246,67 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del)
}
}
-void qemu_chr_fe_set_handlers_full(CharBackend *b,
- IOCanReadHandler *fd_can_read,
- IOReadHandler *fd_read,
- IOEventHandler *fd_event,
- BackendChangeHandler *be_change,
- void *opaque,
- GMainContext *context,
- bool set_open,
- bool sync_state)
+struct MainContextWait {
+ QemuCond cond;
+ QemuMutex lock;
+};
+
+static gboolean
+main_context_wait_cb(gpointer user_data)
+{
+ struct MainContextWait *w = user_data;
+
+ qemu_mutex_lock(&w->lock);
+ qemu_cond_signal(&w->cond);
+ /* wait until switching is over */
+ qemu_cond_wait(&w->cond, &w->lock);
+ qemu_mutex_unlock(&w->lock);
+
+ qemu_mutex_destroy(&w->lock);
+ qemu_cond_destroy(&w->cond);
+ g_free(w);
+
+ return G_SOURCE_REMOVE;
+}
+
+static void
+main_context_wait(struct MainContextWait **wait, GMainContext *ctxt)
+{
+ struct MainContextWait *w = NULL;
+
+ if (!g_main_context_acquire(ctxt)) {
+ w = g_new0(struct MainContextWait, 1);
+ qemu_mutex_init(&w->lock);
+ qemu_cond_init(&w->cond);
+ qemu_mutex_lock(&w->lock);
+ g_main_context_invoke(ctxt, main_context_wait_cb, w);
+ /* wait for the context to freeze */
+ qemu_cond_wait(&w->cond, &w->lock);
+ qemu_mutex_unlock(&w->lock);
+ }
+
+ *wait = w;
+}
+
+static void
+main_context_resume(struct MainContextWait *wait, GMainContext *ctxt)
+{
+ if (wait) {
+ qemu_cond_signal(&wait->cond);
+ } else {
+ g_main_context_release(ctxt);
+ }
+}
+
+void qemu_chr_fe_set_handlers_internal(CharBackend *b,
+ IOCanReadHandler *fd_can_read,
+ IOReadHandler *fd_read,
+ IOEventHandler *fd_event,
+ BackendChangeHandler *be_change,
+ void *opaque,
+ GMainContext *new_context,
+ bool set_open,
+ bool sync_state)
{
Chardev *s;
int fe_open;
@@ -276,7 +328,7 @@ void qemu_chr_fe_set_handlers_full(CharBackend *b,
b->chr_be_change = be_change;
b->opaque = opaque;
- qemu_chr_be_update_read_handlers(s, context);
+ qemu_chr_be_update_read_handlers(s, new_context);
if (set_open) {
qemu_chr_fe_set_open(b, fe_open);
@@ -292,6 +344,34 @@ void qemu_chr_fe_set_handlers_full(CharBackend *b,
}
}
+void qemu_chr_fe_set_handlers_full(CharBackend *b,
+ IOCanReadHandler *fd_can_read,
+ IOReadHandler *fd_read,
+ IOEventHandler *fd_event,
+ BackendChangeHandler *be_change,
+ void *opaque,
+ GMainContext *new_context,
+ bool set_open,
+ bool sync_state)
+{
+ GMainContext *old_context = b->chr->gcontext;
+ struct MainContextWait *old_ctxt_wait, *new_ctxt_wait;
+
+ main_context_wait(&old_ctxt_wait, old_context);
+ if (old_context != new_context) {
+ main_context_wait(&new_ctxt_wait, new_context);
+ }
+
+ qemu_chr_fe_set_handlers_internal(b, fd_can_read, fd_read, fd_event,
+ be_change, opaque, new_context,
+ set_open, sync_state);
+
+ main_context_resume(old_ctxt_wait, old_context);
+ if (old_context != new_context) {
+ main_context_resume(new_ctxt_wait, new_context);
+ }
+}
+
void qemu_chr_fe_set_handlers(CharBackend *b,
IOCanReadHandler *fd_can_read,
IOReadHandler *fd_read,
@@ -302,8 +382,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
bool set_open)
{
qemu_chr_fe_set_handlers_full(b, fd_can_read, fd_read, fd_event, be_change,
- opaque, context, set_open,
- true);
+ opaque, context, set_open, true);
}
void qemu_chr_fe_take_focus(CharBackend *b)
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 23aa82125d..9830cc4b37 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -283,13 +283,13 @@ static void mux_chr_update_read_handlers(Chardev *chr)
MuxChardev *d = MUX_CHARDEV(chr);
/* Fix up the real driver with mux routines */
- qemu_chr_fe_set_handlers_full(&d->chr,
- mux_chr_can_read,
- mux_chr_read,
- mux_chr_event,
- NULL,
- chr,
- chr->gcontext, true, false);
+ qemu_chr_fe_set_handlers_internal(&d->chr,
+ mux_chr_can_read,
+ mux_chr_read,
+ mux_chr_event,
+ NULL,
+ chr,
+ chr->gcontext, true, false);
}
void mux_set_focus(Chardev *chr, int focus)
--
2.21.0.rc1
[Qemu-devel] [PATCH 3/4] monitor: set the chardev context from the main context/thread, Marc-André Lureau, 2019/02/20
[Qemu-devel] [PATCH 4/4] char-socket: restart the reconnect timer to switch context, Marc-André Lureau, 2019/02/20
Re: [Qemu-devel] [PATCH 0/4] RFC: make chardev context switching safer, Peter Xu, 2019/02/21