Re: [Qemu-devel] [PULL 3/5] serial: change retry logic to avoid concurrency

[Paolo Bonzini]

Il 24/07/2014 14:57, Pavel Hrdina ha scritto:
> On 14.7.2014 17:49, Paolo Bonzini wrote:
>> From: Kirill Batuzov <address@hidden>
>>
>> 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 <address@hidden>
>> Signed-off-by: Paolo Bonzini <address@hidden>
> 
> 
> 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