Hao Xiang <hao.xi...@bytedance.com> writes:
> 1. Add zero_pages field in MultiFDPacket_t.
> 2. Implements the zero page detection and handling on the multifd
> threads for non-compression, zlib and zstd compression backends.
> 3. Added a new value 'multifd' in ZeroPageDetection enumeration.
> 4. Handle migration QEMU9.0 -> QEMU8.2 compatibility.
> 5. 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>
> ---
> hw/core/machine.c | 4 +-
> hw/core/qdev-properties-system.c | 2 +-
> migration/meson.build | 1 +
> migration/multifd-zero-page.c | 78 ++++++++++++++++++++++++++++++
> migration/multifd-zlib.c | 21 ++++++--
> migration/multifd-zstd.c | 20 ++++++--
> migration/multifd.c | 83 +++++++++++++++++++++++++++-----
> migration/multifd.h | 24 ++++++++-
> migration/ram.c | 1 -
> migration/trace-events | 8 +--
> qapi/migration.json | 5 +-
> 11 files changed, 214 insertions(+), 33 deletions(-)
> create mode 100644 migration/multifd-zero-page.c
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index fb5afdcae4..746da219a4 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -32,7 +32,9 @@
> #include "hw/virtio/virtio-net.h"
> #include "audio/audio.h"
>
> -GlobalProperty hw_compat_8_2[] = {};
> +GlobalProperty hw_compat_8_2[] = {
> + { "migration", "zero-page-detection", "legacy"},
> +};
> const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>
> GlobalProperty hw_compat_8_1[] = {
> diff --git a/hw/core/qdev-properties-system.c
> b/hw/core/qdev-properties-system.c
> index 228e685f52..6e6f68ae1b 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -682,7 +682,7 @@ const PropertyInfo qdev_prop_mig_mode = {
> const PropertyInfo qdev_prop_zero_page_detection = {
> .name = "ZeroPageDetection",
> .description = "zero_page_detection values, "
> - "none,legacy",
> + "none,legacy,multifd",
> .enum_table = &ZeroPageDetection_lookup,
> .get = qdev_propinfo_get_enum,
> .set = qdev_propinfo_set_enum,
> diff --git a/migration/meson.build b/migration/meson.build
> index 92b1cc4297..1eeb915ff6 100644
> --- a/migration/meson.build
> +++ b/migration/meson.build
> @@ -22,6 +22,7 @@ system_ss.add(files(
> 'migration.c',
> 'multifd.c',
> 'multifd-zlib.c',
> + 'multifd-zero-page.c',
> 'ram-compress.c',
> 'options.c',
> 'postcopy-ram.c',
> diff --git a/migration/multifd-zero-page.c b/migration/multifd-zero-page.c
> new file mode 100644
> index 0000000000..1650c41b26
> --- /dev/null
> +++ b/migration/multifd-zero-page.c
> @@ -0,0 +1,78 @@
> +/*
> + * Multifd zero page detection implementation.
> + *
> + * Copyright (c) 2024 Bytedance Inc
> + *
> + * Authors:
> + * Hao Xiang <hao.xi...@bytedance.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/cutils.h"
> +#include "exec/ramblock.h"
> +#include "migration.h"
> +#include "multifd.h"
> +#include "options.h"
> +#include "ram.h"
> +
> +static void swap_page_offset(ram_addr_t *pages_offset, int a, int b)
> +{
> + ram_addr_t temp;
> +
> + if (a == b) {
> + return;
> + }
> +
> + temp = pages_offset[a];
> + pages_offset[a] = pages_offset[b];
> + pages_offset[b] = temp;
> +}
> +
> +/**
> + * multifd_zero_page_check_send: Perform zero page detection on all pages.
> + *
> + * Sort the page offset array by moving all normal pages to
> + * the left and all zero pages to the right of the array.
Let's move this to the loop as a comment. Here it's better to just
inform about the side effects:
Sorts normal pages before zero pages in p->pages->offset and updates
p->pages->normal_num.
> + *
> + * @param p A pointer to the send params.
> + */
> +void multifd_zero_page_check_send(MultiFDSendParams *p)
Use multifd_send_zero_page_check for consistency with the rest of the code.
> +{
> + /*
> + * QEMU older than 9.0 don't understand zero page
> + * on multifd channel. This switch is required to
> + * maintain backward compatibility.
> + */
> + bool use_multifd_zero_page =
> + (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_MULTIFD);
Could introduce a helper for this.
static bool multifd_zero_page(void)
{
return migrate_zero_page_detection() == ZERO_PAGE_DETECTION_MULTIFD;
}
> + MultiFDPages_t *pages = p->pages;
> + RAMBlock *rb = pages->block;
> + int index_normal = 0;
> + int index_zero = pages->num - 1;
IMHO, i and j are more idiomatic, makes the code easier on the eyes.
> +
> + while (index_normal <= index_zero) {
> + uint64_t offset = pages->offset[index_normal];
> + if (use_multifd_zero_page &&
You don't need to check this at every iteration. Could check at the top
and exit right away.
> + buffer_is_zero(rb->host + offset, p->page_size)) {
> + swap_page_offset(pages->offset, index_normal, index_zero);
> + index_zero--;
> + ram_release_page(rb->idstr, offset);
> + } else {
> + index_normal++;
> + pages->normal_num++;
Not a fan of changing pages->normal_num like this. Let's make the loop
just sort and update normal_num at the end.
Putting all together:
void multifd_send_zero_page_check(MultiFDSendParams *p)
{
MultiFDPages_t *pages = p->pages;
RAMBlock *rb = pages->block;
int i = 0;
int j = pages->num - 1;
if (!multifd_zero_page()) {
pages->normal_num = pages->num;
return;
}
/*
* Sort the offsets array by moving all normal pages to the start
* and all zero pages to the end of the array.
*/
while (i <= j) {
uint64_t offset = pages->offset[i];
if (!buffer_is_zero(rb->host + offset, p->page_size)) {
i++;
continue;
}
swap_page_offset(pages->offset, i, j);
ram_release_page(rb->idstr, offset);
j--;
}
pages->normal_num = i;
}
> + }
> + }
> +}
> +
> +void multifd_zero_page_check_recv(MultiFDRecvParams *p)
> +{
> + for (int i = 0; i < p->zero_num; i++) {
> + void *page = p->host + p->zero[i];
> + if (!buffer_is_zero(page, p->page_size)) {
> + memset(page, 0, p->page_size);
> + }
> + }
> +}
