On Fri, Feb 16, 2024 at 2:41 PM Hao Xiang <hao.xi...@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.xi...@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
