From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44714) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XAJiz-0006PZ-Bf for qemu-devel@nongnu.org; Thu, 24 Jul 2014 10:10:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XAJiq-0000O2-2m for qemu-devel@nongnu.org; Thu, 24 Jul 2014 10:10:25 -0400 Received: from mail-qa0-x231.google.com ([2607:f8b0:400d:c00::231]:46632) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XAJip-0000Nq-SK for qemu-devel@nongnu.org; Thu, 24 Jul 2014 10:10:15 -0400 Received: by mail-qa0-f49.google.com with SMTP id dc16so3000505qab.36 for ; Thu, 24 Jul 2014 07:10:15 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <53D113C4.50405@redhat.com> Date: Thu, 24 Jul 2014 16:10:12 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1405352968-3155-1-git-send-email-pbonzini@redhat.com> <1405352968-3155-4-git-send-email-pbonzini@redhat.com> <53D102D1.3050704@redhat.com> In-Reply-To: <53D102D1.3050704@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 3/5] serial: change retry logic to avoid concurrency List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Hrdina , qemu-devel@nongnu.org Cc: Kirill Batuzov Il 24/07/2014 14:57, Pavel Hrdina ha scritto: > On 14.7.2014 17:49, Paolo Bonzini wrote: >> From: Kirill Batuzov >> >> Whenever serial_xmit fails to transmit a byte it adds a watch that would >> call it again when the "line" becomes ready. This results in a retry >> chain: >> serial_xmit -> add_watch -> serial_xmit >> Each chain is able to transmit one character, and for every character >> passed to serial by the guest driver a new chain is spawned. >> >> The problem lays with the fact that a new chain is spawned even when >> there is one already waiting on the watch. So there can be several retry >> chains waiting concurrently on one "line". Every chain tries to transmit >> current character, so character order is not messed up. But also every >> chain increases retry counter (tsr_retry). If there are enough >> concurrent chains this counter will hit MAX_XMIT_RETRY value and >> the character will be dropped. >> >> To reproduce this bug you need to feed serial output to some program >> consuming it slowly enough. A python script from bug #1335444 >> description is an example of such program. >> >> This commit changes retry logic in the following way to avoid >> concurrency: instead of spawning a new chain for each character being >> transmitted spawn only one and make it transmit characters until FIFO is >> empty. >> >> The change consists of two parts: >> - add a do {} while () loop in serial_xmit (diff is a bit erratic >> for this part, diff -w will show actual change), >> - do not call serial_xmit from serial_ioport_write if there is one >> waiting on the watch already. >> >> This should fix another issue causing bug #1335444. >> >> Signed-off-by: Kirill Batuzov >> Signed-off-by: Paolo Bonzini > > > Hi, this commit introduced a regression with serial console. The issue > is that if you start a guest with serial console: > > -chardev pty,id=charserial0 -device > isa-serial,chardev=charserial0,id=serial0 > > the guest hang during boot for a long time and I'm not even sure it > it ever boot up. The last message printed out by kernel is: > > "[ 0.000000] console [tty0] enabled" > > If you connect to the serial console than the guest continue > booting immediately. Interesting, the patch is actually doing exactly what it was meant to do, but in the wrong circumstances. :) The bug is that G_IO_HUP is not supported by ptys. I have just sent a patch. Paolo