|
From: | Zhang Chen |
Subject: | Re: [Qemu-devel] [RFC PATCH V2] qemu-char: Fix context for g_source_attach() |
Date: | Mon, 18 Jul 2016 09:55:23 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 |
Hi~ All~~ Can you give me some feedback for this patch? We need more comments~~ COLO project depend on this patch to work. Because this patch colo-compare can make handler of qemu_chr_add_handlers()run in compare thread, reduce workload of main_loop in network busy situation.
This idea from Jason. Thanks Zhang Chen On 07/11/2016 10:12 AM, Zhang Chen wrote:
On 07/08/2016 10:27 PM, Paolo Bonzini wrote:On 08/07/2016 10:54, Daniel P. Berrange wrote:On Fri, Jul 08, 2016 at 09:48:23AM +0800, Fam Zheng wrote:I've no real objection to this proposed patch, though it is fairly pointless to take it now without seeing any following patch that actually makes useOn Wed, 06/22 18:49, Zhang Chen wrote:IIRC this opens a gate for your special thread (COLO compare thread?) to useWe want to poll and handle chardev in another thread other than main loop. But qemu_chr_add_handlers() can only work for global default context other than thread default context. So we use g_source_attach(xx, g_main_context_get_thread_default()) replace g_source_attach(xx, NULL) to attach g_source. Comments from jason. Signed-off-by: Zhang Chen <address@hidden> Signed-off-by: Jason Wang <address@hidden> --- io/channel.c | 2 +- qemu-char.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/io/channel.c b/io/channel.c index 692eb17..cd25677 100644 --- a/io/channel.c +++ b/io/channel.c @@ -146,7 +146,7 @@ guint qio_channel_add_watch(QIOChannel *ioc,g_source_set_callback(source, (GSourceFunc)func, user_data, notify);- id = g_source_attach(source, NULL);+ id = g_source_attach(source, g_main_context_get_thread_default());g_source_unref(source); return id; diff --git a/qemu-char.c b/qemu-char.c index 84f49ac..4340457 100644 --- a/qemu-char.c +++ b/qemu-char.c@@ -859,7 +859,7 @@ static gboolean io_watch_poll_prepare(GSource *source, gint *timeout_)iwp->src = qio_channel_create_watch( iwp->ioc, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL);g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL);- g_source_attach(iwp->src, NULL);+ g_source_attach(iwp->src, g_main_context_get_thread_default());} else { g_source_destroy(iwp->src); g_source_unref(iwp->src); @@ -918,7 +918,7 @@ static guint io_add_watch_poll(QIOChannel *ioc, iwp->fd_read = (GSourceFunc) fd_read; iwp->src = NULL; - tag = g_source_attach(&iwp->parent, NULL);+ tag = g_source_attach(&iwp->parent, g_main_context_get_thread_default());g_source_unref(&iwp->parent); return tag; }@@ -3982,7 +3982,7 @@ int qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,}g_source_set_callback(src, (GSourceFunc)func, user_data, NULL);- tag = g_source_attach(src, NULL); + tag = g_source_attach(src, g_main_context_get_thread_default()); g_source_unref(src); return tag; --QIOChannel.of this added feature.I agree.Should I move this patch to the "[RFC PATCH V5 0/4] Introduce COLO-compare"patch set? that can show how it works. you can see this patch for how to use: http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg06754.html
In colo_compare_thread()
I think in the long run it is better to think about allowing integrating QIO to AioContext, to support its usage outside main loop. Given how opaque GSource is, I'm not sure how feasible that is, or how useful it will be. Anyway wePersonally I think it is preferable to stick as close to the standard GSource model as possible, as that's widely used & well understood API, compared to theshould definitely hear more opinions from Daniel and Paolo.QEMU specific AioContext.AioContext is more optimized for the case where the callbacks are static. In general this is not the case for qemu-char.c.I don't sure AioContext can do this job good, but I think we can make qemu more flexible to do same one job. All roads lead to Rome. Thanks Zhang ChenPaolo .
-- Thanks zhangchen
[Prev in Thread] | Current Thread | [Next in Thread] |