Re: [PATCH] fix reading from character devices

[Jan Kiszka <jan.kiszka@siemens.com>]

Dor Laor wrote:
> On Mon, 2008-01-21 at 12:50 +0200, Avi Kivity wrote:
>> Dor Laor wrote:
>>> On Mon, 2008-01-21 at 12:13 +0200, Avi Kivity wrote:
>>>   
>>>> Jan Kiszka wrote:
>>>>     
>>>>> Hi Avi,
>>>>>
>>>>> commit "kvm: qemu: consume all pending I/O in I/O loop"
>>>>> (8ab8bb09f1115b9bf733f885cc92b6c63d83f420) broke reading data bursts
>>>>> from serial devices (and maybe from other character devices as well) by
>>>>> guests. Reason: serial devices do input flow control via fd_read_poll,
>>>>> but qemu now ignores this fact by pushing all data into the virtual
>>>>> device as soon as it is available.
>>>>>
>>>>> Patch below is not really nice (just as the whole internal virtual I/O
>>>>> interface at the moment, IMHO), but it re-enables the serial ports for
>>>>> now.
>>>>>
>>>>>   
>>>>>       
>>>> I'm worried that it will break Dor's hack that speeds up virtio.  Dor?
>>>>
>>>>     
>>> It should be fine. Tap device without my hack has fd_read_poll null and
>>> I hacked it to have a handler that returns false if virtio is used and
>>> true otherwise. 
>>> So Jan's patch should set 'more' to 1 and it will be like before.
>>>
>>>   
>> But if virtio is used with this patch, it won't set 'more' to 1.  Will 
>> virtio handle it or will throughput drop to be related to whatever 
>> timers we have set up?
>>
> 
> Virtio+tap with the performance hack does it's own select on the related
> fd. Virtio+user will remain as before.

I think this got lost somehow. Find refreshed patch below. Avi, could
you apply it kvm-userland to fix the still existent regression?

Jan

---
 qemu/vl.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: b/qemu/vl.c
===================================================================
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -7786,7 +7786,10 @@ void main_loop_wait(int timeout)
         for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) {
             if (!ioh->deleted && ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) {
                 ioh->fd_read(ioh->opaque);
-                more = 1;
+                if (!ioh->fd_read_poll || ioh->fd_read_poll(ioh->opaque))
+                    more = 1;
+                else
+                    FD_CLR(ioh->fd, &rfds);
             }
             if (!ioh->deleted && ioh->fd_write && FD_ISSET(ioh->fd, &wfds)) {
                 ioh->fd_write(ioh->opaque);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/