[Qemu-devel] [PATCH v2 0/8] Migration: Remove copying of guest ram pages

[Qemu-devel] [PATCH v2 0/8] Migration: Remove copying of guest ram pages

[Orit Wasserman]

In migration all data is copied to a static buffer in QEMUFile,
this hurts our network bandwidth and CPU usage especially with large guests.
We switched to iovec for storing different buffers to send (even a byte field is
considered as a buffer) and use writev to send the iovec.
writev was chosen (as apposed to sendmsg) because it supprts non socket fds.
  
Guest memory pages are not copied by calling a new function 
qemu_put_buffer_no_copy.
The page header data and device state data are still copied into the static
buffer. This data consists of a lot of bytes and integer fields and the static
buffer is used to store it during batching.
Another improvement is changing qemu_putbe64/32/16 to create a single
buffer instead of several byte sized buffer.

git repository: git://github.com/oritwas/qemu.git sendv_v2

Changes from v1:
Use iov_send for socket.
Make writev_buffer optional and if it is not implemented use put_buffer

Future work: Make number of iovec changeable

Orit Wasserman (8):
  Add QemuFileWritevBuffer QemuFileOps
  Add socket_writev_buffer function
  Update bytes_xfer in qemu_put_byte
  Store the data to send also in iovec
  Use writev ops if available
  More optimized qemu_put_be64/32/16
  Add qemu_put_buffer_no_copy
  Use qemu_put_buffer_no_copy for guest memory pages

 arch_init.c                   |   2 +-
 include/migration/qemu-file.h |  12 +++++
 savevm.c                      | 113 +++++++++++++++++++++++++++++-------------
 3 files changed, 92 insertions(+), 35 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH v2 1/8] Add QemuFileWritevBuffer QemuFileOps

[Orit Wasserman]

This will allow us to write an iovec

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 include/migration/qemu-file.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index df81261..8d3da9b 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -51,11 +51,18 @@ typedef int (QEMUFileCloseFunc)(void *opaque);
  */
 typedef int (QEMUFileGetFD)(void *opaque);
 
+/*
+ * This function writes an iovec to file.
+ */
+typedef int (QEMUFileWritevBufferFunc)(void *opaque, struct iovec *iov,
+                                       int iovcnt);
+
 typedef struct QEMUFileOps {
     QEMUFilePutBufferFunc *put_buffer;
     QEMUFileGetBufferFunc *get_buffer;
     QEMUFileCloseFunc *close;
     QEMUFileGetFD *get_fd;
+    QEMUFileWritevBufferFunc *writev_buffer;
 } QEMUFileOps;
 
 QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 2/8] Add socket_writev_buffer function

[Orit Wasserman]

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 savevm.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/savevm.c b/savevm.c
index 35c8d1e..baa45ae 100644
--- a/savevm.c
+++ b/savevm.c
@@ -39,6 +39,7 @@
 #include "qmp-commands.h"
 #include "trace.h"
 #include "qemu/bitops.h"
+#include "qemu/iov.h"
 
 #define SELF_ANNOUNCE_ROUNDS 5
 
@@ -171,6 +172,18 @@ static void coroutine_fn yield_until_fd_readable(int fd)
     qemu_coroutine_yield();
 }
 
+static int socket_writev_buffer(void *opaque, struct iovec *iov, int iovcnt)
+{
+    QEMUFileSocket *s = opaque;
+    ssize_t len;
+
+    len = iov_send(s->fd, iov, iovcnt, 0, iov_size(iov, iovcnt));
+    if (len < iov_size(iov, iovcnt)) {
+        len = -socket_error();
+    }
+    return len;
+}
+
 static int socket_get_fd(void *opaque)
 {
     QEMUFileSocket *s = opaque;
@@ -387,6 +400,7 @@ static const QEMUFileOps socket_read_ops = {
 static const QEMUFileOps socket_write_ops = {
     .get_fd =     socket_get_fd,
     .put_buffer = socket_put_buffer,
+    .writev_buffer = socket_writev_buffer,
     .close =      socket_close
 };
 
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 3/8] Update bytes_xfer in qemu_put_byte

[Orit Wasserman]

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 savevm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/savevm.c b/savevm.c
index baa45ae..686c8c8 100644
--- a/savevm.c
+++ b/savevm.c
@@ -647,6 +647,8 @@ void qemu_put_byte(QEMUFile *f, int v)
 
     f->buf[f->buf_index++] = v;
     f->is_write = 1;
+    f->bytes_xfer += 1;
+
     if (f->buf_index >= IO_BUF_SIZE) {
         qemu_fflush(f);
     }
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 4/8] Store the data to send also in iovec

[Orit Wasserman]

All data is still copied into the static buffer.

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 savevm.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/savevm.c b/savevm.c
index 686c8c8..ab81dd3 100644
--- a/savevm.c
+++ b/savevm.c
@@ -114,6 +114,7 @@ void qemu_announce_self(void)
 /* savevm/loadvm support */
 
 #define IO_BUF_SIZE 32768
+#define MAX_IOV_SIZE MIN(IOV_MAX, 64)
 
 struct QEMUFile {
     const QEMUFileOps *ops;
@@ -129,6 +130,9 @@ struct QEMUFile {
     int buf_size; /* 0 when writing */
     uint8_t buf[IO_BUF_SIZE];
 
+    struct iovec iov[MAX_IOV_SIZE];
+    unsigned int iovcnt;
+
     int last_error;
 };
 
@@ -527,6 +531,7 @@ static void qemu_fflush(QEMUFile *f)
             f->pos += f->buf_index;
         }
         f->buf_index = 0;
+        f->iovcnt = 0;
     }
     if (ret < 0) {
         qemu_file_set_error(f, ret);
@@ -619,12 +624,14 @@ 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->iov[f->iovcnt].iov_base = f->buf + f->buf_index;
+        f->iov[f->iovcnt++].iov_len = l;
         f->is_write = 1;
         f->buf_index += l;
         f->bytes_xfer += l;
         buf += l;
         size -= l;
-        if (f->buf_index >= IO_BUF_SIZE) {
+        if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) {
             qemu_fflush(f);
             if (qemu_file_get_error(f)) {
                 break;
@@ -648,8 +655,10 @@ void qemu_put_byte(QEMUFile *f, int v)
     f->buf[f->buf_index++] = v;
     f->is_write = 1;
     f->bytes_xfer += 1;
+    f->iov[f->iovcnt].iov_base = f->buf + (f->buf_index - 1);
+    f->iov[f->iovcnt++].iov_len = 1;
 
-    if (f->buf_index >= IO_BUF_SIZE) {
+    if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) {
         qemu_fflush(f);
     }
 }
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 5/8] Use writev ops if available

[Orit Wasserman]

Update qemu_fflush and stdio_close to use writev ops if they are available

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 savevm.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/savevm.c b/savevm.c
index ab81dd3..fde59d3 100644
--- a/savevm.c
+++ b/savevm.c
@@ -292,7 +292,7 @@ static int stdio_fclose(void *opaque)
     QEMUFileStdio *s = opaque;
     int ret = 0;
 
-    if (s->file->ops->put_buffer) {
+    if (s->file->ops->put_buffer || s->file->ops->writev_buffer) {
         int fd = fileno(s->stdio_file);
         struct stat st;
 
@@ -515,24 +515,34 @@ static void qemu_file_set_error(QEMUFile *f, int ret)
     }
 }
 
-/** Flushes QEMUFile buffer
+/**
+ * Flushes QEMUFile buffer
  *
+ * If there is writev_buffer QEMUFileOps it uses it otherwise uses
+ * put_buffer ops.
  */
 static void qemu_fflush(QEMUFile *f)
 {
     int ret = 0;
 
-    if (!f->ops->put_buffer) {
+    if (f->ops->writev_buffer) {
+        if (f->is_write && f->iovcnt > 0) {
+            ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt);
+        }
+    } else if (f->ops->put_buffer) {
+        if (f->is_write && f->buf_index > 0) {
+            ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
+        }
+    } else {
         return;
     }
-    if (f->is_write && f->buf_index > 0) {
-        ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
-        if (ret >= 0) {
-            f->pos += f->buf_index;
-        }
-        f->buf_index = 0;
-        f->iovcnt = 0;
+
+    if (ret >= 0) {
+        f->pos += f->buf_index;
     }
+    f->buf_index = 0;
+    f->iovcnt = 0;
+
     if (ret < 0) {
         qemu_file_set_error(f, ret);
     }
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 6/8] More optimized qemu_put_be64/32/16

[Orit Wasserman]

qemu_put_be functions used qemu_put_byte this caused lots of 1 bytes buffers
in the iovec.
we move to use cpu_put_be64/32/16wu and put a single buffer per call.

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 savevm.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/savevm.c b/savevm.c
index fde59d3..b724527 100644
--- a/savevm.c
+++ b/savevm.c
@@ -789,22 +789,26 @@ void qemu_file_reset_rate_limit(QEMUFile *f)
 
 void qemu_put_be16(QEMUFile *f, unsigned int v)
 {
-    qemu_put_byte(f, v >> 8);
-    qemu_put_byte(f, v);
+    uint16_t p;
+    cpu_to_be16wu(&p, v);
+
+    qemu_put_buffer(f, (uint8_t *)&p, sizeof(p));
 }
 
 void qemu_put_be32(QEMUFile *f, unsigned int v)
 {
-    qemu_put_byte(f, v >> 24);
-    qemu_put_byte(f, v >> 16);
-    qemu_put_byte(f, v >> 8);
-    qemu_put_byte(f, v);
+    uint32_t p;
+    cpu_to_be32wu(&p, v);
+
+    qemu_put_buffer(f, (uint8_t *)&p, sizeof(p));
 }
 
 void qemu_put_be64(QEMUFile *f, uint64_t v)
 {
-    qemu_put_be32(f, v >> 32);
-    qemu_put_be32(f, v);
+    uint64_t p;
+    cpu_to_be64wu(&p, v);
+
+    qemu_put_buffer(f, (uint8_t *)&p, sizeof(p));
 }
 
 unsigned int qemu_get_be16(QEMUFile *f)
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 7/8] Add qemu_put_buffer_no_copy

[Orit Wasserman]

This allow us to add a buffer to the iovec to send without copying it
into the static buffer.

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 include/migration/qemu-file.h |  5 +++++
 savevm.c                      | 42 ++++++++++++++++++++++++------------------
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 8d3da9b..5168be2 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -75,6 +75,11 @@ int qemu_fclose(QEMUFile *f);
 int64_t qemu_ftell(QEMUFile *f);
 void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
 void qemu_put_byte(QEMUFile *f, int v);
+/*
+ * put_buffer without copying the buffer.
+ * The buffer should be available till it is sent.
+ */
+void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size);
 
 static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
 {
diff --git a/savevm.c b/savevm.c
index b724527..ef5dfe2 100644
--- a/savevm.c
+++ b/savevm.c
@@ -619,6 +619,22 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
 {
     int l;
 
+    while (size > 0) {
+        l = IO_BUF_SIZE - f->buf_index;
+        if (l > size) {
+            l = size;
+        }
+        memcpy(f->buf + f->buf_index, buf, l);
+        f->buf_index += l;
+        f->is_write = 1;
+        qemu_put_buffer_no_copy(f, f->buf + (f->buf_index - l), l);
+        buf += l;
+        size -= l;
+    }
+}
+
+void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size)
+{
     if (f->last_error) {
         return;
     }
@@ -629,24 +645,14 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
         abort();
     }
 
-    while (size > 0) {
-        l = IO_BUF_SIZE - f->buf_index;
-        if (l > size)
-            l = size;
-        memcpy(f->buf + f->buf_index, buf, l);
-        f->iov[f->iovcnt].iov_base = f->buf + f->buf_index;
-        f->iov[f->iovcnt++].iov_len = l;
-        f->is_write = 1;
-        f->buf_index += l;
-        f->bytes_xfer += l;
-        buf += l;
-        size -= l;
-        if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) {
-            qemu_fflush(f);
-            if (qemu_file_get_error(f)) {
-                break;
-            }
-        }
+    f->iov[f->iovcnt].iov_base = (uint8_t *)buf;
+    f->iov[f->iovcnt++].iov_len = size;
+
+    f->is_write = 1;
+    f->bytes_xfer += size;
+
+    if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) {
+        qemu_fflush(f);
     }
 }
 
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 8/8] Use qemu_put_buffer_no_copy for guest memory pages

[Orit Wasserman]

This will remove an unneeded copy of guest memory pages.
For the page header and device state we still copy the data to the
static buffer the other option is to allocate the memory on demand
which is more expensive.

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 arch_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch_init.c b/arch_init.c
index 98e2bc6..27b53eb 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -481,7 +481,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
             /* XBZRLE overflow or normal page */
             if (bytes_sent == -1) {
                 bytes_sent = save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE);
-                qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
+                qemu_put_buffer_no_copy(f, p, TARGET_PAGE_SIZE);
                 bytes_sent += TARGET_PAGE_SIZE;
                 acct_info.norm_pages++;
             }
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH v2 5/8] Use writev ops if available

[Paolo Bonzini]

Il 21/03/2013 14:23, Orit Wasserman ha scritto:
> Update qemu_fflush and stdio_close to use writev ops if they are available
> 
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
> ---
>  savevm.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/savevm.c b/savevm.c
> index ab81dd3..fde59d3 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -292,7 +292,7 @@ static int stdio_fclose(void *opaque)
>      QEMUFileStdio *s = opaque;
>      int ret = 0;
>  
> -    if (s->file->ops->put_buffer) {
> +    if (s->file->ops->put_buffer || s->file->ops->writev_buffer) {
>          int fd = fileno(s->stdio_file);
>          struct stat st;
>  
> @@ -515,24 +515,34 @@ static void qemu_file_set_error(QEMUFile *f, int ret)
>      }
>  }
>  
> -/** Flushes QEMUFile buffer
> +/**
> + * Flushes QEMUFile buffer
>   *
> + * If there is writev_buffer QEMUFileOps it uses it otherwise uses
> + * put_buffer ops.
>   */
>  static void qemu_fflush(QEMUFile *f)
>  {
>      int ret = 0;
>  
> -    if (!f->ops->put_buffer) {
> +    if (f->ops->writev_buffer) {
> +        if (f->is_write && f->iovcnt > 0) {
> +            ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt);

This needs to update f->pos.

> +        }
> +    } else if (f->ops->put_buffer) {
> +        if (f->is_write && f->buf_index > 0) {
> +            ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
> +        }

I think this has to go through all the iovecs (i.e. never look at
f->buf_index, only f->iovcnt).  Otherwise RAM migration does not work,
because the page data is not copied to f->buf.  But that's pretty much
it, the series looks good.

However, I'd still prefer to have qemu_put_buffer_no_copy coalesce
adjacent iovecs.  Otherwise you might end up with only 3-400 bytes in
each sendmsg when serializing device data.

Paolo

> +    } else {
>          return;
>      }
> -    if (f->is_write && f->buf_index > 0) {
> -        ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
> -        if (ret >= 0) {
> -            f->pos += f->buf_index;
> -        }
> -        f->buf_index = 0;
> -        f->iovcnt = 0;
> +
> +    if (ret >= 0) {
> +        f->pos += f->buf_index;
>      }
> +    f->buf_index = 0;
> +    f->iovcnt = 0;
> +
>      if (ret < 0) {
>          qemu_file_set_error(f, ret);
>      }
> 

Re: [Qemu-devel] [PATCH v2 5/8] Use writev ops if available

[Orit Wasserman]

On 03/21/2013 03:44 PM, Paolo Bonzini wrote:
> Il 21/03/2013 14:23, Orit Wasserman ha scritto:
>> Update qemu_fflush and stdio_close to use writev ops if they are available
>>
>> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
>> ---
>>  savevm.c | 30 ++++++++++++++++++++----------
>>  1 file changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/savevm.c b/savevm.c
>> index ab81dd3..fde59d3 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -292,7 +292,7 @@ static int stdio_fclose(void *opaque)
>>      QEMUFileStdio *s = opaque;
>>      int ret = 0;
>>  
>> -    if (s->file->ops->put_buffer) {
>> +    if (s->file->ops->put_buffer || s->file->ops->writev_buffer) {
>>          int fd = fileno(s->stdio_file);
>>          struct stat st;
>>  
>> @@ -515,24 +515,34 @@ static void qemu_file_set_error(QEMUFile *f, int ret)
>>      }
>>  }
>>  
>> -/** Flushes QEMUFile buffer
>> +/**
>> + * Flushes QEMUFile buffer
>>   *
>> + * If there is writev_buffer QEMUFileOps it uses it otherwise uses
>> + * put_buffer ops.
>>   */
>>  static void qemu_fflush(QEMUFile *f)
>>  {
>>      int ret = 0;
>>  
>> -    if (!f->ops->put_buffer) {
>> +    if (f->ops->writev_buffer) {
>> +        if (f->is_write && f->iovcnt > 0) {
>> +            ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt);
> 
> This needs to update f->pos.
> 
>> +        }
>> +    } else if (f->ops->put_buffer) {
>> +        if (f->is_write && f->buf_index > 0) {
>> +            ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
>> +        }
> 
> I think this has to go through all the iovecs (i.e. never look at
> f->buf_index, only f->iovcnt).  Otherwise RAM migration does not work,
> because the page data is not copied to f->buf.  But that's pretty much
> it, the series looks good.
You are right I will fix it.
> 
> However, I'd still prefer to have qemu_put_buffer_no_copy coalesce
> adjacent iovecs.  Otherwise you might end up with only 3-400 bytes in
> each sendmsg when serializing device data.
Yes this will be helpful for the device state.

Orit
> 
> Paolo
> 
>> +    } else {
>>          return;
>>      }
>> -    if (f->is_write && f->buf_index > 0) {
>> -        ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
>> -        if (ret >= 0) {
>> -            f->pos += f->buf_index;
>> -        }
>> -        f->buf_index = 0;
>> -        f->iovcnt = 0;
>> +
>> +    if (ret >= 0) {
>> +        f->pos += f->buf_index;
>>      }
>> +    f->buf_index = 0;
>> +    f->iovcnt = 0;
>> +
>>      if (ret < 0) {
>>          qemu_file_set_error(f, ret);
>>      }
>>
>