From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55987) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UNqFR-0005Mp-IV for qemu-devel@nongnu.org; Thu, 04 Apr 2013 15:55:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UNqFI-0005A9-Te for qemu-devel@nongnu.org; Thu, 04 Apr 2013 15:55:01 -0400 Received: from e23smtp06.au.ibm.com ([202.81.31.148]:59760) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UNqFI-00059i-C4 for qemu-devel@nongnu.org; Thu, 04 Apr 2013 15:54:52 -0400 Received: from /spool/local by e23smtp06.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 5 Apr 2013 05:49:18 +1000 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [9.190.234.120]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id 7B5EA2BB0023 for ; Fri, 5 Apr 2013 06:54:46 +1100 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r34JfYP88782258 for ; Fri, 5 Apr 2013 06:41:34 +1100 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r34Jsj6b020781 for ; Fri, 5 Apr 2013 06:54:46 +1100 From: Anthony Liguori In-Reply-To: <515DCD1E.6050506@redhat.com> References: <1364893452-10604-1-git-send-email-peter.crosthwaite@xilinx.com> <515ABCD1.2070008@redhat.com> <82877867.1048290.1364970927865.JavaMail.root@redhat.com> <515D135B.4020003@redhat.com> <87mwteteot.fsf@codemonkey.ws> <515DCD1E.6050506@redhat.com> Date: Thu, 04 Apr 2013 14:54:41 -0500 Message-ID: <87ppyaysu6.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Amit Shah , Peter Crosthwaite , qemu-devel@nongnu.org, gson@gson.org Paolo Bonzini writes: > Il 04/04/2013 18:59, Anthony Liguori ha scritto: >>> > >>> > The right thing to use would be g_source_add_child_source() and >>> > g_source_remove_child_source(), but that is only present since glib 2.28 >>> > and we currently require 2.12 (2.20 on Windows). >> I don't think a child source fixes the problem. The backend definitely >> has work to do. What we don't know is whether the front end is capable >> of processing the work. >> >> The problem here is that we use polling on the front-end. IOW: >> >> 1) Char backend has data and is ready to write. >> 2) Asks front end if it can write >> 3) Front end says no >> 4) Goto (1) > > What we used to do is this, however: > > 1) Char backend asks front end if it can write > 2) Front end says no You're missing a step here. Previously we would drop the data silently. > 3) poll() goes on without char backend's descriptor Which is what enabled this. > 4) Goto (1), then: > 5) Front end is now available, calls qemu_notify_event() This still exists FWIW. > 6) Char backend asks front end if it can write > 7) Front end says yes > > No busy waiting here. Though we don't really do (5) everywhere, because > for example this patch slipped through the cracks: Ack. > http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg03208.html (but > for the monitor and every consumer running in iothread context it's ok). > > You could achieve the same thing by making the io watch a child source > of the QEMU wrapper. All the wrapper does is add/remove the io watch > source from its children, depending on can_read. I don't understand how this solves the problem. There aren't two sources. There is only a single source (the GIOChannel source). We override the function pointers to basically do monkey patching of the GSource. If I understand your suggestion, it's to add the source as a child of itself. > > Perhaps it's possible without child sources though, by setting the > callbacks on the glib source directly. That's what we're doing... src = g_io_create_watch(channel, G_IO_IN | G_IO_ERR | G_IO_HUP); g_source_set_funcs(src, &io_watch_poll_funcs); g_source_set_callback(src, (GSourceFunc)fd_read, user_data, NULL); There is only one GSource active here. Regards, Anthony Liguori > Paolo