[v8,7/7] virtio-console: Enable port throttling when chardev is slow to consume data

Submitted by Amit Shah on Dec. 1, 2010, 9:54 a.m.

Details

Message ID 90e8c0b7db1086c38283f684a159a892b4a66228.1291196789.git.amit.shah@redhat.com
State New
Headers show

Commit Message

Amit Shah Dec. 1, 2010, 9:54 a.m.
When a chardev indicates it can't accept more data, we tell the
virtio-serial code to stop sending us any more data till we tell
otherwise.  This helps in guests continuing to run normally while the vq
keeps getting full and eventually the guest stops queueing more data.
As soon as the chardev indicates it can accept more data, start pushing!

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

Comments

Paul Brook Dec. 1, 2010, 11:30 a.m.
>  /* Callback function that's called when the guest sends us data */
>  static void flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t
> len) {
>      VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
> +    int ret;
> 
> -    qemu_chr_write(vcon->chr, buf, len);
> +    ret = qemu_chr_write(vcon->chr, buf, len);
> +    if (ret == -EAGAIN) {
> +        virtio_serial_throttle_port(port, true);
> +    }
>  }

This looks wrong. It will loose data in the case of a partial write
(i.e. ret < len)

Paul
Amit Shah Dec. 1, 2010, 11:48 a.m.
On (Wed) Dec 01 2010 [11:30:58], Paul Brook wrote:
> >  /* Callback function that's called when the guest sends us data */
> >  static void flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t
> > len) {
> >      VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
> > +    int ret;
> > 
> > -    qemu_chr_write(vcon->chr, buf, len);
> > +    ret = qemu_chr_write(vcon->chr, buf, len);
> > +    if (ret == -EAGAIN) {
> > +        virtio_serial_throttle_port(port, true);
> > +    }
> >  }
> 
> This looks wrong. It will loose data in the case of a partial write
> (i.e. ret < len)

That doesn't happen currently (qemu_chr_write doesn't return a value > 0
but < len).

I had code in there to handle it, but that would change behaviour for
current users of qemu_chr_write(), which is a risk.

		Amit

Patch hide | download patch | download mbox

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 749ed59..ec85ad5 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -18,13 +18,27 @@  typedef struct VirtConsole {
     CharDriverState *chr;
 } VirtConsole;
 
+/*
+ * Callback function that's called from chardevs when backend becomes
+ * writable.
+ */
+static void chr_write_unblocked(void *opaque)
+{
+    VirtConsole *vcon = opaque;
+
+    virtio_serial_throttle_port(&vcon->port, false);
+}
 
 /* Callback function that's called when the guest sends us data */
 static void flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
 {
     VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+    int ret;
 
-    qemu_chr_write(vcon->chr, buf, len);
+    ret = qemu_chr_write(vcon->chr, buf, len);
+    if (ret == -EAGAIN) {
+        virtio_serial_throttle_port(port, true);
+    }
 }
 
 /* Readiness of the guest to accept data on a port */
@@ -62,6 +76,7 @@  static QemuChrHandlers chr_handlers = {
     .fd_can_read = chr_can_read,
     .fd_read = chr_read,
     .fd_event = chr_event,
+    .fd_write_unblocked = chr_write_unblocked,
 };
 
 static int generic_port_init(VirtConsole *vcon, VirtIOSerialDevice *dev)