[PATCH 00/11] migration: Make multifd use only one write on the send path

[PATCH 00/11] migration: Make multifd use only one write on the send path

[Juan Quintela]

Hi

Right now, multifd does a write() for the header and a writev() for
each group of pages.  Simplify it so we send the header as another
member of the IOV.

Once there, I got several simplifications:
* is_zero_range() was used only once, just use its body.
* same with is_zero_page().
* Be consintent and use offset insed the ramblock everywhere.
* Now that we have the offsets of the ramblock, we can drop the iov.
* Now that nothing uses iov's except NOCOMP method, move the iovs
  from pages to methods.
* Now we can use iov's with a single field for zlib/zstd.
* send_write() method is the same in all the implementaitons, so use
  it directly.
* Now, we can use a single writev() to write everything.

ToDo: Move zero page detection to the multifd thrteads.

With RAM in the Terabytes size, the detection of the zero page takes
too much time on the main thread.

Last patch on the series removes the detection of zero pages in the
main thread for multifd.  In the next series post, I will add how to
detect the zero pages and send them on multifd channels.

Please review.

Later, Juan.

Juan Quintela (11):
  migration: Remove is_zero_range()
  dump: Remove is_zero_page()
  multifd: Fill offset and block for reception
  multifd: Make zstd compression method not use iovs
  multifd: Make zlib compression method not use iovs
  migration: Move iov from pages to params
  multifd: Make zlib use iov's
  multifd: Make zstd use iov's
  multifd: Remove send_write() method
  multifd: Use a single writev on the send side
  migration: Use multifd before we check for the zero page

 migration/multifd.h      | 10 ++++---
 dump/dump.c              | 10 +------
 migration/multifd-zlib.c | 40 ++++++++------------------
 migration/multifd-zstd.c | 41 ++++++++------------------
 migration/multifd.c      | 62 ++++++++++++++++++----------------------
 migration/ram.c          | 31 +++++++++-----------
 6 files changed, 72 insertions(+), 122 deletions(-)

-- 
2.33.1

[PATCH 01/11] migration: Remove is_zero_range()

[Juan Quintela]

It just calls buffer_is_zero().  Just change the callers.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 863035d235..f2a799ff83 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -81,11 +81,6 @@
 /* 0x80 is reserved in migration.h start with 0x100 next */
 #define RAM_SAVE_FLAG_COMPRESS_PAGE    0x100
 
-static inline bool is_zero_range(uint8_t *p, uint64_t size)
-{
-    return buffer_is_zero(p, size);
-}
-
 XBZRLECacheStats xbzrle_counters;
 
 /* struct contains XBZRLE cache and a static page
@@ -1180,7 +1175,7 @@ static int save_zero_page_to_file(RAMState *rs, QEMUFile *file,
     uint8_t *p = block->host + offset;
     int len = 0;
 
-    if (is_zero_range(p, TARGET_PAGE_SIZE)) {
+    if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
         len += save_page_header(rs, file, block, offset | RAM_SAVE_FLAG_ZERO);
         qemu_put_byte(file, 0);
         len += 1;
@@ -3367,7 +3362,7 @@ static inline void *colo_cache_from_block_offset(RAMBlock *block,
  */
 void ram_handle_compressed(void *host, uint8_t ch, uint64_t size)
 {
-    if (ch != 0 || !is_zero_range(host, size)) {
+    if (ch != 0 || !buffer_is_zero(host, size)) {
         memset(host, ch, size);
     }
 }
-- 
2.33.1

[PATCH 02/11] dump: Remove is_zero_page()

[Juan Quintela]

It just calls buffer_is_zero().  Just change the callers.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 dump/dump.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index 662d0a62cd..a84d8b1598 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -1293,14 +1293,6 @@ static size_t get_len_buf_out(size_t page_size, uint32_t flag_compress)
     return 0;
 }
 
-/*
- * check if the page is all 0
- */
-static inline bool is_zero_page(const uint8_t *buf, size_t page_size)
-{
-    return buffer_is_zero(buf, page_size);
-}
-
 static void write_dump_pages(DumpState *s, Error **errp)
 {
     int ret = 0;
@@ -1357,7 +1349,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
      */
     while (get_next_page(&block_iter, &pfn_iter, &buf, s)) {
         /* check zero page */
-        if (is_zero_page(buf, s->dump_info.page_size)) {
+        if (buffer_is_zero(buf, s->dump_info.page_size)) {
             ret = write_cache(&page_desc, &pd_zero, sizeof(PageDescriptor),
                               false);
             if (ret < 0) {
-- 
2.33.1

[PATCH 03/11] multifd: Fill offset and block for reception

[Juan Quintela]

We were using the iov directly, but we will need this info on the
following patch.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/multifd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration/multifd.c b/migration/multifd.c
index 7c9deb1921..e2adcdffa1 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -364,6 +364,8 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
                        offset, block->used_length);
             return -1;
         }
+        p->pages->offset[i] = offset;
+        p->pages->block = block;
         p->pages->iov[i].iov_base = block->host + offset;
         p->pages->iov[i].iov_len = qemu_target_page_size();
     }
-- 
2.33.1

[PATCH 04/11] multifd: Make zstd compression method not use iovs

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/multifd-zstd.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
index 693bddf8c9..1dccdbd733 100644
--- a/migration/multifd-zstd.c
+++ b/migration/multifd-zstd.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include <zstd.h>
 #include "qemu/rcu.h"
+#include "exec/ramblock.h"
 #include "exec/target_page.h"
 #include "qapi/error.h"
 #include "migration.h"
@@ -112,8 +113,8 @@ static void zstd_send_cleanup(MultiFDSendParams *p, Error **errp)
  */
 static int zstd_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
 {
-    struct iovec *iov = p->pages->iov;
     struct zstd_data *z = p->data;
+    size_t page_size = qemu_target_page_size();
     int ret;
     uint32_t i;
 
@@ -127,8 +128,8 @@ static int zstd_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
         if (i == used - 1) {
             flush = ZSTD_e_flush;
         }
-        z->in.src = iov[i].iov_base;
-        z->in.size = iov[i].iov_len;
+        z->in.src = p->pages->block->host + p->pages->offset[i];
+        z->in.size = page_size;
         z->in.pos = 0;
 
         /*
@@ -264,6 +265,7 @@ static int zstd_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
     uint32_t expected_size = used * qemu_target_page_size();
     uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
     struct zstd_data *z = p->data;
+    size_t page_size = qemu_target_page_size();
     int ret;
     int i;
 
@@ -283,10 +285,8 @@ static int zstd_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
     z->in.pos = 0;
 
     for (i = 0; i < used; i++) {
-        struct iovec *iov = &p->pages->iov[i];
-
-        z->out.dst = iov->iov_base;
-        z->out.size = iov->iov_len;
+        z->out.dst = p->pages->block->host + p->pages->offset[i];
+        z->out.size = page_size;
         z->out.pos = 0;
 
         /*
@@ -300,8 +300,8 @@ static int zstd_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
         do {
             ret = ZSTD_decompressStream(z->zds, &z->out, &z->in);
         } while (ret > 0 && (z->in.size - z->in.pos > 0)
-                         && (z->out.pos < iov->iov_len));
-        if (ret > 0 && (z->out.pos < iov->iov_len)) {
+                         && (z->out.pos < page_size));
+        if (ret > 0 && (z->out.pos < page_size)) {
             error_setg(errp, "multifd %d: decompressStream buffer too small",
                        p->id);
             return -1;
-- 
2.33.1

[PATCH 05/11] multifd: Make zlib compression method not use iovs

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/multifd-zlib.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
index ab4ba75d75..40d4cbd0b8 100644
--- a/migration/multifd-zlib.c
+++ b/migration/multifd-zlib.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include <zlib.h>
 #include "qemu/rcu.h"
+#include "exec/ramblock.h"
 #include "exec/target_page.h"
 #include "qapi/error.h"
 #include "migration.h"
@@ -99,8 +100,8 @@ static void zlib_send_cleanup(MultiFDSendParams *p, Error **errp)
  */
 static int zlib_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
 {
-    struct iovec *iov = p->pages->iov;
     struct zlib_data *z = p->data;
+    size_t page_size = qemu_target_page_size();
     z_stream *zs = &z->zs;
     uint32_t out_size = 0;
     int ret;
@@ -114,8 +115,8 @@ static int zlib_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
             flush = Z_SYNC_FLUSH;
         }
 
-        zs->avail_in = iov[i].iov_len;
-        zs->next_in = iov[i].iov_base;
+        zs->avail_in = page_size;
+        zs->next_in = p->pages->block->host + p->pages->offset[i];
 
         zs->avail_out = available;
         zs->next_out = z->zbuff + out_size;
@@ -240,6 +241,7 @@ static void zlib_recv_cleanup(MultiFDRecvParams *p)
 static int zlib_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
 {
     struct zlib_data *z = p->data;
+    size_t page_size = qemu_target_page_size();
     z_stream *zs = &z->zs;
     uint32_t in_size = p->next_packet_size;
     /* we measure the change of total_out */
@@ -264,7 +266,6 @@ static int zlib_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
     zs->next_in = z->zbuff;
 
     for (i = 0; i < used; i++) {
-        struct iovec *iov = &p->pages->iov[i];
         int flush = Z_NO_FLUSH;
         unsigned long start = zs->total_out;
 
@@ -272,8 +273,8 @@ static int zlib_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
             flush = Z_SYNC_FLUSH;
         }
 
-        zs->avail_out = iov->iov_len;
-        zs->next_out = iov->iov_base;
+        zs->avail_out = page_size;
+        zs->next_out = p->pages->block->host + p->pages->offset[i];
 
         /*
          * Welcome to inflate semantics
@@ -286,8 +287,8 @@ static int zlib_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
         do {
             ret = inflate(zs, flush);
         } while (ret == Z_OK && zs->avail_in
-                             && (zs->total_out - start) < iov->iov_len);
-        if (ret == Z_OK && (zs->total_out - start) < iov->iov_len) {
+                             && (zs->total_out - start) < page_size);
+        if (ret == Z_OK && (zs->total_out - start) < page_size) {
             error_setg(errp, "multifd %d: inflate generated too few output",
                        p->id);
             return -1;
-- 
2.33.1

[PATCH 06/11] migration: Move iov from pages to params

[Juan Quintela]

This will allow us to reduce the number of system calls on the next patch.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/multifd.h |  8 ++++++--
 migration/multifd.c | 30 +++++++++++++++++++++---------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/migration/multifd.h b/migration/multifd.h
index 15c50ca0b2..67f9104051 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -62,8 +62,6 @@ typedef struct {
     uint64_t packet_num;
     /* offset of each page */
     ram_addr_t *offset;
-    /* pointer to each page */
-    struct iovec *iov;
     RAMBlock *block;
 } MultiFDPages_t;
 
@@ -110,6 +108,10 @@ typedef struct {
     uint64_t num_pages;
     /* syncs main thread and channels */
     QemuSemaphore sem_sync;
+    /* buffers to send */
+    struct iovec *iov;
+    /* used for compression methods */
+    uint32_t iovs_used;
     /* used for compression methods */
     void *data;
 }  MultiFDSendParams;
@@ -149,6 +151,8 @@ typedef struct {
     uint64_t num_pages;
     /* syncs main thread and channels */
     QemuSemaphore sem_sync;
+    /* buffers to recv */
+    struct iovec *iov;
     /* used for de-compression methods */
     void *data;
 } MultiFDRecvParams;
diff --git a/migration/multifd.c b/migration/multifd.c
index e2adcdffa1..4368b7f855 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -87,6 +87,14 @@ static void nocomp_send_cleanup(MultiFDSendParams *p, Error **errp)
 static int nocomp_send_prepare(MultiFDSendParams *p, uint32_t used,
                                Error **errp)
 {
+    MultiFDPages_t *pages = p->pages;
+
+    for (int i = 0; i < used; i++) {
+        p->iov[p->iovs_used].iov_base = pages->block->host + pages->offset[i];
+        p->iov[p->iovs_used].iov_len = qemu_target_page_size();
+        p->iovs_used++;
+    }
+
     p->next_packet_size = used * qemu_target_page_size();
     p->flags |= MULTIFD_FLAG_NOCOMP;
     return 0;
@@ -105,7 +113,7 @@ static int nocomp_send_prepare(MultiFDSendParams *p, uint32_t used,
  */
 static int nocomp_send_write(MultiFDSendParams *p, uint32_t used, Error **errp)
 {
-    return qio_channel_writev_all(p->c, p->pages->iov, used, errp);
+    return qio_channel_writev_all(p->c, p->iov, p->iovs_used, errp);
 }
 
 /**
@@ -154,7 +162,11 @@ static int nocomp_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
                    p->id, flags, MULTIFD_FLAG_NOCOMP);
         return -1;
     }
-    return qio_channel_readv_all(p->c, p->pages->iov, used, errp);
+    for (int i = 0; i < p->pages->used; i++) {
+        p->iov[i].iov_base = p->pages->block->host + p->pages->offset[i];
+        p->iov[i].iov_len = qemu_target_page_size();
+    }
+    return qio_channel_readv_all(p->c, p->iov, used, errp);
 }
 
 static MultiFDMethods multifd_nocomp_ops = {
@@ -244,7 +256,6 @@ static MultiFDPages_t *multifd_pages_init(size_t size)
     MultiFDPages_t *pages = g_new0(MultiFDPages_t, 1);
 
     pages->allocated = size;
-    pages->iov = g_new0(struct iovec, size);
     pages->offset = g_new0(ram_addr_t, size);
 
     return pages;
@@ -256,8 +267,6 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
     pages->allocated = 0;
     pages->packet_num = 0;
     pages->block = NULL;
-    g_free(pages->iov);
-    pages->iov = NULL;
     g_free(pages->offset);
     pages->offset = NULL;
     g_free(pages);
@@ -366,8 +375,6 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
         }
         p->pages->offset[i] = offset;
         p->pages->block = block;
-        p->pages->iov[i].iov_base = block->host + offset;
-        p->pages->iov[i].iov_len = qemu_target_page_size();
     }
 
     return 0;
@@ -471,8 +478,6 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset)
 
     if (pages->block == block) {
         pages->offset[pages->used] = offset;
-        pages->iov[pages->used].iov_base = block->host + offset;
-        pages->iov[pages->used].iov_len = qemu_target_page_size();
         pages->used++;
 
         if (pages->used < pages->allocated) {
@@ -565,6 +570,8 @@ void multifd_save_cleanup(void)
         p->packet_len = 0;
         g_free(p->packet);
         p->packet = NULL;
+        g_free(p->iov);
+        p->iov = NULL;
         multifd_send_state->ops->send_cleanup(p, &local_err);
         if (local_err) {
             migrate_set_error(migrate_get_current(), local_err);
@@ -653,6 +660,7 @@ static void *multifd_send_thread(void *opaque)
             uint32_t used = p->pages->used;
             uint64_t packet_num = p->packet_num;
             flags = p->flags;
+            p->iovs_used = 0;
 
             if (used) {
                 ret = multifd_send_state->ops->send_prepare(p, used,
@@ -922,6 +930,7 @@ int multifd_save_setup(Error **errp)
         p->packet->version = cpu_to_be32(MULTIFD_VERSION);
         p->name = g_strdup_printf("multifdsend_%d", i);
         p->tls_hostname = g_strdup(s->hostname);
+        p->iov = g_new0(struct iovec, page_count);
         socket_send_channel_create(multifd_new_send_channel_async, p);
     }
 
@@ -1021,6 +1030,8 @@ int multifd_load_cleanup(Error **errp)
         p->packet_len = 0;
         g_free(p->packet);
         p->packet = NULL;
+        g_free(p->iov);
+        p->iov = NULL;
         multifd_recv_state->ops->recv_cleanup(p);
     }
     qemu_sem_destroy(&multifd_recv_state->sem_sync);
@@ -1161,6 +1172,7 @@ int multifd_load_setup(Error **errp)
                       + sizeof(uint64_t) * page_count;
         p->packet = g_malloc0(p->packet_len);
         p->name = g_strdup_printf("multifdrecv_%d", i);
+        p->iov = g_new0(struct iovec, page_count);
     }
 
     for (i = 0; i < thread_count; i++) {
-- 
2.33.1

[PATCH 07/11] multifd: Make zlib use iov's

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/multifd-zlib.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
index 40d4cbd0b8..2c385e57bc 100644
--- a/migration/multifd-zlib.c
+++ b/migration/multifd-zlib.c
@@ -144,6 +144,9 @@ static int zlib_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
         }
         out_size += available - zs->avail_out;
     }
+    p->iov[p->iovs_used].iov_base = z->zbuff;
+    p->iov[p->iovs_used].iov_len = out_size;
+    p->iovs_used++;
     p->next_packet_size = out_size;
     p->flags |= MULTIFD_FLAG_ZLIB;
 
@@ -163,10 +166,7 @@ static int zlib_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
  */
 static int zlib_send_write(MultiFDSendParams *p, uint32_t used, Error **errp)
 {
-    struct zlib_data *z = p->data;
-
-    return qio_channel_write_all(p->c, (void *)z->zbuff, p->next_packet_size,
-                                 errp);
+    return qio_channel_writev_all(p->c, p->iov, p->iovs_used, errp);
 }
 
 /**
-- 
2.33.1

[PATCH 08/11] multifd: Make zstd use iov's

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/multifd-zstd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
index 1dccdbd733..6183d0b26d 100644
--- a/migration/multifd-zstd.c
+++ b/migration/multifd-zstd.c
@@ -155,6 +155,9 @@ static int zstd_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
             return -1;
         }
     }
+    p->iov[p->iovs_used].iov_base = z->zbuff;
+    p->iov[p->iovs_used].iov_len = z->out.pos;
+    p->iovs_used++;
     p->next_packet_size = z->out.pos;
     p->flags |= MULTIFD_FLAG_ZSTD;
 
@@ -174,10 +177,7 @@ static int zstd_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
  */
 static int zstd_send_write(MultiFDSendParams *p, uint32_t used, Error **errp)
 {
-    struct zstd_data *z = p->data;
-
-    return qio_channel_write_all(p->c, (void *)z->zbuff, p->next_packet_size,
-                                 errp);
+    return qio_channel_writev_all(p->c, p->iov, p->iovs_used, errp);
 }
 
 /**
-- 
2.33.1

[PATCH 09/11] multifd: Remove send_write() method

[Juan Quintela]

Everything use now iov's.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/multifd.h      |  2 --
 migration/multifd-zlib.c | 17 -----------------
 migration/multifd-zstd.c | 17 -----------------
 migration/multifd.c      | 20 ++------------------
 4 files changed, 2 insertions(+), 54 deletions(-)

diff --git a/migration/multifd.h b/migration/multifd.h
index 67f9104051..f50c51f365 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -164,8 +164,6 @@ typedef struct {
     void (*send_cleanup)(MultiFDSendParams *p, Error **errp);
     /* Prepare the send packet */
     int (*send_prepare)(MultiFDSendParams *p, uint32_t used, Error **errp);
-    /* Write the send packet */
-    int (*send_write)(MultiFDSendParams *p, uint32_t used, Error **errp);
     /* Setup for receiving side */
     int (*recv_setup)(MultiFDRecvParams *p, Error **errp);
     /* Cleanup for receiving side */
diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
index 2c385e57bc..1201473fa4 100644
--- a/migration/multifd-zlib.c
+++ b/migration/multifd-zlib.c
@@ -153,22 +153,6 @@ static int zlib_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
     return 0;
 }
 
-/**
- * zlib_send_write: do the actual write of the data
- *
- * Do the actual write of the comprresed buffer.
- *
- * Returns 0 for success or -1 for error
- *
- * @p: Params for the channel that we are using
- * @used: number of pages used
- * @errp: pointer to an error
- */
-static int zlib_send_write(MultiFDSendParams *p, uint32_t used, Error **errp)
-{
-    return qio_channel_writev_all(p->c, p->iov, p->iovs_used, errp);
-}
-
 /**
  * zlib_recv_setup: setup receive side
  *
@@ -312,7 +296,6 @@ static MultiFDMethods multifd_zlib_ops = {
     .send_setup = zlib_send_setup,
     .send_cleanup = zlib_send_cleanup,
     .send_prepare = zlib_send_prepare,
-    .send_write = zlib_send_write,
     .recv_setup = zlib_recv_setup,
     .recv_cleanup = zlib_recv_cleanup,
     .recv_pages = zlib_recv_pages
diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
index 6183d0b26d..1532450204 100644
--- a/migration/multifd-zstd.c
+++ b/migration/multifd-zstd.c
@@ -164,22 +164,6 @@ static int zstd_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
     return 0;
 }
 
-/**
- * zstd_send_write: do the actual write of the data
- *
- * Do the actual write of the comprresed buffer.
- *
- * Returns 0 for success or -1 for error
- *
- * @p: Params for the channel that we are using
- * @used: number of pages used
- * @errp: pointer to an error
- */
-static int zstd_send_write(MultiFDSendParams *p, uint32_t used, Error **errp)
-{
-    return qio_channel_writev_all(p->c, p->iov, p->iovs_used, errp);
-}
-
 /**
  * zstd_recv_setup: setup receive side
  *
@@ -325,7 +309,6 @@ static MultiFDMethods multifd_zstd_ops = {
     .send_setup = zstd_send_setup,
     .send_cleanup = zstd_send_cleanup,
     .send_prepare = zstd_send_prepare,
-    .send_write = zstd_send_write,
     .recv_setup = zstd_recv_setup,
     .recv_cleanup = zstd_recv_cleanup,
     .recv_pages = zstd_recv_pages
diff --git a/migration/multifd.c b/migration/multifd.c
index 4368b7f855..f748250c0e 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -100,22 +100,6 @@ static int nocomp_send_prepare(MultiFDSendParams *p, uint32_t used,
     return 0;
 }
 
-/**
- * nocomp_send_write: do the actual write of the data
- *
- * For no compression we just have to write the data.
- *
- * Returns 0 for success or -1 for error
- *
- * @p: Params for the channel that we are using
- * @used: number of pages used
- * @errp: pointer to an error
- */
-static int nocomp_send_write(MultiFDSendParams *p, uint32_t used, Error **errp)
-{
-    return qio_channel_writev_all(p->c, p->iov, p->iovs_used, errp);
-}
-
 /**
  * nocomp_recv_setup: setup receive side
  *
@@ -173,7 +157,6 @@ static MultiFDMethods multifd_nocomp_ops = {
     .send_setup = nocomp_send_setup,
     .send_cleanup = nocomp_send_cleanup,
     .send_prepare = nocomp_send_prepare,
-    .send_write = nocomp_send_write,
     .recv_setup = nocomp_recv_setup,
     .recv_cleanup = nocomp_recv_cleanup,
     .recv_pages = nocomp_recv_pages
@@ -688,7 +671,8 @@ static void *multifd_send_thread(void *opaque)
             }
 
             if (used) {
-                ret = multifd_send_state->ops->send_write(p, used, &local_err);
+                ret = qio_channel_writev_all(p->c, p->iov, p->iovs_used,
+                                             &local_err);
                 if (ret != 0) {
                     break;
                 }
-- 
2.33.1

Re: [PATCH 01/11] migration: Remove is_zero_range()

[Richard Henderson]

On 11/19/21 5:58 PM, Juan Quintela wrote:
> It just calls buffer_is_zero().  Just change the callers.
> 
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
>   migration/ram.c | 9 ++-------
>   1 file changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 02/11] dump: Remove is_zero_page()

[Richard Henderson]

On 11/19/21 5:58 PM, Juan Quintela wrote:
> It just calls buffer_is_zero().  Just change the callers.
> 
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
>   dump/dump.c | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 03/11] multifd: Fill offset and block for reception

[Richard Henderson]

On 11/19/21 5:58 PM, Juan Quintela wrote:
> We were using the iov directly, but we will need this info on the
> following patch.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>   migration/multifd.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 7c9deb1921..e2adcdffa1 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -364,6 +364,8 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
>                          offset, block->used_length);
>               return -1;
>           }
> +        p->pages->offset[i] = offset;
> +        p->pages->block = block;
>           p->pages->iov[i].iov_base = block->host + offset;
>           p->pages->iov[i].iov_len = qemu_target_page_size();
>       }
> 

Block should be stored one outside the loop.


r~

Re: [PATCH 06/11] migration: Move iov from pages to params

[Richard Henderson]

On 11/19/21 5:58 PM, Juan Quintela wrote:
>   static int nocomp_send_prepare(MultiFDSendParams *p, uint32_t used,
>                                  Error **errp)
>   {
> +    MultiFDPages_t *pages = p->pages;
> +
> +    for (int i = 0; i < used; i++) {
> +        p->iov[p->iovs_used].iov_base = pages->block->host + pages->offset[i];
> +        p->iov[p->iovs_used].iov_len = qemu_target_page_size();
> +        p->iovs_used++;
> +    }
> +
>       p->next_packet_size = used * qemu_target_page_size();

Compute qemu_target_page_size once in the function.
Hoist p->iovs_used to a local variable around the loop.

> @@ -154,7 +162,11 @@ static int nocomp_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
>                      p->id, flags, MULTIFD_FLAG_NOCOMP);
>           return -1;
>       }
> -    return qio_channel_readv_all(p->c, p->pages->iov, used, errp);
> +    for (int i = 0; i < p->pages->used; i++) {
> +        p->iov[i].iov_base = p->pages->block->host + p->pages->offset[i];
> +        p->iov[i].iov_len = qemu_target_page_size();
> +    }

Similarly.


r~

Re: [PATCH 02/11] dump: Remove is_zero_page()

[Marc-André Lureau]

On Fri, Nov 19, 2021 at 8:59 PM Juan Quintela <quintela@redhat.com> wrote:
>
> It just calls buffer_is_zero().  Just change the callers.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  dump/dump.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 662d0a62cd..a84d8b1598 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -1293,14 +1293,6 @@ static size_t get_len_buf_out(size_t page_size, uint32_t flag_compress)
>      return 0;
>  }
>
> -/*
> - * check if the page is all 0
> - */
> -static inline bool is_zero_page(const uint8_t *buf, size_t page_size)
> -{
> -    return buffer_is_zero(buf, page_size);
> -}
> -
>  static void write_dump_pages(DumpState *s, Error **errp)
>  {
>      int ret = 0;
> @@ -1357,7 +1349,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
>       */
>      while (get_next_page(&block_iter, &pfn_iter, &buf, s)) {
>          /* check zero page */
> -        if (is_zero_page(buf, s->dump_info.page_size)) {
> +        if (buffer_is_zero(buf, s->dump_info.page_size)) {
>              ret = write_cache(&page_desc, &pd_zero, sizeof(PageDescriptor),
>                                false);
>              if (ret < 0) {
> --
> 2.33.1
>

Re: [PATCH 03/11] multifd: Fill offset and block for reception

[Juan Quintela]

Richard Henderson <richard.henderson@linaro.org> wrote:
> On 11/19/21 5:58 PM, Juan Quintela wrote:
>> We were using the iov directly, but we will need this info on the
>> following patch.
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>   migration/multifd.c | 2 ++
>>   1 file changed, 2 insertions(+)
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index 7c9deb1921..e2adcdffa1 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -364,6 +364,8 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
>>                          offset, block->used_length);
>>               return -1;
>>           }
>> +        p->pages->offset[i] = offset;
>> +        p->pages->block = block;
>>           p->pages->iov[i].iov_base = block->host + offset;
>>           p->pages->iov[i].iov_len = qemu_target_page_size();
>>       }
>> 
>
> Block should be stored one outside the loop.

Done.

Thanks, Juan.

Re: [PATCH 06/11] migration: Move iov from pages to params

[Juan Quintela]

Richard Henderson <richard.henderson@linaro.org> wrote:
> On 11/19/21 5:58 PM, Juan Quintela wrote:
>>   static int nocomp_send_prepare(MultiFDSendParams *p, uint32_t used,
>>                                  Error **errp)
>>   {
>> +    MultiFDPages_t *pages = p->pages;
>> +
>> +    for (int i = 0; i < used; i++) {
>> +        p->iov[p->iovs_used].iov_base = pages->block->host + pages->offset[i];
>> +        p->iov[p->iovs_used].iov_len = qemu_target_page_size();
>> +        p->iovs_used++;
>> +    }
>> +
>>       p->next_packet_size = used * qemu_target_page_size();
>
> Compute qemu_target_page_size once in the function.
> Hoist p->iovs_used to a local variable around the loop.
>
>> @@ -154,7 +162,11 @@ static int nocomp_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
>>                      p->id, flags, MULTIFD_FLAG_NOCOMP);
>>           return -1;
>>       }
>> -    return qio_channel_readv_all(p->c, p->pages->iov, used, errp);
>> +    for (int i = 0; i < p->pages->used; i++) {
>> +        p->iov[i].iov_base = p->pages->block->host + p->pages->offset[i];
>> +        p->iov[i].iov_len = qemu_target_page_size();
>> +    }
>
> Similarly.

Done both.

Thank again, Juan.