From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41419) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTdTL-0006sM-JN for qemu-devel@nongnu.org; Tue, 08 Oct 2013 16:01:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VTdTF-00067D-Iz for qemu-devel@nongnu.org; Tue, 08 Oct 2013 16:01:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:8253) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTdTF-000670-AG for qemu-devel@nongnu.org; Tue, 08 Oct 2013 16:01:29 -0400 Message-ID: <52546492.7000809@redhat.com> Date: Tue, 08 Oct 2013 22:01:22 +0200 From: Hans de Goede MIME-Version: 1.0 References: <1381259403-7386-1-git-send-email-hdegoede@redhat.com> <0863CECC-BE13-4B55-8FFF-593D942A5195@alex.org.uk> In-Reply-To: <0863CECC-BE13-4B55-8FFF-593D942A5195@alex.org.uk> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Bligh Cc: qemu-devel@nongnu.org Hi, On 10/08/2013 09:48 PM, Alex Bligh wrote: > > On 8 Oct 2013, at 20:10, Hans de Goede wrote: > >> I noticed today that current qemu master would hang as soon as Xorg starts in >> the guest when using qxl + a Linux guest. This message would be printed: >> main-loop: WARNING: I/O thread spun for 1000 iterations >> >> And from then on the guest hangs and qemu consumes 100% cpu, bisecting pointed >> out commit 7b595f35d89d73bc69c35bf3980a89c420e8a44b: >> "aio / timers: Convert mainloop to use timeout" >> >> After looking at that commit I had a hunch the problem might be blocking >> main_loop_wait calls being turned into non-blocking ones (and thus never >> releasing the io-lock), a debug printf confirmed this was happening at >> the moment of the hang, so I wrote this patch which fixes the hang for me >> and seems like a good idea in general. >> >> Signed-off-by: Hans de Goede >> --- >> main-loop.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/main-loop.c b/main-loop.c >> index c3c9c28..921c939 100644 >> --- a/main-loop.c >> +++ b/main-loop.c >> @@ -480,6 +480,11 @@ int main_loop_wait(int nonblocking) >> timerlistgroup_deadline_ns( >> &main_loop_tlg)); >> >> + /* When not non-blocking always allow io-threads to acquire the lock */ >> + if (timeout != 0 && timeout_ns == 0) { >> + timeout_ns = 1; >> + } >> + >> ret = os_host_main_loop_wait(timeout_ns); >> qemu_iohandler_poll(gpollfds, ret); >> #ifdef CONFIG_SLIRP > > I /think/ you might mean "if (!blocking && timeout_ns == 0)" > as timeout can be zero on a blocking call at this stage (i.e. > when there is a timer which has already expired. timeout does not get modified, except by the slirp-stuff, which won't ever set it to 0 I think. But your right writing if (!nonblocking && timeout_ns == 0) would be much clearer, so I'll do that in v2 of the patch. Regards, Hans