From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54614) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFcJ2-0006Sm-9T for qemu-devel@nongnu.org; Wed, 22 Jun 2016 03:10:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bFcIw-0003eo-Uf for qemu-devel@nongnu.org; Wed, 22 Jun 2016 03:10:36 -0400 Received: from [59.151.112.132] (port=53645 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFcIv-0003cp-0t for qemu-devel@nongnu.org; Wed, 22 Jun 2016 03:10:30 -0400 References: <1464180653-12637-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> <574BB14F.6050303@redhat.com> <574D0AEF.7080804@cn.fujitsu.com> <574D2C3B.4000603@redhat.com> <574D4B24.3010603@cn.fujitsu.com> <574D7040.6090602@redhat.com> <57635FA4.60907@cn.fujitsu.com> <57675D18.6030207@redhat.com> <576762A5.9000906@cn.fujitsu.com> <57677E09.6090004@redhat.com> <576A2F03.90901@cn.fujitsu.com> <576A34CD.4080704@redhat.com> From: Zhang Chen Message-ID: <576A39E9.8070205@cn.fujitsu.com> Date: Wed, 22 Jun 2016 15:10:33 +0800 MIME-Version: 1.0 In-Reply-To: <576A34CD.4080704@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH V4 0/4] Introduce COLO-compare List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu devel Cc: "eddie . dong" , zhanghailiang , Li Zhijian , "Dr . David Alan Gilbert" On 06/22/2016 02:48 PM, Jason Wang wrote: > > > On 2016年06月22日 14:24, Zhang Chen wrote: >> >> >> On 06/20/2016 01:24 PM, Jason Wang wrote: >>> >>> >>> On 2016年06月20日 11:27, Zhang Chen wrote: >>>> >>>> >>>> On 06/20/2016 11:03 AM, Jason Wang wrote: >>>>> >>>>> >>>>> On 2016年06月17日 10:25, Zhang Chen wrote: >>>>>> Hi~ jason. >>>>>> >>>>>> I tried a lot of ways to make it run in compare thread, but it >>>>>> not work. >>>>>> >>>>>> Because that: >>>>>> void g_main_context_push_thread_default (GMainContext *context); >>>>>> Acquires context and sets it as the thread-default context for >>>>>> the current thread. >>>>>> >>>>>> So, this function g_main_context_push_thread_default() just can >>>>>> set thread-default context, >>>>>> not the global default context. and I can't find a function to >>>>>> set global default context in glib. >>>>>> The qemu's QIOChannel uses g_source_attach(source, NULL); to >>>>>> attach GSource. >>>>>> g_source_attach (GSource *source, >>>>>> GMainContext *context); >>>>>> >>>>>> context >>>>>> a GMainContext (if NULL, the default context will be used). >>>>>> >>>>>> But he not say "the default context" is global default context or >>>>>> thread-default context. >>>>>> So I read glib codes find that: >>>>>> guint >>>>>> g_source_attach (GSource *source, >>>>>> GMainContext *context) >>>>>> { >>>>>> ..... >>>>>> if (!context) >>>>>> context = g_main_context_default (); >>>>>> ..... >>>>>> } >>>>>> >>>>>> g_main_context_default () >>>>>> Returns the global default main context. >>>>>> >>>>>> So...I think we can't make qemu_chr_add_handlers() run in colo >>>>>> thread in this way. >>>>>> Do you have any comments about that? >>>>> >>>>> How about (like I suggest previously) just use >>>>> >>>>> g_souce_attach(x, g_main_context_get_thread_default()); >>>>> >>>>> in qemu's QIOChannel code? >>>>> >>>>> >>>>> >>>> >>>> I feel it seems can work, and will try it. >>>> but I don't know how this change in qemu's QIOChannel code >>>> will affect the other code. needs other people confirm it? >>> >>> Yes, we need convince them. I believe we don't do this in the past >>> is because there's no users. >>> >>>> If no affect and works good,I will send a independent patch >>>> for this. >>>> >>> >>> Right, if it works, please send a RFC and explain why it is needed. >>> >> >> Hi~ Jason. >> It work! I have send a patch for this and signed-off-by you: >> >> [RFC PATCH] Change g_source_attach(xx, NULL) to g_souce_attach(xx, >> g_main_context_get_thread_default()) >> >> But I find a problem when fix colo-compare. >> if we want poll and handle chardev in compare thread, >> we must use g_main_loop_run(compare_loop). and this function >> block here, we can not run other job in compare thread... >> like that: >> >> static void *colo_compare_thread(void *opaque) >> { >> ....... >> >> worker_context = g_main_context_new(); >> g_main_context_push_thread_default(worker_context); >> qemu_chr_add_handlers(s->chr_pri_in, compare_chr_can_read, >> compare_pri_chr_in, NULL, s); >> qemu_chr_add_handlers(s->chr_sec_in, compare_chr_can_read, >> compare_sec_chr_in, NULL, s); >> >> compare_loop = g_main_loop_new(worker_context, FALSE); >> >> g_main_loop_run(compare_loop); >> >> //will block here... >> >> while (s->thread_status == COMPARE_THREAD_RUNNING) { >> qemu_event_wait(&s->event); >> qemu_event_reset(&s->event); >> rcu_read_lock(); >> g_queue_foreach(&s->conn_list, colo_compare_connection, s); >> rcu_read_unlock(); >> } >> >> ..... >> >> >> Have any comments? > > Why not just trigger the comparing in compare_{pri|sec}_chr_in() ? Make sence! Thanks Zhang Chen > >> >> >> Thanks >> Zhang Chen >> >> >>> Thanks >>> >>>> >>>> Thanks >>>> Zhang Chen >>> >>> >>> >>> . >>> >> > > > > . > -- Thanks zhangchen