From: Wang Yong <wang.yong155@zte.com.cn>
IOThread uses AioContext event loop and does not run a GMainContext.
Therefore,chardev cannot work in IOThread,such as the chardev is
used for colo-compare packets reception.
This patch makes the IOThread run the GMainContext event loop,
chardev and IOThread can work together.
Signed-off-by: Wang Yong<wang.yong155@zte.com.cn>
Signed-off-by: Wang Guang<wang.guang55@zte.com.cn>
---
include/sysemu/iothread.h | 10 +++++++++
iothread.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+)
diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index e6da1a4..89e913c 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -20,10 +20,19 @@
#define TYPE_IOTHREAD "iothread"
typedef struct {
+ GMainContext *worker_context;
+ GMainLoop *main_loop;
+ GOnce once;
+
+ QEMUBH *bh;
+} GMainOnce;
+
+typedef struct {
Object parent_obj;
QemuThread thread;
AioContext *ctx;
+ GMainOnce thread_gonce;
QemuMutex init_done_lock;
QemuCond init_done_cond; /* is thread initialization done? */
bool stopping;
@@ -41,5 +50,6 @@ typedef struct {
char *iothread_get_id(IOThread *iothread);
AioContext *iothread_get_aio_context(IOThread *iothread);
void iothread_stop_all(void);
+GMainContext *iothread_get_g_main_context(IOThread *iothread);
#endif /* IOTHREAD_H */
diff --git a/iothread.c b/iothread.c
index beeb870..9995eb0 100644
--- a/iothread.c
+++ b/iothread.c
@@ -72,6 +72,9 @@ static int iothread_stop(Object *object, void *opaque)
return 0;
}
iothread->stopping = true;
+ if (iothread->thread_gonce.main_loop) {
+ g_main_loop_quit(iothread->thread_gonce.main_loop);
+ }
aio_notify(iothread->ctx);
qemu_thread_join(&iothread->thread);
return 0;
@@ -125,6 +128,7 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
qemu_mutex_init(&iothread->init_done_lock);
qemu_cond_init(&iothread->init_done_cond);
+ iothread->thread_gonce.once = (GOnce) G_ONCE_INIT;
/* This assumes we are called from a thread with useful CPU affinity for us
* to inherit.
@@ -309,3 +313,53 @@ void iothread_stop_all(void)
object_child_foreach(container, iothread_stop, NULL);
}
+
+static void iothread_g_main_context_bh(void *opaque)
+{
+ GMainOnce *g = opaque;
+
+ qemu_bh_delete(g->bh);
+ g->bh = NULL;
+
+ g_main_context_push_thread_default(g->worker_context);
+
+ g->main_loop = g_main_loop_new(g->worker_context, TRUE);
+ g_main_loop_run(g->main_loop);
+
+ g_main_loop_unref(g->main_loop);
+ g->main_loop = NULL;
+
+ g_main_context_pop_thread_default(g->worker_context);
+ g_main_context_unref(g->worker_context);
+ g->worker_context = NULL;
+}
+
+static gpointer iothread_g_main_context_init(gpointer g_data)
+{
+ AioContext *ctx;
+ IOThread *iothread = (IOThread *)g_data;
+ GMainOnce *g = &iothread->thread_gonce;
+ GSource *source;
+
+ g->worker_context = g_main_context_new();
+
+ ctx = iothread_get_aio_context(iothread);
+ source = aio_get_g_source(ctx);
+ g_source_attach(source, g->worker_context);
+ g_source_unref(source);
+
+ g->bh = aio_bh_new(ctx,
+ iothread_g_main_context_bh, g);
+ qemu_bh_schedule(g->bh);
+
+ return (gpointer) g->worker_context;
+}
+
+GMainContext *iothread_get_g_main_context(IOThread *iothread)
+{
+ GMainOnce *g = &iothread->thread_gonce;
+
+ g_once(&g->once, iothread_g_main_context_init, iothread);
+
+ return (GMainContext *) g->once.retval;
+}
--
1.8.3.1
On Tue, Aug 15, 2017 at 03:28:15PM +0800, Wang yong wrote: > From: Wang Yong <wang.yong155@zte.com.cn> > > IOThread uses AioContext event loop and does not run a GMainContext. > Therefore,chardev cannot work in IOThread,such as the chardev is > used for colo-compare packets reception. > > This patch makes the IOThread run the GMainContext event loop, > chardev and IOThread can work together. > > Signed-off-by: Wang Yong<wang.yong155@zte.com.cn> > Signed-off-by: Wang Guang<wang.guang55@zte.com.cn> There is usually a space between the name and email address. Tools and humans searching git log might not find you if you format the S-o-b line without a space. > --- > include/sysemu/iothread.h | 10 +++++++++ > iothread.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 64 insertions(+) > > diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h > index e6da1a4..89e913c 100644 > --- a/include/sysemu/iothread.h > +++ b/include/sysemu/iothread.h > @@ -20,10 +20,19 @@ > #define TYPE_IOTHREAD "iothread" > > typedef struct { > + GMainContext *worker_context; > + GMainLoop *main_loop; > + GOnce once; > + > + QEMUBH *bh; > +} GMainOnce; Please do not use the glib namespace (G*, g_*) because it's easy to confuse GMainOnce with glib APIs like GOnce. I suggest inlining these fields in IOThread instead of defining a new struct. > + > +typedef struct { > Object parent_obj; > > QemuThread thread; > AioContext *ctx; > + GMainOnce thread_gonce; > QemuMutex init_done_lock; > QemuCond init_done_cond; /* is thread initialization done? */ > bool stopping; > @@ -41,5 +50,6 @@ typedef struct { > char *iothread_get_id(IOThread *iothread); > AioContext *iothread_get_aio_context(IOThread *iothread); > void iothread_stop_all(void); > +GMainContext *iothread_get_g_main_context(IOThread *iothread); > > #endif /* IOTHREAD_H */ > diff --git a/iothread.c b/iothread.c > index beeb870..9995eb0 100644 > --- a/iothread.c > +++ b/iothread.c > @@ -72,6 +72,9 @@ static int iothread_stop(Object *object, void *opaque) > return 0; > } > iothread->stopping = true; > + if (iothread->thread_gonce.main_loop) { > + g_main_loop_quit(iothread->thread_gonce.main_loop); > + } > aio_notify(iothread->ctx); > qemu_thread_join(&iothread->thread); > return 0; > @@ -125,6 +128,7 @@ static void iothread_complete(UserCreatable *obj, Error **errp) > > qemu_mutex_init(&iothread->init_done_lock); > qemu_cond_init(&iothread->init_done_cond); > + iothread->thread_gonce.once = (GOnce) G_ONCE_INIT; > > /* This assumes we are called from a thread with useful CPU affinity for us > * to inherit. > @@ -309,3 +313,53 @@ void iothread_stop_all(void) > > object_child_foreach(container, iothread_stop, NULL); > } > + > +static void iothread_g_main_context_bh(void *opaque) > +{ > + GMainOnce *g = opaque; > + > + qemu_bh_delete(g->bh); > + g->bh = NULL; > + > + g_main_context_push_thread_default(g->worker_context); > + > + g->main_loop = g_main_loop_new(g->worker_context, TRUE); > + g_main_loop_run(g->main_loop); Running the glib main loop from a BH function means deleted BHs can never be freed in aio-posix.c:aio_bh_poll(): if (qemu_lockcnt_dec_if_lock(&ctx->list_lock)) { bhp = &ctx->first_bh; while (*bhp) { bh = *bhp; if (bh->deleted && !bh->scheduled) { *bhp = bh->next; g_free(bh); } else { bhp = &bh->next; } } qemu_lockcnt_inc_and_unlock(&ctx->list_lock); } ctx->list_lock is held by iothread_run() -> aio_poll() so qemu_lockcnt_dec_if_lock() always returns false. Instead of running the glib event loop inside aio_poll() I would modify iothread_run(): while (!atomic_read(&iothread->stopping)) { if (atomic_read(&iothread->use_glib_event_loop)) { iothread_glib_event_loop(); break; } aio_poll(iothread->ctx, true); } > + > + g_main_loop_unref(g->main_loop); > + g->main_loop = NULL; > + > + g_main_context_pop_thread_default(g->worker_context); > + g_main_context_unref(g->worker_context); > + g->worker_context = NULL; > +} > + > +static gpointer iothread_g_main_context_init(gpointer g_data) > +{ > + AioContext *ctx; > + IOThread *iothread = (IOThread *)g_data; No cast is necessary from void * to another pointer type in C. Please remove the cast. > + GMainOnce *g = &iothread->thread_gonce; > + GSource *source; > + > + g->worker_context = g_main_context_new(); > + > + ctx = iothread_get_aio_context(iothread); > + source = aio_get_g_source(ctx); > + g_source_attach(source, g->worker_context); > + g_source_unref(source); > + > + g->bh = aio_bh_new(ctx, > + iothread_g_main_context_bh, g); > + qemu_bh_schedule(g->bh); > + > + return (gpointer) g->worker_context; No cast is necessary from a pointer type to void * in C. Please remove the cast. > +} > + > +GMainContext *iothread_get_g_main_context(IOThread *iothread) > +{ > + GMainOnce *g = &iothread->thread_gonce; > + > + g_once(&g->once, iothread_g_main_context_init, iothread); > + > + return (GMainContext *) g->once.retval; No cast is necessary from a pointer type to void * in C. Please remove the cast. A dangling pointer is returned if this this function is called after iothread_g_main_context_bh() has terminated. It would be safer to return g->worker_context instead of g->once.retval.
On 16/08/2017 11:52, Stefan Hajnoczi wrote: > > while (!atomic_read(&iothread->stopping)) { > if (atomic_read(&iothread->use_glib_event_loop)) { > iothread_glib_event_loop(); > break; > } > > aio_poll(iothread->ctx, true); > } Alternatively: while (!atomic_read(&iothread->stopping)) { aio_poll(iothread->ctx, true); } if (atomic_read(&iothread->worker_context)) { iothread_glib_event_loop(); } and you can use this to enter the glib event loop: iothread->stopping = true; aio_notify(iothread->ctx); (similar to iothread_stop). Thanks, Paolo
© 2016 - 2024 Red Hat, Inc.