From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55900) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TS1nR-00063X-HO for qemu-devel@nongnu.org; Sat, 27 Oct 2012 04:31:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TS1nP-0008DY-0S for qemu-devel@nongnu.org; Sat, 27 Oct 2012 04:31:09 -0400 Message-ID: <508B9BC7.7040905@msgid.tls.msk.ru> Date: Sat, 27 Oct 2012 12:31:03 +0400 From: Michael Tokarev MIME-Version: 1.0 References: <1348042111-16143-1-git-send-email-mjt@msgid.tls.msk.ru> In-Reply-To: <1348042111-16143-1-git-send-email-mjt@msgid.tls.msk.ru> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Revert "serial: fix retry logic" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Stefano Stabellini , qemu-devel@nongnu.org, qemu-stable@nongnu.org Ping? /mjt On 19.09.2012 12:08, Michael Tokarev wrote: > This reverts commit 67c5322d7000fd105a926eec44bc1765b7d70bdd: > > I'm not sure if the retry logic has ever worked when not using FIFO mode. I > found this while writing a test case although code inspection confirms it is > definitely broken. > > The TSR retry logic will never actually happen because it is guarded by an > 'if (s->tsr_rety > 0)' but this is the only place that can ever make the > variable greater than zero. That effectively makes the retry logic an 'if (0) > > I believe this is a typo and the intention was >= 0. Once this is fixed thoug > I see double transmits with my test case. This is because in the non FIFO > case, serial_xmit may get invoked while LSR.THRE is still high because the > character was processed but the retransmit timer was still active. > > We can handle this by simply checking for LSR.THRE and returning early. It's > possible that the FIFO paths also need some attention. > > Cc: Stefano Stabellini > Signed-off-by: Anthony Liguori > > Even if the previous logic was never worked, new logic breaks stuff - > namely, > > qemu -enable-kvm -nographic -kernel /boot/vmlinuz-$(uname -r) -append console=ttyS0 -serial pty > > the above command will cause the virtual machine to stuck at startup > using 100% CPU till one connects to the pty and sends any char to it. > > Note this is rather typical invocation for various headless virtual > machines by libvirt. > > So revert this change for now, till a better solution will be found. > > Signed-off-by: Michael Tokarev > --- > hw/serial.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/hw/serial.c b/hw/serial.c > index a421d1e..df54de2 100644 > --- a/hw/serial.c > +++ b/hw/serial.c > @@ -327,8 +327,6 @@ static void serial_xmit(void *opaque) > s->tsr = fifo_get(s,XMIT_FIFO); > if (!s->xmit_fifo.count) > s->lsr |= UART_LSR_THRE; > - } else if ((s->lsr & UART_LSR_THRE)) { > - return; > } else { > s->tsr = s->thr; > s->lsr |= UART_LSR_THRE; > @@ -340,7 +338,7 @@ static void serial_xmit(void *opaque) > /* in loopback mode, say that we just received a char */ > serial_receive1(s, &s->tsr, 1); > } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) { > - if ((s->tsr_retry >= 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) { > + if ((s->tsr_retry > 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) { > s->tsr_retry++; > qemu_mod_timer(s->transmit_timer, new_xmit_ts + s->char_transmit_time); > return;