qemu-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]