Skip to contentSkip to conversation

fix reading from character devices

PATCH Applied Jan Kiszka 5 participants 8 messages Jan 21, 2008 – Feb 22, 2008 4 messages not in the archive KVM

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 fix reading from character devices

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.

Jan

Signature · 28 lines
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

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

Index: kvm-userspace/qemu/vl.c
===================================================================
--- kvm-userspace.orig/qemu/vl.c
+++ kvm-userspace/qemu/vl.c
@@ -7751,7 +7751,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/

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw
Quoted 9 lines · Jan Kiszka

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.

Quoted 5 lines

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?

Signature · 7 lines
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
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/

A message that is not in the archive (sent off-list or not yet received): 47947045.6040106-atKUWr5tajBWk0Htik3J/[email protected]

#4 lore raw
Quoted 10 lines · Avi Kivity
Quoted 9 lines · Jan Kiszka

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.

Quoted 8 lines
Quoted 5 lines

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.

-------------------------------------------------------------------------
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/

A message that is not in the archive (sent off-list or not yet received): 1200912198.3188.6.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/[email protected]

#6 lore raw
Quoted 19 lines · Dor Laor
Quoted 17 lines · Avi Kivity
Quoted 15 lines · Jan Kiszka

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.

Quoted 10 lines
Quoted 3 lines

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?

Signature · 7 lines
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
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/

A message that is not in the archive (sent off-list or not yet received): 479478E6.6030903-atKUWr5tajBWk0Htik3J/[email protected]

#8 lore raw
Quoted 31 lines · Avi Kivity
Quoted 19 lines · Dor Laor
Quoted 17 lines · Avi Kivity
Quoted 15 lines · Jan Kiszka

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.

Quoted 10 lines
Quoted 3 lines

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.

Quoted 4 lines

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.

-------------------------------------------------------------------------
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/

#9 lore raw
Quoted 30 lines · Dor Laor
Quoted 29 lines · Avi Kivity
Quoted 28 lines · Dor Laor
Quoted 11 lines · Avi Kivity
Quoted 9 lines · Jan Kiszka

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.

Quoted 9 lines
Quoted 6 lines

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.

Quoted 7 lines
Quoted 4 lines

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(-)
qemu/vl.c +4 -1
Index: 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/

Applied
#10 lore raw
Quoted 4 lines · Jan Kiszka

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

Applied, thanks.

Signature · 7 lines
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
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/
author
#11 lore raw
In reply to a message not in the archive
Quoted 30 lines · Avi Kivity
Quoted 18 lines · Dor Laor
Quoted 16 lines · Avi Kivity
Quoted 14 lines · Jan Kiszka

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.

Quoted 10 lines
Quoted 3 lines

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.

Quoted 3 lines

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?

As long as virtio_net_can_receive returns 1, the loop will keep on feeding also this device. And if it returns 0, I see no reason for ignoring this signal. Anythings else would be, well, weird. However, the current io-polling loop in git urgently needs fixing, or even a redesign.

Jan

Signature · 7 lines
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

-------------------------------------------------------------------------
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/