From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33655) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TXviA-0001Ry-Bk for qemu-devel@nongnu.org; Mon, 12 Nov 2012 10:14:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TXvi7-0003aR-99 for qemu-devel@nongnu.org; Mon, 12 Nov 2012 10:14:06 -0500 Message-ID: <50A11231.8070505@msgid.tls.msk.ru> Date: Mon, 12 Nov 2012 19:13:53 +0400 From: Michael Tokarev MIME-Version: 1.0 References: <1348042111-16143-1-git-send-email-mjt@msgid.tls.msk.ru> <508B9BC7.7040905@msgid.tls.msk.ru> In-Reply-To: <508B9BC7.7040905@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: qemu-stable@nongnu.org, qemu-devel@nongnu.org, Stefano Stabellini Ping^2 ? /mjt 27.10.2012 12:31, Michael Tokarev wrote: > Ping? > > 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; > >