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

login
register
mail settings
Submitter Amit Shah
Date Dec. 1, 2010, 9:54 a.m.
Message ID <90e8c0b7db1086c38283f684a159a892b4a66228.1291196789.git.amit.shah@redhat.com>
Download mbox | patch
Permalink /patch/73777/
State New
Headers show

Comments

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(-)
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

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)