[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-stable] [PATCH] Revert "serial: fix retry logic"
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [Qemu-stable] [PATCH] Revert "serial: fix retry logic" |
Date: |
Thu, 24 Jan 2013 11:03:36 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130105 Thunderbird/17.0.2 |
Am 22.01.2013 12:01, schrieb Michael Tokarev:
> Ping^3?
>
> This issue is still present in qemu 1.3 and current git (1.4-tobe)
> versions,
> and the said commit is still revertable, and reverting it still fixes the
> problem...
>
> I wonder why only debian users suffer from this problem ;)
It was reported for openSUSE as well [1], but Anthony promised me it
would get reverted for 1.3... I admit, I simply assumed it to be fixed
with v1.3.0.
Regards,
Andreas
[1] https://bugzilla.novell.com/show_bug.cgi?id=779727
>
> Thanks,
>
> /mjt
>
> 12.11.2012 19:13, Michael Tokarev wrote:
>> 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 <address@hidden>
>>>> Signed-off-by: Anthony Liguori <address@hidden>
>>>>
>>>> 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 <address@hidden>
>>>> ---
>>>> 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;
>>>
>>>
>>
>>
>
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg