[PATCH v4 0/7] Introduce multifd zero page checking.

[PATCH v4 0/7] Introduce multifd zero page checking.

[Hao Xiang]

v4 update:
* Fix documentation for interface ZeroPageDetection.
* Fix implementation in multifd_send_zero_page_check.
* Rebase on top of c0c6a0e3528b88aaad0b9d333e295707a195587b.

v3 update:
* Change "zero" to "zero-pages" and use type size for "zero-bytes".
* Fixed ZeroPageDetection interface description.
* Move zero page unit tests to its own path.
* Removed some asserts.
* Added backward compatibility support for migration 9.0 -> 8.2.
* Removed fields "zero" and "normal" page address arrays from v2. Now
multifd_zero_page_check_send sorts normal/zero pages in the "offset" array.

v2 update:
* Implement zero-page-detection switch with enumeration "legacy",
"none" and "multifd".
* Move normal/zero pages from MultiFDSendParams to MultiFDPages_t.
* Add zeros and zero_bytes accounting.

This patchset is based on Juan Quintela's old series here
https://lore.kernel.org/all/20220802063907.18882-1-quintela@redhat.com/

In the multifd live migration model, there is a single migration main
thread scanning the page map, queuing the pages to multiple multifd
sender threads. The migration main thread runs zero page checking on
every page before queuing the page to the sender threads. Zero page
checking is a CPU intensive task and hence having a single thread doing
all that doesn't scale well. This change introduces a new function
to run the zero page checking on the multifd sender threads. This
patchset also lays the ground work for future changes to offload zero
page checking task to accelerator hardwares.

Use two Intel 4th generation Xeon servers for testing.

Architecture:        x86_64
CPU(s):              192
Thread(s) per core:  2
Core(s) per socket:  48
Socket(s):           2
NUMA node(s):        2
Vendor ID:           GenuineIntel
CPU family:          6
Model:               143
Model name:          Intel(R) Xeon(R) Platinum 8457C
Stepping:            8
CPU MHz:             2538.624
CPU max MHz:         3800.0000
CPU min MHz:         800.0000

Perform multifd live migration with below setup:
1. VM has 100GB memory. All pages in the VM are zero pages.
2. Use tcp socket for live migration.
3. Use 4 multifd channels and zero page checking on migration main thread.
4. Use 1/2/4 multifd channels and zero page checking on multifd sender
threads.
5. Record migration total time from sender QEMU console's "info migrate"
command.

+------------------------------------+
|zero-page-checking | total-time(ms) |
+------------------------------------+
|main-thread        | 9629           |
+------------------------------------+
|multifd-1-threads  | 6182           |
+------------------------------------+
|multifd-2-threads  | 4643           |
+------------------------------------+
|multifd-4-threads  | 4143           |
+------------------------------------+

Apply this patchset on top of commit
c0c6a0e3528b88aaad0b9d333e295707a195587b

Hao Xiang (7):
  migration/multifd: Add new migration option zero-page-detection.
  migration/multifd: Implement zero page transmission on the multifd
    thread.
  migration/multifd: Implement ram_save_target_page_multifd to handle
    multifd version of MigrationOps::ram_save_target_page.
  migration/multifd: Enable multifd zero page checking by default.
  migration/multifd: Add new migration test cases for legacy zero page
    checking.
  migration/multifd: Add zero pages and zero bytes counter to migration
    status interface.
  Update maintainer contact for migration multifd zero page checking
    acceleration.

 MAINTAINERS                         |  5 ++
 hw/core/machine.c                   |  4 +-
 hw/core/qdev-properties-system.c    | 10 ++++
 include/hw/qdev-properties-system.h |  4 ++
 migration/meson.build               |  1 +
 migration/migration-hmp-cmds.c      | 13 ++++
 migration/migration.c               |  2 +
 migration/multifd-zero-page.c       | 92 +++++++++++++++++++++++++++++
 migration/multifd-zlib.c            | 21 +++++--
 migration/multifd-zstd.c            | 20 +++++--
 migration/multifd.c                 | 83 ++++++++++++++++++++++----
 migration/multifd.h                 | 24 +++++++-
 migration/options.c                 | 21 +++++++
 migration/options.h                 |  1 +
 migration/ram.c                     | 40 +++++++++----
 migration/trace-events              |  8 +--
 qapi/migration.json                 | 53 +++++++++++++++--
 tests/migration/guestperf/engine.py |  2 +
 tests/qtest/migration-test.c        | 52 ++++++++++++++++
 19 files changed, 412 insertions(+), 44 deletions(-)
 create mode 100644 migration/multifd-zero-page.c

-- 
2.30.2

[PATCH v4 1/7] migration/multifd: Add new migration option zero-page-detection.

[Hao Xiang]

This new parameter controls where the zero page checking is running.
1. If this parameter is set to 'legacy', zero page checking is
done in the migration main thread.
2. If this parameter is set to 'none', zero page checking is disabled.

Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
---
 hw/core/qdev-properties-system.c    | 10 +++++++++
 include/hw/qdev-properties-system.h |  4 ++++
 migration/migration-hmp-cmds.c      |  9 ++++++++
 migration/options.c                 | 21 ++++++++++++++++++
 migration/options.h                 |  1 +
 migration/ram.c                     |  4 ++++
 qapi/migration.json                 | 33 ++++++++++++++++++++++++++---
 7 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 1a396521d5..228e685f52 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -679,6 +679,16 @@ const PropertyInfo qdev_prop_mig_mode = {
     .set_default_value = qdev_propinfo_set_default_value_enum,
 };
 
+const PropertyInfo qdev_prop_zero_page_detection = {
+    .name = "ZeroPageDetection",
+    .description = "zero_page_detection values, "
+                   "none,legacy",
+    .enum_table = &ZeroPageDetection_lookup,
+    .get = qdev_propinfo_get_enum,
+    .set = qdev_propinfo_set_enum,
+    .set_default_value = qdev_propinfo_set_default_value_enum,
+};
+
 /* --- Reserved Region --- */
 
 /*
diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h
index 06c359c190..839b170235 100644
--- a/include/hw/qdev-properties-system.h
+++ b/include/hw/qdev-properties-system.h
@@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr;
 extern const PropertyInfo qdev_prop_reserved_region;
 extern const PropertyInfo qdev_prop_multifd_compression;
 extern const PropertyInfo qdev_prop_mig_mode;
+extern const PropertyInfo qdev_prop_zero_page_detection;
 extern const PropertyInfo qdev_prop_losttickpolicy;
 extern const PropertyInfo qdev_prop_blockdev_on_error;
 extern const PropertyInfo qdev_prop_bios_chs_trans;
@@ -47,6 +48,9 @@ extern const PropertyInfo qdev_prop_iothread_vq_mapping_list;
 #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \
     DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \
                        MigMode)
+#define DEFINE_PROP_ZERO_PAGE_DETECTION(_n, _s, _f, _d) \
+    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_zero_page_detection, \
+                       ZeroPageDetection)
 #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
     DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
                         LostTickPolicy)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 99b49df5dd..7e96ae6ffd 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -344,6 +344,11 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "%s: %s\n",
             MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION),
             MultiFDCompression_str(params->multifd_compression));
+        assert(params->has_zero_page_detection);
+        monitor_printf(mon, "%s: %s\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_ZERO_PAGE_DETECTION),
+            qapi_enum_lookup(&ZeroPageDetection_lookup,
+                params->zero_page_detection));
         monitor_printf(mon, "%s: %" PRIu64 " bytes\n",
             MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
             params->xbzrle_cache_size);
@@ -634,6 +639,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
         p->has_multifd_zstd_level = true;
         visit_type_uint8(v, param, &p->multifd_zstd_level, &err);
         break;
+    case MIGRATION_PARAMETER_ZERO_PAGE_DETECTION:
+        p->has_zero_page_detection = true;
+        visit_type_ZeroPageDetection(v, param, &p->zero_page_detection, &err);
+        break;
     case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
         p->has_xbzrle_cache_size = true;
         if (!visit_type_size(v, param, &cache_size, &err)) {
diff --git a/migration/options.c b/migration/options.c
index 3e3e0b93b4..3c603391b0 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -179,6 +179,9 @@ Property migration_properties[] = {
     DEFINE_PROP_MIG_MODE("mode", MigrationState,
                       parameters.mode,
                       MIG_MODE_NORMAL),
+    DEFINE_PROP_ZERO_PAGE_DETECTION("zero-page-detection", MigrationState,
+                       parameters.zero_page_detection,
+                       ZERO_PAGE_DETECTION_LEGACY),
 
     /* Migration capabilities */
     DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
@@ -903,6 +906,13 @@ uint64_t migrate_xbzrle_cache_size(void)
     return s->parameters.xbzrle_cache_size;
 }
 
+ZeroPageDetection migrate_zero_page_detection(void)
+{
+    MigrationState *s = migrate_get_current();
+
+    return s->parameters.zero_page_detection;
+}
+
 /* parameter setters */
 
 void migrate_set_block_incremental(bool value)
@@ -1013,6 +1023,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit;
     params->has_mode = true;
     params->mode = s->parameters.mode;
+    params->has_zero_page_detection = true;
+    params->zero_page_detection = s->parameters.zero_page_detection;
 
     return params;
 }
@@ -1049,6 +1061,7 @@ void migrate_params_init(MigrationParameters *params)
     params->has_x_vcpu_dirty_limit_period = true;
     params->has_vcpu_dirty_limit = true;
     params->has_mode = true;
+    params->has_zero_page_detection = true;
 }
 
 /*
@@ -1350,6 +1363,10 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
     if (params->has_mode) {
         dest->mode = params->mode;
     }
+
+    if (params->has_zero_page_detection) {
+        dest->zero_page_detection = params->zero_page_detection;
+    }
 }
 
 static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
@@ -1494,6 +1511,10 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
     if (params->has_mode) {
         s->parameters.mode = params->mode;
     }
+
+    if (params->has_zero_page_detection) {
+        s->parameters.zero_page_detection = params->zero_page_detection;
+    }
 }
 
 void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
diff --git a/migration/options.h b/migration/options.h
index 246c160aee..b7c4fb3861 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -93,6 +93,7 @@ const char *migrate_tls_authz(void);
 const char *migrate_tls_creds(void);
 const char *migrate_tls_hostname(void);
 uint64_t migrate_xbzrle_cache_size(void);
+ZeroPageDetection migrate_zero_page_detection(void);
 
 /* parameters setters */
 
diff --git a/migration/ram.c b/migration/ram.c
index 45a00b45ed..2581ee1e04 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1122,6 +1122,10 @@ static int save_zero_page(RAMState *rs, PageSearchStatus *pss,
     QEMUFile *file = pss->pss_channel;
     int len = 0;
 
+    if (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_NONE) {
+        return 0;
+    }
+
     if (!buffer_is_zero(p, TARGET_PAGE_SIZE)) {
         return 0;
     }
diff --git a/qapi/migration.json b/qapi/migration.json
index 0b33a71ab4..8da05dba47 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -664,6 +664,18 @@
 { 'enum': 'MigMode',
   'data': [ 'normal', 'cpr-reboot' ] }
 
+##
+# @ZeroPageDetection:
+#
+# @none: Do not perform zero page checking.
+#
+# @legacy: Perform zero page checking in main migration thread.
+#
+# Since: 9.0
+##
+{ 'enum': 'ZeroPageDetection',
+  'data': [ 'none', 'legacy' ] }
+
 ##
 # @BitmapMigrationBitmapAliasTransform:
 #
@@ -885,6 +897,10 @@
 # @mode: Migration mode. See description in @MigMode. Default is 'normal'.
 #        (Since 8.2)
 #
+# @zero-page-detection: Whether and how to detect zero pages.
+#     See description in @ZeroPageDetection.  Default is 'legacy'.
+#     (since 9.0)
+#
 # Features:
 #
 # @deprecated: Member @block-incremental is deprecated.  Use
@@ -918,7 +934,8 @@
            'block-bitmap-mapping',
            { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] },
            'vcpu-dirty-limit',
-           'mode'] }
+           'mode',
+           'zero-page-detection'] }
 
 ##
 # @MigrateSetParameters:
@@ -1077,6 +1094,10 @@
 # @mode: Migration mode. See description in @MigMode. Default is 'normal'.
 #        (Since 8.2)
 #
+# @zero-page-detection: Whether and how to detect zero pages.
+#     See description in @ZeroPageDetection.  Default is 'legacy'.
+#     (since 9.0)
+#
 # Features:
 #
 # @deprecated: Member @block-incremental is deprecated.  Use
@@ -1130,7 +1151,8 @@
             '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
                                             'features': [ 'unstable' ] },
             '*vcpu-dirty-limit': 'uint64',
-            '*mode': 'MigMode'} }
+            '*mode': 'MigMode',
+            '*zero-page-detection': 'ZeroPageDetection'} }
 
 ##
 # @migrate-set-parameters:
@@ -1305,6 +1327,10 @@
 # @mode: Migration mode. See description in @MigMode. Default is 'normal'.
 #        (Since 8.2)
 #
+# @zero-page-detection: Whether and how to detect zero pages.
+#     See description in @ZeroPageDetection.  Default is 'legacy'.
+#     (since 9.0)
+#
 # Features:
 #
 # @deprecated: Member @block-incremental is deprecated.  Use
@@ -1355,7 +1381,8 @@
             '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
                                             'features': [ 'unstable' ] },
             '*vcpu-dirty-limit': 'uint64',
-            '*mode': 'MigMode'} }
+            '*mode': 'MigMode',
+            '*zero-page-detection': 'ZeroPageDetection'} }
 
 ##
 # @query-migrate-parameters:
-- 
2.30.2

[PATCH v4 2/7] migration/multifd: Implement zero page transmission on the multifd thread.

[Hao Xiang]

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.xiang@bytedance.com>
---
 hw/core/machine.c                |  4 +-
 hw/core/qdev-properties-system.c |  2 +-
 migration/meson.build            |  1 +
 migration/multifd-zero-page.c    | 92 ++++++++++++++++++++++++++++++++
 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              |  7 ++-
 11 files changed, 230 insertions(+), 33 deletions(-)
 create mode 100644 migration/multifd-zero-page.c

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 9ac5d5389a..0e9d646b61 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..e695f6ff7d
--- /dev/null
+++ b/migration/multifd-zero-page.c
@@ -0,0 +1,92 @@
+/*
+ * Multifd zero page detection implementation.
+ *
+ * Copyright (c) 2024 Bytedance Inc
+ *
+ * Authors:
+ *  Hao Xiang <hao.xiang@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 bool multifd_zero_page(void)
+{
+    return migrate_zero_page_detection() == ZERO_PAGE_DETECTION_MULTIFD;
+}
+
+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_send_zero_page_check: Perform zero page detection on all pages.
+ *
+ * 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_send_zero_page_check(MultiFDSendParams *p)
+{
+    MultiFDPages_t *pages = p->pages;
+    RAMBlock *rb = pages->block;
+    int i = 0;
+    int j = pages->num - 1;
+
+    /*
+     * QEMU older than 9.0 don't understand zero page
+     * on multifd channel. This switch is required to
+     * maintain backward compatibility.
+     */
+    if (multifd_zero_page()) {
+        pages->normal_num = pages->num;
+        return;
+    }
+
+    /*
+     * Sort the page offset array by moving all normal pages to
+     * the left and all zero pages to the right 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_recv_zero_page_check(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);
+        }
+    }
+}
diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
index 012e3bdea1..f749e703a9 100644
--- a/migration/multifd-zlib.c
+++ b/migration/multifd-zlib.c
@@ -123,13 +123,15 @@ static int zlib_send_prepare(MultiFDSendParams *p, Error **errp)
     int ret;
     uint32_t i;
 
-    multifd_send_prepare_header(p);
+    if (!multifd_send_prepare_common(p)) {
+        goto out;
+    }
 
-    for (i = 0; i < pages->num; i++) {
+    for (i = 0; i < pages->normal_num; i++) {
         uint32_t available = z->zbuff_len - out_size;
         int flush = Z_NO_FLUSH;
 
-        if (i == pages->num - 1) {
+        if (i == pages->normal_num - 1) {
             flush = Z_SYNC_FLUSH;
         }
 
@@ -172,10 +174,10 @@ static int zlib_send_prepare(MultiFDSendParams *p, Error **errp)
     p->iov[p->iovs_num].iov_len = out_size;
     p->iovs_num++;
     p->next_packet_size = out_size;
-    p->flags |= MULTIFD_FLAG_ZLIB;
 
+out:
+    p->flags |= MULTIFD_FLAG_ZLIB;
     multifd_send_fill_packet(p);
-
     return 0;
 }
 
@@ -261,6 +263,14 @@ static int zlib_recv_pages(MultiFDRecvParams *p, Error **errp)
                    p->id, flags, MULTIFD_FLAG_ZLIB);
         return -1;
     }
+
+    multifd_recv_zero_page_check(p);
+
+    if (!p->normal_num) {
+        assert(in_size == 0);
+        return 0;
+    }
+
     ret = qio_channel_read_all(p->c, (void *)z->zbuff, in_size, errp);
 
     if (ret != 0) {
@@ -310,6 +320,7 @@ static int zlib_recv_pages(MultiFDRecvParams *p, Error **errp)
                    p->id, out_size, expected_size);
         return -1;
     }
+
     return 0;
 }
 
diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
index dc8fe43e94..80635e19ef 100644
--- a/migration/multifd-zstd.c
+++ b/migration/multifd-zstd.c
@@ -118,16 +118,18 @@ static int zstd_send_prepare(MultiFDSendParams *p, Error **errp)
     int ret;
     uint32_t i;
 
-    multifd_send_prepare_header(p);
+    if (!multifd_send_prepare_common(p)) {
+        goto out;
+    }
 
     z->out.dst = z->zbuff;
     z->out.size = z->zbuff_len;
     z->out.pos = 0;
 
-    for (i = 0; i < pages->num; i++) {
+    for (i = 0; i < pages->normal_num; i++) {
         ZSTD_EndDirective flush = ZSTD_e_continue;
 
-        if (i == pages->num - 1) {
+        if (i == pages->normal_num - 1) {
             flush = ZSTD_e_flush;
         }
         z->in.src = p->pages->block->host + pages->offset[i];
@@ -161,10 +163,10 @@ static int zstd_send_prepare(MultiFDSendParams *p, Error **errp)
     p->iov[p->iovs_num].iov_len = z->out.pos;
     p->iovs_num++;
     p->next_packet_size = z->out.pos;
-    p->flags |= MULTIFD_FLAG_ZSTD;
 
+out:
+    p->flags |= MULTIFD_FLAG_ZSTD;
     multifd_send_fill_packet(p);
-
     return 0;
 }
 
@@ -257,6 +259,14 @@ static int zstd_recv_pages(MultiFDRecvParams *p, Error **errp)
                    p->id, flags, MULTIFD_FLAG_ZSTD);
         return -1;
     }
+
+    multifd_recv_zero_page_check(p);
+
+    if (!p->normal_num) {
+        assert(in_size == 0);
+        return 0;
+    }
+
     ret = qio_channel_read_all(p->c, (void *)z->zbuff, in_size, errp);
 
     if (ret != 0) {
diff --git a/migration/multifd.c b/migration/multifd.c
index 6c07f19af1..a95c763df8 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qemu/rcu.h"
 #include "exec/target_page.h"
 #include "sysemu/sysemu.h"
@@ -139,6 +140,8 @@ static int nocomp_send_prepare(MultiFDSendParams *p, Error **errp)
     MultiFDPages_t *pages = p->pages;
     int ret;
 
+    multifd_send_zero_page_check(p);
+
     if (!use_zero_copy_send) {
         /*
          * Only !zerocopy needs the header in IOV; zerocopy will
@@ -147,13 +150,13 @@ static int nocomp_send_prepare(MultiFDSendParams *p, Error **errp)
         multifd_send_prepare_header(p);
     }
 
-    for (int i = 0; i < pages->num; i++) {
+    for (int i = 0; i < pages->normal_num; i++) {
         p->iov[p->iovs_num].iov_base = pages->block->host + pages->offset[i];
         p->iov[p->iovs_num].iov_len = p->page_size;
         p->iovs_num++;
     }
 
-    p->next_packet_size = pages->num * p->page_size;
+    p->next_packet_size = pages->normal_num * p->page_size;
     p->flags |= MULTIFD_FLAG_NOCOMP;
 
     multifd_send_fill_packet(p);
@@ -215,6 +218,13 @@ static int nocomp_recv_pages(MultiFDRecvParams *p, Error **errp)
                    p->id, flags, MULTIFD_FLAG_NOCOMP);
         return -1;
     }
+
+    multifd_recv_zero_page_check(p);
+
+    if (!p->normal_num) {
+        return 0;
+    }
+
     for (int i = 0; i < p->normal_num; i++) {
         p->iov[i].iov_base = p->host + p->normal[i];
         p->iov[i].iov_len = p->page_size;
@@ -249,6 +259,7 @@ static void multifd_pages_reset(MultiFDPages_t *pages)
      * overwritten later when reused.
      */
     pages->num = 0;
+    pages->normal_num = 0;
     pages->block = NULL;
 }
 
@@ -340,11 +351,13 @@ void multifd_send_fill_packet(MultiFDSendParams *p)
     MultiFDPacket_t *packet = p->packet;
     MultiFDPages_t *pages = p->pages;
     uint64_t packet_num;
+    uint32_t zero_num = pages->num - pages->normal_num;
     int i;
 
     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->normal_pages = cpu_to_be32(pages->normal_num);
+    packet->zero_pages = cpu_to_be32(zero_num);
     packet->next_packet_size = cpu_to_be32(p->next_packet_size);
 
     packet_num = qatomic_fetch_inc(&multifd_send_state->packet_num);
@@ -362,10 +375,11 @@ void multifd_send_fill_packet(MultiFDSendParams *p)
     }
 
     p->packets_sent++;
-    p->total_normal_pages += pages->num;
+    p->total_normal_pages += pages->normal_num;
+    p->total_zero_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->normal_num, zero_num,
+                       p->flags, p->next_packet_size);
 }
 
 static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
@@ -406,20 +420,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;
     }
 
@@ -445,6 +468,18 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
         p->normal[i] = offset;
     }
 
+    for (i = 0; i < p->zero_num; i++) {
+        uint64_t offset = be64_to_cpu(packet->offset[p->normal_num + i]);
+
+        if (offset > (p->block->used_length - p->page_size)) {
+            error_setg(errp, "multifd: offset too long %" PRIu64
+                       " (max " RAM_ADDR_FMT ")",
+                       offset, p->block->used_length);
+            return -1;
+        }
+        p->zero[i] = offset;
+    }
+
     return 0;
 }
 
@@ -837,6 +872,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->normal_num);
+            stat64_add(&mig_stats.zero_pages, pages->num - pages->normal_num);
 
             multifd_pages_reset(p->pages);
             p->next_packet_size = 0;
@@ -880,7 +917,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;
 }
@@ -1147,6 +1185,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);
 }
 
@@ -1241,7 +1281,7 @@ static void *multifd_recv_thread(void *opaque)
         p->flags &= ~MULTIFD_FLAG_SYNC;
         qemu_mutex_unlock(&p->mutex);
 
-        if (p->normal_num) {
+        if (p->normal_num + p->zero_num) {
             ret = multifd_recv_state->ops->recv_pages(p, &local_err);
             if (ret != 0) {
                 break;
@@ -1260,7 +1300,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;
 }
@@ -1299,6 +1341,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();
     }
@@ -1368,3 +1411,17 @@ void multifd_recv_new_channel(QIOChannel *ioc, Error **errp)
                        QEMU_THREAD_JOINABLE);
     qatomic_inc(&multifd_recv_state->count);
 }
+
+bool multifd_send_prepare_common(MultiFDSendParams *p)
+{
+    multifd_send_zero_page_check(p);
+
+    if (!p->pages->normal_num) {
+        p->next_packet_size = 0;
+        return false;
+    }
+
+    multifd_send_prepare_header(p);
+
+    return true;
+}
diff --git a/migration/multifd.h b/migration/multifd.h
index b3fe27ae93..4b59d5569c 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -48,14 +48,24 @@ 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];
+    /*
+     * This array contains the pointers to:
+     *  - normal pages (initial normal_pages entries)
+     *  - zero pages (following zero_pages entries)
+     */
     uint64_t offset[];
 } __attribute__((packed)) MultiFDPacket_t;
 
 typedef struct {
     /* number of used pages */
     uint32_t num;
+    /* number of normal pages */
+    uint32_t normal_num;
     /* number of allocated pages */
     uint32_t allocated;
     /* offset of each page */
@@ -122,6 +132,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 */
@@ -176,12 +188,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;
@@ -203,6 +221,9 @@ typedef struct {
 
 void multifd_register_ops(int method, MultiFDMethods *ops);
 void multifd_send_fill_packet(MultiFDSendParams *p);
+bool multifd_send_prepare_common(MultiFDSendParams *p);
+void multifd_send_zero_page_check(MultiFDSendParams *p);
+void multifd_recv_zero_page_check(MultiFDRecvParams *p);
 
 static inline void multifd_send_prepare_header(MultiFDSendParams *p)
 {
@@ -211,5 +232,4 @@ static inline void multifd_send_prepare_header(MultiFDSendParams *p)
     p->iovs_num++;
 }
 
-
 #endif
diff --git a/migration/ram.c b/migration/ram.c
index 2581ee1e04..e1fa229acf 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1258,7 +1258,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"
diff --git a/qapi/migration.json b/qapi/migration.json
index 8da05dba47..846d0411d5 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -671,10 +671,15 @@
 #
 # @legacy: Perform zero page checking in main migration thread.
 #
+# @multifd: Perform zero page checking in multifd sender thread.
+#     This option only takes effect if migration capability multifd
+#     is set.  Otherwise, it will have the same effect as legacy.
+#
 # Since: 9.0
+#
 ##
 { 'enum': 'ZeroPageDetection',
-  'data': [ 'none', 'legacy' ] }
+  'data': [ 'none', 'legacy', 'multifd' ] }
 
 ##
 # @BitmapMigrationBitmapAliasTransform:
-- 
2.30.2

[PATCH v4 3/7] migration/multifd: Implement ram_save_target_page_multifd to handle multifd version of MigrationOps::ram_save_target_page.

[Hao Xiang]

1. Add a dedicated handler for MigrationOps::ram_save_target_page in
multifd live migration.
2. Refactor ram_save_target_page_legacy so that the legacy and multifd
handlers don't have internal functions calling into each other.

Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Message-Id: <20240226195654.934709-4-hao.xiang@bytedance.com>
---
 migration/ram.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index e1fa229acf..f9d6ea65cc 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1122,10 +1122,6 @@ static int save_zero_page(RAMState *rs, PageSearchStatus *pss,
     QEMUFile *file = pss->pss_channel;
     int len = 0;
 
-    if (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_NONE) {
-        return 0;
-    }
-
     if (!buffer_is_zero(p, TARGET_PAGE_SIZE)) {
         return 0;
     }
@@ -2045,7 +2041,6 @@ static bool save_compress_page(RAMState *rs, PageSearchStatus *pss,
  */
 static int ram_save_target_page_legacy(RAMState *rs, PageSearchStatus *pss)
 {
-    RAMBlock *block = pss->block;
     ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
     int res;
 
@@ -2061,17 +2056,34 @@ static int ram_save_target_page_legacy(RAMState *rs, PageSearchStatus *pss)
         return 1;
     }
 
+    return ram_save_page(rs, pss);
+}
+
+/**
+ * ram_save_target_page_multifd: send one target page to multifd workers
+ *
+ * Returns 1 if the page was queued, -1 otherwise.
+ *
+ * @rs: current RAM state
+ * @pss: data about the page we want to send
+ */
+static int ram_save_target_page_multifd(RAMState *rs, PageSearchStatus *pss)
+{
+    RAMBlock *block = pss->block;
+    ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
+
     /*
-     * Do not use multifd in postcopy as one whole host page should be
-     * placed.  Meanwhile postcopy requires atomic update of pages, so even
-     * if host page size == guest page size the dest guest during run may
-     * still see partially copied pages which is data corruption.
+     * Backward compatibility support. While using multifd live
+     * migration, we still need to handle zero page checking on the
+     * migration main thread.
      */
-    if (migrate_multifd() && !migration_in_postcopy()) {
-        return ram_save_multifd_page(block, offset);
+    if (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_LEGACY) {
+        if (save_zero_page(rs, pss, offset)) {
+            return 1;
+        }
     }
 
-    return ram_save_page(rs, pss);
+    return ram_save_multifd_page(block, offset);
 }
 
 /* Should be called before sending a host page */
@@ -2983,7 +2995,12 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     }
 
     migration_ops = g_malloc0(sizeof(MigrationOps));
-    migration_ops->ram_save_target_page = ram_save_target_page_legacy;
+
+    if (migrate_multifd()) {
+        migration_ops->ram_save_target_page = ram_save_target_page_multifd;
+    } else {
+        migration_ops->ram_save_target_page = ram_save_target_page_legacy;
+    }
 
     bql_unlock();
     ret = multifd_send_sync_main();
-- 
2.30.2

[PATCH v4 4/7] migration/multifd: Enable multifd zero page checking by default.

[Hao Xiang]

Set default "zero-page-detection" option to "multifd". Now zero page
checking can be done in the multifd threads and this becomes the
default configuration. We still provide backward compatibility
where zero page checking is done from the migration main thread.

Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
---
 migration/options.c | 2 +-
 qapi/migration.json | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/options.c b/migration/options.c
index 3c603391b0..3c79b6ccd4 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -181,7 +181,7 @@ Property migration_properties[] = {
                       MIG_MODE_NORMAL),
     DEFINE_PROP_ZERO_PAGE_DETECTION("zero-page-detection", MigrationState,
                        parameters.zero_page_detection,
-                       ZERO_PAGE_DETECTION_LEGACY),
+                       ZERO_PAGE_DETECTION_MULTIFD),
 
     /* Migration capabilities */
     DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
diff --git a/qapi/migration.json b/qapi/migration.json
index 846d0411d5..ca9561fbf1 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -903,7 +903,7 @@
 #        (Since 8.2)
 #
 # @zero-page-detection: Whether and how to detect zero pages.
-#     See description in @ZeroPageDetection.  Default is 'legacy'.
+#     See description in @ZeroPageDetection.  Default is 'multifd'.
 #     (since 9.0)
 #
 # Features:
@@ -1100,7 +1100,7 @@
 #        (Since 8.2)
 #
 # @zero-page-detection: Whether and how to detect zero pages.
-#     See description in @ZeroPageDetection.  Default is 'legacy'.
+#     See description in @ZeroPageDetection.  Default is 'multifd'.
 #     (since 9.0)
 #
 # Features:
@@ -1333,7 +1333,7 @@
 #        (Since 8.2)
 #
 # @zero-page-detection: Whether and how to detect zero pages.
-#     See description in @ZeroPageDetection.  Default is 'legacy'.
+#     See description in @ZeroPageDetection.  Default is 'multifd'.
 #     (since 9.0)
 #
 # Features:
-- 
2.30.2

[PATCH v4 5/7] migration/multifd: Add new migration test cases for legacy zero page checking.

[Hao Xiang]

Now that zero page checking is done on the multifd sender threads by
default, we still provide an option for backward compatibility. This
change adds a qtest migration test case to set the zero-page-detection
option to "legacy" and run multifd migration with zero page checking on the
migration main thread.

Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
---
 tests/qtest/migration-test.c | 52 ++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 83512bce85..8a966364b5 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2660,6 +2660,24 @@ test_migrate_precopy_tcp_multifd_start(QTestState *from,
     return test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
 }
 
+static void *
+test_migrate_precopy_tcp_multifd_start_zero_page_legacy(QTestState *from,
+                                                        QTestState *to)
+{
+    test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
+    migrate_set_parameter_str(from, "zero-page-detection", "legacy");
+    return NULL;
+}
+
+static void *
+test_migration_precopy_tcp_multifd_start_no_zero_page(QTestState *from,
+                                                      QTestState *to)
+{
+    test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
+    migrate_set_parameter_str(from, "zero-page-detection", "none");
+    return NULL;
+}
+
 static void *
 test_migrate_precopy_tcp_multifd_zlib_start(QTestState *from,
                                             QTestState *to)
@@ -2691,6 +2709,36 @@ static void test_multifd_tcp_none(void)
     test_precopy_common(&args);
 }
 
+static void test_multifd_tcp_zero_page_legacy(void)
+{
+    MigrateCommon args = {
+        .listen_uri = "defer",
+        .start_hook = test_migrate_precopy_tcp_multifd_start_zero_page_legacy,
+        /*
+         * Multifd is more complicated than most of the features, it
+         * directly takes guest page buffers when sending, make sure
+         * everything will work alright even if guest page is changing.
+         */
+        .live = true,
+    };
+    test_precopy_common(&args);
+}
+
+static void test_multifd_tcp_no_zero_page(void)
+{
+    MigrateCommon args = {
+        .listen_uri = "defer",
+        .start_hook = test_migration_precopy_tcp_multifd_start_no_zero_page,
+        /*
+         * Multifd is more complicated than most of the features, it
+         * directly takes guest page buffers when sending, make sure
+         * everything will work alright even if guest page is changing.
+         */
+        .live = true,
+    };
+    test_precopy_common(&args);
+}
+
 static void test_multifd_tcp_zlib(void)
 {
     MigrateCommon args = {
@@ -3592,6 +3640,10 @@ int main(int argc, char **argv)
     }
     migration_test_add("/migration/multifd/tcp/plain/none",
                        test_multifd_tcp_none);
+    migration_test_add("/migration/multifd/tcp/plain/zero-page/legacy",
+                       test_multifd_tcp_zero_page_legacy);
+    migration_test_add("/migration/multifd/tcp/plain/zero-page/none",
+                       test_multifd_tcp_no_zero_page);
     migration_test_add("/migration/multifd/tcp/plain/cancel",
                        test_multifd_tcp_cancel);
     migration_test_add("/migration/multifd/tcp/plain/zlib",
-- 
2.30.2

[PATCH v4 6/7] migration/multifd: Add zero pages and zero bytes counter to migration status interface.

[Hao Xiang]

This change extends the MigrationStatus interface to track zero pages
and zero bytes counter.

Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
---
 migration/migration-hmp-cmds.c      |  4 ++++
 migration/migration.c               |  2 ++
 qapi/migration.json                 | 15 ++++++++++++++-
 tests/migration/guestperf/engine.py |  2 ++
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 7e96ae6ffd..a38ad0255d 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -111,6 +111,10 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
                        info->ram->normal);
         monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
                        info->ram->normal_bytes >> 10);
+        monitor_printf(mon, "zero pages: %" PRIu64 " pages\n",
+                       info->ram->zero_pages);
+        monitor_printf(mon, "zero bytes: %" PRIu64 " kbytes\n",
+                       info->ram->zero_bytes >> 10);
         monitor_printf(mon, "dirty sync count: %" PRIu64 "\n",
                        info->ram->dirty_sync_count);
         monitor_printf(mon, "page size: %" PRIu64 " kbytes\n",
diff --git a/migration/migration.c b/migration/migration.c
index bab68bcbef..fc4e3ef52d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1126,6 +1126,8 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s)
     info->ram->skipped = 0;
     info->ram->normal = stat64_get(&mig_stats.normal_pages);
     info->ram->normal_bytes = info->ram->normal * page_size;
+    info->ram->zero_pages = stat64_get(&mig_stats.zero_pages);
+    info->ram->zero_bytes = info->ram->zero_pages * page_size;
     info->ram->mbps = s->mbps;
     info->ram->dirty_sync_count =
         stat64_get(&mig_stats.dirty_sync_count);
diff --git a/qapi/migration.json b/qapi/migration.json
index ca9561fbf1..03b850bab7 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -63,6 +63,10 @@
 #     between 0 and @dirty-sync-count * @multifd-channels.  (since
 #     7.1)
 #
+# @zero-pages: number of zero pages (since 9.0)
+#
+# @zero-bytes: number of zero bytes sent (since 9.0)
+#
 # Features:
 #
 # @deprecated: Member @skipped is always zero since 1.5.3
@@ -81,7 +85,8 @@
            'multifd-bytes': 'uint64', 'pages-per-second': 'uint64',
            'precopy-bytes': 'uint64', 'downtime-bytes': 'uint64',
            'postcopy-bytes': 'uint64',
-           'dirty-sync-missed-zero-copy': 'uint64' } }
+           'dirty-sync-missed-zero-copy': 'uint64',
+           'zero-pages': 'int', 'zero-bytes': 'size' } }
 
 ##
 # @XBZRLECacheStats:
@@ -332,6 +337,8 @@
 #               "duplicate":123,
 #               "normal":123,
 #               "normal-bytes":123456,
+#               "zero-pages":123,
+#               "zero-bytes":123456,
 #               "dirty-sync-count":15
 #             }
 #          }
@@ -358,6 +365,8 @@
 #                 "duplicate":123,
 #                 "normal":123,
 #                 "normal-bytes":123456,
+#                 "zero-pages":123,
+#                 "zero-bytes":123456,
 #                 "dirty-sync-count":15
 #              }
 #           }
@@ -379,6 +388,8 @@
 #                 "duplicate":123,
 #                 "normal":123,
 #                 "normal-bytes":123456,
+#                 "zero-pages":123,
+#                 "zero-bytes":123456,
 #                 "dirty-sync-count":15
 #              },
 #              "disk":{
@@ -405,6 +416,8 @@
 #                 "duplicate":10,
 #                 "normal":3333,
 #                 "normal-bytes":3412992,
+#                 "zero-pages":3333,
+#                 "zero-bytes":3412992,
 #                 "dirty-sync-count":15
 #              },
 #              "xbzrle-cache":{
diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
index 608d7270f6..693e07c227 100644
--- a/tests/migration/guestperf/engine.py
+++ b/tests/migration/guestperf/engine.py
@@ -92,6 +92,8 @@ def _migrate_progress(self, vm):
                 info["ram"].get("skipped", 0),
                 info["ram"].get("normal", 0),
                 info["ram"].get("normal-bytes", 0),
+                info["ram"].get("zero-pages", 0);
+                info["ram"].get("zero-bytes", 0);
                 info["ram"].get("dirty-pages-rate", 0),
                 info["ram"].get("mbps", 0),
                 info["ram"].get("dirty-sync-count", 0)
-- 
2.30.2

[PATCH v4 7/7] Update maintainer contact for migration multifd zero page checking acceleration.

[Hao Xiang]

Add myself to maintain multifd zero page checking acceleration function.

Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 65dfdc9677..b547918e4d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3414,6 +3414,11 @@ F: tests/migration/
 F: util/userfaultfd.c
 X: migration/rdma*
 
+Migration multifd zero page checking acceleration
+M: Hao Xiang <hao.xiang@bytedance.com>
+S: Maintained
+F: migration/multifd-zero-page.c
+
 RDMA Migration
 R: Li Zhijian <lizhijian@fujitsu.com>
 R: Peter Xu <peterx@redhat.com>
-- 
2.30.2

Re: [PATCH v4 1/7] migration/multifd: Add new migration option zero-page-detection.

[Markus Armbruster]

Hao Xiang <hao.xiang@bytedance.com> writes:

> This new parameter controls where the zero page checking is running.
> 1. If this parameter is set to 'legacy', zero page checking is
> done in the migration main thread.
> 2. If this parameter is set to 'none', zero page checking is disabled.
>
> Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>

QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v4 2/7] migration/multifd: Implement zero page transmission on the multifd thread.

[Markus Armbruster]

Hao Xiang <hao.xiang@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.xiang@bytedance.com>

[...]

> diff --git a/qapi/migration.json b/qapi/migration.json
> index 8da05dba47..846d0411d5 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -671,10 +671,15 @@
>  #
>  # @legacy: Perform zero page checking in main migration thread.
>  #
> +# @multifd: Perform zero page checking in multifd sender thread.
> +#     This option only takes effect if migration capability multifd
> +#     is set.  Otherwise, it will have the same effect as legacy.

Suggest

   # @multifd: Perform zero page checking in multifd sender thread if
   #     multifd migration is enabled, else in the main migration
   #     thread as for @legacy.

Thoughts?

> +#
>  # Since: 9.0
> +#
>  ##
>  { 'enum': 'ZeroPageDetection',
> -  'data': [ 'none', 'legacy' ] }
> +  'data': [ 'none', 'legacy', 'multifd' ] }
>  
>  ##
>  # @BitmapMigrationBitmapAliasTransform:

QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v4 6/7] migration/multifd: Add zero pages and zero bytes counter to migration status interface.

[Markus Armbruster]

Hao Xiang <hao.xiang@bytedance.com> writes:

> This change extends the MigrationStatus interface to track zero pages
> and zero bytes counter.
>
> Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>

[...]

> diff --git a/qapi/migration.json b/qapi/migration.json
> index ca9561fbf1..03b850bab7 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -63,6 +63,10 @@
>  #     between 0 and @dirty-sync-count * @multifd-channels.  (since
>  #     7.1)
>  #
> +# @zero-pages: number of zero pages (since 9.0)
> +#
> +# @zero-bytes: number of zero bytes sent (since 9.0)
> +#

Discussion of v3 has led me to believe:

1. A page is either migrated as a normal page or as a zero page.

2. The following equations hold:

    @normal-bytes = @normal * @page-size

    @zero-bytes = @zero-pages * @page-size

3. @zero-pages is the same as @duplicate, with a better name.  We intend
   to drop @duplicate eventually.

If this is correct, I'd like you to

A. Name it @zero for consistency with @normal.  Disregard my advice to
   name it @zero-pages; two consistent bad names are better than one bad
   name, one good name, and inconsistency.

B. Add @zero and @zero-bytes next to @normal and @normal-bytes.

C. Deprecate @duplicate (item 3).  Separate patch, please.

D. Consider documenting more clearly what normal and zero pages are
   (item 1), and how @FOO, @FOO-pages and @page-size are related (item
   2).  Could be done in a followup patch.

>  # Features:
>  #
>  # @deprecated: Member @skipped is always zero since 1.5.3
> @@ -81,7 +85,8 @@
>             'multifd-bytes': 'uint64', 'pages-per-second': 'uint64',
>             'precopy-bytes': 'uint64', 'downtime-bytes': 'uint64',
>             'postcopy-bytes': 'uint64',
> -           'dirty-sync-missed-zero-copy': 'uint64' } }
> +           'dirty-sync-missed-zero-copy': 'uint64',
> +           'zero-pages': 'int', 'zero-bytes': 'size' } }
>  

[...]

Re: [External] Re: [PATCH v4 2/7] migration/multifd: Implement zero page transmission on the multifd thread.

[Hao Xiang]

On Thu, Feb 29, 2024 at 11:28 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Hao Xiang <hao.xiang@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.xiang@bytedance.com>
>
> [...]
>
> > diff --git a/qapi/migration.json b/qapi/migration.json
> > index 8da05dba47..846d0411d5 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -671,10 +671,15 @@
> >  #
> >  # @legacy: Perform zero page checking in main migration thread.
> >  #
> > +# @multifd: Perform zero page checking in multifd sender thread.
> > +#     This option only takes effect if migration capability multifd
> > +#     is set.  Otherwise, it will have the same effect as legacy.
>
> Suggest
>
>    # @multifd: Perform zero page checking in multifd sender thread if
>    #     multifd migration is enabled, else in the main migration
>    #     thread as for @legacy.
>
> Thoughts?

Sounds good. Will change that.

>
> > +#
> >  # Since: 9.0
> > +#
> >  ##
> >  { 'enum': 'ZeroPageDetection',
> > -  'data': [ 'none', 'legacy' ] }
> > +  'data': [ 'none', 'legacy', 'multifd' ] }
> >
> >  ##
> >  # @BitmapMigrationBitmapAliasTransform:
>
> QAPI schema
> Acked-by: Markus Armbruster <armbru@redhat.com>
>

Re: [PATCH v4 1/7] migration/multifd: Add new migration option zero-page-detection.

[Peter Xu]

On Fri, Mar 01, 2024 at 02:28:23AM +0000, Hao Xiang wrote:
> This new parameter controls where the zero page checking is running.
> 1. If this parameter is set to 'legacy', zero page checking is
> done in the migration main thread.
> 2. If this parameter is set to 'none', zero page checking is disabled.
> 
> Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [PATCH v4 2/7] migration/multifd: Implement zero page transmission on the multifd thread.

[Peter Xu]

On Fri, Mar 01, 2024 at 02:28:24AM +0000, Hao Xiang wrote:
> -GlobalProperty hw_compat_8_2[] = {};
> +GlobalProperty hw_compat_8_2[] = {
> +    { "migration", "zero-page-detection", "legacy"},
> +};

I hope we can make it for 9.0, then this (and many rest places) can be kept
as-is.  Let's see..  soft-freeze is March 12th.

One thing to mention is I just sent a pull which has mapped-ram feature
merged.  You may need a rebase onto that, and hopefully mapped-ram can also
use your feature too within the same patch when you repost.

https://lore.kernel.org/all/20240229153017.2221-1-farosas@suse.de/

That rebase may or may not need much caution, I apologize for that:
mapped-ram as a feature was discussed 1+ years, so it was a plan to merge
it (actually still partly of it) into QEMU 9.0.

[...]

> +static bool multifd_zero_page(void)

multifd_zero_page_enabled()?

> +{
> +    return migrate_zero_page_detection() == ZERO_PAGE_DETECTION_MULTIFD;
> +}
> +
> +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_send_zero_page_check: Perform zero page detection on all pages.
> + *
> + * Sorts normal pages before zero pages in p->pages->offset and updates
> + * p->pages->normal_num.
> + *
> + * @param p A pointer to the send params.

Nit: the majority of doc style in QEMU (it seems to me) is:

  @p: pointer to @MultiFDSendParams.

> + */
> +void multifd_send_zero_page_check(MultiFDSendParams *p)

multifd_send_zero_page_detect()?

This patch used "check" on both sides, but neither of them is a pure check
to me.  For the other side, maybe multifd_recv_zero_page_process()?  As
that one applies the zero pages.

> +{
> +    MultiFDPages_t *pages = p->pages;
> +    RAMBlock *rb = pages->block;
> +    int i = 0;
> +    int j = pages->num - 1;
> +
> +    /*
> +     * QEMU older than 9.0 don't understand zero page
> +     * on multifd channel. This switch is required to
> +     * maintain backward compatibility.
> +     */

IMHO we can drop this comment; it is not accurate as the user can disable
it explicitly through the parameter, then it may not always about compatibility.

> +    if (multifd_zero_page()) {

Shouldn't this be "!multifd_zero_page_enabled()"?

> +        pages->normal_num = pages->num;
> +        return;
> +    }

The rest looks all sane.

Thanks,

-- 
Peter Xu

Re: [PATCH v4 4/7] migration/multifd: Enable multifd zero page checking by default.

[Peter Xu]

On Fri, Mar 01, 2024 at 02:28:26AM +0000, Hao Xiang wrote:
> Set default "zero-page-detection" option to "multifd". Now zero page
> checking can be done in the multifd threads and this becomes the
> default configuration. We still provide backward compatibility
> where zero page checking is done from the migration main thread.
> 
> Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>

Either this patch can be squashed into patch 2, or you can move
hw_compat_8_2[] changes from patch 2 into this patch.  The change itself
looks good.

-- 
Peter Xu

Re: [PATCH v4 5/7] migration/multifd: Add new migration test cases for legacy zero page checking.

[Peter Xu]

On Fri, Mar 01, 2024 at 02:28:27AM +0000, Hao Xiang wrote:
> Now that zero page checking is done on the multifd sender threads by
> default, we still provide an option for backward compatibility. This
> change adds a qtest migration test case to set the zero-page-detection
> option to "legacy" and run multifd migration with zero page checking on the
> migration main thread.
> 
> Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [PATCH v4 7/7] Update maintainer contact for migration multifd zero page checking acceleration.

[Peter Xu]

On Fri, Mar 01, 2024 at 02:28:29AM +0000, Hao Xiang wrote:
> Add myself to maintain multifd zero page checking acceleration function.
> 
> Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
> ---
>  MAINTAINERS | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 65dfdc9677..b547918e4d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3414,6 +3414,11 @@ F: tests/migration/
>  F: util/userfaultfd.c
>  X: migration/rdma*
>  
> +Migration multifd zero page checking acceleration
> +M: Hao Xiang <hao.xiang@bytedance.com>
> +S: Maintained
> +F: migration/multifd-zero-page.c
> +

Firstly appreciate a lot for volunteering!

My fault to not have made it clear.  This file alone so far will need to be
closely related to the multifd core, so whoever maintains migration should
look after this.  It's also slightly weird to have a separate entry for a
file that is tens of LOC if it's already covered by another upper entry.

What I worry is about vendor/library specific parts that will be harder to
maintain, and migration maintainers (no matter who does the job in the
future) may not always cover those areas.  So I was expecting we could have
volunteers covering e.g. QAT / DSA / IAA accelerators.  Since all these
accelerators will all be part of Intel's new chips, there's also one way
that we have "Intel accelerators" section to cover vendor specific codes
and then cover all those areas no matter it's zero detect accelerator or HW
compressors.

I'd suggest we discuss this with Intel people to check out a solid plan
later when we start to merge HW/LIB specific codes.  For now I suggest we
can drop this patch and stick with the feature implementation, to see
whether it can catch the train for 9.0.  IMHO this is a good feature even
without HW accelerators (and I think it's close to ready), so I hope it can
still make it.

Thanks,

-- 
Peter Xu

Re: [PATCH v4 3/7] migration/multifd: Implement ram_save_target_page_multifd to handle multifd version of MigrationOps::ram_save_target_page.

[Peter Xu]

On Fri, Mar 01, 2024 at 02:28:25AM +0000, Hao Xiang wrote:
> 1. Add a dedicated handler for MigrationOps::ram_save_target_page in
> multifd live migration.
> 2. Refactor ram_save_target_page_legacy so that the legacy and multifd
> handlers don't have internal functions calling into each other.
> 
> Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> Message-Id: <20240226195654.934709-4-hao.xiang@bytedance.com>
> ---
>  migration/ram.c | 43 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index e1fa229acf..f9d6ea65cc 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1122,10 +1122,6 @@ static int save_zero_page(RAMState *rs, PageSearchStatus *pss,
>      QEMUFile *file = pss->pss_channel;
>      int len = 0;
>  
> -    if (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_NONE) {
> -        return 0;
> -    }

We need to keep this to disable zero-page-detect on !multifd?

> -
>      if (!buffer_is_zero(p, TARGET_PAGE_SIZE)) {
>          return 0;
>      }
> @@ -2045,7 +2041,6 @@ static bool save_compress_page(RAMState *rs, PageSearchStatus *pss,
>   */
>  static int ram_save_target_page_legacy(RAMState *rs, PageSearchStatus *pss)
>  {
> -    RAMBlock *block = pss->block;
>      ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
>      int res;
>  
> @@ -2061,17 +2056,34 @@ static int ram_save_target_page_legacy(RAMState *rs, PageSearchStatus *pss)
>          return 1;
>      }
>  
> +    return ram_save_page(rs, pss);
> +}
> +
> +/**
> + * ram_save_target_page_multifd: send one target page to multifd workers
> + *
> + * Returns 1 if the page was queued, -1 otherwise.
> + *
> + * @rs: current RAM state
> + * @pss: data about the page we want to send
> + */
> +static int ram_save_target_page_multifd(RAMState *rs, PageSearchStatus *pss)
> +{
> +    RAMBlock *block = pss->block;
> +    ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
> +
>      /*
> -     * Do not use multifd in postcopy as one whole host page should be
> -     * placed.  Meanwhile postcopy requires atomic update of pages, so even
> -     * if host page size == guest page size the dest guest during run may
> -     * still see partially copied pages which is data corruption.
> +     * Backward compatibility support. While using multifd live

We can also avoid mentioning "compatibility support" here - it's a
parameter, user can legally set it to anything.

> +     * migration, we still need to handle zero page checking on the
> +     * migration main thread.
>       */
> -    if (migrate_multifd() && !migration_in_postcopy()) {
> -        return ram_save_multifd_page(block, offset);
> +    if (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_LEGACY) {
> +        if (save_zero_page(rs, pss, offset)) {
> +            return 1;
> +        }
>      }
>  
> -    return ram_save_page(rs, pss);
> +    return ram_save_multifd_page(block, offset);
>  }
>  
>  /* Should be called before sending a host page */
> @@ -2983,7 +2995,12 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>      }
>  
>      migration_ops = g_malloc0(sizeof(MigrationOps));
> -    migration_ops->ram_save_target_page = ram_save_target_page_legacy;
> +
> +    if (migrate_multifd()) {
> +        migration_ops->ram_save_target_page = ram_save_target_page_multifd;
> +    } else {
> +        migration_ops->ram_save_target_page = ram_save_target_page_legacy;
> +    }
>  
>      bql_unlock();
>      ret = multifd_send_sync_main();
> -- 
> 2.30.2
> 

-- 
Peter Xu

Re: [PATCH v4 2/7] migration/multifd: Implement zero page transmission on the multifd thread.

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Fri, Mar 01, 2024 at 02:28:24AM +0000, Hao Xiang wrote:
>> -GlobalProperty hw_compat_8_2[] = {};
>> +GlobalProperty hw_compat_8_2[] = {
>> +    { "migration", "zero-page-detection", "legacy"},
>> +};
>
> I hope we can make it for 9.0, then this (and many rest places) can be kept
> as-is.  Let's see..  soft-freeze is March 12th.
>
> One thing to mention is I just sent a pull which has mapped-ram feature
> merged.  You may need a rebase onto that, and hopefully mapped-ram can also
> use your feature too within the same patch when you repost.

The key points are:

- The socket migration is under "use_packets", the mapped-ram is under
"!use_packets" always.

- mapped-ram doesn't trasmit zero-pages, it just clears the
corresponding bit in block->file_bmap.

> https://lore.kernel.org/all/20240229153017.2221-1-farosas@suse.de/
>
> That rebase may or may not need much caution, I apologize for that:
> mapped-ram as a feature was discussed 1+ years, so it was a plan to merge
> it (actually still partly of it) into QEMU 9.0.

I started doing that rebase last week and saw issues with a sender
thread always getting -EPIPE at the sendmsg() on the regular socket
migration. Let's hope it was just me being tired. I'll try to get
something ready this week.

Re: [PATCH v4 2/7] migration/multifd: Implement zero page transmission on the multifd thread.

[Fabiano Rosas]

Fabiano Rosas <farosas@suse.de> writes:

> Peter Xu <peterx@redhat.com> writes:
>
>> On Fri, Mar 01, 2024 at 02:28:24AM +0000, Hao Xiang wrote:
>>> -GlobalProperty hw_compat_8_2[] = {};
>>> +GlobalProperty hw_compat_8_2[] = {
>>> +    { "migration", "zero-page-detection", "legacy"},
>>> +};
>>
>> I hope we can make it for 9.0, then this (and many rest places) can be kept
>> as-is.  Let's see..  soft-freeze is March 12th.
>>
>> One thing to mention is I just sent a pull which has mapped-ram feature
>> merged.  You may need a rebase onto that, and hopefully mapped-ram can also
>> use your feature too within the same patch when you repost.
>
> The key points are:
>
> - The socket migration is under "use_packets", the mapped-ram is under
> "!use_packets" always.
>
> - mapped-ram doesn't trasmit zero-pages, it just clears the
> corresponding bit in block->file_bmap.
>
>> https://lore.kernel.org/all/20240229153017.2221-1-farosas@suse.de/
>>
>> That rebase may or may not need much caution, I apologize for that:
>> mapped-ram as a feature was discussed 1+ years, so it was a plan to merge
>> it (actually still partly of it) into QEMU 9.0.
>
> I started doing that rebase last week and saw issues with a sender
> thread always getting -EPIPE at the sendmsg() on the regular socket
> migration. Let's hope it was just me being tired. I'll try to get
> something ready this week.

This was just a rebase mistake.

While debugging it I noticed that migration-test doesn't really test
zero page code properly. The guest workload dirties all memory right
away, so I'm not sure we ever see a zero page. A quick test with
multifd, shows p->zero_num=0 all the time.

Any ideas on how to introduce some holes for zero page testing?

Re: [PATCH v4 2/7] migration/multifd: Implement zero page transmission on the multifd thread.

[Fabiano Rosas]

Fabiano Rosas <farosas@suse.de> writes:

> Fabiano Rosas <farosas@suse.de> writes:
>
>> Peter Xu <peterx@redhat.com> writes:
>>
>>> On Fri, Mar 01, 2024 at 02:28:24AM +0000, Hao Xiang wrote:
>>>> -GlobalProperty hw_compat_8_2[] = {};
>>>> +GlobalProperty hw_compat_8_2[] = {
>>>> +    { "migration", "zero-page-detection", "legacy"},
>>>> +};
>>>
>>> I hope we can make it for 9.0, then this (and many rest places) can be kept
>>> as-is.  Let's see..  soft-freeze is March 12th.
>>>
>>> One thing to mention is I just sent a pull which has mapped-ram feature
>>> merged.  You may need a rebase onto that, and hopefully mapped-ram can also
>>> use your feature too within the same patch when you repost.
>>
>> The key points are:
>>
>> - The socket migration is under "use_packets", the mapped-ram is under
>> "!use_packets" always.
>>
>> - mapped-ram doesn't trasmit zero-pages, it just clears the
>> corresponding bit in block->file_bmap.
>>
>>> https://lore.kernel.org/all/20240229153017.2221-1-farosas@suse.de/
>>>
>>> That rebase may or may not need much caution, I apologize for that:
>>> mapped-ram as a feature was discussed 1+ years, so it was a plan to merge
>>> it (actually still partly of it) into QEMU 9.0.
>>
>> I started doing that rebase last week and saw issues with a sender
>> thread always getting -EPIPE at the sendmsg() on the regular socket
>> migration. Let's hope it was just me being tired. I'll try to get
>> something ready this week.
>
> This was just a rebase mistake.
>
> While debugging it I noticed that migration-test doesn't really test
> zero page code properly. The guest workload dirties all memory right
> away, so I'm not sure we ever see a zero page. A quick test with
> multifd, shows p->zero_num=0 all the time.
>
> Any ideas on how to introduce some holes for zero page testing?

Aaaand that's another mistake on my part. Scratch that. The tests work
just fine.

Re: [PATCH v4 2/7] migration/multifd: Implement zero page transmission on the multifd thread.

[Fabiano Rosas]

Fabiano Rosas <farosas@suse.de> writes:

> Peter Xu <peterx@redhat.com> writes:
>
>> On Fri, Mar 01, 2024 at 02:28:24AM +0000, Hao Xiang wrote:
>>> -GlobalProperty hw_compat_8_2[] = {};
>>> +GlobalProperty hw_compat_8_2[] = {
>>> +    { "migration", "zero-page-detection", "legacy"},
>>> +};
>>
>> I hope we can make it for 9.0, then this (and many rest places) can be kept
>> as-is.  Let's see..  soft-freeze is March 12th.
>>
>> One thing to mention is I just sent a pull which has mapped-ram feature
>> merged.  You may need a rebase onto that, and hopefully mapped-ram can also
>> use your feature too within the same patch when you repost.
>
> The key points are:
>
> - The socket migration is under "use_packets", the mapped-ram is under
> "!use_packets" always.
>
> - mapped-ram doesn't trasmit zero-pages, it just clears the
> corresponding bit in block->file_bmap.
>
>> https://lore.kernel.org/all/20240229153017.2221-1-farosas@suse.de/
>>
>> That rebase may or may not need much caution, I apologize for that:
>> mapped-ram as a feature was discussed 1+ years, so it was a plan to merge
>> it (actually still partly of it) into QEMU 9.0.
>
> I started doing that rebase last week and saw issues with a sender
> thread always getting -EPIPE at the sendmsg() on the regular socket
> migration. Let's hope it was just me being tired.

> I'll try to get something ready this week.

@Hao Xiang:

Here's a branch with the rebase. Please include the first two commits
when you repost:

 migration/multifd: Allow clearing of the file_bmap from multifd
 migration/multifd: Allow zero pages in file migration

There are also two small additions and some conflict resolution at the
"Implement zero page..." commit. Make sure you don't miss them.

https://gitlab.com/farosas/qemu/-/commits/migration-multifd-zero-page

Let me know if you encounter any issues.

Re: [PATCH v4 3/7] migration/multifd: Implement ram_save_target_page_multifd to handle multifd version of MigrationOps::ram_save_target_page.

[hao.xiang]

> 
> On Sun, Mar 3, 2024 at 11:46 PM Peter Xu <peterx@redhat.com> wrote:
> 
> > 
> > On Fri, Mar 01, 2024 at 02:28:25AM +0000, Hao Xiang wrote:
> > 
> >  1. Add a dedicated handler for MigrationOps::ram_save_target_page in
> > 
> >  multifd live migration.
> > 
> >  2. Refactor ram_save_target_page_legacy so that the legacy and multifd
> > 
> >  handlers don't have internal functions calling into each other.
> > 
> >  Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
> > 
> >  Reviewed-by: Fabiano Rosas <farosas@suse.de>
> > 
> >  Message-Id: <20240226195654.934709-4-hao.xiang@bytedance.com>
> > 
> >  ---
> > 
> >  migration/ram.c | 43 ++++++++++++++++++++++++++++++-------------
> > 
> >  1 file changed, 30 insertions(+), 13 deletions(-)
> > 
> >  diff --git a/migration/ram.c b/migration/ram.c
> > 
> >  index e1fa229acf..f9d6ea65cc 100644
> > 
> >  --- a/migration/ram.c
> > 
> >  +++ b/migration/ram.c
> > 
> >  @@ -1122,10 +1122,6 @@ static int save_zero_page(RAMState *rs, PageSearchStatus *pss,
> > 
> >  QEMUFile *file = pss->pss_channel;
> > 
> >  int len = 0;
> > 
> >  - if (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_NONE) {
> > 
> >  - return 0;
> > 
> >  - }
> > 
> >  We need to keep this to disable zero-page-detect on !multifd?

So if multifd is enabled, the new parameter takes effect. If multifd is not enabled, zero page checking will always be done in the main thread, which is exactly the behavior it is now. I thought legacy migration is a deprecated feature so I am trying to not add new stuff to it.

> > 
> >  -
> > 
> >  if (!buffer_is_zero(p, TARGET_PAGE_SIZE)) {
> > 
> >  return 0;
> > 
> >  }
> > 
> >  @@ -2045,7 +2041,6 @@ static bool save_compress_page(RAMState *rs, PageSearchStatus *pss,
> > 
> >  */
> > 
> >  static int ram_save_target_page_legacy(RAMState *rs, PageSearchStatus *pss)
> > 
> >  {
> > 
> >  - RAMBlock *block = pss->block;
> > 
> >  ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
> > 
> >  int res;
> > 
> >  @@ -2061,17 +2056,34 @@ static int ram_save_target_page_legacy(RAMState *rs, PageSearchStatus *pss)
> > 
> >  return 1;
> > 
> >  }
> > 
> >  + return ram_save_page(rs, pss);
> > 
> >  +}
> > 
> >  +
> > 
> >  +/**
> > 
> >  + * ram_save_target_page_multifd: send one target page to multifd workers
> > 
> >  + *
> > 
> >  + * Returns 1 if the page was queued, -1 otherwise.
> > 
> >  + *
> > 
> >  + * @rs: current RAM state
> > 
> >  + * @pss: data about the page we want to send
> > 
> >  + */
> > 
> >  +static int ram_save_target_page_multifd(RAMState *rs, PageSearchStatus *pss)
> > 
> >  +{
> > 
> >  + RAMBlock *block = pss->block;
> > 
> >  + ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
> > 
> >  +
> > 
> >  /*
> > 
> >  - * Do not use multifd in postcopy as one whole host page should be
> > 
> >  - * placed. Meanwhile postcopy requires atomic update of pages, so even
> > 
> >  - * if host page size == guest page size the dest guest during run may
> > 
> >  - * still see partially copied pages which is data corruption.
> > 
> >  + * Backward compatibility support. While using multifd live
> > 
> >  We can also avoid mentioning "compatibility support" here - it's a
> > 
> >  parameter, user can legally set it to anything.

Will drop that.

> > 
> >  + * migration, we still need to handle zero page checking on the
> > 
> >  + * migration main thread.
> > 
> >  */
> > 
> >  - if (migrate_multifd() && !migration_in_postcopy()) {
> > 
> >  - return ram_save_multifd_page(block, offset);
> > 
> >  + if (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_LEGACY) {
> > 
> >  + if (save_zero_page(rs, pss, offset)) {
> > 
> >  + return 1;
> > 
> >  + }
> > 
> >  }
> > 
> >  - return ram_save_page(rs, pss);
> > 
> >  + return ram_save_multifd_page(block, offset);
> > 
> >  }
> > 
> >  /* Should be called before sending a host page */
> > 
> >  @@ -2983,7 +2995,12 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> > 
> >  }
> > 
> >  migration_ops = g_malloc0(sizeof(MigrationOps));
> > 
> >  - migration_ops->ram_save_target_page = ram_save_target_page_legacy;
> > 
> >  +
> > 
> >  + if (migrate_multifd()) {
> > 
> >  + migration_ops->ram_save_target_page = ram_save_target_page_multifd;
> > 
> >  + } else {
> > 
> >  + migration_ops->ram_save_target_page = ram_save_target_page_legacy;
> > 
> >  + }
> > 
> >  bql_unlock();
> > 
> >  ret = multifd_send_sync_main();
> > 
> >  --
> > 
> >  2.30.2
> > 
> >  --
> > 
> >  Peter Xu
> >
>

Re: [External] Re: [PATCH v4 2/7] migration/multifd: Implement zero page transmission on the multifd thread.

[hao.xiang]

> 
> On Sun, Mar 3, 2024 at 11:16 PM Peter Xu <peterx@redhat.com> wrote:
> 
> > 
> > On Fri, Mar 01, 2024 at 02:28:24AM +0000, Hao Xiang wrote:
> > 
> >  -GlobalProperty hw_compat_8_2[] = {};
> > 
> >  +GlobalProperty hw_compat_8_2[] = {
> > 
> >  + { "migration", "zero-page-detection", "legacy"},
> > 
> >  +};
> > 
> >  I hope we can make it for 9.0, then this (and many rest places) can be kept
> > 
> >  as-is. Let's see.. soft-freeze is March 12th.
> > 
> >  One thing to mention is I just sent a pull which has mapped-ram feature
> > 
> >  merged. You may need a rebase onto that, and hopefully mapped-ram can also
> > 
> >  use your feature too within the same patch when you repost.
> > 
> >  https://lore.kernel.org/all/20240229153017.2221-1-farosas@suse.de/
> > 
> >  That rebase may or may not need much caution, I apologize for that:
> > 
> >  mapped-ram as a feature was discussed 1+ years, so it was a plan to merge
> > 
> >  it (actually still partly of it) into QEMU 9.0.

Let's see if we can catch that.

> > 
> >  [...]
> > 
> >  +static bool multifd_zero_page(void)
> > 
> >  multifd_zero_page_enabled()?

Changed.

> > 
> >  +{
> > 
> >  + return migrate_zero_page_detection() == ZERO_PAGE_DETECTION_MULTIFD;
> > 
> >  +}
> > 
> >  +
> > 
> >  +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_send_zero_page_check: Perform zero page detection on all pages.
> > 
> >  + *
> > 
> >  + * Sorts normal pages before zero pages in p->pages->offset and updates
> > 
> >  + * p->pages->normal_num.
> > 
> >  + *
> > 
> >  + * @param p A pointer to the send params.
> > 
> >  Nit: the majority of doc style in QEMU (it seems to me) is:
> > 
> >  @p: pointer to @MultiFDSendParams.
> > 
> >  + */
> > 
> >  +void multifd_send_zero_page_check(MultiFDSendParams *p)
> > 
> >  multifd_send_zero_page_detect()?
> > 
> >  This patch used "check" on both sides, but neither of them is a pure check
> > 
> >  to me. For the other side, maybe multifd_recv_zero_page_process()? As
> > 
> >  that one applies the zero pages.


Renamed.

> > 
> >  +{
> > 
> >  + MultiFDPages_t *pages = p->pages;
> > 
> >  + RAMBlock *rb = pages->block;
> > 
> >  + int i = 0;
> > 
> >  + int j = pages->num - 1;
> > 
> >  +
> > 
> >  + /*
> > 
> >  + * QEMU older than 9.0 don't understand zero page
> > 
> >  + * on multifd channel. This switch is required to
> > 
> >  + * maintain backward compatibility.
> > 
> >  + */
> > 
> >  IMHO we can drop this comment; it is not accurate as the user can disable
> > 
> >  it explicitly through the parameter, then it may not always about compatibility.

Dropped.

> > 
> >  + if (multifd_zero_page()) {
> > 
> >  Shouldn't this be "!multifd_zero_page_enabled()"?

Thanks for catching this! My bad. Fixed.

> > 
> >  + pages->normal_num = pages->num;
> > 
> >  + return;
> > 
> >  + }
> > 
> >  The rest looks all sane.
> > 
> >  Thanks,
> > 
> >  --
> > 
> >  Peter Xu
> >
>

Re: [External] Re: [PATCH v4 6/7] migration/multifd: Add zero pages and zero bytes counter to migration status interface.

[hao.xiang]

> 
> On Thu, Feb 29, 2024 at 11:40 PM Markus Armbruster <armbru@redhat.com> wrote:
> 
> > 
> > Hao Xiang <hao.xiang@bytedance.com> writes:
> > 
> >  This change extends the MigrationStatus interface to track zero pages
> > 
> >  and zero bytes counter.
> > 
> >  Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
> > 
> >  [...]
> > 
> >  diff --git a/qapi/migration.json b/qapi/migration.json
> > 
> >  index ca9561fbf1..03b850bab7 100644
> > 
> >  --- a/qapi/migration.json
> > 
> >  +++ b/qapi/migration.json
> > 
> >  @@ -63,6 +63,10 @@
> > 
> >  # between 0 and @dirty-sync-count * @multifd-channels. (since
> > 
> >  # 7.1)
> > 
> >  #
> > 
> >  +# @zero-pages: number of zero pages (since 9.0)
> > 
> >  +#
> > 
> >  +# @zero-bytes: number of zero bytes sent (since 9.0)
> > 
> >  +#
> > 
> >  Discussion of v3 has led me to believe:
> > 
> >  1. A page is either migrated as a normal page or as a zero page.
> > 
> >  2. The following equations hold:
> > 
> >  @normal-bytes = @normal * @page-size
> > 
> >  @zero-bytes = @zero-pages * @page-size
> > 
> >  3. @zero-pages is the same as @duplicate, with a better name. We intend
> > 
> >  to drop @duplicate eventually.
> > 
> >  If this is correct, I'd like you to
> > 
> >  A. Name it @zero for consistency with @normal. Disregard my advice to
> > 
> >  name it @zero-pages; two consistent bad names are better than one bad
> > 
> >  name, one good name, and inconsistency.
> > 
> >  B. Add @zero and @zero-bytes next to @normal and @normal-bytes.
> > 
> >  C. Deprecate @duplicate (item 3). Separate patch, please.
> > 
> >  D. Consider documenting more clearly what normal and zero pages are
> > 
> >  (item 1), and how @FOO, @FOO-pages and @page-size are related (item
> > 
> >  2). Could be done in a followup patch.

I will move this out of the current patchset and put them into a seperate patchset. I think I am not totally understanding the exact process of deprecating an interface and hence will need your help to probably go a few more versions. And I read from earlier conversation the soft release for QEMU9.0 is 3/12 so hopefully the rest of this patchset can catch it.

> > 
> >  # Features:
> > 
> >  #
> > 
> >  # @deprecated: Member @skipped is always zero since 1.5.3
> > 
> >  @@ -81,7 +85,8 @@
> > 
> >  'multifd-bytes': 'uint64', 'pages-per-second': 'uint64',
> > 
> >  'precopy-bytes': 'uint64', 'downtime-bytes': 'uint64',
> > 
> >  'postcopy-bytes': 'uint64',
> > 
> >  - 'dirty-sync-missed-zero-copy': 'uint64' } }
> > 
> >  + 'dirty-sync-missed-zero-copy': 'uint64',
> > 
> >  + 'zero-pages': 'int', 'zero-bytes': 'size' } }
> > 
> >  [...]
> >
>

Re: [PATCH v4 2/7] migration/multifd: Implement zero page transmission on the multifd thread.

[hao.xiang]

> 
> On Mon, Mar 4, 2024 at 10:24 AM Fabiano Rosas <farosas@suse.de> wrote:
> 
>  
> 
>  
> 
>  Fabiano Rosas <farosas@suse.de> writes:
> 
>  
> 
>  Peter Xu <peterx@redhat.com> writes:
> 
>  
> 
>  On Fri, Mar 01, 2024 at 02:28:24AM +0000, Hao Xiang wrote:
> 
>  
> 
>  -GlobalProperty hw_compat_8_2[] = {};
> 
>  
> 
>  +GlobalProperty hw_compat_8_2[] = {
> 
>  
> 
>  + { "migration", "zero-page-detection", "legacy"},
> 
>  
> 
>  +};
> 
>  
> 
>  I hope we can make it for 9.0, then this (and many rest places) can be kept
> 
>  
> 
>  as-is. Let's see.. soft-freeze is March 12th.
> 
>  
> 
>  One thing to mention is I just sent a pull which has mapped-ram feature
> 
>  
> 
>  merged. You may need a rebase onto that, and hopefully mapped-ram can also
> 
>  
> 
>  use your feature too within the same patch when you repost.
> 
>  
> 
>  The key points are:
> 
>  
> 
>  - The socket migration is under "use_packets", the mapped-ram is under
> 
>  
> 
>  "!use_packets" always.
> 
>  
> 
>  - mapped-ram doesn't trasmit zero-pages, it just clears the
> 
>  
> 
>  corresponding bit in block->file_bmap.
> 
>  
> 
>  https://lore.kernel.org/all/20240229153017.2221-1-farosas@suse.de/
> 
>  
> 
>  That rebase may or may not need much caution, I apologize for that:
> 
>  
> 
>  mapped-ram as a feature was discussed 1+ years, so it was a plan to merge
> 
>  
> 
>  it (actually still partly of it) into QEMU 9.0.
> 
>  
> 
>  I started doing that rebase last week and saw issues with a sender
> 
>  
> 
>  thread always getting -EPIPE at the sendmsg() on the regular socket
> 
>  
> 
>  migration. Let's hope it was just me being tired.
> 
>  
> 
>  I'll try to get something ready this week.
> 
>  
> 
>  @Hao Xiang:
> 
>  
> 
>  Here's a branch with the rebase. Please include the first two commits
> 
>  
> 
>  when you repost:
> 
>  
> 
>  migration/multifd: Allow clearing of the file_bmap from multifd
> 
>  
> 
>  migration/multifd: Allow zero pages in file migration
> 
>  
> 
>  There are also two small additions and some conflict resolution at the
> 
>  
> 
>  "Implement zero page..." commit. Make sure you don't miss them.
> 
>  
> 
>  https://gitlab.com/farosas/qemu/-/commits/migration-multifd-zero-page
> 
>  
> 
>  Let me know if you encounter any issues.
> 

Sorry about the delay. I have rebased and pulled in the two commits you mentioned. Test works fine. I just sent out a new version.

I removed the zero/zero-bytes interface changes out of this patchset but will follow up with a separate one.

Re: [PATCH v4 7/7] Update maintainer contact for migration multifd zero page checking acceleration.

[hao.xiang]

> 
> On Sun, Mar 3, 2024 at 11:34 PM Peter Xu <peterx@redhat.com> wrote:
> 
> > 
> > On Fri, Mar 01, 2024 at 02:28:29AM +0000, Hao Xiang wrote:
> > 
> >  Add myself to maintain multifd zero page checking acceleration function.
> > 
> >  Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
> > 
> >  ---
> > 
> >  MAINTAINERS | 5 +++++
> > 
> >  1 file changed, 5 insertions(+)
> > 
> >  diff --git a/MAINTAINERS b/MAINTAINERS
> > 
> >  index 65dfdc9677..b547918e4d 100644
> > 
> >  --- a/MAINTAINERS
> > 
> >  +++ b/MAINTAINERS
> > 
> >  @@ -3414,6 +3414,11 @@ F: tests/migration/
> > 
> >  F: util/userfaultfd.c
> > 
> >  X: migration/rdma*
> > 
> >  +Migration multifd zero page checking acceleration
> > 
> >  +M: Hao Xiang <hao.xiang@bytedance.com>
> > 
> >  +S: Maintained
> > 
> >  +F: migration/multifd-zero-page.c
> > 
> >  +
> > 
> >  Firstly appreciate a lot for volunteering!
> > 
> >  My fault to not have made it clear. This file alone so far will need to be
> > 
> >  closely related to the multifd core, so whoever maintains migration should
> > 
> >  look after this. It's also slightly weird to have a separate entry for a
> > 
> >  file that is tens of LOC if it's already covered by another upper entry.
> > 
> >  What I worry is about vendor/library specific parts that will be harder to
> > 
> >  maintain, and migration maintainers (no matter who does the job in the
> > 
> >  future) may not always cover those areas. So I was expecting we could have
> > 
> >  volunteers covering e.g. QAT / DSA / IAA accelerators. Since all these
> > 
> >  accelerators will all be part of Intel's new chips, there's also one way
> > 
> >  that we have "Intel accelerators" section to cover vendor specific codes
> > 
> >  and then cover all those areas no matter it's zero detect accelerator or HW
> > 
> >  compressors.
> > 
> >  I'd suggest we discuss this with Intel people to check out a solid plan
> > 
> >  later when we start to merge HW/LIB specific codes. For now I suggest we
> > 
> >  can drop this patch and stick with the feature implementation, to see
> > 
> >  whether it can catch the train for 9.0. IMHO this is a good feature even
> > 
> >  without HW accelerators (and I think it's close to ready), so I hope it can
> > 
> >  still make it.
> > 
> >  Thanks,

No worries. I misunderstood you. We can talk about maintenance later on when we have the accelerator changes ready.

> > 
> >  --
> > 
> >  Peter Xu
> >
>

Re: [PATCH v4 3/7] migration/multifd: Implement ram_save_target_page_multifd to handle multifd version of MigrationOps::ram_save_target_page.

[Peter Xu]

On Sat, Mar 09, 2024 at 02:06:33AM +0000, hao.xiang@linux.dev wrote:
> > >  @@ -1122,10 +1122,6 @@ static int save_zero_page(RAMState *rs, PageSearchStatus *pss,
> > >  QEMUFile *file = pss->pss_channel;
> > >  int len = 0;
> > >
> > >  - if (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_NONE) {
> > >  - return 0;
> > >  - }
> > > 
> > >  We need to keep this to disable zero-page-detect on !multifd?
> 
> So if multifd is enabled, the new parameter takes effect. If multifd is
> not enabled, zero page checking will always be done in the main thread,
> which is exactly the behavior it is now. I thought legacy migration is a
> deprecated feature so I am trying to not add new stuff to it.

There's no plan to deprecate legacy migrations, I think there was a plan to
make multifd the default, but I don't yet think it all thorougly yet, and
even if it happens it doesn't mean we'll remove legacy migration code.

When repost please still make sure this parameter works for both multifd
and !multifd.

Thanks,

-- 
Peter Xu

Re: [PATCH v4 3/7] migration/multifd: Implement ram_save_target_page_multifd to handle multifd version of MigrationOps::ram_save_target_page.

[hao.xiang]

March 11, 2024 at 6:20 AM, "Peter Xu" <peterx@redhat.com> wrote:



> 
> On Sat, Mar 09, 2024 at 02:06:33AM +0000, hao.xiang@linux.dev wrote:
> 
> > 
> > > @@ -1122,10 +1122,6 @@ static int save_zero_page(RAMState *rs, PageSearchStatus *pss,
> > 
> >  > QEMUFile *file = pss->pss_channel;
> > 
> >  > int len = 0;
> > 
> >  >
> > 
> >  > - if (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_NONE) {
> > 
> >  > - return 0;
> > 
> >  > - }
> > 
> >  > 
> > 
> >  > We need to keep this to disable zero-page-detect on !multifd?
> > 
> >  
> > 
> >  So if multifd is enabled, the new parameter takes effect. If multifd is
> > 
> >  not enabled, zero page checking will always be done in the main thread,
> > 
> >  which is exactly the behavior it is now. I thought legacy migration is a
> > 
> >  deprecated feature so I am trying to not add new stuff to it.
> > 
> 
> There's no plan to deprecate legacy migrations, I think there was a plan to
> 
> make multifd the default, but I don't yet think it all thorougly yet, and
> 
> even if it happens it doesn't mean we'll remove legacy migration code.
> 
> When repost please still make sure this parameter works for both multifd
> 
> and !multifd.
> 
> Thanks,
> 
> -- 
> 
> Peter Xu


Sure. Fixed the issue now and reposted a new patchset.

>