Patchwork [v5,7/7] Use qemu_put_buffer_async for guest memory pages

login
register
mail settings
Submitter Paolo Bonzini
Date April 5, 2013, 3:42 p.m.
Message ID <515EF0E5.9090205@redhat.com>
Download mbox | patch
Permalink /patch/234183/
State New
Headers show

Comments

Paolo Bonzini - April 5, 2013, 3:42 p.m.
Il 05/04/2013 17:39, Kevin Wolf ha scritto:
>> > The solution could be to make bdrv_load_vmstate take an iov/iovcnt pair.
> Ah, so you're saying that instead of linearising the buffer it breaks up
> the requests in tiny pieces?

Only for RAM (header/page/header/page...), because the page comes
straight from the guest memory.  Device state is still buffered and fast.

> Implementing vectored bdrv_load/save_vmstate should be easy in theory.
> 
>> > Alternatively, you can try the attached patch.  I haven't yet tested it
>> > though, and won't be able to do so today.
> Attempted to write to buffer while read buffer is not empty
> 
> Program received signal SIGABRT, Aborted.

Second try.

Paolo
Kevin Wolf - April 5, 2013, 3:56 p.m.
Am 05.04.2013 um 17:42 hat Paolo Bonzini geschrieben:
> Il 05/04/2013 17:39, Kevin Wolf ha scritto:
> >> > The solution could be to make bdrv_load_vmstate take an iov/iovcnt pair.
> > Ah, so you're saying that instead of linearising the buffer it breaks up
> > the requests in tiny pieces?
> 
> Only for RAM (header/page/header/page...), because the page comes
> straight from the guest memory.  Device state is still buffered and fast.

And quite small in comparison. ;-)

> > Implementing vectored bdrv_load/save_vmstate should be easy in theory.
> > 
> >> > Alternatively, you can try the attached patch.  I haven't yet tested it
> >> > though, and won't be able to do so today.
> > Attempted to write to buffer while read buffer is not empty
> > 
> > Program received signal SIGABRT, Aborted.
> 
> Second try.

Okay, this doesn't seem to crash any more. Now I'm at 2.5s instead of
10s, which is obviously much better, but still worse than the initial
0.6s. The rest of the performance regression seems to come from a
different patch, though, so I guess I should do another bisect.

Kevin

Patch

diff --git a/savevm.c b/savevm.c
index b1d8988..5871642 100644
--- a/savevm.c
+++ b/savevm.c
@@ -525,27 +525,24 @@  static void qemu_file_set_error(QEMUFile *f, int ret)
 static void qemu_fflush(QEMUFile *f)
 {
     ssize_t ret = 0;
-    int i = 0;
 
     if (!f->ops->writev_buffer && !f->ops->put_buffer) {
         return;
     }
 
-    if (f->is_write && f->iovcnt > 0) {
+    if (f->is_write) {
         if (f->ops->writev_buffer) {
-            ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt);
-            if (ret >= 0) {
-                f->pos += ret;
+            if (f->iovcnt > 0) {
+                ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt);
             }
         } else {
-            for (i = 0; i < f->iovcnt && ret >= 0; i++) {
-                ret = f->ops->put_buffer(f->opaque, f->iov[i].iov_base, f->pos,
-                                         f->iov[i].iov_len);
-                if (ret >= 0) {
-                    f->pos += ret;
-                }
+            if (f->buf_index > 0) {
+                ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
             }
         }
+        if (ret >= 0) {
+            f->pos += ret;
+        }
         f->buf_index = 0;
         f->iovcnt = 0;
     }
@@ -631,6 +628,11 @@  static void add_to_iovec(QEMUFile *f, const uint8_t *buf, int size)
         f->iov[f->iovcnt].iov_base = (uint8_t *)buf;
         f->iov[f->iovcnt++].iov_len = size;
     }
+
+    f->is_write = 1;
+    if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) {
+        qemu_fflush(f);
+    }
 }
 
 void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size)
@@ -645,13 +647,11 @@  void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size)
         abort();
     }
 
-    add_to_iovec(f, buf, size);
-
-    f->is_write = 1;
-    f->bytes_xfer += size;
-
-    if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) {
-        qemu_fflush(f);
+    if (f->ops->writev_buffer) {
+        f->bytes_xfer += size;
+        add_to_iovec(f, buf, size);
+    } else {
+        qemu_put_buffer(f, buf, size);
     }
 }
 
@@ -674,9 +674,17 @@  void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
         if (l > size)
             l = size;
         memcpy(f->buf + f->buf_index, buf, l);
-        f->is_write = 1;
-        f->buf_index += l;
-        qemu_put_buffer_async(f, f->buf + (f->buf_index - l), l);
+        f->bytes_xfer += size;
+        if (f->ops->writev_buffer) {
+            add_to_iovec(f, f->buf + f->buf_index, l);
+            f->buf_index += l;
+        } else {
+            f->is_write = 1;
+            f->buf_index += l;
+            if (f->buf_index == IO_BUF_SIZE) {
+                qemu_fflush(f);
+            }
+        }
         if (qemu_file_get_error(f)) {
             break;
         }
@@ -697,14 +705,17 @@  void qemu_put_byte(QEMUFile *f, int v)
         abort();
     }
 
-    f->buf[f->buf_index++] = v;
-    f->is_write = 1;
+    f->buf[f->buf_index] = v;
     f->bytes_xfer++;
-
-    add_to_iovec(f, f->buf + (f->buf_index - 1), 1);
-
-    if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) {
-        qemu_fflush(f);
+    if (f->ops->writev_buffer) {
+        add_to_iovec(f, f->buf + f->buf_index, 1);
+        f->buf_index++;
+    } else {
+        f->is_write = 1;
+        f->buf_index++;
+        if (f->buf_index == IO_BUF_SIZE) {
+            qemu_fflush(f);
+        }
     }
 }