This change adds zero page counters and updates multifd send/receive
tracing format to track the newly added counters.

Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
---
 migration/multifd.c    | 43 ++++++++++++++++++++++++++++++++++--------
 migration/multifd.h    | 21 ++++++++++++++++++++-
 migration/ram.c        |  1 -
 migration/trace-events |  8 ++++----
 4 files changed, 59 insertions(+), 14 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index adfe8c9a0a..a33dba40d9 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -236,6 +236,8 @@ static void multifd_pages_reset(MultiFDPages_t *pages)
      * overwritten later when reused.
      */
     pages->num = 0;
+    pages->normal_num = 0;
+    pages->zero_num = 0;
     pages->block = NULL;
 }
 
@@ -309,6 +311,8 @@ static MultiFDPages_t *multifd_pages_init(uint32_t n)
 
     pages->allocated = n;
     pages->offset = g_new0(ram_addr_t, n);
+    pages->normal = g_new0(ram_addr_t, n);
+    pages->zero = g_new0(ram_addr_t, n);
 
     return pages;
 }
@@ -319,6 +323,10 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
     pages->allocated = 0;
     g_free(pages->offset);
     pages->offset = NULL;
+    g_free(pages->normal);
+    pages->normal = NULL;
+    g_free(pages->zero);
+    pages->zero = NULL;
     g_free(pages);
 }
 
@@ -332,6 +340,7 @@ void multifd_send_fill_packet(MultiFDSendParams *p)
     packet->flags = cpu_to_be32(p->flags);
     packet->pages_alloc = cpu_to_be32(p->pages->allocated);
     packet->normal_pages = cpu_to_be32(pages->num);
+    packet->zero_pages = cpu_to_be32(pages->zero_num);
     packet->next_packet_size = cpu_to_be32(p->next_packet_size);
 
     packet_num = qatomic_fetch_inc(&multifd_send_state->packet_num);
@@ -350,9 +359,10 @@ void multifd_send_fill_packet(MultiFDSendParams *p)
 
     p->packets_sent++;
     p->total_normal_pages += pages->num;
+    p->total_zero_pages += pages->zero_num;
 
-    trace_multifd_send(p->id, packet_num, pages->num, p->flags,
-                       p->next_packet_size);
+    trace_multifd_send(p->id, packet_num, pages->num, pages->zero_num,
+                       p->flags, p->next_packet_size);
 }
 
 static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
@@ -393,20 +403,29 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
     p->normal_num = be32_to_cpu(packet->normal_pages);
     if (p->normal_num > packet->pages_alloc) {
         error_setg(errp, "multifd: received packet "
-                   "with %u pages and expected maximum pages are %u",
+                   "with %u normal pages and expected maximum pages are %u",
                    p->normal_num, packet->pages_alloc) ;
         return -1;
     }
 
+    p->zero_num = be32_to_cpu(packet->zero_pages);
+    if (p->zero_num > packet->pages_alloc - p->normal_num) {
+        error_setg(errp, "multifd: received packet "
+                   "with %u zero pages and expected maximum zero pages are %u",
+                   p->zero_num, packet->pages_alloc - p->normal_num) ;
+        return -1;
+    }
+
     p->next_packet_size = be32_to_cpu(packet->next_packet_size);
     p->packet_num = be64_to_cpu(packet->packet_num);
     p->packets_recved++;
     p->total_normal_pages += p->normal_num;
+    p->total_zero_pages += p->zero_num;
 
-    trace_multifd_recv(p->id, p->packet_num, p->normal_num, p->flags,
-                       p->next_packet_size);
+    trace_multifd_recv(p->id, p->packet_num, p->normal_num, p->zero_num,
+                       p->flags, p->next_packet_size);
 
-    if (p->normal_num == 0) {
+    if (p->normal_num == 0 && p->zero_num == 0) {
         return 0;
     }
 
@@ -823,6 +842,8 @@ static void *multifd_send_thread(void *opaque)
 
             stat64_add(&mig_stats.multifd_bytes,
                        p->next_packet_size + p->packet_len);
+            stat64_add(&mig_stats.normal_pages, pages->num);
+            stat64_add(&mig_stats.zero_pages, pages->zero_num);
 
             multifd_pages_reset(p->pages);
             p->next_packet_size = 0;
@@ -866,7 +887,8 @@ out:
 
     rcu_unregister_thread();
     migration_threads_remove(thread);
-    trace_multifd_send_thread_end(p->id, p->packets_sent, p->total_normal_pages);
+    trace_multifd_send_thread_end(p->id, p->packets_sent, p->total_normal_pages,
+                                  p->total_zero_pages);
 
     return NULL;
 }
@@ -1132,6 +1154,8 @@ static void multifd_recv_cleanup_channel(MultiFDRecvParams *p)
     p->iov = NULL;
     g_free(p->normal);
     p->normal = NULL;
+    g_free(p->zero);
+    p->zero = NULL;
     multifd_recv_state->ops->recv_cleanup(p);
 }
 
@@ -1251,7 +1275,9 @@ static void *multifd_recv_thread(void *opaque)
     }
 
     rcu_unregister_thread();
-    trace_multifd_recv_thread_end(p->id, p->packets_recved, p->total_normal_pages);
+    trace_multifd_recv_thread_end(p->id, p->packets_recved,
+                                  p->total_normal_pages,
+                                  p->total_zero_pages);
 
     return NULL;
 }
@@ -1290,6 +1316,7 @@ int multifd_recv_setup(Error **errp)
         p->name = g_strdup_printf("multifdrecv_%d", i);
         p->iov = g_new0(struct iovec, page_count);
         p->normal = g_new0(ram_addr_t, page_count);
+        p->zero = g_new0(ram_addr_t, page_count);
         p->page_count = page_count;
         p->page_size = qemu_target_page_size();
     }
diff --git a/migration/multifd.h b/migration/multifd.h
index 8a1cad0996..9822ff298a 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -48,7 +48,10 @@ typedef struct {
     /* size of the next packet that contains pages */
     uint32_t next_packet_size;
     uint64_t packet_num;
-    uint64_t unused[4];    /* Reserved for future use */
+    /* zero pages */
+    uint32_t zero_pages;
+    uint32_t unused32[1];    /* Reserved for future use */
+    uint64_t unused64[3];    /* Reserved for future use */
     char ramblock[256];
     uint64_t offset[];
 } __attribute__((packed)) MultiFDPacket_t;
@@ -56,10 +59,18 @@ typedef struct {
 typedef struct {
     /* number of used pages */
     uint32_t num;
+    /* number of normal pages */
+    uint32_t normal_num;
+    /* number of zero pages */
+    uint32_t zero_num;
     /* number of allocated pages */
     uint32_t allocated;
     /* offset of each page */
     ram_addr_t *offset;
+    /* offset of normal page */
+    ram_addr_t *normal;
+    /* offset of zero page */
+    ram_addr_t *zero;
     RAMBlock *block;
 } MultiFDPages_t;
 
@@ -124,6 +135,8 @@ typedef struct {
     uint64_t packets_sent;
     /* non zero pages sent through this channel */
     uint64_t total_normal_pages;
+    /* zero pages sent through this channel */
+    uint64_t total_zero_pages;
     /* buffers to send */
     struct iovec *iov;
     /* number of iovs used */
@@ -178,12 +191,18 @@ typedef struct {
     uint8_t *host;
     /* non zero pages recv through this channel */
     uint64_t total_normal_pages;
+    /* zero pages recv through this channel */
+    uint64_t total_zero_pages;
     /* buffers to recv */
     struct iovec *iov;
     /* Pages that are not zero */
     ram_addr_t *normal;
     /* num of non zero pages */
     uint32_t normal_num;
+    /* Pages that are zero */
+    ram_addr_t *zero;
+    /* num of zero pages */
+    uint32_t zero_num;
     /* used for de-compression methods */
     void *data;
 } MultiFDRecvParams;
diff --git a/migration/ram.c b/migration/ram.c
index 556725c30f..5ece9f042e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1259,7 +1259,6 @@ static int ram_save_multifd_page(RAMBlock *block, ram_addr_t offset)
     if (!multifd_queue_page(block, offset)) {
         return -1;
     }
-    stat64_add(&mig_stats.normal_pages, 1);
 
     return 1;
 }
diff --git a/migration/trace-events b/migration/trace-events
index 298ad2b0dd..9f1d7ae71a 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -128,21 +128,21 @@ postcopy_preempt_reset_channel(void) ""
 # multifd.c
 multifd_new_send_channel_async(uint8_t id) "channel %u"
 multifd_new_send_channel_async_error(uint8_t id, void *err) "channel=%u err=%p"
-multifd_recv(uint8_t id, uint64_t packet_num, uint32_t used, uint32_t flags, uint32_t next_packet_size) "channel %u packet_num %" PRIu64 " pages %u flags 0x%x next packet size %u"
+multifd_recv(uint8_t id, uint64_t packet_num, uint32_t normal, uint32_t zero, uint32_t flags, uint32_t next_packet_size) "channel %u packet_num %" PRIu64 " normal pages %u zero pages %u flags 0x%x next packet size %u"
 multifd_recv_new_channel(uint8_t id) "channel %u"
 multifd_recv_sync_main(long packet_num) "packet num %ld"
 multifd_recv_sync_main_signal(uint8_t id) "channel %u"
 multifd_recv_sync_main_wait(uint8_t id) "channel %u"
 multifd_recv_terminate_threads(bool error) "error %d"
-multifd_recv_thread_end(uint8_t id, uint64_t packets, uint64_t pages) "channel %u packets %" PRIu64 " pages %" PRIu64
+multifd_recv_thread_end(uint8_t id, uint64_t packets, uint64_t normal_pages, uint64_t zero_pages) "channel %u packets %" PRIu64 " normal pages %" PRIu64 " zero pages %" PRIu64
 multifd_recv_thread_start(uint8_t id) "%u"
-multifd_send(uint8_t id, uint64_t packet_num, uint32_t normal, uint32_t flags, uint32_t next_packet_size) "channel %u packet_num %" PRIu64 " normal pages %u flags 0x%x next packet size %u"
+multifd_send(uint8_t id, uint64_t packet_num, uint32_t normal_pages, uint32_t zero_pages, uint32_t flags, uint32_t next_packet_size) "channel %u packet_num %" PRIu64 " normal pages %u zero pages %u flags 0x%x next packet size %u"
 multifd_send_error(uint8_t id) "channel %u"
 multifd_send_sync_main(long packet_num) "packet num %ld"
 multifd_send_sync_main_signal(uint8_t id) "channel %u"
 multifd_send_sync_main_wait(uint8_t id) "channel %u"
 multifd_send_terminate_threads(void) ""
-multifd_send_thread_end(uint8_t id, uint64_t packets, uint64_t normal_pages) "channel %u packets %" PRIu64 " normal pages %"  PRIu64
+multifd_send_thread_end(uint8_t id, uint64_t packets, uint64_t normal_pages, uint64_t zero_pages) "channel %u packets %" PRIu64 " normal pages %"  PRIu64 " zero pages %"  PRIu64
 multifd_send_thread_start(uint8_t id) "%u"
 multifd_tls_outgoing_handshake_start(void *ioc, void *tioc, const char *hostname) "ioc=%p tioc=%p hostname=%s"
 multifd_tls_outgoing_handshake_error(void *ioc, const char *err) "ioc=%p err=%s"
-- 
2.30.2

On Fri, Feb 16, 2024 at 2:41 PM Hao Xiang <hao.xiang@bytedance.com> wrote:

> This change adds zero page counters and updates multifd send/receive
> tracing format to track the newly added counters.
>
> Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
> ---
>  migration/multifd.c    | 43 ++++++++++++++++++++++++++++++++++--------
>  migration/multifd.h    | 21 ++++++++++++++++++++-
>  migration/ram.c        |  1 -
>  migration/trace-events |  8 ++++----
>  4 files changed, 59 insertions(+), 14 deletions(-)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index adfe8c9a0a..a33dba40d9 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -236,6 +236,8 @@ static void multifd_pages_reset(MultiFDPages_t *pages)
>       * overwritten later when reused.
>       */
>      pages->num = 0;
> +    pages->normal_num = 0;
> +    pages->zero_num = 0;
>      pages->block = NULL;
>  }
>

> @@ -309,6 +311,8 @@ static MultiFDPages_t *multifd_pages_init(uint32_t n)
>
>      pages->allocated = n;
>      pages->offset = g_new0(ram_addr_t, n);
> +    pages->normal = g_new0(ram_addr_t, n);
> +    pages->zero = g_new0(ram_addr_t, n);
>
>
     return pages;
>  }
> @@ -319,6 +323,10 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
>      pages->allocated = 0;
>      g_free(pages->offset);
>      pages->offset = NULL;
> +    g_free(pages->normal);
> +    pages->normal = NULL;
> +    g_free(pages->zero);
> +    pages->zero = NULL;
>      g_free(pages);
>  }
>
> @@ -332,6 +340,7 @@ void multifd_send_fill_packet(MultiFDSendParams *p)
>      packet->flags = cpu_to_be32(p->flags);
>      packet->pages_alloc = cpu_to_be32(p->pages->allocated);
>      packet->normal_pages = cpu_to_be32(pages->num);
> +    packet->zero_pages = cpu_to_be32(pages->zero_num);
>      packet->next_packet_size = cpu_to_be32(p->next_packet_size);
>
>      packet_num = qatomic_fetch_inc(&multifd_send_state->packet_num);
> @@ -350,9 +359,10 @@ void multifd_send_fill_packet(MultiFDSendParams *p)
>
>      p->packets_sent++;
>      p->total_normal_pages += pages->num;
> +    p->total_zero_pages += pages->zero_num;
>
> -    trace_multifd_send(p->id, packet_num, pages->num, p->flags,
> -                       p->next_packet_size);
> +    trace_multifd_send(p->id, packet_num, pages->num, pages->zero_num,
> +                       p->flags, p->next_packet_size);
>  }
>
>  static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
> @@ -393,20 +403,29 @@ static int
> multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
>      p->normal_num = be32_to_cpu(packet->normal_pages);
>      if (p->normal_num > packet->pages_alloc) {
>          error_setg(errp, "multifd: received packet "
> -                   "with %u pages and expected maximum pages are %u",
> +                   "with %u normal pages and expected maximum pages are
> %u",
>                     p->normal_num, packet->pages_alloc) ;
>          return -1;
>      }
>
> +    p->zero_num = be32_to_cpu(packet->zero_pages);
> +    if (p->zero_num > packet->pages_alloc - p->normal_num) {
> +        error_setg(errp, "multifd: received packet "
> +                   "with %u zero pages and expected maximum zero pages
> are %u",
> +                   p->zero_num, packet->pages_alloc - p->normal_num) ;
> +        return -1;
> +    }


You could probably combine this check with normal_num against pages_alloc.

> +
>      p->next_packet_size = be32_to_cpu(packet->next_packet_size);
>      p->packet_num = be64_to_cpu(packet->packet_num);
>      p->packets_recved++;
>      p->total_normal_pages += p->normal_num;
> +    p->total_zero_pages += p->zero_num;
>
> -    trace_multifd_recv(p->id, p->packet_num, p->normal_num, p->flags,
> -                       p->next_packet_size);
> +    trace_multifd_recv(p->id, p->packet_num, p->normal_num, p->zero_num,
> +                       p->flags, p->next_packet_size);
>
> -    if (p->normal_num == 0) {
> +    if (p->normal_num == 0 && p->zero_num == 0) {
>          return 0;
>      }
>
> @@ -823,6 +842,8 @@ static void *multifd_send_thread(void *opaque)
>
>              stat64_add(&mig_stats.multifd_bytes,
>                         p->next_packet_size + p->packet_len);
> +            stat64_add(&mig_stats.normal_pages, pages->num);
>

That seems wrong. pages->num is the number of pages total in the packet.
But next patch changes it, so I suggest or change it here and not in 3/7.

+            stat64_add(&mig_stats.zero_pages, pages->zero_num);
>
>              multifd_pages_reset(p->pages);
>              p->next_packet_size = 0;
> @@ -866,7 +887,8 @@ out:
>
>      rcu_unregister_thread();
>      migration_threads_remove(thread);
> -    trace_multifd_send_thread_end(p->id, p->packets_sent,
> p->total_normal_pages);
> +    trace_multifd_send_thread_end(p->id, p->packets_sent,
> p->total_normal_pages,
> +                                  p->total_zero_pages);
>
>      return NULL;
>  }
> @@ -1132,6 +1154,8 @@ static void
> multifd_recv_cleanup_channel(MultiFDRecvParams *p)
>      p->iov = NULL;
>      g_free(p->normal);
>      p->normal = NULL;
> +    g_free(p->zero);
> +    p->zero = NULL;
>      multifd_recv_state->ops->recv_cleanup(p);
>  }
>
> @@ -1251,7 +1275,9 @@ static void *multifd_recv_thread(void *opaque)
>      }
>
>      rcu_unregister_thread();
> -    trace_multifd_recv_thread_end(p->id, p->packets_recved,
> p->total_normal_pages);
> +    trace_multifd_recv_thread_end(p->id, p->packets_recved,
> +                                  p->total_normal_pages,
> +                                  p->total_zero_pages);
>
>      return NULL;
>  }
> @@ -1290,6 +1316,7 @@ int multifd_recv_setup(Error **errp)
>          p->name = g_strdup_printf("multifdrecv_%d", i);
>          p->iov = g_new0(struct iovec, page_count);
>          p->normal = g_new0(ram_addr_t, page_count);
> +        p->zero = g_new0(ram_addr_t, page_count);
>          p->page_count = page_count;
>          p->page_size = qemu_target_page_size();
>      }
> diff --git a/migration/multifd.h b/migration/multifd.h
> index 8a1cad0996..9822ff298a 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -48,7 +48,10 @@ typedef struct {
>      /* size of the next packet that contains pages */
>      uint32_t next_packet_size;
>      uint64_t packet_num;
> -    uint64_t unused[4];    /* Reserved for future use */
> +    /* zero pages */
> +    uint32_t zero_pages;
> +    uint32_t unused32[1];    /* Reserved for future use */
> +    uint64_t unused64[3];    /* Reserved for future use */
>      char ramblock[256];
>      uint64_t offset[];
>  } __attribute__((packed)) MultiFDPacket_t;
> @@ -56,10 +59,18 @@ typedef struct {
>  typedef struct {
>      /* number of used pages */
>      uint32_t num;
> +    /* number of normal pages */
> +    uint32_t normal_num;
> +    /* number of zero pages */
> +    uint32_t zero_num;
>      /* number of allocated pages */
>      uint32_t allocated;
>      /* offset of each page */
>      ram_addr_t *offset;
> +    /* offset of normal page */
> +    ram_addr_t *normal;
> +    /* offset of zero page */
> +    ram_addr_t *zero;
>      RAMBlock *block;
>  } MultiFDPages_t;
>
> @@ -124,6 +135,8 @@ typedef struct {
>      uint64_t packets_sent;
>      /* non zero pages sent through this channel */
>      uint64_t total_normal_pages;
> +    /* zero pages sent through this channel */
> +    uint64_t total_zero_pages;
>

Can we initialize these to zero when threads are being set up?
Also, I have a strong desire to rename these.. later.


>      /* buffers to send */
>      struct iovec *iov;
>      /* number of iovs used */
> @@ -178,12 +191,18 @@ typedef struct {
>      uint8_t *host;
>      /* non zero pages recv through this channel */
>      uint64_t total_normal_pages;
> +    /* zero pages recv through this channel */
> +    uint64_t total_zero_pages;
>      /* buffers to recv */
>      struct iovec *iov;
>      /* Pages that are not zero */
>      ram_addr_t *normal;
>      /* num of non zero pages */
>      uint32_t normal_num;
> +    /* Pages that are zero */
> +    ram_addr_t *zero;
> +    /* num of zero pages */
> +    uint32_t zero_num;
>      /* used for de-compression methods */
>      void *data;
>  } MultiFDRecvParams;
> diff --git a/migration/ram.c b/migration/ram.c
> index 556725c30f..5ece9f042e 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1259,7 +1259,6 @@ static int ram_save_multifd_page(RAMBlock *block,
> ram_addr_t offset)
>      if (!multifd_queue_page(block, offset)) {
>          return -1;
>      }
> -    stat64_add(&mig_stats.normal_pages, 1);
>
>      return 1;
>  }
> diff --git a/migration/trace-events b/migration/trace-events
> index 298ad2b0dd..9f1d7ae71a 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -128,21 +128,21 @@ postcopy_preempt_reset_channel(void) ""
>  # multifd.c
>  multifd_new_send_channel_async(uint8_t id) "channel %u"
>  multifd_new_send_channel_async_error(uint8_t id, void *err) "channel=%u
> err=%p"
> -multifd_recv(uint8_t id, uint64_t packet_num, uint32_t used, uint32_t
> flags, uint32_t next_packet_size) "channel %u packet_num %" PRIu64 " pages
> %u flags 0x%x next packet size %u"
> +multifd_recv(uint8_t id, uint64_t packet_num, uint32_t normal, uint32_t
> zero, uint32_t flags, uint32_t next_packet_size) "channel %u packet_num %"
> PRIu64 " normal pages %u zero pages %u flags 0x%x next packet size %u"
>  multifd_recv_new_channel(uint8_t id) "channel %u"
>  multifd_recv_sync_main(long packet_num) "packet num %ld"
>  multifd_recv_sync_main_signal(uint8_t id) "channel %u"
>  multifd_recv_sync_main_wait(uint8_t id) "channel %u"
>  multifd_recv_terminate_threads(bool error) "error %d"
> -multifd_recv_thread_end(uint8_t id, uint64_t packets, uint64_t pages)
> "channel %u packets %" PRIu64 " pages %" PRIu64
> +multifd_recv_thread_end(uint8_t id, uint64_t packets, uint64_t
> normal_pages, uint64_t zero_pages) "channel %u packets %" PRIu64 " normal
> pages %" PRIu64 " zero pages %" PRIu64
>  multifd_recv_thread_start(uint8_t id) "%u"
> -multifd_send(uint8_t id, uint64_t packet_num, uint32_t normal, uint32_t
> flags, uint32_t next_packet_size) "channel %u packet_num %" PRIu64 " normal
> pages %u flags 0x%x next packet size %u"
> +multifd_send(uint8_t id, uint64_t packet_num, uint32_t normal_pages,
> uint32_t zero_pages, uint32_t flags, uint32_t next_packet_size) "channel %u
> packet_num %" PRIu64 " normal pages %u zero pages %u flags 0x%x next packet
> size %u"
>  multifd_send_error(uint8_t id) "channel %u"
>  multifd_send_sync_main(long packet_num) "packet num %ld"
>  multifd_send_sync_main_signal(uint8_t id) "channel %u"
>  multifd_send_sync_main_wait(uint8_t id) "channel %u"
>  multifd_send_terminate_threads(void) ""
> -multifd_send_thread_end(uint8_t id, uint64_t packets, uint64_t
> normal_pages) "channel %u packets %" PRIu64 " normal pages %"  PRIu64
> +multifd_send_thread_end(uint8_t id, uint64_t packets, uint64_t
> normal_pages, uint64_t zero_pages) "channel %u packets %" PRIu64 " normal
> pages %"  PRIu64 " zero pages %"  PRIu64
>  multifd_send_thread_start(uint8_t id) "%u"
>  multifd_tls_outgoing_handshake_start(void *ioc, void *tioc, const char
> *hostname) "ioc=%p tioc=%p hostname=%s"
>  multifd_tls_outgoing_handshake_error(void *ioc, const char *err) "ioc=%p
> err=%s"
> --
> 2.30.2
>
>
>

-- 
Elena

On Wed, Feb 21, 2024 at 7:37 AM Elena Ufimtseva <ufimtseva@gmail.com> wrote:
>
>
>
> On Fri, Feb 16, 2024 at 2:41 PM Hao Xiang <hao.xiang@bytedance.com> wrote:
>>
>> This change adds zero page counters and updates multifd send/receive
>> tracing format to track the newly added counters.
>>
>> Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
>> ---
>>  migration/multifd.c    | 43 ++++++++++++++++++++++++++++++++++--------
>>  migration/multifd.h    | 21 ++++++++++++++++++++-
>>  migration/ram.c        |  1 -
>>  migration/trace-events |  8 ++++----
>>  4 files changed, 59 insertions(+), 14 deletions(-)
>>
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index adfe8c9a0a..a33dba40d9 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -236,6 +236,8 @@ static void multifd_pages_reset(MultiFDPages_t *pages)
>>       * overwritten later when reused.
>>       */
>>      pages->num = 0;
>> +    pages->normal_num = 0;
>> +    pages->zero_num = 0;
>>      pages->block = NULL;
>>  }
>>
>>
>> @@ -309,6 +311,8 @@ static MultiFDPages_t *multifd_pages_init(uint32_t n)
>>
>>      pages->allocated = n;
>>      pages->offset = g_new0(ram_addr_t, n);
>> +    pages->normal = g_new0(ram_addr_t, n);
>> +    pages->zero = g_new0(ram_addr_t, n);
>>
>>
>>      return pages;
>>  }
>> @@ -319,6 +323,10 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
>>      pages->allocated = 0;
>>      g_free(pages->offset);
>>      pages->offset = NULL;
>> +    g_free(pages->normal);
>> +    pages->normal = NULL;
>> +    g_free(pages->zero);
>> +    pages->zero = NULL;
>>      g_free(pages);
>>  }
>>
>> @@ -332,6 +340,7 @@ void multifd_send_fill_packet(MultiFDSendParams *p)
>>      packet->flags = cpu_to_be32(p->flags);
>>      packet->pages_alloc = cpu_to_be32(p->pages->allocated);
>>      packet->normal_pages = cpu_to_be32(pages->num);
>> +    packet->zero_pages = cpu_to_be32(pages->zero_num);
>>      packet->next_packet_size = cpu_to_be32(p->next_packet_size);
>>
>>      packet_num = qatomic_fetch_inc(&multifd_send_state->packet_num);
>> @@ -350,9 +359,10 @@ void multifd_send_fill_packet(MultiFDSendParams *p)
>>
>>      p->packets_sent++;
>>      p->total_normal_pages += pages->num;
>> +    p->total_zero_pages += pages->zero_num;
>>
>> -    trace_multifd_send(p->id, packet_num, pages->num, p->flags,
>> -                       p->next_packet_size);
>> +    trace_multifd_send(p->id, packet_num, pages->num, pages->zero_num,
>> +                       p->flags, p->next_packet_size);
>>  }
>>
>>  static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
>> @@ -393,20 +403,29 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
>>      p->normal_num = be32_to_cpu(packet->normal_pages);
>>      if (p->normal_num > packet->pages_alloc) {
>>          error_setg(errp, "multifd: received packet "
>> -                   "with %u pages and expected maximum pages are %u",
>> +                   "with %u normal pages and expected maximum pages are %u",
>>                     p->normal_num, packet->pages_alloc) ;
>>          return -1;
>>      }
>>
>> +    p->zero_num = be32_to_cpu(packet->zero_pages);
>> +    if (p->zero_num > packet->pages_alloc - p->normal_num) {
>> +        error_setg(errp, "multifd: received packet "
>> +                   "with %u zero pages and expected maximum zero pages are %u",
>> +                   p->zero_num, packet->pages_alloc - p->normal_num) ;
>> +        return -1;
>> +    }
>
>
> You could probably combine this check with normal_num against pages_alloc.
>>
>> +
>>      p->next_packet_size = be32_to_cpu(packet->next_packet_size);
>>      p->packet_num = be64_to_cpu(packet->packet_num);
>>      p->packets_recved++;
>>      p->total_normal_pages += p->normal_num;
>> +    p->total_zero_pages += p->zero_num;
>>
>> -    trace_multifd_recv(p->id, p->packet_num, p->normal_num, p->flags,
>> -                       p->next_packet_size);
>> +    trace_multifd_recv(p->id, p->packet_num, p->normal_num, p->zero_num,
>> +                       p->flags, p->next_packet_size);
>>
>> -    if (p->normal_num == 0) {
>> +    if (p->normal_num == 0 && p->zero_num == 0) {
>>          return 0;
>>      }
>>
>> @@ -823,6 +842,8 @@ static void *multifd_send_thread(void *opaque)
>>
>>              stat64_add(&mig_stats.multifd_bytes,
>>                         p->next_packet_size + p->packet_len);
>> +            stat64_add(&mig_stats.normal_pages, pages->num);
>
>
> That seems wrong. pages->num is the number of pages total in the packet.
> But next patch changes it, so I suggest or change it here and not in 3/7.

In this patch, multifd zero pages are not enabled yet. So pages->num
are the number of normal pages not pages total in the packet. The zero
pages were send in a different format in save_zero_page. Later on,
when multifd zero page is enabled, pages->normal_num counts the number
of normal pages and hence the accounting is changed.

>
>> +            stat64_add(&mig_stats.zero_pages, pages->zero_num);
>>
>>              multifd_pages_reset(p->pages);
>>              p->next_packet_size = 0;
>> @@ -866,7 +887,8 @@ out:
>>
>>      rcu_unregister_thread();
>>      migration_threads_remove(thread);
>> -    trace_multifd_send_thread_end(p->id, p->packets_sent, p->total_normal_pages);
>> +    trace_multifd_send_thread_end(p->id, p->packets_sent, p->total_normal_pages,
>> +                                  p->total_zero_pages);
>>
>>      return NULL;
>>  }
>> @@ -1132,6 +1154,8 @@ static void multifd_recv_cleanup_channel(MultiFDRecvParams *p)
>>      p->iov = NULL;
>>      g_free(p->normal);
>>      p->normal = NULL;
>> +    g_free(p->zero);
>> +    p->zero = NULL;
>>      multifd_recv_state->ops->recv_cleanup(p);
>>  }
>>
>> @@ -1251,7 +1275,9 @@ static void *multifd_recv_thread(void *opaque)
>>      }
>>
>>      rcu_unregister_thread();
>> -    trace_multifd_recv_thread_end(p->id, p->packets_recved, p->total_normal_pages);
>> +    trace_multifd_recv_thread_end(p->id, p->packets_recved,
>> +                                  p->total_normal_pages,
>> +                                  p->total_zero_pages);
>>
>>      return NULL;
>>  }
>> @@ -1290,6 +1316,7 @@ int multifd_recv_setup(Error **errp)
>>          p->name = g_strdup_printf("multifdrecv_%d", i);
>>          p->iov = g_new0(struct iovec, page_count);
>>          p->normal = g_new0(ram_addr_t, page_count);
>> +        p->zero = g_new0(ram_addr_t, page_count);
>>          p->page_count = page_count;
>>          p->page_size = qemu_target_page_size();
>>      }
>> diff --git a/migration/multifd.h b/migration/multifd.h
>> index 8a1cad0996..9822ff298a 100644
>> --- a/migration/multifd.h
>> +++ b/migration/multifd.h
>> @@ -48,7 +48,10 @@ typedef struct {
>>      /* size of the next packet that contains pages */
>>      uint32_t next_packet_size;
>>      uint64_t packet_num;
>> -    uint64_t unused[4];    /* Reserved for future use */
>> +    /* zero pages */
>> +    uint32_t zero_pages;
>> +    uint32_t unused32[1];    /* Reserved for future use */
>> +    uint64_t unused64[3];    /* Reserved for future use */
>>      char ramblock[256];
>>      uint64_t offset[];
>>  } __attribute__((packed)) MultiFDPacket_t;
>> @@ -56,10 +59,18 @@ typedef struct {
>>  typedef struct {
>>      /* number of used pages */
>>      uint32_t num;
>> +    /* number of normal pages */
>> +    uint32_t normal_num;
>> +    /* number of zero pages */
>> +    uint32_t zero_num;
>>      /* number of allocated pages */
>>      uint32_t allocated;
>>      /* offset of each page */
>>      ram_addr_t *offset;
>> +    /* offset of normal page */
>> +    ram_addr_t *normal;
>> +    /* offset of zero page */
>> +    ram_addr_t *zero;
>>      RAMBlock *block;
>>  } MultiFDPages_t;
>>
>> @@ -124,6 +135,8 @@ typedef struct {
>>      uint64_t packets_sent;
>>      /* non zero pages sent through this channel */
>>      uint64_t total_normal_pages;
>> +    /* zero pages sent through this channel */
>> +    uint64_t total_zero_pages;
>
>
> Can we initialize these to zero when threads are being set up?
> Also, I have a strong desire to rename these.. later.

When MultiFDSendParams are allocated in multifd_send_setup, g_new0
will initialize them to zero.

>
>>
>>      /* buffers to send */
>>      struct iovec *iov;
>>      /* number of iovs used */
>> @@ -178,12 +191,18 @@ typedef struct {
>>      uint8_t *host;
>>      /* non zero pages recv through this channel */
>>      uint64_t total_normal_pages;
>> +    /* zero pages recv through this channel */
>> +    uint64_t total_zero_pages;
>>      /* buffers to recv */
>>      struct iovec *iov;
>>      /* Pages that are not zero */
>>      ram_addr_t *normal;
>>      /* num of non zero pages */
>>      uint32_t normal_num;
>> +    /* Pages that are zero */
>> +    ram_addr_t *zero;
>> +    /* num of zero pages */
>> +    uint32_t zero_num;
>>      /* used for de-compression methods */
>>      void *data;
>>  } MultiFDRecvParams;
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 556725c30f..5ece9f042e 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -1259,7 +1259,6 @@ static int ram_save_multifd_page(RAMBlock *block, ram_addr_t offset)
>>      if (!multifd_queue_page(block, offset)) {
>>          return -1;
>>      }
>> -    stat64_add(&mig_stats.normal_pages, 1);
>>
>>      return 1;
>>  }
>> diff --git a/migration/trace-events b/migration/trace-events
>> index 298ad2b0dd..9f1d7ae71a 100644
>> --- a/migration/trace-events
>> +++ b/migration/trace-events
>> @@ -128,21 +128,21 @@ postcopy_preempt_reset_channel(void) ""
>>  # multifd.c
>>  multifd_new_send_channel_async(uint8_t id) "channel %u"
>>  multifd_new_send_channel_async_error(uint8_t id, void *err) "channel=%u err=%p"
>> -multifd_recv(uint8_t id, uint64_t packet_num, uint32_t used, uint32_t flags, uint32_t next_packet_size) "channel %u packet_num %" PRIu64 " pages %u flags 0x%x next packet size %u"
>> +multifd_recv(uint8_t id, uint64_t packet_num, uint32_t normal, uint32_t zero, uint32_t flags, uint32_t next_packet_size) "channel %u packet_num %" PRIu64 " normal pages %u zero pages %u flags 0x%x next packet size %u"
>>  multifd_recv_new_channel(uint8_t id) "channel %u"
>>  multifd_recv_sync_main(long packet_num) "packet num %ld"
>>  multifd_recv_sync_main_signal(uint8_t id) "channel %u"
>>  multifd_recv_sync_main_wait(uint8_t id) "channel %u"
>>  multifd_recv_terminate_threads(bool error) "error %d"
>> -multifd_recv_thread_end(uint8_t id, uint64_t packets, uint64_t pages) "channel %u packets %" PRIu64 " pages %" PRIu64
>> +multifd_recv_thread_end(uint8_t id, uint64_t packets, uint64_t normal_pages, uint64_t zero_pages) "channel %u packets %" PRIu64 " normal pages %" PRIu64 " zero pages %" PRIu64
>>  multifd_recv_thread_start(uint8_t id) "%u"
>> -multifd_send(uint8_t id, uint64_t packet_num, uint32_t normal, uint32_t flags, uint32_t next_packet_size) "channel %u packet_num %" PRIu64 " normal pages %u flags 0x%x next packet size %u"
>> +multifd_send(uint8_t id, uint64_t packet_num, uint32_t normal_pages, uint32_t zero_pages, uint32_t flags, uint32_t next_packet_size) "channel %u packet_num %" PRIu64 " normal pages %u zero pages %u flags 0x%x next packet size %u"
>>  multifd_send_error(uint8_t id) "channel %u"
>>  multifd_send_sync_main(long packet_num) "packet num %ld"
>>  multifd_send_sync_main_signal(uint8_t id) "channel %u"
>>  multifd_send_sync_main_wait(uint8_t id) "channel %u"
>>  multifd_send_terminate_threads(void) ""
>> -multifd_send_thread_end(uint8_t id, uint64_t packets, uint64_t normal_pages) "channel %u packets %" PRIu64 " normal pages %"  PRIu64
>> +multifd_send_thread_end(uint8_t id, uint64_t packets, uint64_t normal_pages, uint64_t zero_pages) "channel %u packets %" PRIu64 " normal pages %"  PRIu64 " zero pages %"  PRIu64
>>  multifd_send_thread_start(uint8_t id) "%u"
>>  multifd_tls_outgoing_handshake_start(void *ioc, void *tioc, const char *hostname) "ioc=%p tioc=%p hostname=%s"
>>  multifd_tls_outgoing_handshake_error(void *ioc, const char *err) "ioc=%p err=%s"
>> --
>> 2.30.2
>>
>>
>
>
> --
> Elena