[PATCH v4 00/18] Support Multifd for RDMA migration

[PATCH v4 00/18] Support Multifd for RDMA migration

[Chuan Zheng]

The RDMA bandwidth is not fully utilized for over 25Gigabit NIC because
of single channel for RDMA migration. This patch series is going to support
multifd for RDMA migration based on multifd framework.

Comparsion is between origion and multifd RDMA migration is re-tested for v3.
The VM specifications for migration are as follows:
- VM use 4k page;
- the number of VCPU is 4;
- the total memory is 16Gigabit;
- use 'mempress' tool to pressurize VM(mempress 8000 500);
- use 25Gigabit network card to migrate;

For origin RDMA and MultiRDMA migration, the total migration times of
VM are as follows:
+++++++++++++++++++++++++++++++++++++++++++++++++
|             | NOT rdma-pin-all | rdma-pin-all |
+++++++++++++++++++++++++++++++++++++++++++++++++
| origin RDMA |       26 s       |     29 s     |
-------------------------------------------------
|  MultiRDMA  |       16 s       |     17 s     |
+++++++++++++++++++++++++++++++++++++++++++++++++

Test the multifd RDMA migration like this:
virsh migrate --live --parallel --migrateuri
rdma://192.168.1.100 [VM] --listen-address 0.0.0.0  qemu+tcp://192.168.1.100/system --verbose

v3 -> v4:
    modify some function names
    export multifd_rdma_ops instead of a function
    fix minior codestyle issues

v2 -> v3:
    create multifd ops for both tcp and rdma
    do not export rdma to avoid multifd code in mess
    fix build issue for non-rdma
    fix some codestyle and buggy code

Chuan Zheng (18):
  migration/rdma: add the 'migrate_rdma_pin_all' function
  migration/rdma: judge whether or not the RDMA is used for migration
  migration/rdma: create multifd_setup_ops for Tx/Rx thread
  migration/rdma: add multifd_setup_ops for rdma
  migration/rdma: do not need sync main for rdma
  migration/rdma: export MultiFDSendParams/MultiFDRecvParams
  migration/rdma: add rdma field into multifd send/recv param
  migration/rdma: export getQIOChannel to get QIOchannel in rdma
  migration/rdma: add multifd_rdma_load_setup() to setup multifd rdma
  migration/rdma: Create the multifd recv channels for RDMA
  migration/rdma: record host_port for multifd RDMA
  migration/rdma: Create the multifd send channels for RDMA
  migration/rdma: Add the function for dynamic page registration
  migration/rdma: register memory for multifd RDMA channels
  migration/rdma: only register the memory for multifd channels
  migration/rdma: add rdma_channel into Migrationstate field
  migration/rdma: send data for both rdma-pin-all and NOT rdma-pin-all
    mode
  migration/rdma: RDMA cleanup for multifd migration

 migration/migration.c |  24 +++
 migration/migration.h |  11 ++
 migration/multifd.c   |  97 +++++++++-
 migration/multifd.h   |  25 +++
 migration/qemu-file.c |   5 +
 migration/qemu-file.h |   1 +
 migration/rdma.c      | 490 +++++++++++++++++++++++++++++++++++++++++++++++++-
 7 files changed, 641 insertions(+), 12 deletions(-)

-- 
1.8.3.1

[PATCH v4 01/18] migration/rdma: add the 'migrate_rdma_pin_all' function

[Chuan Zheng]

Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 9 +++++++++
 migration/migration.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index 1986cb8..447dfb9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2382,6 +2382,15 @@ bool migrate_use_events(void)
     return s->enabled_capabilities[MIGRATION_CAPABILITY_EVENTS];
 }
 
+bool migrate_rdma_pin_all(void)
+{
+    MigrationState *s;
+
+    s = migrate_get_current();
+
+    return s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL];
+}
+
 bool migrate_use_multifd(void)
 {
     MigrationState *s;
diff --git a/migration/migration.h b/migration/migration.h
index d096b77..22b36f3 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -316,6 +316,7 @@ bool migrate_ignore_shared(void);
 bool migrate_validate_uuid(void);
 
 bool migrate_auto_converge(void);
+bool migrate_rdma_pin_all(void);
 bool migrate_use_multifd(void);
 bool migrate_pause_before_switchover(void);
 int migrate_multifd_channels(void);
-- 
1.8.3.1

[PATCH v4 02/18] migration/rdma: judge whether or not the RDMA is used for migration

[Chuan Zheng]

Add enabled_rdma_migration into MigrationState to judge
whether or not the RDMA is used for migration.

Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/migration.c | 13 +++++++++++++
 migration/migration.h |  6 ++++++
 2 files changed, 19 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index 447dfb9..129c81a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -418,11 +418,13 @@ void migrate_add_address(SocketAddress *address)
 static void qemu_start_incoming_migration(const char *uri, Error **errp)
 {
     const char *p = NULL;
+    MigrationState *s = migrate_get_current();
 
     if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
         return;
     }
 
+    s->enabled_rdma_migration = false;
     qapi_event_send_migration(MIGRATION_STATUS_SETUP);
     if (strstart(uri, "tcp:", &p) ||
         strstart(uri, "unix:", NULL) ||
@@ -430,6 +432,7 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
         socket_start_incoming_migration(p ? p : uri, errp);
 #ifdef CONFIG_RDMA
     } else if (strstart(uri, "rdma:", &p)) {
+        s->enabled_rdma_migration = true;
         rdma_start_incoming_migration(p, errp);
 #endif
     } else if (strstart(uri, "exec:", &p)) {
@@ -1921,6 +1924,7 @@ void migrate_init(MigrationState *s)
     s->start_postcopy = false;
     s->postcopy_after_devices = false;
     s->migration_thread_running = false;
+    s->enabled_rdma_migration = false;
     error_free(s->error);
     s->error = NULL;
     s->hostname = NULL;
@@ -2162,6 +2166,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
         socket_start_outgoing_migration(s, p ? p : uri, &local_err);
 #ifdef CONFIG_RDMA
     } else if (strstart(uri, "rdma:", &p)) {
+        s->enabled_rdma_migration = true;
         rdma_start_outgoing_migration(s, p, &local_err);
 #endif
     } else if (strstart(uri, "exec:", &p)) {
@@ -2391,6 +2396,14 @@ bool migrate_rdma_pin_all(void)
     return s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL];
 }
 
+bool migrate_use_rdma(void)
+{
+    MigrationState *s;
+    s = migrate_get_current();
+
+    return s->enabled_rdma_migration;
+}
+
 bool migrate_use_multifd(void)
 {
     MigrationState *s;
diff --git a/migration/migration.h b/migration/migration.h
index 22b36f3..da5681b 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -280,6 +280,11 @@ struct MigrationState {
      * This save hostname when out-going migration starts
      */
     char *hostname;
+
+    /*
+     * Enable RDMA migration
+     */
+    bool enabled_rdma_migration;
 };
 
 void migrate_set_state(int *state, int old_state, int new_state);
@@ -317,6 +322,7 @@ bool migrate_validate_uuid(void);
 
 bool migrate_auto_converge(void);
 bool migrate_rdma_pin_all(void);
+bool migrate_use_rdma(void);
 bool migrate_use_multifd(void);
 bool migrate_pause_before_switchover(void);
 int migrate_multifd_channels(void);
-- 
1.8.3.1

[PATCH v4 03/18] migration/rdma: create multifd_setup_ops for Tx/Rx thread

[Chuan Zheng]

Create multifd_setup_ops for TxRx thread, no logic change.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/multifd.c | 44 +++++++++++++++++++++++++++++++++++++++-----
 migration/multifd.h |  7 +++++++
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 1a1e589..cb1fc01 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -386,6 +386,8 @@ struct {
     int exiting;
     /* multifd ops */
     MultiFDMethods *ops;
+    /* multifd setup ops */
+    MultiFDSetup *setup_ops;
 } *multifd_send_state;
 
 /*
@@ -805,8 +807,9 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
         } else {
             /* update for tls qio channel */
             p->c = ioc;
-            qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
-                                   QEMU_THREAD_JOINABLE);
+            qemu_thread_create(&p->thread, p->name,
+                               multifd_send_state->setup_ops->send_thread,
+                               p, QEMU_THREAD_JOINABLE);
        }
        return false;
     }
@@ -854,6 +857,11 @@ cleanup:
     multifd_new_send_channel_cleanup(p, sioc, local_err);
 }
 
+static void multifd_send_channel_setup(MultiFDSendParams *p)
+{
+    socket_send_channel_create(multifd_new_send_channel_async, p);
+}
+
 int multifd_save_setup(Error **errp)
 {
     int thread_count;
@@ -871,6 +879,7 @@ int multifd_save_setup(Error **errp)
     multifd_send_state->pages = multifd_pages_init(page_count);
     qemu_sem_init(&multifd_send_state->channels_ready, 0);
     qatomic_set(&multifd_send_state->exiting, 0);
+    multifd_send_state->setup_ops = multifd_setup_ops_init();
     multifd_send_state->ops = multifd_ops[migrate_multifd_compression()];
 
     for (i = 0; i < thread_count; i++) {
@@ -890,7 +899,7 @@ int multifd_save_setup(Error **errp)
         p->packet->version = cpu_to_be32(MULTIFD_VERSION);
         p->name = g_strdup_printf("multifdsend_%d", i);
         p->tls_hostname = g_strdup(s->hostname);
-        socket_send_channel_create(multifd_new_send_channel_async, p);
+        multifd_send_state->setup_ops->send_channel_setup(p);
     }
 
     for (i = 0; i < thread_count; i++) {
@@ -917,6 +926,8 @@ struct {
     uint64_t packet_num;
     /* multifd ops */
     MultiFDMethods *ops;
+    /* multifd setup ops */
+    MultiFDSetup *setup_ops;
 } *multifd_recv_state;
 
 static void multifd_recv_terminate_threads(Error *err)
@@ -1117,6 +1128,7 @@ int multifd_load_setup(Error **errp)
     multifd_recv_state->params = g_new0(MultiFDRecvParams, thread_count);
     qatomic_set(&multifd_recv_state->count, 0);
     qemu_sem_init(&multifd_recv_state->sem_sync, 0);
+    multifd_recv_state->setup_ops = multifd_setup_ops_init();
     multifd_recv_state->ops = multifd_ops[migrate_multifd_compression()];
 
     for (i = 0; i < thread_count; i++) {
@@ -1195,9 +1207,31 @@ bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp)
     p->num_packets = 1;
 
     p->running = true;
-    qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
-                       QEMU_THREAD_JOINABLE);
+    multifd_recv_state->setup_ops->recv_channel_setup(ioc, p);
+    qemu_thread_create(&p->thread, p->name,
+                       multifd_recv_state->setup_ops->recv_thread,
+                       p, QEMU_THREAD_JOINABLE);
     qatomic_inc(&multifd_recv_state->count);
     return qatomic_read(&multifd_recv_state->count) ==
            migrate_multifd_channels();
 }
+
+static void multifd_recv_channel_setup(QIOChannel *ioc, MultiFDRecvParams *p)
+{
+    return;
+}
+
+static MultiFDSetup multifd_socket_ops = {
+    .send_thread = multifd_send_thread,
+    .recv_thread = multifd_recv_thread,
+    .send_channel_setup = multifd_send_channel_setup,
+    .recv_channel_setup = multifd_recv_channel_setup
+};
+
+MultiFDSetup *multifd_setup_ops_init(void)
+{
+    MultiFDSetup *ops;
+
+    ops = &multifd_socket_ops;
+    return ops;
+}
diff --git a/migration/multifd.h b/migration/multifd.h
index 8d6751f..1d2dc90 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -166,6 +166,13 @@ typedef struct {
     int (*recv_pages)(MultiFDRecvParams *p, uint32_t used, Error **errp);
 } MultiFDMethods;
 
+typedef struct {
+    void *(*send_thread)(void *opaque);
+    void *(*recv_thread)(void *opaque);
+    void (*send_channel_setup)(MultiFDSendParams *p);
+    void (*recv_channel_setup)(QIOChannel *ioc, MultiFDRecvParams *p);
+} MultiFDSetup;
+
 void multifd_register_ops(int method, MultiFDMethods *ops);
 
 #endif
-- 
1.8.3.1

[PATCH v4 04/18] migration/rdma: add multifd_setup_ops for rdma

[Chuan Zheng]

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/multifd.c |  6 +++++
 migration/multifd.h |  5 ++++
 migration/rdma.c    | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+)

diff --git a/migration/multifd.c b/migration/multifd.c
index cb1fc01..4820702 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -1232,6 +1232,12 @@ MultiFDSetup *multifd_setup_ops_init(void)
 {
     MultiFDSetup *ops;
 
+#ifdef CONFIG_RDMA
+    if (migrate_use_rdma()) {
+        ops = &multifd_rdma_ops;
+        return ops;
+    }
+#endif
     ops = &multifd_socket_ops;
     return ops;
 }
diff --git a/migration/multifd.h b/migration/multifd.h
index 1d2dc90..e3ab4b0 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -173,6 +173,11 @@ typedef struct {
     void (*recv_channel_setup)(QIOChannel *ioc, MultiFDRecvParams *p);
 } MultiFDSetup;
 
+#ifdef CONFIG_RDMA
+extern MultiFDSetup multifd_rdma_ops;
+#endif
+MultiFDSetup *multifd_setup_ops_init(void);
+
 void multifd_register_ops(int method, MultiFDMethods *ops);
 
 #endif
diff --git a/migration/rdma.c b/migration/rdma.c
index 00eac34..e0ea86d 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -19,6 +19,7 @@
 #include "qemu/cutils.h"
 #include "rdma.h"
 #include "migration.h"
+#include "multifd.h"
 #include "qemu-file.h"
 #include "ram.h"
 #include "qemu-file-channel.h"
@@ -4139,3 +4140,73 @@ err:
     g_free(rdma);
     g_free(rdma_return_path);
 }
+
+static void *multifd_rdma_send_thread(void *opaque)
+{
+    MultiFDSendParams *p = opaque;
+
+    while (true) {
+        WITH_QEMU_LOCK_GUARD(&p->mutex) {
+            if (p->quit) {
+                break;
+            }
+        }
+        qemu_sem_wait(&p->sem);
+    }
+
+    WITH_QEMU_LOCK_GUARD(&p->mutex) {
+        p->running = false;
+    }
+
+    return NULL;
+}
+
+static void multifd_rdma_send_channel_setup(MultiFDSendParams *p)
+{
+    Error *local_err = NULL;
+
+    if (p->quit) {
+        error_setg(&local_err, "multifd: send id %d already quit", p->id);
+        return ;
+    }
+    p->running = true;
+
+    qemu_thread_create(&p->thread, p->name, multifd_rdma_send_thread, p,
+                       QEMU_THREAD_JOINABLE);
+}
+
+static void *multifd_rdma_recv_thread(void *opaque)
+{
+    MultiFDRecvParams *p = opaque;
+
+    while (true) {
+        WITH_QEMU_LOCK_GUARD(&p->mutex) {
+            if (p->quit) {
+                break;
+            }
+        }
+        qemu_sem_wait(&p->sem_sync);
+    }
+
+    WITH_QEMU_LOCK_GUARD(&p->mutex) {
+        p->running = false;
+    }
+
+    return NULL;
+}
+
+static void multifd_rdma_recv_channel_setup(QIOChannel *ioc,
+                                            MultiFDRecvParams *p)
+{
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
+
+    p->file = rioc->file;
+    return;
+}
+
+MultiFDSetup multifd_rdma_ops = {
+    .send_thread = multifd_rdma_send_thread,
+    .recv_thread = multifd_rdma_recv_thread,
+    .send_channel_setup = multifd_rdma_send_channel_setup,
+    .recv_channel_setup = multifd_rdma_recv_channel_setup
+};
-- 
1.8.3.1

[PATCH v4 05/18] migration/rdma: do not need sync main for rdma

[Chuan Zheng]

Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/multifd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/migration/multifd.c b/migration/multifd.c
index 4820702..5d34950 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -583,6 +583,10 @@ void multifd_send_sync_main(QEMUFile *f)
     if (!migrate_use_multifd()) {
         return;
     }
+     /* Do not need sync for rdma */
+    if (migrate_use_rdma()) {
+        return;
+    }
     if (multifd_send_state->pages->used) {
         if (multifd_send_pages(f) < 0) {
             error_report("%s: multifd_send_pages fail", __func__);
@@ -1024,6 +1028,10 @@ void multifd_recv_sync_main(void)
     if (!migrate_use_multifd()) {
         return;
     }
+    /* Do not need sync for rdma */
+    if (migrate_use_rdma()) {
+        return;
+    }
     for (i = 0; i < migrate_multifd_channels(); i++) {
         MultiFDRecvParams *p = &multifd_recv_state->params[i];
 
-- 
1.8.3.1

[PATCH v4 06/18] migration/rdma: export MultiFDSendParams/MultiFDRecvParams

[Chuan Zheng]

MultiFDSendParams and MultiFDRecvParams is need for rdma, export it

Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/multifd.c | 26 ++++++++++++++++++++++++++
 migration/multifd.h |  2 ++
 2 files changed, 28 insertions(+)

diff --git a/migration/multifd.c b/migration/multifd.c
index 5d34950..ae0b7f0 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -390,6 +390,19 @@ struct {
     MultiFDSetup *setup_ops;
 } *multifd_send_state;
 
+int get_multifd_send_param(int id, MultiFDSendParams **param)
+{
+    int ret = 0;
+
+    if (id < 0 || id >= migrate_multifd_channels()) {
+        ret = -1;
+    } else {
+        *param = &(multifd_send_state->params[id]);
+    }
+
+    return ret;
+}
+
 /*
  * How we use multifd_send_state->pages and channel->pages?
  *
@@ -934,6 +947,19 @@ struct {
     MultiFDSetup *setup_ops;
 } *multifd_recv_state;
 
+int get_multifd_recv_param(int id, MultiFDRecvParams **param)
+{
+    int ret = 0;
+
+    if (id < 0 || id >= migrate_multifd_channels()) {
+        ret = -1;
+    } else {
+        *param = &(multifd_recv_state->params[id]);
+    }
+
+    return ret;
+}
+
 static void multifd_recv_terminate_threads(Error *err)
 {
     int i;
diff --git a/migration/multifd.h b/migration/multifd.h
index e3ab4b0..d57756c 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -176,6 +176,8 @@ typedef struct {
 #ifdef CONFIG_RDMA
 extern MultiFDSetup multifd_rdma_ops;
 #endif
+int get_multifd_send_param(int id, MultiFDSendParams **param);
+int get_multifd_recv_param(int id, MultiFDRecvParams **param);
 MultiFDSetup *multifd_setup_ops_init(void);
 
 void multifd_register_ops(int method, MultiFDMethods *ops);
-- 
1.8.3.1

[PATCH v4 07/18] migration/rdma: add rdma field into multifd send/recv param

[Chuan Zheng]

Note we do want to export any rdma struct, take void * instead.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/multifd.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/migration/multifd.h b/migration/multifd.h
index d57756c..b17a2c1 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -108,6 +108,10 @@ typedef struct {
     QemuSemaphore sem_sync;
     /* used for compression methods */
     void *data;
+    /* used for multifd rdma */
+    void *rdma;
+    /* communication channel */
+    QEMUFile *file;
 }  MultiFDSendParams;
 
 typedef struct {
@@ -147,6 +151,10 @@ typedef struct {
     QemuSemaphore sem_sync;
     /* used for de-compression methods */
     void *data;
+    /* used for multifd rdma */
+    void *rdma;
+    /* communication channel */
+    QEMUFile *file;
 } MultiFDRecvParams;
 
 typedef struct {
-- 
1.8.3.1

[PATCH v4 08/18] migration/rdma: export getQIOChannel to get QIOchannel in rdma

[Chuan Zheng]

Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/qemu-file.c | 5 +++++
 migration/qemu-file.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index be21518..37f6201 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -260,6 +260,11 @@ void ram_control_before_iterate(QEMUFile *f, uint64_t flags)
     }
 }
 
+void *getQIOChannel(QEMUFile *f)
+{
+    return f->opaque;
+}
+
 void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
 {
     int ret = 0;
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index a9b6d6c..4cef043 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -165,6 +165,7 @@ void qemu_file_set_blocking(QEMUFile *f, bool block);
 void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
 void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
 void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data);
+void *getQIOChannel(QEMUFile *f);
 
 /* Whenever this is found in the data stream, the flags
  * will be passed to ram_control_load_hook in the incoming-migration
-- 
1.8.3.1

[PATCH v4 09/18] migration/rdma: add multifd_rdma_load_setup() to setup multifd rdma

[Chuan Zheng]

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/rdma.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/migration/rdma.c b/migration/rdma.c
index e0ea86d..996afb0 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -4011,6 +4011,48 @@ static void rdma_accept_incoming_migration(void *opaque)
     }
 }
 
+static bool multifd_rdma_load_setup(const char *host_port,
+                                    RDMAContext *rdma, Error **errp)
+{
+    int thread_count;
+    int i;
+    int idx;
+    MultiFDRecvParams *multifd_recv_param;
+    RDMAContext *multifd_rdma;
+
+    if (!migrate_use_multifd()) {
+        return true;
+    }
+
+    if (multifd_load_setup(errp) != 0) {
+        /*
+         * We haven't been able to create multifd threads
+         * nothing better to do
+         */
+        return false;
+    }
+
+    thread_count = migrate_multifd_channels();
+    for (i = 0; i < thread_count; i++) {
+        if (get_multifd_recv_param(i, &multifd_recv_param) < 0) {
+            ERROR(errp, "rdma: error getting multifd_recv_param(%d)", i);
+            return false;
+        }
+
+        multifd_rdma = qemu_rdma_data_init(host_port, errp);
+        for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
+            multifd_rdma->wr_data[idx].control_len = 0;
+            multifd_rdma->wr_data[idx].control_curr = NULL;
+        }
+        /* the CM channel and CM id is shared */
+        multifd_rdma->channel = rdma->channel;
+        multifd_rdma->listen_id = rdma->listen_id;
+        multifd_recv_param->rdma = (void *)multifd_rdma;
+    }
+
+    return true;
+}
+
 void rdma_start_incoming_migration(const char *host_port, Error **errp)
 {
     int ret;
@@ -4058,6 +4100,16 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)
         qemu_rdma_return_path_dest_init(rdma_return_path, rdma);
     }
 
+    /* multifd rdma setup */
+    if (!multifd_rdma_load_setup(host_port, rdma, &local_err)) {
+        /*
+         * We haven't been able to create multifd threads
+         * nothing better to do
+         */
+        error_report_err(local_err);
+        goto err;
+    }
+
     qemu_set_fd_handler(rdma->channel->fd, rdma_accept_incoming_migration,
                         NULL, (void *)(intptr_t)rdma);
     return;
-- 
1.8.3.1

[PATCH v4 10/18] migration/rdma: Create the multifd recv channels for RDMA

[Chuan Zheng]

We still don't transmit anything through them, and we only build
the RDMA connections.

Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/rdma.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 67 insertions(+), 2 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 996afb0..ed8a015 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3267,6 +3267,40 @@ static void rdma_cm_poll_handler(void *opaque)
     }
 }
 
+static bool qemu_rdma_accept_setup(RDMAContext *rdma)
+{
+    RDMAContext *multifd_rdma = NULL;
+    int thread_count;
+    int i;
+    MultiFDRecvParams *multifd_recv_param;
+    thread_count = migrate_multifd_channels();
+    /* create the multifd channels for RDMA */
+    for (i = 0; i < thread_count; i++) {
+        if (get_multifd_recv_param(i, &multifd_recv_param) < 0) {
+            error_report("rdma: error getting multifd_recv_param(%d)", i);
+            return false;
+        }
+
+        multifd_rdma = (RDMAContext *) multifd_recv_param->rdma;
+        if (multifd_rdma->cm_id == NULL) {
+            break;
+        } else {
+            multifd_rdma = NULL;
+        }
+    }
+
+    if (multifd_rdma) {
+        qemu_set_fd_handler(rdma->channel->fd,
+                            rdma_accept_incoming_migration,
+                            NULL, (void *)(intptr_t)multifd_rdma);
+    } else {
+        qemu_set_fd_handler(rdma->channel->fd, rdma_cm_poll_handler,
+                            NULL, rdma);
+    }
+
+    return true;
+}
+
 static int qemu_rdma_accept(RDMAContext *rdma)
 {
     RDMACapabilities cap;
@@ -3366,6 +3400,10 @@ static int qemu_rdma_accept(RDMAContext *rdma)
         qemu_set_fd_handler(rdma->channel->fd, rdma_accept_incoming_migration,
                             NULL,
                             (void *)(intptr_t)rdma->return_path);
+    } else if (migrate_use_multifd()) {
+        if (!qemu_rdma_accept_setup(rdma)) {
+            goto err_rdma_dest_wait;
+        }
     } else {
         qemu_set_fd_handler(rdma->channel->fd, rdma_cm_poll_handler,
                             NULL, rdma);
@@ -3976,6 +4014,34 @@ static QEMUFile *qemu_fopen_rdma(RDMAContext *rdma, const char *mode)
     return rioc->file;
 }
 
+static void migration_rdma_process_incoming(QEMUFile *f,
+                                            RDMAContext *rdma, Error **errp)
+{
+    MigrationIncomingState *mis = migration_incoming_get_current();
+    QIOChannel *ioc = NULL;
+    bool start_migration = false;
+
+    if (!migrate_use_multifd()) {
+        rdma->migration_started_on_destination = 1;
+        migration_fd_process_incoming(f, errp);
+        return;
+    }
+
+    if (!mis->from_src_file) {
+        mis->from_src_file = f;
+        qemu_file_set_blocking(f, false);
+    } else {
+        ioc = QIO_CHANNEL(getQIOChannel(f));
+        /* Multiple connections */
+        assert(migrate_use_multifd());
+        start_migration = multifd_recv_new_channel(ioc, errp);
+    }
+
+    if (start_migration) {
+        migration_incoming_process();
+    }
+}
+
 static void rdma_accept_incoming_migration(void *opaque)
 {
     RDMAContext *rdma = opaque;
@@ -4004,8 +4070,7 @@ static void rdma_accept_incoming_migration(void *opaque)
         return;
     }
 
-    rdma->migration_started_on_destination = 1;
-    migration_fd_process_incoming(f, &local_err);
+    migration_rdma_process_incoming(f, rdma, &local_err);
     if (local_err) {
         error_reportf_err(local_err, "RDMA ERROR:");
     }
-- 
1.8.3.1

[PATCH v4 11/18] migration/rdma: record host_port for multifd RDMA

[Chuan Zheng]

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/migration.c | 1 +
 migration/migration.h | 3 +++
 migration/rdma.c      | 3 +++
 3 files changed, 7 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index 129c81a..b8f4844 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1925,6 +1925,7 @@ void migrate_init(MigrationState *s)
     s->postcopy_after_devices = false;
     s->migration_thread_running = false;
     s->enabled_rdma_migration = false;
+    s->host_port = NULL;
     error_free(s->error);
     s->error = NULL;
     s->hostname = NULL;
diff --git a/migration/migration.h b/migration/migration.h
index da5681b..537ee09 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -285,6 +285,9 @@ struct MigrationState {
      * Enable RDMA migration
      */
     bool enabled_rdma_migration;
+
+    /* Need by Multi-RDMA */
+    char *host_port;
 };
 
 void migrate_set_state(int *state, int old_state, int new_state);
diff --git a/migration/rdma.c b/migration/rdma.c
index ed8a015..9654b87 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -4206,6 +4206,8 @@ void rdma_start_outgoing_migration(void *opaque,
         goto err;
     }
 
+    s->host_port = g_strdup(host_port);
+
     ret = qemu_rdma_source_init(rdma,
         s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL], errp);
 
@@ -4250,6 +4252,7 @@ void rdma_start_outgoing_migration(void *opaque,
 
     s->to_dst_file = qemu_fopen_rdma(rdma, "wb");
     migrate_fd_connect(s, NULL);
+    g_free(s->host_port);
     return;
 return_path_err:
     qemu_rdma_cleanup(rdma);
-- 
1.8.3.1

[PATCH v4 12/18] migration/rdma: Create the multifd send channels for RDMA

[Chuan Zheng]

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/multifd.c |  4 ++--
 migration/multifd.h |  2 ++
 migration/rdma.c    | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index ae0b7f0..919a414 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -176,7 +176,7 @@ void multifd_register_ops(int method, MultiFDMethods *ops)
     multifd_ops[method] = ops;
 }
 
-static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
+int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
 {
     MultiFDInit_t msg = {};
     int ret;
@@ -503,7 +503,7 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset)
     return 1;
 }
 
-static void multifd_send_terminate_threads(Error *err)
+void multifd_send_terminate_threads(Error *err)
 {
     int i;
 
diff --git a/migration/multifd.h b/migration/multifd.h
index b17a2c1..26d4489 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -184,6 +184,8 @@ typedef struct {
 #ifdef CONFIG_RDMA
 extern MultiFDSetup multifd_rdma_ops;
 #endif
+void multifd_send_terminate_threads(Error *err);
+int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp);
 int get_multifd_send_param(int id, MultiFDSendParams **param);
 int get_multifd_recv_param(int id, MultiFDRecvParams **param);
 MultiFDSetup *multifd_setup_ops_init(void);
diff --git a/migration/rdma.c b/migration/rdma.c
index 9654b87..cff9446 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -4261,9 +4261,54 @@ err:
     g_free(rdma_return_path);
 }
 
+static int multifd_channel_rdma_connect(void *opaque)
+{
+    MultiFDSendParams *p = opaque;
+    Error *local_err = NULL;
+    int ret = 0;
+    MigrationState *s = migrate_get_current();
+
+    p->rdma = qemu_rdma_data_init(s->host_port, &local_err);
+    if (p->rdma == NULL) {
+        goto out;
+    }
+
+    ret = qemu_rdma_source_init(p->rdma,
+                                migrate_rdma_pin_all(),
+                                &local_err);
+    if (ret) {
+        goto out;
+    }
+
+    ret = qemu_rdma_connect(p->rdma, &local_err);
+    if (ret) {
+        goto out;
+    }
+
+    p->file = qemu_fopen_rdma(p->rdma, "wb");
+    if (p->file == NULL) {
+        goto out;
+    }
+
+    p->c = QIO_CHANNEL(getQIOChannel(p->file));
+
+out:
+    if (local_err) {
+        trace_multifd_send_error(p->id);
+    }
+
+    return ret;
+}
+
 static void *multifd_rdma_send_thread(void *opaque)
 {
     MultiFDSendParams *p = opaque;
+    Error *local_err = NULL;
+
+    trace_multifd_send_thread_start(p->id);
+    if (multifd_send_initial_packet(p, &local_err) < 0) {
+        goto out;
+    }
 
     while (true) {
         WITH_QEMU_LOCK_GUARD(&p->mutex) {
@@ -4274,6 +4319,12 @@ static void *multifd_rdma_send_thread(void *opaque)
         qemu_sem_wait(&p->sem);
     }
 
+out:
+    if (local_err) {
+        trace_multifd_send_error(p->id);
+        multifd_send_terminate_threads(local_err);
+    }
+
     WITH_QEMU_LOCK_GUARD(&p->mutex) {
         p->running = false;
     }
@@ -4285,6 +4336,12 @@ static void multifd_rdma_send_channel_setup(MultiFDSendParams *p)
 {
     Error *local_err = NULL;
 
+    if (multifd_channel_rdma_connect(p)) {
+        error_setg(&local_err, "multifd: rdma channel %d not established",
+                   p->id);
+        return ;
+    }
+
     if (p->quit) {
         error_setg(&local_err, "multifd: send id %d already quit", p->id);
         return ;
-- 
1.8.3.1

[PATCH v4 13/18] migration/rdma: Add the function for dynamic page registration

[Chuan Zheng]

Add the 'qemu_rdma_registration' function, multifd send threads
call it to register memory.

Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/rdma.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/migration/rdma.c b/migration/rdma.c
index cff9446..1095a8f 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3739,6 +3739,57 @@ out:
     return ret;
 }
 
+/*
+ * Dynamic page registrations for multifd RDMA threads.
+ */
+static int qemu_rdma_registration(void *opaque)
+{
+    RDMAContext *rdma = opaque;
+    RDMAControlHeader resp = {.type = RDMA_CONTROL_RAM_BLOCKS_RESULT };
+    RDMALocalBlocks *local = &rdma->local_ram_blocks;
+    int reg_result_idx, i, nb_dest_blocks;
+    RDMAControlHeader head = { .len = 0, .repeat = 1 };
+    int ret = 0;
+
+    head.type = RDMA_CONTROL_RAM_BLOCKS_REQUEST;
+
+    ret = qemu_rdma_exchange_send(rdma, &head, NULL, &resp,
+            &reg_result_idx, rdma->pin_all ?
+            qemu_rdma_reg_whole_ram_blocks : NULL);
+    if (ret < 0) {
+        goto out;
+    }
+
+    nb_dest_blocks = resp.len / sizeof(RDMADestBlock);
+
+    if (local->nb_blocks != nb_dest_blocks) {
+        rdma->error_state = -EINVAL;
+        ret = -1;
+        goto out;
+    }
+
+    qemu_rdma_move_header(rdma, reg_result_idx, &resp);
+    memcpy(rdma->dest_blocks,
+           rdma->wr_data[reg_result_idx].control_curr, resp.len);
+
+    for (i = 0; i < nb_dest_blocks; i++) {
+        network_to_dest_block(&rdma->dest_blocks[i]);
+
+        /* We require that the blocks are in the same order */
+        if (rdma->dest_blocks[i].length != local->block[i].length) {
+            rdma->error_state = -EINVAL;
+            ret = -1;
+            goto out;
+        }
+        local->block[i].remote_host_addr =
+            rdma->dest_blocks[i].remote_host_addr;
+        local->block[i].remote_rkey = rdma->dest_blocks[i].remote_rkey;
+    }
+
+out:
+    return ret;
+}
+
 /* Destination:
  * Called via a ram_control_load_hook during the initial RAM load section which
  * lists the RAMBlocks by name.  This lets us know the order of the RAMBlocks
-- 
1.8.3.1

[PATCH v4 14/18] migration/rdma: register memory for multifd RDMA channels

[Chuan Zheng]

Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/multifd.c |  3 ++
 migration/rdma.c    | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 919a414..1186246 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -537,6 +537,9 @@ void multifd_send_terminate_threads(Error *err)
         qemu_mutex_lock(&p->mutex);
         p->quit = true;
         qemu_sem_post(&p->sem);
+        if (migrate_use_rdma()) {
+            qemu_sem_post(&p->sem_sync);
+        }
         qemu_mutex_unlock(&p->mutex);
     }
 }
diff --git a/migration/rdma.c b/migration/rdma.c
index 1095a8f..c906cc7 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3838,6 +3838,19 @@ static int rdma_load_hook(QEMUFile *f, void *opaque, uint64_t flags, void *data)
         return rdma_block_notification_handle(opaque, data);
 
     case RAM_CONTROL_HOOK:
+        if (migrate_use_multifd()) {
+            int i;
+            MultiFDRecvParams *multifd_recv_param = NULL;
+            int thread_count = migrate_multifd_channels();
+            /* Inform dest recv_thread to poll */
+            for (i = 0; i < thread_count; i++) {
+                if (get_multifd_recv_param(i, &multifd_recv_param)) {
+                    return -1;
+                }
+                qemu_sem_post(&multifd_recv_param->sem_sync);
+            }
+        }
+
         return qemu_rdma_registration_handle(f, opaque);
 
     default:
@@ -3910,6 +3923,24 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
         head.type = RDMA_CONTROL_RAM_BLOCKS_REQUEST;
         trace_qemu_rdma_registration_stop_ram();
 
+        if (migrate_use_multifd()) {
+            /*
+             * Inform the multifd channels to register memory
+             */
+            int i;
+            int thread_count = migrate_multifd_channels();
+            MultiFDSendParams *multifd_send_param = NULL;
+            for (i = 0; i < thread_count; i++) {
+                ret = get_multifd_send_param(i, &multifd_send_param);
+                if (ret) {
+                    error_report("rdma: error getting multifd(%d)", i);
+                    return ret;
+                }
+
+                qemu_sem_post(&multifd_send_param->sem_sync);
+            }
+        }
+
         /*
          * Make sure that we parallelize the pinning on both sides.
          * For very large guests, doing this serially takes a really
@@ -3968,6 +3999,21 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
                     rdma->dest_blocks[i].remote_host_addr;
             local->block[i].remote_rkey = rdma->dest_blocks[i].remote_rkey;
         }
+        /* Wait for all multifd channels to complete registration */
+        if (migrate_use_multifd()) {
+            int i;
+            int thread_count = migrate_multifd_channels();
+            MultiFDSendParams *multifd_send_param = NULL;
+            for (i = 0; i < thread_count; i++) {
+                ret = get_multifd_send_param(i, &multifd_send_param);
+                if (ret) {
+                    error_report("rdma: error getting multifd(%d)", i);
+                    return ret;
+                }
+
+                qemu_sem_wait(&multifd_send_param->sem);
+            }
+        }
     }
 
     trace_qemu_rdma_registration_stop(flags);
@@ -3979,6 +4025,24 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
         goto err;
     }
 
+    if (migrate_use_multifd()) {
+        /*
+         * Inform src send_thread to send FINISHED signal.
+         * Wait for multifd RDMA send threads to poll the CQE.
+         */
+        int i;
+        int thread_count = migrate_multifd_channels();
+        MultiFDSendParams *multifd_send_param = NULL;
+        for (i = 0; i < thread_count; i++) {
+            ret = get_multifd_send_param(i, &multifd_send_param);
+            if (ret < 0) {
+                goto err;
+            }
+
+            qemu_sem_post(&multifd_send_param->sem_sync);
+        }
+    }
+
     return 0;
 err:
     rdma->error_state = ret;
@@ -4355,19 +4419,37 @@ static void *multifd_rdma_send_thread(void *opaque)
 {
     MultiFDSendParams *p = opaque;
     Error *local_err = NULL;
+    int ret = 0;
+    RDMAControlHeader head = { .len = 0, .repeat = 1 };
 
     trace_multifd_send_thread_start(p->id);
     if (multifd_send_initial_packet(p, &local_err) < 0) {
         goto out;
     }
 
+    /* wait for semaphore notification to register memory */
+    qemu_sem_wait(&p->sem_sync);
+    if (qemu_rdma_registration(p->rdma) < 0) {
+        goto out;
+    }
+    /*
+     * Inform the main RDMA thread to run when multifd
+     * RDMA thread have completed registration.
+     */
+    qemu_sem_post(&p->sem);
     while (true) {
+        qemu_sem_wait(&p->sem_sync);
         WITH_QEMU_LOCK_GUARD(&p->mutex) {
             if (p->quit) {
                 break;
             }
         }
-        qemu_sem_wait(&p->sem);
+        /* Send FINISHED to the destination */
+        head.type = RDMA_CONTROL_REGISTER_FINISHED;
+        ret = qemu_rdma_exchange_send(p->rdma, &head, NULL, NULL, NULL, NULL);
+        if (ret < 0) {
+            return NULL;
+        }
     }
 
 out:
@@ -4406,14 +4488,20 @@ static void multifd_rdma_send_channel_setup(MultiFDSendParams *p)
 static void *multifd_rdma_recv_thread(void *opaque)
 {
     MultiFDRecvParams *p = opaque;
+    int ret = 0;
 
     while (true) {
+        qemu_sem_wait(&p->sem_sync);
         WITH_QEMU_LOCK_GUARD(&p->mutex) {
             if (p->quit) {
                 break;
             }
         }
-        qemu_sem_wait(&p->sem_sync);
+        ret = qemu_rdma_registration_handle(p->file, p->c);
+        if (ret < 0) {
+            qemu_file_set_error(p->file, ret);
+            break;
+        }
     }
 
     WITH_QEMU_LOCK_GUARD(&p->mutex) {
-- 
1.8.3.1

[PATCH v4 15/18] migration/rdma: only register the memory for multifd channels

[Chuan Zheng]

All data is sent by multifd Channels, so we only register its for
multifd channels and main channel don't register its.

Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/rdma.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/migration/rdma.c b/migration/rdma.c
index c906cc7..f5eb563 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3939,6 +3939,12 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
 
                 qemu_sem_post(&multifd_send_param->sem_sync);
             }
+
+            /*
+             * Use multifd to migrate, we only register memory for
+             * multifd RDMA channel and main channel don't register it.
+             */
+            goto wait_reg_complete;
         }
 
         /*
@@ -3999,6 +4005,8 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
                     rdma->dest_blocks[i].remote_host_addr;
             local->block[i].remote_rkey = rdma->dest_blocks[i].remote_rkey;
         }
+
+wait_reg_complete:
         /* Wait for all multifd channels to complete registration */
         if (migrate_use_multifd()) {
             int i;
-- 
1.8.3.1

[PATCH v4 16/18] migration/rdma: add rdma_channel into Migrationstate field

[Chuan Zheng]

Multifd RDMA is need to poll when we send data, record it.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/migration.c |  1 +
 migration/migration.h |  1 +
 migration/rdma.c      | 14 ++++++++++++++
 3 files changed, 16 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index b8f4844..47bd11d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1926,6 +1926,7 @@ void migrate_init(MigrationState *s)
     s->migration_thread_running = false;
     s->enabled_rdma_migration = false;
     s->host_port = NULL;
+    s->rdma_channel = 0;
     error_free(s->error);
     s->error = NULL;
     s->hostname = NULL;
diff --git a/migration/migration.h b/migration/migration.h
index 537ee09..5ff46e6 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -288,6 +288,7 @@ struct MigrationState {
 
     /* Need by Multi-RDMA */
     char *host_port;
+    int rdma_channel;
 };
 
 void migrate_set_state(int *state, int old_state, int new_state);
diff --git a/migration/rdma.c b/migration/rdma.c
index f5eb563..2097839 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -183,6 +183,20 @@ typedef struct {
 } RDMAWorkRequestData;
 
 /*
+ * Get the multifd RDMA channel used to send data.
+ */
+static int get_multifd_RDMA_channel(void)
+{
+    int thread_count = migrate_multifd_channels();
+    MigrationState *s = migrate_get_current();
+
+    s->rdma_channel++;
+    s->rdma_channel %= thread_count;
+
+    return s->rdma_channel;
+}
+
+/*
  * Negotiate RDMA capabilities during connection-setup time.
  */
 typedef struct {
-- 
1.8.3.1

[PATCH v4 17/18] migration/rdma: send data for both rdma-pin-all and NOT rdma-pin-all mode

[Chuan Zheng]

Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/rdma.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 2097839..c19a91f 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2002,6 +2002,20 @@ static int qemu_rdma_write_one(QEMUFile *f, RDMAContext *rdma,
                                .repeat = 1,
                              };
 
+    /* use multifd to send data */
+    if (migrate_use_multifd()) {
+        int channel = get_multifd_RDMA_channel();
+        int ret = 0;
+        MultiFDSendParams *multifd_send_param = NULL;
+        ret = get_multifd_send_param(channel, &multifd_send_param);
+        if (ret) {
+            error_report("rdma: error getting multifd_send_param(%d)", channel);
+            return -EINVAL;
+        }
+        rdma = (RDMAContext *)multifd_send_param->rdma;
+        block = &(rdma->local_ram_blocks.block[current_index]);
+    }
+
 retry:
     sge.addr = (uintptr_t)(block->local_host_addr +
                             (current_addr - block->offset));
@@ -2197,6 +2211,27 @@ retry:
     return 0;
 }
 
+static int multifd_rdma_write_flush(void)
+{
+    /* The multifd RDMA threads send data */
+    MultiFDSendParams *multifd_send_param = NULL;
+    RDMAContext *rdma = NULL;
+    MigrationState *s = migrate_get_current();
+    int ret = 0;
+
+    ret = get_multifd_send_param(s->rdma_channel,
+                                 &multifd_send_param);
+    if (ret) {
+        error_report("rdma: error getting multifd_send_param(%d)",
+                     s->rdma_channel);
+        return ret;
+    }
+    rdma = (RDMAContext *)(multifd_send_param->rdma);
+    rdma->nb_sent++;
+
+    return ret;
+}
+
 /*
  * Push out any unwritten RDMA operations.
  *
@@ -2219,8 +2254,15 @@ static int qemu_rdma_write_flush(QEMUFile *f, RDMAContext *rdma)
     }
 
     if (ret == 0) {
-        rdma->nb_sent++;
-        trace_qemu_rdma_write_flush(rdma->nb_sent);
+        if (migrate_use_multifd()) {
+            ret = multifd_rdma_write_flush();
+            if (ret) {
+                return ret;
+            }
+        } else {
+            rdma->nb_sent++;
+            trace_qemu_rdma_write_flush(rdma->nb_sent);
+        }
     }
 
     rdma->current_length = 0;
@@ -4062,6 +4104,7 @@ wait_reg_complete:
             }
 
             qemu_sem_post(&multifd_send_param->sem_sync);
+            qemu_sem_wait(&multifd_send_param->sem);
         }
     }
 
@@ -4443,6 +4486,7 @@ static void *multifd_rdma_send_thread(void *opaque)
     Error *local_err = NULL;
     int ret = 0;
     RDMAControlHeader head = { .len = 0, .repeat = 1 };
+    RDMAContext *rdma = p->rdma;
 
     trace_multifd_send_thread_start(p->id);
     if (multifd_send_initial_packet(p, &local_err) < 0) {
@@ -4451,7 +4495,7 @@ static void *multifd_rdma_send_thread(void *opaque)
 
     /* wait for semaphore notification to register memory */
     qemu_sem_wait(&p->sem_sync);
-    if (qemu_rdma_registration(p->rdma) < 0) {
+    if (qemu_rdma_registration(rdma) < 0) {
         goto out;
     }
     /*
@@ -4466,12 +4510,25 @@ static void *multifd_rdma_send_thread(void *opaque)
                 break;
             }
         }
+        /* To complete polling(CQE) */
+        while (rdma->nb_sent) {
+            ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
+            if (ret < 0) {
+                error_report("multifd RDMA migration: "
+                             "complete polling error!");
+                return NULL;
+            }
+        }
         /* Send FINISHED to the destination */
         head.type = RDMA_CONTROL_REGISTER_FINISHED;
-        ret = qemu_rdma_exchange_send(p->rdma, &head, NULL, NULL, NULL, NULL);
+        ret = qemu_rdma_exchange_send(rdma, &head, NULL, NULL, NULL, NULL);
         if (ret < 0) {
+            error_report("multifd RDMA migration: "
+                         "sending remote error!");
             return NULL;
         }
+        /* sync main thread */
+        qemu_sem_post(&p->sem);
     }
 
 out:
-- 
1.8.3.1

[PATCH v4 18/18] migration/rdma: RDMA cleanup for multifd migration

[Chuan Zheng]

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/multifd.c |  6 ++++++
 migration/multifd.h |  1 +
 migration/rdma.c    | 16 +++++++++++++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 1186246..4031648 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -577,6 +577,9 @@ void multifd_save_cleanup(void)
         p->packet_len = 0;
         g_free(p->packet);
         p->packet = NULL;
+#ifdef CONFIG_RDMA
+        multifd_rdma_cleanup(p->rdma);
+#endif
         multifd_send_state->ops->send_cleanup(p, &local_err);
         if (local_err) {
             migrate_set_error(migrate_get_current(), local_err);
@@ -1039,6 +1042,9 @@ int multifd_load_cleanup(Error **errp)
         p->packet_len = 0;
         g_free(p->packet);
         p->packet = NULL;
+#ifdef CONFIG_RDMA
+        multifd_rdma_cleanup(p->rdma);
+#endif
         multifd_recv_state->ops->recv_cleanup(p);
     }
     qemu_sem_destroy(&multifd_recv_state->sem_sync);
diff --git a/migration/multifd.h b/migration/multifd.h
index 26d4489..0ecec5e 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -183,6 +183,7 @@ typedef struct {
 
 #ifdef CONFIG_RDMA
 extern MultiFDSetup multifd_rdma_ops;
+void multifd_rdma_cleanup(void *opaque);
 #endif
 void multifd_send_terminate_threads(Error *err);
 int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp);
diff --git a/migration/rdma.c b/migration/rdma.c
index c19a91f..f14357f 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2369,7 +2369,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
 {
     int idx;
 
-    if (rdma->cm_id && rdma->connected) {
+    if (rdma->channel && rdma->cm_id && rdma->connected) {
         if ((rdma->error_state ||
              migrate_get_current()->state == MIGRATION_STATUS_CANCELLING) &&
             !rdma->received_error) {
@@ -4599,6 +4599,20 @@ static void multifd_rdma_recv_channel_setup(QIOChannel *ioc,
     return;
 }
 
+void multifd_rdma_cleanup(void *opaque)
+{
+    RDMAContext *rdma = (RDMAContext *)opaque;
+
+    if (!migrate_use_rdma()) {
+        return;
+    }
+
+    rdma->listen_id = NULL;
+    rdma->channel = NULL;
+    qemu_rdma_cleanup(rdma);
+    g_free(rdma);
+}
+
 MultiFDSetup multifd_rdma_ops = {
     .send_thread = multifd_rdma_send_thread,
     .recv_thread = multifd_rdma_recv_thread,
-- 
1.8.3.1

Re: [PATCH v4 02/18] migration/rdma: judge whether or not the RDMA is used for migration

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Add enabled_rdma_migration into MigrationState to judge
> whether or not the RDMA is used for migration.
> 
> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>

I'd rather see a separate flag added to each of the MigrationState and
MigrationIncomingState separately for outoging and incoming migration.

It's also probably better to call it 'is_rdma_migration' rather than
enabled.

Dave

> ---
>  migration/migration.c | 13 +++++++++++++
>  migration/migration.h |  6 ++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 447dfb9..129c81a 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -418,11 +418,13 @@ void migrate_add_address(SocketAddress *address)
>  static void qemu_start_incoming_migration(const char *uri, Error **errp)
>  {
>      const char *p = NULL;
> +    MigrationState *s = migrate_get_current();
>  
>      if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
>          return;
>      }
>  
> +    s->enabled_rdma_migration = false;
>      qapi_event_send_migration(MIGRATION_STATUS_SETUP);
>      if (strstart(uri, "tcp:", &p) ||
>          strstart(uri, "unix:", NULL) ||
> @@ -430,6 +432,7 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
>          socket_start_incoming_migration(p ? p : uri, errp);
>  #ifdef CONFIG_RDMA
>      } else if (strstart(uri, "rdma:", &p)) {
> +        s->enabled_rdma_migration = true;
>          rdma_start_incoming_migration(p, errp);
>  #endif
>      } else if (strstart(uri, "exec:", &p)) {
> @@ -1921,6 +1924,7 @@ void migrate_init(MigrationState *s)
>      s->start_postcopy = false;
>      s->postcopy_after_devices = false;
>      s->migration_thread_running = false;
> +    s->enabled_rdma_migration = false;
>      error_free(s->error);
>      s->error = NULL;
>      s->hostname = NULL;
> @@ -2162,6 +2166,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>          socket_start_outgoing_migration(s, p ? p : uri, &local_err);
>  #ifdef CONFIG_RDMA
>      } else if (strstart(uri, "rdma:", &p)) {
> +        s->enabled_rdma_migration = true;
>          rdma_start_outgoing_migration(s, p, &local_err);
>  #endif
>      } else if (strstart(uri, "exec:", &p)) {
> @@ -2391,6 +2396,14 @@ bool migrate_rdma_pin_all(void)
>      return s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL];
>  }
>  
> +bool migrate_use_rdma(void)
> +{
> +    MigrationState *s;
> +    s = migrate_get_current();
> +
> +    return s->enabled_rdma_migration;
> +}
> +
>  bool migrate_use_multifd(void)
>  {
>      MigrationState *s;
> diff --git a/migration/migration.h b/migration/migration.h
> index 22b36f3..da5681b 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -280,6 +280,11 @@ struct MigrationState {
>       * This save hostname when out-going migration starts
>       */
>      char *hostname;
> +
> +    /*
> +     * Enable RDMA migration
> +     */
> +    bool enabled_rdma_migration;
>  };
>  
>  void migrate_set_state(int *state, int old_state, int new_state);
> @@ -317,6 +322,7 @@ bool migrate_validate_uuid(void);
>  
>  bool migrate_auto_converge(void);
>  bool migrate_rdma_pin_all(void);
> +bool migrate_use_rdma(void);
>  bool migrate_use_multifd(void);
>  bool migrate_pause_before_switchover(void);
>  int migrate_multifd_channels(void);
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 04/18] migration/rdma: add multifd_setup_ops for rdma

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/multifd.c |  6 +++++
>  migration/multifd.h |  5 ++++
>  migration/rdma.c    | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 82 insertions(+)

I think that's OK, although will need minor changes with my suggested
change to 'migrate_use' in the previous patch.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index cb1fc01..4820702 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -1232,6 +1232,12 @@ MultiFDSetup *multifd_setup_ops_init(void)
>  {
>      MultiFDSetup *ops;
>  
> +#ifdef CONFIG_RDMA
> +    if (migrate_use_rdma()) {
> +        ops = &multifd_rdma_ops;
> +        return ops;
> +    }
> +#endif
>      ops = &multifd_socket_ops;
>      return ops;
>  }
> diff --git a/migration/multifd.h b/migration/multifd.h
> index 1d2dc90..e3ab4b0 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -173,6 +173,11 @@ typedef struct {
>      void (*recv_channel_setup)(QIOChannel *ioc, MultiFDRecvParams *p);
>  } MultiFDSetup;
>  
> +#ifdef CONFIG_RDMA
> +extern MultiFDSetup multifd_rdma_ops;
> +#endif
> +MultiFDSetup *multifd_setup_ops_init(void);
> +
>  void multifd_register_ops(int method, MultiFDMethods *ops);
>  
>  #endif
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 00eac34..e0ea86d 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -19,6 +19,7 @@
>  #include "qemu/cutils.h"
>  #include "rdma.h"
>  #include "migration.h"
> +#include "multifd.h"
>  #include "qemu-file.h"
>  #include "ram.h"
>  #include "qemu-file-channel.h"
> @@ -4139,3 +4140,73 @@ err:
>      g_free(rdma);
>      g_free(rdma_return_path);
>  }
> +
> +static void *multifd_rdma_send_thread(void *opaque)
> +{
> +    MultiFDSendParams *p = opaque;
> +
> +    while (true) {
> +        WITH_QEMU_LOCK_GUARD(&p->mutex) {
> +            if (p->quit) {
> +                break;
> +            }
> +        }
> +        qemu_sem_wait(&p->sem);
> +    }
> +
> +    WITH_QEMU_LOCK_GUARD(&p->mutex) {
> +        p->running = false;
> +    }
> +
> +    return NULL;
> +}
> +
> +static void multifd_rdma_send_channel_setup(MultiFDSendParams *p)
> +{
> +    Error *local_err = NULL;
> +
> +    if (p->quit) {
> +        error_setg(&local_err, "multifd: send id %d already quit", p->id);
> +        return ;
> +    }
> +    p->running = true;
> +
> +    qemu_thread_create(&p->thread, p->name, multifd_rdma_send_thread, p,
> +                       QEMU_THREAD_JOINABLE);
> +}
> +
> +static void *multifd_rdma_recv_thread(void *opaque)
> +{
> +    MultiFDRecvParams *p = opaque;
> +
> +    while (true) {
> +        WITH_QEMU_LOCK_GUARD(&p->mutex) {
> +            if (p->quit) {
> +                break;
> +            }
> +        }
> +        qemu_sem_wait(&p->sem_sync);
> +    }
> +
> +    WITH_QEMU_LOCK_GUARD(&p->mutex) {
> +        p->running = false;
> +    }
> +
> +    return NULL;
> +}
> +
> +static void multifd_rdma_recv_channel_setup(QIOChannel *ioc,
> +                                            MultiFDRecvParams *p)
> +{
> +    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
> +
> +    p->file = rioc->file;
> +    return;
> +}
> +
> +MultiFDSetup multifd_rdma_ops = {
> +    .send_thread = multifd_rdma_send_thread,
> +    .recv_thread = multifd_rdma_recv_thread,
> +    .send_channel_setup = multifd_rdma_send_channel_setup,
> +    .recv_channel_setup = multifd_rdma_recv_channel_setup
> +};
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 05/18] migration/rdma: do not need sync main for rdma

[Dr. David Alan Gilbert]

This patch needs to explain why the sync isn't needed for RDMA.

Dave

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/multifd.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 4820702..5d34950 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -583,6 +583,10 @@ void multifd_send_sync_main(QEMUFile *f)
>      if (!migrate_use_multifd()) {
>          return;
>      }
> +     /* Do not need sync for rdma */
> +    if (migrate_use_rdma()) {
> +        return;
> +    }
>      if (multifd_send_state->pages->used) {
>          if (multifd_send_pages(f) < 0) {
>              error_report("%s: multifd_send_pages fail", __func__);
> @@ -1024,6 +1028,10 @@ void multifd_recv_sync_main(void)
>      if (!migrate_use_multifd()) {
>          return;
>      }
> +    /* Do not need sync for rdma */
> +    if (migrate_use_rdma()) {
> +        return;
> +    }
>      for (i = 0; i < migrate_multifd_channels(); i++) {
>          MultiFDRecvParams *p = &multifd_recv_state->params[i];
>  
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 06/18] migration/rdma: export MultiFDSendParams/MultiFDRecvParams

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> MultiFDSendParams and MultiFDRecvParams is need for rdma, export it
> 
> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>

I think these become simpler if you just return a NULL on error,
also I think you can make 'id' unsigned and then you don't have
to worry about it being negative.

Also, please make it start with multifd_ so we know where it's coming
from; so:

MultiFDSendParams *multifd_send_param_get(unsigned channel);

Dave

> ---
>  migration/multifd.c | 26 ++++++++++++++++++++++++++
>  migration/multifd.h |  2 ++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 5d34950..ae0b7f0 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -390,6 +390,19 @@ struct {
>      MultiFDSetup *setup_ops;
>  } *multifd_send_state;
>  
> +int get_multifd_send_param(int id, MultiFDSendParams **param)
> +{
> +    int ret = 0;
> +
> +    if (id < 0 || id >= migrate_multifd_channels()) {
> +        ret = -1;
> +    } else {
> +        *param = &(multifd_send_state->params[id]);
> +    }
> +
> +    return ret;
> +}
> +
>  /*
>   * How we use multifd_send_state->pages and channel->pages?
>   *
> @@ -934,6 +947,19 @@ struct {
>      MultiFDSetup *setup_ops;
>  } *multifd_recv_state;
>  
> +int get_multifd_recv_param(int id, MultiFDRecvParams **param)
> +{
> +    int ret = 0;
> +
> +    if (id < 0 || id >= migrate_multifd_channels()) {
> +        ret = -1;
> +    } else {
> +        *param = &(multifd_recv_state->params[id]);
> +    }
> +
> +    return ret;
> +}
> +
>  static void multifd_recv_terminate_threads(Error *err)
>  {
>      int i;
> diff --git a/migration/multifd.h b/migration/multifd.h
> index e3ab4b0..d57756c 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -176,6 +176,8 @@ typedef struct {
>  #ifdef CONFIG_RDMA
>  extern MultiFDSetup multifd_rdma_ops;
>  #endif
> +int get_multifd_send_param(int id, MultiFDSendParams **param);
> +int get_multifd_recv_param(int id, MultiFDRecvParams **param);
>  MultiFDSetup *multifd_setup_ops_init(void);
>  
>  void multifd_register_ops(int method, MultiFDMethods *ops);
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 07/18] migration/rdma: add rdma field into multifd send/recv param

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Note we do want to export any rdma struct, take void * instead.

You don't need to make this a void *; add a typedef struct RDMAContext
into include/qemu/typedefs.h  and then you can use the right type here
without actually exporting the interesting contents of the type or
being dependent on rdma being built.

Dave

> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/multifd.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/migration/multifd.h b/migration/multifd.h
> index d57756c..b17a2c1 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -108,6 +108,10 @@ typedef struct {
>      QemuSemaphore sem_sync;
>      /* used for compression methods */
>      void *data;
> +    /* used for multifd rdma */
> +    void *rdma;
> +    /* communication channel */
> +    QEMUFile *file;
>  }  MultiFDSendParams;
>  
>  typedef struct {
> @@ -147,6 +151,10 @@ typedef struct {
>      QemuSemaphore sem_sync;
>      /* used for de-compression methods */
>      void *data;
> +    /* used for multifd rdma */
> +    void *rdma;
> +    /* communication channel */
> +    QEMUFile *file;
>  } MultiFDRecvParams;
>  
>  typedef struct {
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 08/18] migration/rdma: export getQIOChannel to get QIOchannel in rdma

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/qemu-file.c | 5 +++++
>  migration/qemu-file.h | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index be21518..37f6201 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -260,6 +260,11 @@ void ram_control_before_iterate(QEMUFile *f, uint64_t flags)
>      }
>  }
>  
> +void *getQIOChannel(QEMUFile *f)
> +{
> +    return f->opaque;
> +}
> +

Unfortunately that's not right, since the opaque isn't always a
QUIChannel, so getOpaque would be a suitable name here.

It's a shame this is needed; I'm surprised you ever have a QEMUFIle* in
the rdma code in somewhere you don't have the QIOChannel; could you
avoid this by adding a QIOChannel pointer into the RDAMContext to point
back to the channel which it's for?

Dave

>  void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
>  {
>      int ret = 0;
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index a9b6d6c..4cef043 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -165,6 +165,7 @@ void qemu_file_set_blocking(QEMUFile *f, bool block);
>  void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
>  void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
>  void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data);
> +void *getQIOChannel(QEMUFile *f);
>  
>  /* Whenever this is found in the data stream, the flags
>   * will be passed to ram_control_load_hook in the incoming-migration
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 10/18] migration/rdma: Create the multifd recv channels for RDMA

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> We still don't transmit anything through them, and we only build
> the RDMA connections.
> 
> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/rdma.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 67 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 996afb0..ed8a015 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3267,6 +3267,40 @@ static void rdma_cm_poll_handler(void *opaque)
>      }
>  }
>  
> +static bool qemu_rdma_accept_setup(RDMAContext *rdma)
> +{
> +    RDMAContext *multifd_rdma = NULL;
> +    int thread_count;
> +    int i;
> +    MultiFDRecvParams *multifd_recv_param;
> +    thread_count = migrate_multifd_channels();
> +    /* create the multifd channels for RDMA */
> +    for (i = 0; i < thread_count; i++) {
> +        if (get_multifd_recv_param(i, &multifd_recv_param) < 0) {
> +            error_report("rdma: error getting multifd_recv_param(%d)", i);
> +            return false;
> +        }
> +
> +        multifd_rdma = (RDMAContext *) multifd_recv_param->rdma;
> +        if (multifd_rdma->cm_id == NULL) {
> +            break;
> +        } else {
> +            multifd_rdma = NULL;
> +        }

I'm confused by what this if is doing - what are the two cases?

> +    }
> +
> +    if (multifd_rdma) {
> +        qemu_set_fd_handler(rdma->channel->fd,
> +                            rdma_accept_incoming_migration,
> +                            NULL, (void *)(intptr_t)multifd_rdma);
> +    } else {
> +        qemu_set_fd_handler(rdma->channel->fd, rdma_cm_poll_handler,
> +                            NULL, rdma);
> +    }
> +
> +    return true;
> +}
> +
>  static int qemu_rdma_accept(RDMAContext *rdma)
>  {
>      RDMACapabilities cap;
> @@ -3366,6 +3400,10 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>          qemu_set_fd_handler(rdma->channel->fd, rdma_accept_incoming_migration,
>                              NULL,
>                              (void *)(intptr_t)rdma->return_path);
> +    } else if (migrate_use_multifd()) {
> +        if (!qemu_rdma_accept_setup(rdma)) {
> +            goto err_rdma_dest_wait;
> +        }
>      } else {
>          qemu_set_fd_handler(rdma->channel->fd, rdma_cm_poll_handler,
>                              NULL, rdma);
> @@ -3976,6 +4014,34 @@ static QEMUFile *qemu_fopen_rdma(RDMAContext *rdma, const char *mode)
>      return rioc->file;
>  }
>  
> +static void migration_rdma_process_incoming(QEMUFile *f,
> +                                            RDMAContext *rdma, Error **errp)
> +{
> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +    QIOChannel *ioc = NULL;
> +    bool start_migration = false;
> +
> +    if (!migrate_use_multifd()) {
> +        rdma->migration_started_on_destination = 1;
> +        migration_fd_process_incoming(f, errp);
> +        return;
> +    }
> +
> +    if (!mis->from_src_file) {
> +        mis->from_src_file = f;
> +        qemu_file_set_blocking(f, false);
> +    } else {
> +        ioc = QIO_CHANNEL(getQIOChannel(f));
> +        /* Multiple connections */
> +        assert(migrate_use_multifd());

Are you sure that's never triggerable by something trying to connect
badly? If it was it would be better to error than abort.

> +        start_migration = multifd_recv_new_channel(ioc, errp);

And what does 'start_migration' mean here - is that meaning that we have
a full set of connections?

Dave

> +    }
> +
> +    if (start_migration) {
> +        migration_incoming_process();
> +    }
> +}
> +
>  static void rdma_accept_incoming_migration(void *opaque)
>  {
>      RDMAContext *rdma = opaque;
> @@ -4004,8 +4070,7 @@ static void rdma_accept_incoming_migration(void *opaque)
>          return;
>      }
>  
> -    rdma->migration_started_on_destination = 1;
> -    migration_fd_process_incoming(f, &local_err);
> +    migration_rdma_process_incoming(f, rdma, &local_err);
>      if (local_err) {
>          error_reportf_err(local_err, "RDMA ERROR:");
>      }
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 11/18] migration/rdma: record host_port for multifd RDMA

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/migration.c | 1 +
>  migration/migration.h | 3 +++
>  migration/rdma.c      | 3 +++
>  3 files changed, 7 insertions(+)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 129c81a..b8f4844 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1925,6 +1925,7 @@ void migrate_init(MigrationState *s)
>      s->postcopy_after_devices = false;
>      s->migration_thread_running = false;
>      s->enabled_rdma_migration = false;
> +    s->host_port = NULL;
>      error_free(s->error);
>      s->error = NULL;
>      s->hostname = NULL;
> diff --git a/migration/migration.h b/migration/migration.h
> index da5681b..537ee09 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -285,6 +285,9 @@ struct MigrationState {
>       * Enable RDMA migration
>       */
>      bool enabled_rdma_migration;
> +
> +    /* Need by Multi-RDMA */
> +    char *host_port;

Please keep that next to the char *hostname, since they go together.
Also, 'Needed'

Dave

>  };
>  
>  void migrate_set_state(int *state, int old_state, int new_state);
> diff --git a/migration/rdma.c b/migration/rdma.c
> index ed8a015..9654b87 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -4206,6 +4206,8 @@ void rdma_start_outgoing_migration(void *opaque,
>          goto err;
>      }
>  
> +    s->host_port = g_strdup(host_port);
> +
>      ret = qemu_rdma_source_init(rdma,
>          s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL], errp);
>  
> @@ -4250,6 +4252,7 @@ void rdma_start_outgoing_migration(void *opaque,
>  
>      s->to_dst_file = qemu_fopen_rdma(rdma, "wb");
>      migrate_fd_connect(s, NULL);
> +    g_free(s->host_port);
>      return;
>  return_path_err:
>      qemu_rdma_cleanup(rdma);
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 12/18] migration/rdma: Create the multifd send channels for RDMA

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/multifd.c |  4 ++--
>  migration/multifd.h |  2 ++
>  migration/rdma.c    | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 61 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index ae0b7f0..919a414 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -176,7 +176,7 @@ void multifd_register_ops(int method, MultiFDMethods *ops)
>      multifd_ops[method] = ops;
>  }
>  
> -static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
> +int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
>  {
>      MultiFDInit_t msg = {};
>      int ret;
> @@ -503,7 +503,7 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset)
>      return 1;
>  }
>  
> -static void multifd_send_terminate_threads(Error *err)
> +void multifd_send_terminate_threads(Error *err)
>  {
>      int i;
>  
> diff --git a/migration/multifd.h b/migration/multifd.h
> index b17a2c1..26d4489 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -184,6 +184,8 @@ typedef struct {
>  #ifdef CONFIG_RDMA
>  extern MultiFDSetup multifd_rdma_ops;
>  #endif
> +void multifd_send_terminate_threads(Error *err);
> +int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp);
>  int get_multifd_send_param(int id, MultiFDSendParams **param);
>  int get_multifd_recv_param(int id, MultiFDRecvParams **param);
>  MultiFDSetup *multifd_setup_ops_init(void);
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 9654b87..cff9446 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -4261,9 +4261,54 @@ err:
>      g_free(rdma_return_path);
>  }
>  
> +static int multifd_channel_rdma_connect(void *opaque)
> +{
> +    MultiFDSendParams *p = opaque;
> +    Error *local_err = NULL;
> +    int ret = 0;
> +    MigrationState *s = migrate_get_current();
> +
> +    p->rdma = qemu_rdma_data_init(s->host_port, &local_err);
> +    if (p->rdma == NULL) {
> +        goto out;
> +    }
> +
> +    ret = qemu_rdma_source_init(p->rdma,
> +                                migrate_rdma_pin_all(),
> +                                &local_err);
> +    if (ret) {
> +        goto out;
> +    }
> +
> +    ret = qemu_rdma_connect(p->rdma, &local_err);
> +    if (ret) {
> +        goto out;
> +    }
> +
> +    p->file = qemu_fopen_rdma(p->rdma, "wb");
> +    if (p->file == NULL) {
> +        goto out;
> +    }
> +
> +    p->c = QIO_CHANNEL(getQIOChannel(p->file));
> +
> +out:
> +    if (local_err) {
> +        trace_multifd_send_error(p->id);
> +    }

If any of the previous steps have failed, but not the first step,
what cleans up?

> +    return ret;
> +}
> +
>  static void *multifd_rdma_send_thread(void *opaque)
>  {
>      MultiFDSendParams *p = opaque;
> +    Error *local_err = NULL;
> +
> +    trace_multifd_send_thread_start(p->id);
> +    if (multifd_send_initial_packet(p, &local_err) < 0) {
> +        goto out;
> +    }
>  
>      while (true) {
>          WITH_QEMU_LOCK_GUARD(&p->mutex) {
> @@ -4274,6 +4319,12 @@ static void *multifd_rdma_send_thread(void *opaque)
>          qemu_sem_wait(&p->sem);
>      }
>  
> +out:
> +    if (local_err) {
> +        trace_multifd_send_error(p->id);
> +        multifd_send_terminate_threads(local_err);
> +    }
> +
>      WITH_QEMU_LOCK_GUARD(&p->mutex) {
>          p->running = false;
>      }
> @@ -4285,6 +4336,12 @@ static void multifd_rdma_send_channel_setup(MultiFDSendParams *p)
>  {
>      Error *local_err = NULL;
>  
> +    if (multifd_channel_rdma_connect(p)) {
> +        error_setg(&local_err, "multifd: rdma channel %d not established",
> +                   p->id);
> +        return ;
> +    }
> +
>      if (p->quit) {
>          error_setg(&local_err, "multifd: send id %d already quit", p->id);
>          return ;
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 13/18] migration/rdma: Add the function for dynamic page registration

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Add the 'qemu_rdma_registration' function, multifd send threads
> call it to register memory.

This function is a copy of the code out of qemu_rdma_registration_stop;
with some of the comments removed.
It's OK to split this code out so you can use it as well; but then why
not make qemu_rdma_registration_stop use this function as well to stop
having two copies ?  And keep the comments!

> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/rdma.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index cff9446..1095a8f 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3739,6 +3739,57 @@ out:
>      return ret;
>  }
>  
> +/*
> + * Dynamic page registrations for multifd RDMA threads.
> + */
> +static int qemu_rdma_registration(void *opaque)
> +{
> +    RDMAContext *rdma = opaque;

Can't you keep that as qemu_rdma_registration(RDMAContext *rdma) ?

> +    RDMAControlHeader resp = {.type = RDMA_CONTROL_RAM_BLOCKS_RESULT };
> +    RDMALocalBlocks *local = &rdma->local_ram_blocks;
> +    int reg_result_idx, i, nb_dest_blocks;
> +    RDMAControlHeader head = { .len = 0, .repeat = 1 };
> +    int ret = 0;
> +
> +    head.type = RDMA_CONTROL_RAM_BLOCKS_REQUEST;
> +
> +    ret = qemu_rdma_exchange_send(rdma, &head, NULL, &resp,
> +            &reg_result_idx, rdma->pin_all ?
> +            qemu_rdma_reg_whole_ram_blocks : NULL);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    nb_dest_blocks = resp.len / sizeof(RDMADestBlock);
> +
> +    if (local->nb_blocks != nb_dest_blocks) {
> +        rdma->error_state = -EINVAL;
> +        ret = -1;
> +        goto out;
> +    }
> +
> +    qemu_rdma_move_header(rdma, reg_result_idx, &resp);
> +    memcpy(rdma->dest_blocks,
> +           rdma->wr_data[reg_result_idx].control_curr, resp.len);
> +
> +    for (i = 0; i < nb_dest_blocks; i++) {
> +        network_to_dest_block(&rdma->dest_blocks[i]);
> +
> +        /* We require that the blocks are in the same order */
> +        if (rdma->dest_blocks[i].length != local->block[i].length) {
> +            rdma->error_state = -EINVAL;
> +            ret = -1;
> +            goto out;
> +        }
> +        local->block[i].remote_host_addr =
> +            rdma->dest_blocks[i].remote_host_addr;
> +        local->block[i].remote_rkey = rdma->dest_blocks[i].remote_rkey;
> +    }
> +
> +out:
> +    return ret;
> +}
> +
>  /* Destination:
>   * Called via a ram_control_load_hook during the initial RAM load section which
>   * lists the RAMBlocks by name.  This lets us know the order of the RAMBlocks
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 14/18] migration/rdma: register memory for multifd RDMA channels

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>

This could do with a description in the commit message of the sequence;
I think you're waiting for the semaphore; doing the registratin, then
waiting again to say that everyone has finished???

> ---
>  migration/multifd.c |  3 ++
>  migration/rdma.c    | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 93 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 919a414..1186246 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -537,6 +537,9 @@ void multifd_send_terminate_threads(Error *err)
>          qemu_mutex_lock(&p->mutex);
>          p->quit = true;
>          qemu_sem_post(&p->sem);
> +        if (migrate_use_rdma()) {
> +            qemu_sem_post(&p->sem_sync);
> +        }
>          qemu_mutex_unlock(&p->mutex);
>      }
>  }
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 1095a8f..c906cc7 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3838,6 +3838,19 @@ static int rdma_load_hook(QEMUFile *f, void *opaque, uint64_t flags, void *data)
>          return rdma_block_notification_handle(opaque, data);
>  
>      case RAM_CONTROL_HOOK:
> +        if (migrate_use_multifd()) {
> +            int i;
> +            MultiFDRecvParams *multifd_recv_param = NULL;
> +            int thread_count = migrate_multifd_channels();
> +            /* Inform dest recv_thread to poll */
> +            for (i = 0; i < thread_count; i++) {
> +                if (get_multifd_recv_param(i, &multifd_recv_param)) {
> +                    return -1;
> +                }
> +                qemu_sem_post(&multifd_recv_param->sem_sync);
> +            }
> +        }
> +
>          return qemu_rdma_registration_handle(f, opaque);
>  
>      default:
> @@ -3910,6 +3923,24 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>          head.type = RDMA_CONTROL_RAM_BLOCKS_REQUEST;
>          trace_qemu_rdma_registration_stop_ram();
>  
> +        if (migrate_use_multifd()) {
> +            /*
> +             * Inform the multifd channels to register memory
> +             */
> +            int i;
> +            int thread_count = migrate_multifd_channels();
> +            MultiFDSendParams *multifd_send_param = NULL;
> +            for (i = 0; i < thread_count; i++) {
> +                ret = get_multifd_send_param(i, &multifd_send_param);
> +                if (ret) {
> +                    error_report("rdma: error getting multifd(%d)", i);
> +                    return ret;
> +                }
> +
> +                qemu_sem_post(&multifd_send_param->sem_sync);
> +            }
> +        }
> +
>          /*
>           * Make sure that we parallelize the pinning on both sides.
>           * For very large guests, doing this serially takes a really
> @@ -3968,6 +3999,21 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>                      rdma->dest_blocks[i].remote_host_addr;
>              local->block[i].remote_rkey = rdma->dest_blocks[i].remote_rkey;
>          }
> +        /* Wait for all multifd channels to complete registration */
> +        if (migrate_use_multifd()) {
> +            int i;
> +            int thread_count = migrate_multifd_channels();
> +            MultiFDSendParams *multifd_send_param = NULL;
> +            for (i = 0; i < thread_count; i++) {
> +                ret = get_multifd_send_param(i, &multifd_send_param);
> +                if (ret) {
> +                    error_report("rdma: error getting multifd(%d)", i);
> +                    return ret;
> +                }
> +
> +                qemu_sem_wait(&multifd_send_param->sem);
> +            }
> +        }
>      }
>  
>      trace_qemu_rdma_registration_stop(flags);
> @@ -3979,6 +4025,24 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>          goto err;
>      }
>  
> +    if (migrate_use_multifd()) {
> +        /*
> +         * Inform src send_thread to send FINISHED signal.
> +         * Wait for multifd RDMA send threads to poll the CQE.
> +         */
> +        int i;
> +        int thread_count = migrate_multifd_channels();
> +        MultiFDSendParams *multifd_send_param = NULL;
> +        for (i = 0; i < thread_count; i++) {
> +            ret = get_multifd_send_param(i, &multifd_send_param);
> +            if (ret < 0) {
> +                goto err;
> +            }
> +
> +            qemu_sem_post(&multifd_send_param->sem_sync);
> +        }
> +    }
> +
>      return 0;
>  err:
>      rdma->error_state = ret;
> @@ -4355,19 +4419,37 @@ static void *multifd_rdma_send_thread(void *opaque)
>  {
>      MultiFDSendParams *p = opaque;
>      Error *local_err = NULL;
> +    int ret = 0;
> +    RDMAControlHeader head = { .len = 0, .repeat = 1 };
>  
>      trace_multifd_send_thread_start(p->id);
>      if (multifd_send_initial_packet(p, &local_err) < 0) {
>          goto out;
>      }
>  
> +    /* wait for semaphore notification to register memory */
> +    qemu_sem_wait(&p->sem_sync);
> +    if (qemu_rdma_registration(p->rdma) < 0) {
> +        goto out;
> +    }
> +    /*
> +     * Inform the main RDMA thread to run when multifd
> +     * RDMA thread have completed registration.
> +     */
> +    qemu_sem_post(&p->sem);
>      while (true) {
> +        qemu_sem_wait(&p->sem_sync);
>          WITH_QEMU_LOCK_GUARD(&p->mutex) {
>              if (p->quit) {
>                  break;
>              }
>          }
> -        qemu_sem_wait(&p->sem);

Is this something where you put the line in just a few patches earlier -
if so, put it in the right place in the original patch.

Dave

> +        /* Send FINISHED to the destination */
> +        head.type = RDMA_CONTROL_REGISTER_FINISHED;
> +        ret = qemu_rdma_exchange_send(p->rdma, &head, NULL, NULL, NULL, NULL);
> +        if (ret < 0) {
> +            return NULL;
> +        }
>      }
>  
>  out:
> @@ -4406,14 +4488,20 @@ static void multifd_rdma_send_channel_setup(MultiFDSendParams *p)
>  static void *multifd_rdma_recv_thread(void *opaque)
>  {
>      MultiFDRecvParams *p = opaque;
> +    int ret = 0;
>  
>      while (true) {
> +        qemu_sem_wait(&p->sem_sync);
>          WITH_QEMU_LOCK_GUARD(&p->mutex) {
>              if (p->quit) {
>                  break;
>              }
>          }
> -        qemu_sem_wait(&p->sem_sync);
> +        ret = qemu_rdma_registration_handle(p->file, p->c);
> +        if (ret < 0) {
> +            qemu_file_set_error(p->file, ret);
> +            break;
> +        }
>      }
>  
>      WITH_QEMU_LOCK_GUARD(&p->mutex) {
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 16/18] migration/rdma: add rdma_channel into Migrationstate field

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Multifd RDMA is need to poll when we send data, record it.

This looks like it's trying to be the equivalent of the 'static int
next_channel' in multifd_send_pages.

If so, why not mkae this 'multifd_channel' and make the function
'multifd_next_channel' and replace the code in multifd_send_pages to use
this as well, rather than make it a special for rdma.

Dave

> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/migration.c |  1 +
>  migration/migration.h |  1 +
>  migration/rdma.c      | 14 ++++++++++++++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index b8f4844..47bd11d 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1926,6 +1926,7 @@ void migrate_init(MigrationState *s)
>      s->migration_thread_running = false;
>      s->enabled_rdma_migration = false;
>      s->host_port = NULL;
> +    s->rdma_channel = 0;
>      error_free(s->error);
>      s->error = NULL;
>      s->hostname = NULL;
> diff --git a/migration/migration.h b/migration/migration.h
> index 537ee09..5ff46e6 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -288,6 +288,7 @@ struct MigrationState {
>  
>      /* Need by Multi-RDMA */
>      char *host_port;
> +    int rdma_channel;
>  };
>  
>  void migrate_set_state(int *state, int old_state, int new_state);
> diff --git a/migration/rdma.c b/migration/rdma.c
> index f5eb563..2097839 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -183,6 +183,20 @@ typedef struct {
>  } RDMAWorkRequestData;
>  
>  /*
> + * Get the multifd RDMA channel used to send data.
> + */
> +static int get_multifd_RDMA_channel(void)
> +{
> +    int thread_count = migrate_multifd_channels();
> +    MigrationState *s = migrate_get_current();
> +
> +    s->rdma_channel++;
> +    s->rdma_channel %= thread_count;
> +
> +    return s->rdma_channel;
> +}
> +
> +/*
>   * Negotiate RDMA capabilities during connection-setup time.
>   */
>  typedef struct {
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 15/18] migration/rdma: only register the memory for multifd channels

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> All data is sent by multifd Channels, so we only register its for
> multifd channels and main channel don't register its.
> 
> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/rdma.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index c906cc7..f5eb563 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3939,6 +3939,12 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>  
>                  qemu_sem_post(&multifd_send_param->sem_sync);
>              }
> +
> +            /*
> +             * Use multifd to migrate, we only register memory for
> +             * multifd RDMA channel and main channel don't register it.
> +             */
> +            goto wait_reg_complete;

No! No goto's for control flow except for error exits.

>          }
>  
>          /*
> @@ -3999,6 +4005,8 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>                      rdma->dest_blocks[i].remote_host_addr;
>              local->block[i].remote_rkey = rdma->dest_blocks[i].remote_rkey;
>          }
> +
> +wait_reg_complete:
>          /* Wait for all multifd channels to complete registration */
>          if (migrate_use_multifd()) {
>              int i;
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 17/18] migration/rdma: send data for both rdma-pin-all and NOT rdma-pin-all mode

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/rdma.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 61 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 2097839..c19a91f 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2002,6 +2002,20 @@ static int qemu_rdma_write_one(QEMUFile *f, RDMAContext *rdma,
>                                 .repeat = 1,
>                               };
>  
> +    /* use multifd to send data */
> +    if (migrate_use_multifd()) {
> +        int channel = get_multifd_RDMA_channel();
> +        int ret = 0;
> +        MultiFDSendParams *multifd_send_param = NULL;
> +        ret = get_multifd_send_param(channel, &multifd_send_param);
> +        if (ret) {
> +            error_report("rdma: error getting multifd_send_param(%d)", channel);
> +            return -EINVAL;
> +        }
> +        rdma = (RDMAContext *)multifd_send_param->rdma;
> +        block = &(rdma->local_ram_blocks.block[current_index]);
> +    }
> +
>  retry:
>      sge.addr = (uintptr_t)(block->local_host_addr +
>                              (current_addr - block->offset));
> @@ -2197,6 +2211,27 @@ retry:
>      return 0;
>  }
>  
> +static int multifd_rdma_write_flush(void)
> +{
> +    /* The multifd RDMA threads send data */
> +    MultiFDSendParams *multifd_send_param = NULL;
> +    RDMAContext *rdma = NULL;
> +    MigrationState *s = migrate_get_current();
> +    int ret = 0;
> +
> +    ret = get_multifd_send_param(s->rdma_channel,
> +                                 &multifd_send_param);
> +    if (ret) {
> +        error_report("rdma: error getting multifd_send_param(%d)",
> +                     s->rdma_channel);

Do we need these error_report's for get_multifd_send_param calls - how
can they fail in practice?

> +        return ret;
> +    }
> +    rdma = (RDMAContext *)(multifd_send_param->rdma);
> +    rdma->nb_sent++;
> +
> +    return ret;

But this doesn't actually 'flush' anything?

> +}
> +
>  /*
>   * Push out any unwritten RDMA operations.
>   *
> @@ -2219,8 +2254,15 @@ static int qemu_rdma_write_flush(QEMUFile *f, RDMAContext *rdma)
>      }
>  
>      if (ret == 0) {
> -        rdma->nb_sent++;
> -        trace_qemu_rdma_write_flush(rdma->nb_sent);
> +        if (migrate_use_multifd()) {
> +            ret = multifd_rdma_write_flush();
> +            if (ret) {
> +                return ret;
> +            }
> +        } else {
> +            rdma->nb_sent++;
> +            trace_qemu_rdma_write_flush(rdma->nb_sent);
> +        }
>      }
>  
>      rdma->current_length = 0;
> @@ -4062,6 +4104,7 @@ wait_reg_complete:
>              }
>  
>              qemu_sem_post(&multifd_send_param->sem_sync);
> +            qemu_sem_wait(&multifd_send_param->sem);

why?

>          }
>      }
>  
> @@ -4443,6 +4486,7 @@ static void *multifd_rdma_send_thread(void *opaque)
>      Error *local_err = NULL;
>      int ret = 0;
>      RDMAControlHeader head = { .len = 0, .repeat = 1 };
> +    RDMAContext *rdma = p->rdma;
>  
>      trace_multifd_send_thread_start(p->id);
>      if (multifd_send_initial_packet(p, &local_err) < 0) {
> @@ -4451,7 +4495,7 @@ static void *multifd_rdma_send_thread(void *opaque)
>  
>      /* wait for semaphore notification to register memory */
>      qemu_sem_wait(&p->sem_sync);
> -    if (qemu_rdma_registration(p->rdma) < 0) {
> +    if (qemu_rdma_registration(rdma) < 0) {
>          goto out;
>      }
>      /*
> @@ -4466,12 +4510,25 @@ static void *multifd_rdma_send_thread(void *opaque)
>                  break;
>              }
>          }
> +        /* To complete polling(CQE) */
> +        while (rdma->nb_sent) {

Where is nb_sent decremented?

> +            ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
> +            if (ret < 0) {
> +                error_report("multifd RDMA migration: "
> +                             "complete polling error!");
> +                return NULL;
> +            }
> +        }
>          /* Send FINISHED to the destination */
>          head.type = RDMA_CONTROL_REGISTER_FINISHED;
> -        ret = qemu_rdma_exchange_send(p->rdma, &head, NULL, NULL, NULL, NULL);
> +        ret = qemu_rdma_exchange_send(rdma, &head, NULL, NULL, NULL, NULL);
>          if (ret < 0) {
> +            error_report("multifd RDMA migration: "
> +                         "sending remote error!");
>              return NULL;
>          }
> +        /* sync main thread */
> +        qemu_sem_post(&p->sem);
>      }
>  
>  out:
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 18/18] migration/rdma: RDMA cleanup for multifd migration

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---
>  migration/multifd.c |  6 ++++++
>  migration/multifd.h |  1 +
>  migration/rdma.c    | 16 +++++++++++++++-
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 1186246..4031648 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -577,6 +577,9 @@ void multifd_save_cleanup(void)
>          p->packet_len = 0;
>          g_free(p->packet);
>          p->packet = NULL;
> +#ifdef CONFIG_RDMA
> +        multifd_rdma_cleanup(p->rdma);
> +#endif

You may find it easier to add an entry into stubs/ for
multifd_rdma_cleanup; it then avoids the need for the ifdef.

>          multifd_send_state->ops->send_cleanup(p, &local_err);
>          if (local_err) {
>              migrate_set_error(migrate_get_current(), local_err);
> @@ -1039,6 +1042,9 @@ int multifd_load_cleanup(Error **errp)
>          p->packet_len = 0;
>          g_free(p->packet);
>          p->packet = NULL;
> +#ifdef CONFIG_RDMA
> +        multifd_rdma_cleanup(p->rdma);
> +#endif
>          multifd_recv_state->ops->recv_cleanup(p);
>      }
>      qemu_sem_destroy(&multifd_recv_state->sem_sync);
> diff --git a/migration/multifd.h b/migration/multifd.h
> index 26d4489..0ecec5e 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -183,6 +183,7 @@ typedef struct {
>  
>  #ifdef CONFIG_RDMA
>  extern MultiFDSetup multifd_rdma_ops;
> +void multifd_rdma_cleanup(void *opaque);
>  #endif
>  void multifd_send_terminate_threads(Error *err);
>  int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp);
> diff --git a/migration/rdma.c b/migration/rdma.c
> index c19a91f..f14357f 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2369,7 +2369,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
>  {
>      int idx;
>  
> -    if (rdma->cm_id && rdma->connected) {
> +    if (rdma->channel && rdma->cm_id && rdma->connected) {
>          if ((rdma->error_state ||
>               migrate_get_current()->state == MIGRATION_STATUS_CANCELLING) &&
>              !rdma->received_error) {
> @@ -4599,6 +4599,20 @@ static void multifd_rdma_recv_channel_setup(QIOChannel *ioc,
>      return;
>  }
>  
> +void multifd_rdma_cleanup(void *opaque)

I think you need to make it clear that this is only to cleanup one
channel, rather than the whole multifd-rdma connection;
multifd_load_cleanup for example cleans up all the channels, where as I
think this is only doing one?

Don't use a 'void *opaque' except for something that's called via
a registration/callback scheme that's designed to be generic
(e.g. multifd_send_thread does it because it's called from
qemu_thread_create that doesn't know the type).  Where you know
the type, use it!

> +{
> +    RDMAContext *rdma = (RDMAContext *)opaque;
> +
> +    if (!migrate_use_rdma()) {
> +        return;
> +    }
> +
> +    rdma->listen_id = NULL;
> +    rdma->channel = NULL;
> +    qemu_rdma_cleanup(rdma);
> +    g_free(rdma);
> +}
> +
>  MultiFDSetup multifd_rdma_ops = {
>      .send_thread = multifd_rdma_send_thread,
>      .recv_thread = multifd_rdma_recv_thread,
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 02/18] migration/rdma: judge whether or not the RDMA is used for migration

[Zheng Chuan]

On 2021/2/4 1:49, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Add enabled_rdma_migration into MigrationState to judge
>> whether or not the RDMA is used for migration.
>>
>> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> 
Hi, Dave. Sorry for late reply due to Spring Festival.

> I'd rather see a separate flag added to each of the MigrationState and
> MigrationIncomingState separately for outoging and incoming migration.
>
We use enabled_rdma_migration in migrate_use_rdma() to judge whether or not the RDMA is used for for both Src and Dst.
As far as i see, function like migrate_use_multifd() is used also for both sides.
If we use separate flag added to each of the MigrationState and MigrationIncomingState, we need to add two function to do that for each side.
I am not sure if that is really what you want.

> It's also probably better to call it 'is_rdma_migration' rather than
> enabled.
> 

Yes, I agree with you, it is better to use is_rdma_migration, will use it in next version.

> Dave
> 
>> ---
>>  migration/migration.c | 13 +++++++++++++
>>  migration/migration.h |  6 ++++++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 447dfb9..129c81a 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -418,11 +418,13 @@ void migrate_add_address(SocketAddress *address)
>>  static void qemu_start_incoming_migration(const char *uri, Error **errp)
>>  {
>>      const char *p = NULL;
>> +    MigrationState *s = migrate_get_current();
>>  
>>      if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
>>          return;
>>      }
>>  
>> +    s->enabled_rdma_migration = false;
>>      qapi_event_send_migration(MIGRATION_STATUS_SETUP);
>>      if (strstart(uri, "tcp:", &p) ||
>>          strstart(uri, "unix:", NULL) ||
>> @@ -430,6 +432,7 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
>>          socket_start_incoming_migration(p ? p : uri, errp);
>>  #ifdef CONFIG_RDMA
>>      } else if (strstart(uri, "rdma:", &p)) {
>> +        s->enabled_rdma_migration = true;
>>          rdma_start_incoming_migration(p, errp);
>>  #endif
>>      } else if (strstart(uri, "exec:", &p)) {
>> @@ -1921,6 +1924,7 @@ void migrate_init(MigrationState *s)
>>      s->start_postcopy = false;
>>      s->postcopy_after_devices = false;
>>      s->migration_thread_running = false;
>> +    s->enabled_rdma_migration = false;
>>      error_free(s->error);
>>      s->error = NULL;
>>      s->hostname = NULL;
>> @@ -2162,6 +2166,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>>          socket_start_outgoing_migration(s, p ? p : uri, &local_err);
>>  #ifdef CONFIG_RDMA
>>      } else if (strstart(uri, "rdma:", &p)) {
>> +        s->enabled_rdma_migration = true;
>>          rdma_start_outgoing_migration(s, p, &local_err);
>>  #endif
>>      } else if (strstart(uri, "exec:", &p)) {
>> @@ -2391,6 +2396,14 @@ bool migrate_rdma_pin_all(void)
>>      return s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL];
>>  }
>>  
>> +bool migrate_use_rdma(void)
>> +{
>> +    MigrationState *s;
>> +    s = migrate_get_current();
>> +
>> +    return s->enabled_rdma_migration;
>> +}
>> +
>>  bool migrate_use_multifd(void)
>>  {
>>      MigrationState *s;
>> diff --git a/migration/migration.h b/migration/migration.h
>> index 22b36f3..da5681b 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -280,6 +280,11 @@ struct MigrationState {
>>       * This save hostname when out-going migration starts
>>       */
>>      char *hostname;
>> +
>> +    /*
>> +     * Enable RDMA migration
>> +     */
>> +    bool enabled_rdma_migration;
>>  };
>>  
>>  void migrate_set_state(int *state, int old_state, int new_state);
>> @@ -317,6 +322,7 @@ bool migrate_validate_uuid(void);
>>  
>>  bool migrate_auto_converge(void);
>>  bool migrate_rdma_pin_all(void);
>> +bool migrate_use_rdma(void);
>>  bool migrate_use_multifd(void);
>>  bool migrate_pause_before_switchover(void);
>>  int migrate_multifd_channels(void);
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 06/18] migration/rdma: export MultiFDSendParams/MultiFDRecvParams

[Zheng Chuan]

On 2021/2/4 2:23, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> MultiFDSendParams and MultiFDRecvParams is need for rdma, export it
>>
>> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> 
> I think these become simpler if you just return a NULL on error,
> also I think you can make 'id' unsigned and then you don't have
> to worry about it being negative.
> 
Yes, that's a good point, I will do that in v5.

> Also, please make it start with multifd_ so we know where it's coming
> from; so:
> 
> MultiFDSendParams *multifd_send_param_get(unsigned channel);
> 
> Dave

OK, will do that in next version.

>> ---
>>  migration/multifd.c | 26 ++++++++++++++++++++++++++
>>  migration/multifd.h |  2 ++
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index 5d34950..ae0b7f0 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -390,6 +390,19 @@ struct {
>>      MultiFDSetup *setup_ops;
>>  } *multifd_send_state;
>>  
>> +int get_multifd_send_param(int id, MultiFDSendParams **param)
>> +{
>> +    int ret = 0;
>> +
>> +    if (id < 0 || id >= migrate_multifd_channels()) {
>> +        ret = -1;
>> +    } else {
>> +        *param = &(multifd_send_state->params[id]);
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>>  /*
>>   * How we use multifd_send_state->pages and channel->pages?
>>   *
>> @@ -934,6 +947,19 @@ struct {
>>      MultiFDSetup *setup_ops;
>>  } *multifd_recv_state;
>>  
>> +int get_multifd_recv_param(int id, MultiFDRecvParams **param)
>> +{
>> +    int ret = 0;
>> +
>> +    if (id < 0 || id >= migrate_multifd_channels()) {
>> +        ret = -1;
>> +    } else {
>> +        *param = &(multifd_recv_state->params[id]);
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>>  static void multifd_recv_terminate_threads(Error *err)
>>  {
>>      int i;
>> diff --git a/migration/multifd.h b/migration/multifd.h
>> index e3ab4b0..d57756c 100644
>> --- a/migration/multifd.h
>> +++ b/migration/multifd.h
>> @@ -176,6 +176,8 @@ typedef struct {
>>  #ifdef CONFIG_RDMA
>>  extern MultiFDSetup multifd_rdma_ops;
>>  #endif
>> +int get_multifd_send_param(int id, MultiFDSendParams **param);
>> +int get_multifd_recv_param(int id, MultiFDRecvParams **param);
>>  MultiFDSetup *multifd_setup_ops_init(void);
>>  
>>  void multifd_register_ops(int method, MultiFDMethods *ops);
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 07/18] migration/rdma: add rdma field into multifd send/recv param

[Zheng Chuan]

On 2021/2/4 2:32, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Note we do want to export any rdma struct, take void * instead.
> 
> You don't need to make this a void *; add a typedef struct RDMAContext
> into include/qemu/typedefs.h  and then you can use the right type here
> without actually exporting the interesting contents of the type or
> being dependent on rdma being built.
> 
> Dave
>

OK, good to know it, will do it in v5.

>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>>  migration/multifd.h | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/migration/multifd.h b/migration/multifd.h
>> index d57756c..b17a2c1 100644
>> --- a/migration/multifd.h
>> +++ b/migration/multifd.h
>> @@ -108,6 +108,10 @@ typedef struct {
>>      QemuSemaphore sem_sync;
>>      /* used for compression methods */
>>      void *data;
>> +    /* used for multifd rdma */
>> +    void *rdma;
>> +    /* communication channel */
>> +    QEMUFile *file;
>>  }  MultiFDSendParams;
>>  
>>  typedef struct {
>> @@ -147,6 +151,10 @@ typedef struct {
>>      QemuSemaphore sem_sync;
>>      /* used for de-compression methods */
>>      void *data;
>> +    /* used for multifd rdma */
>> +    void *rdma;
>> +    /* communication channel */
>> +    QEMUFile *file;
>>  } MultiFDRecvParams;
>>  
>>  typedef struct {
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 08/18] migration/rdma: export getQIOChannel to get QIOchannel in rdma

[Zheng Chuan]

On 2021/2/4 2:49, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>>  migration/qemu-file.c | 5 +++++
>>  migration/qemu-file.h | 1 +
>>  2 files changed, 6 insertions(+)
>>
>> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
>> index be21518..37f6201 100644
>> --- a/migration/qemu-file.c
>> +++ b/migration/qemu-file.c
>> @@ -260,6 +260,11 @@ void ram_control_before_iterate(QEMUFile *f, uint64_t flags)
>>      }
>>  }
>>  
>> +void *getQIOChannel(QEMUFile *f)
>> +{
>> +    return f->opaque;
>> +}
>> +
> 
> Unfortunately that's not right, since the opaque isn't always a
> QUIChannel, so getOpaque would be a suitable name here.
> 
> It's a shame this is needed; I'm surprised you ever have a QEMUFIle* in
> the rdma code in somewhere you don't have the QIOChannel; could you
> avoid this by adding a QIOChannel pointer into the RDAMContext to point
> back to the channel which it's for?
> 
> Dave
> 
OK, i'll try it.
>>  void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
>>  {
>>      int ret = 0;
>> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
>> index a9b6d6c..4cef043 100644
>> --- a/migration/qemu-file.h
>> +++ b/migration/qemu-file.h
>> @@ -165,6 +165,7 @@ void qemu_file_set_blocking(QEMUFile *f, bool block);
>>  void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
>>  void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
>>  void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data);
>> +void *getQIOChannel(QEMUFile *f);
>>  
>>  /* Whenever this is found in the data stream, the flags
>>   * will be passed to ram_control_load_hook in the incoming-migration
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 11/18] migration/rdma: record host_port for multifd RDMA

[Zheng Chuan]

On 2021/2/4 3:04, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>>  migration/migration.c | 1 +
>>  migration/migration.h | 3 +++
>>  migration/rdma.c      | 3 +++
>>  3 files changed, 7 insertions(+)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 129c81a..b8f4844 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -1925,6 +1925,7 @@ void migrate_init(MigrationState *s)
>>      s->postcopy_after_devices = false;
>>      s->migration_thread_running = false;
>>      s->enabled_rdma_migration = false;
>> +    s->host_port = NULL;
>>      error_free(s->error);
>>      s->error = NULL;
>>      s->hostname = NULL;
>> diff --git a/migration/migration.h b/migration/migration.h
>> index da5681b..537ee09 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -285,6 +285,9 @@ struct MigrationState {
>>       * Enable RDMA migration
>>       */
>>      bool enabled_rdma_migration;
>> +
>> +    /* Need by Multi-RDMA */
>> +    char *host_port;
> 
> Please keep that next to the char *hostname, since they go together.
> Also, 'Needed'
> 
> Dave
> 
OK, will fix it in V5.
>>  };
>>  
>>  void migrate_set_state(int *state, int old_state, int new_state);
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index ed8a015..9654b87 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -4206,6 +4206,8 @@ void rdma_start_outgoing_migration(void *opaque,
>>          goto err;
>>      }
>>  
>> +    s->host_port = g_strdup(host_port);
>> +
>>      ret = qemu_rdma_source_init(rdma,
>>          s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL], errp);
>>  
>> @@ -4250,6 +4252,7 @@ void rdma_start_outgoing_migration(void *opaque,
>>  
>>      s->to_dst_file = qemu_fopen_rdma(rdma, "wb");
>>      migrate_fd_connect(s, NULL);
>> +    g_free(s->host_port);
>>      return;
>>  return_path_err:
>>      qemu_rdma_cleanup(rdma);
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 12/18] migration/rdma: Create the multifd send channels for RDMA

[Zheng Chuan]

On 2021/2/4 3:52, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>>  migration/multifd.c |  4 ++--
>>  migration/multifd.h |  2 ++
>>  migration/rdma.c    | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 61 insertions(+), 2 deletions(-)
>>
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index ae0b7f0..919a414 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -176,7 +176,7 @@ void multifd_register_ops(int method, MultiFDMethods *ops)
>>      multifd_ops[method] = ops;
>>  }
>>  
>> -static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
>> +int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
>>  {
>>      MultiFDInit_t msg = {};
>>      int ret;
>> @@ -503,7 +503,7 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset)
>>      return 1;
>>  }
>>  
>> -static void multifd_send_terminate_threads(Error *err)
>> +void multifd_send_terminate_threads(Error *err)
>>  {
>>      int i;
>>  
>> diff --git a/migration/multifd.h b/migration/multifd.h
>> index b17a2c1..26d4489 100644
>> --- a/migration/multifd.h
>> +++ b/migration/multifd.h
>> @@ -184,6 +184,8 @@ typedef struct {
>>  #ifdef CONFIG_RDMA
>>  extern MultiFDSetup multifd_rdma_ops;
>>  #endif
>> +void multifd_send_terminate_threads(Error *err);
>> +int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp);
>>  int get_multifd_send_param(int id, MultiFDSendParams **param);
>>  int get_multifd_recv_param(int id, MultiFDRecvParams **param);
>>  MultiFDSetup *multifd_setup_ops_init(void);
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index 9654b87..cff9446 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -4261,9 +4261,54 @@ err:
>>      g_free(rdma_return_path);
>>  }
>>  
>> +static int multifd_channel_rdma_connect(void *opaque)
>> +{
>> +    MultiFDSendParams *p = opaque;
>> +    Error *local_err = NULL;
>> +    int ret = 0;
>> +    MigrationState *s = migrate_get_current();
>> +
>> +    p->rdma = qemu_rdma_data_init(s->host_port, &local_err);
>> +    if (p->rdma == NULL) {
>> +        goto out;
>> +    }
>> +
>> +    ret = qemu_rdma_source_init(p->rdma,
>> +                                migrate_rdma_pin_all(),
>> +                                &local_err);
>> +    if (ret) {
>> +        goto out;
>> +    }
>> +
>> +    ret = qemu_rdma_connect(p->rdma, &local_err);
>> +    if (ret) {
>> +        goto out;
>> +    }
>> +
>> +    p->file = qemu_fopen_rdma(p->rdma, "wb");
>> +    if (p->file == NULL) {
>> +        goto out;
>> +    }
>> +
>> +    p->c = QIO_CHANNEL(getQIOChannel(p->file));
>> +
>> +out:
>> +    if (local_err) {
>> +        trace_multifd_send_error(p->id);
>> +    }
> 
> If any of the previous steps have failed, but not the first step,
> what cleans up?
> 
Yes，Sorry for that. I'll add cleanup in next v5.

>> +    return ret;
>> +}
>> +
>>  static void *multifd_rdma_send_thread(void *opaque)
>>  {
>>      MultiFDSendParams *p = opaque;
>> +    Error *local_err = NULL;
>> +
>> +    trace_multifd_send_thread_start(p->id);
>> +    if (multifd_send_initial_packet(p, &local_err) < 0) {
>> +        goto out;
>> +    }
>>  
>>      while (true) {
>>          WITH_QEMU_LOCK_GUARD(&p->mutex) {
>> @@ -4274,6 +4319,12 @@ static void *multifd_rdma_send_thread(void *opaque)
>>          qemu_sem_wait(&p->sem);
>>      }
>>  
>> +out:
>> +    if (local_err) {
>> +        trace_multifd_send_error(p->id);
>> +        multifd_send_terminate_threads(local_err);
>> +    }
>> +
>>      WITH_QEMU_LOCK_GUARD(&p->mutex) {
>>          p->running = false;
>>      }
>> @@ -4285,6 +4336,12 @@ static void multifd_rdma_send_channel_setup(MultiFDSendParams *p)
>>  {
>>      Error *local_err = NULL;
>>  
>> +    if (multifd_channel_rdma_connect(p)) {
>> +        error_setg(&local_err, "multifd: rdma channel %d not established",
>> +                   p->id);
>> +        return ;
>> +    }
>> +
>>      if (p->quit) {
>>          error_setg(&local_err, "multifd: send id %d already quit", p->id);
>>          return ;
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 13/18] migration/rdma: Add the function for dynamic page registration

[Zheng Chuan]

On 2021/2/4 4:06, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Add the 'qemu_rdma_registration' function, multifd send threads
>> call it to register memory.
> 
> This function is a copy of the code out of qemu_rdma_registration_stop;
> with some of the comments removed.
> It's OK to split this code out so you can use it as well; but then why
> not make qemu_rdma_registration_stop use this function as well to stop
> having two copies ?  And keep the comments!
> 
OK, That's good to me. Will do that in V5.

>> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>>  migration/rdma.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 51 insertions(+)
>>
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index cff9446..1095a8f 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -3739,6 +3739,57 @@ out:
>>      return ret;
>>  }
>>  
>> +/*
>> + * Dynamic page registrations for multifd RDMA threads.
>> + */
>> +static int qemu_rdma_registration(void *opaque)
>> +{
>> +    RDMAContext *rdma = opaque;
> 
> Can't you keep that as qemu_rdma_registration(RDMAContext *rdma) ?
> 
>> +    RDMAControlHeader resp = {.type = RDMA_CONTROL_RAM_BLOCKS_RESULT };
>> +    RDMALocalBlocks *local = &rdma->local_ram_blocks;
>> +    int reg_result_idx, i, nb_dest_blocks;
>> +    RDMAControlHeader head = { .len = 0, .repeat = 1 };
>> +    int ret = 0;
>> +
>> +    head.type = RDMA_CONTROL_RAM_BLOCKS_REQUEST;
>> +
>> +    ret = qemu_rdma_exchange_send(rdma, &head, NULL, &resp,
>> +            &reg_result_idx, rdma->pin_all ?
>> +            qemu_rdma_reg_whole_ram_blocks : NULL);
>> +    if (ret < 0) {
>> +        goto out;
>> +    }
>> +
>> +    nb_dest_blocks = resp.len / sizeof(RDMADestBlock);
>> +
>> +    if (local->nb_blocks != nb_dest_blocks) {
>> +        rdma->error_state = -EINVAL;
>> +        ret = -1;
>> +        goto out;
>> +    }
>> +
>> +    qemu_rdma_move_header(rdma, reg_result_idx, &resp);
>> +    memcpy(rdma->dest_blocks,
>> +           rdma->wr_data[reg_result_idx].control_curr, resp.len);
>> +
>> +    for (i = 0; i < nb_dest_blocks; i++) {
>> +        network_to_dest_block(&rdma->dest_blocks[i]);
>> +
>> +        /* We require that the blocks are in the same order */
>> +        if (rdma->dest_blocks[i].length != local->block[i].length) {
>> +            rdma->error_state = -EINVAL;
>> +            ret = -1;
>> +            goto out;
>> +        }
>> +        local->block[i].remote_host_addr =
>> +            rdma->dest_blocks[i].remote_host_addr;
>> +        local->block[i].remote_rkey = rdma->dest_blocks[i].remote_rkey;
>> +    }
>> +
>> +out:
>> +    return ret;
>> +}
>> +
>>  /* Destination:
>>   * Called via a ram_control_load_hook during the initial RAM load section which
>>   * lists the RAMBlocks by name.  This lets us know the order of the RAMBlocks
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 16/18] migration/rdma: add rdma_channel into Migrationstate field

[Zheng Chuan]

On 2021/2/4 4:19, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Multifd RDMA is need to poll when we send data, record it.
> 
> This looks like it's trying to be the equivalent of the 'static int
> next_channel' in multifd_send_pages.
> 
> If so, why not mkae this 'multifd_channel' and make the function
> 'multifd_next_channel' and replace the code in multifd_send_pages to use
> this as well, rather than make it a special for rdma.
> 
> Dave
> 
Yes, that's a good suggestion. I'll do it in V5.

>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>>  migration/migration.c |  1 +
>>  migration/migration.h |  1 +
>>  migration/rdma.c      | 14 ++++++++++++++
>>  3 files changed, 16 insertions(+)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index b8f4844..47bd11d 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -1926,6 +1926,7 @@ void migrate_init(MigrationState *s)
>>      s->migration_thread_running = false;
>>      s->enabled_rdma_migration = false;
>>      s->host_port = NULL;
>> +    s->rdma_channel = 0;
>>      error_free(s->error);
>>      s->error = NULL;
>>      s->hostname = NULL;
>> diff --git a/migration/migration.h b/migration/migration.h
>> index 537ee09..5ff46e6 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -288,6 +288,7 @@ struct MigrationState {
>>  
>>      /* Need by Multi-RDMA */
>>      char *host_port;
>> +    int rdma_channel;
>>  };
>>  
>>  void migrate_set_state(int *state, int old_state, int new_state);
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index f5eb563..2097839 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -183,6 +183,20 @@ typedef struct {
>>  } RDMAWorkRequestData;
>>  
>>  /*
>> + * Get the multifd RDMA channel used to send data.
>> + */
>> +static int get_multifd_RDMA_channel(void)
>> +{
>> +    int thread_count = migrate_multifd_channels();
>> +    MigrationState *s = migrate_get_current();
>> +
>> +    s->rdma_channel++;
>> +    s->rdma_channel %= thread_count;
>> +
>> +    return s->rdma_channel;
>> +}
>> +
>> +/*
>>   * Negotiate RDMA capabilities during connection-setup time.
>>   */
>>  typedef struct {
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 05/18] migration/rdma: do not need sync main for rdma

[Zheng Chuan]

On 2021/2/4 2:10, Dr. David Alan Gilbert wrote:
> This patch needs to explain why the sync isn't needed for RDMA.
> 
> Dave
> 
OK. the multifd with tcp will send pages if it has pages to send by the record of multifd_send_state->pages->used while
RDMA is using rdma_write_hooks.

> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>>  migration/multifd.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index 4820702..5d34950 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -583,6 +583,10 @@ void multifd_send_sync_main(QEMUFile *f)
>>      if (!migrate_use_multifd()) {
>>          return;
>>      }
>> +     /* Do not need sync for rdma */
>> +    if (migrate_use_rdma()) {
>> +        return;
>> +    }
>>      if (multifd_send_state->pages->used) {
>>          if (multifd_send_pages(f) < 0) {
>>              error_report("%s: multifd_send_pages fail", __func__);
>> @@ -1024,6 +1028,10 @@ void multifd_recv_sync_main(void)
>>      if (!migrate_use_multifd()) {
>>          return;
>>      }
>> +    /* Do not need sync for rdma */
>> +    if (migrate_use_rdma()) {
>> +        return;
>> +    }
>>      for (i = 0; i < migrate_multifd_channels(); i++) {
>>          MultiFDRecvParams *p = &multifd_recv_state->params[i];
>>  
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 10/18] migration/rdma: Create the multifd recv channels for RDMA

[Zheng Chuan]

On 2021/2/4 2:59, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> We still don't transmit anything through them, and we only build
>> the RDMA connections.
>>
>> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>>  migration/rdma.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 67 insertions(+), 2 deletions(-)
>>
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index 996afb0..ed8a015 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -3267,6 +3267,40 @@ static void rdma_cm_poll_handler(void *opaque)
>>      }
>>  }
>>  
>> +static bool qemu_rdma_accept_setup(RDMAContext *rdma)
>> +{
>> +    RDMAContext *multifd_rdma = NULL;
>> +    int thread_count;
>> +    int i;
>> +    MultiFDRecvParams *multifd_recv_param;
>> +    thread_count = migrate_multifd_channels();
>> +    /* create the multifd channels for RDMA */
>> +    for (i = 0; i < thread_count; i++) {
>> +        if (get_multifd_recv_param(i, &multifd_recv_param) < 0) {
>> +            error_report("rdma: error getting multifd_recv_param(%d)", i);
>> +            return false;
>> +        }
>> +
>> +        multifd_rdma = (RDMAContext *) multifd_recv_param->rdma;
>> +        if (multifd_rdma->cm_id == NULL) {
>> +            break;
>> +        } else {
>> +            multifd_rdma = NULL;
>> +        }
> 
> I'm confused by what this if is doing - what are the two cases?
> 
Since we share the CM channel and CM id with main thread,
we assign the cmd_id through the callback rdma_accept_incoming_migration() for the multifd thread if cm_id is NULL.
Once it is assigned, we could go to the normal rdma_cm_poll_handler() set handler.

>> +    }
>> +
>> +    if (multifd_rdma) {
>> +        qemu_set_fd_handler(rdma->channel->fd,
>> +                            rdma_accept_incoming_migration,
>> +                            NULL, (void *)(intptr_t)multifd_rdma);
>> +    } else {
>> +        qemu_set_fd_handler(rdma->channel->fd, rdma_cm_poll_handler,
>> +                            NULL, rdma);
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>  static int qemu_rdma_accept(RDMAContext *rdma)
>>  {
>>      RDMACapabilities cap;
>> @@ -3366,6 +3400,10 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>>          qemu_set_fd_handler(rdma->channel->fd, rdma_accept_incoming_migration,
>>                              NULL,
>>                              (void *)(intptr_t)rdma->return_path);
>> +    } else if (migrate_use_multifd()) {
>> +        if (!qemu_rdma_accept_setup(rdma)) {
>> +            goto err_rdma_dest_wait;
>> +        }
>>      } else {
>>          qemu_set_fd_handler(rdma->channel->fd, rdma_cm_poll_handler,
>>                              NULL, rdma);
>> @@ -3976,6 +4014,34 @@ static QEMUFile *qemu_fopen_rdma(RDMAContext *rdma, const char *mode)
>>      return rioc->file;
>>  }
>>  
>> +static void migration_rdma_process_incoming(QEMUFile *f,
>> +                                            RDMAContext *rdma, Error **errp)
>> +{
>> +    MigrationIncomingState *mis = migration_incoming_get_current();
>> +    QIOChannel *ioc = NULL;
>> +    bool start_migration = false;
>> +
>> +    if (!migrate_use_multifd()) {
>> +        rdma->migration_started_on_destination = 1;
>> +        migration_fd_process_incoming(f, errp);
>> +        return;
>> +    }
>> +
>> +    if (!mis->from_src_file) {
>> +        mis->from_src_file = f;
>> +        qemu_file_set_blocking(f, false);
>> +    } else {
>> +        ioc = QIO_CHANNEL(getQIOChannel(f));
>> +        /* Multiple connections */
>> +        assert(migrate_use_multifd());
> 
> Are you sure that's never triggerable by something trying to connect
> badly? If it was it would be better to error than abort.
> 
This is the similiar action with tcp multifd which is introduced by a429e7f4887313370,
However we will never get there if migrate_use_multifd is false because of return at the first judgement of function, we could not do it or just put a warning.

>> +        start_migration = multifd_recv_new_channel(ioc, errp);
> 
> And what does 'start_migration' mean here - is that meaning that we have
> a full set of connections?
> 
Yes, multifd_recv_new_channel returns true when correctly receiving all channels.

> Dave
> 
>> +    }
>> +
>> +    if (start_migration) {
>> +        migration_incoming_process();
>> +    }
>> +}
>> +
>>  static void rdma_accept_incoming_migration(void *opaque)
>>  {
>>      RDMAContext *rdma = opaque;
>> @@ -4004,8 +4070,7 @@ static void rdma_accept_incoming_migration(void *opaque)
>>          return;
>>      }
>>  
>> -    rdma->migration_started_on_destination = 1;
>> -    migration_fd_process_incoming(f, &local_err);
>> +    migration_rdma_process_incoming(f, rdma, &local_err);
>>      if (local_err) {
>>          error_reportf_err(local_err, "RDMA ERROR:");
>>      }
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 14/18] migration/rdma: register memory for multifd RDMA channels

[Zheng Chuan]

On 2021/2/4 4:12, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> 
> This could do with a description in the commit message of the sequence;
> I think you're waiting for the semaphore; doing the registratin, then
> waiting again to say that everyone has finished???
> 
Yes. The detailed description will be added in v5.
>> ---
>>  migration/multifd.c |  3 ++
>>  migration/rdma.c    | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 93 insertions(+), 2 deletions(-)
>>
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index 919a414..1186246 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -537,6 +537,9 @@ void multifd_send_terminate_threads(Error *err)
>>          qemu_mutex_lock(&p->mutex);
>>          p->quit = true;
>>          qemu_sem_post(&p->sem);
>> +        if (migrate_use_rdma()) {
>> +            qemu_sem_post(&p->sem_sync);
>> +        }
>>          qemu_mutex_unlock(&p->mutex);
>>      }
>>  }
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index 1095a8f..c906cc7 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -3838,6 +3838,19 @@ static int rdma_load_hook(QEMUFile *f, void *opaque, uint64_t flags, void *data)
>>          return rdma_block_notification_handle(opaque, data);
>>  
>>      case RAM_CONTROL_HOOK:
>> +        if (migrate_use_multifd()) {
>> +            int i;
>> +            MultiFDRecvParams *multifd_recv_param = NULL;
>> +            int thread_count = migrate_multifd_channels();
>> +            /* Inform dest recv_thread to poll */
>> +            for (i = 0; i < thread_count; i++) {
>> +                if (get_multifd_recv_param(i, &multifd_recv_param)) {
>> +                    return -1;
>> +                }
>> +                qemu_sem_post(&multifd_recv_param->sem_sync);
>> +            }
>> +        }
>> +
>>          return qemu_rdma_registration_handle(f, opaque);
>>  
>>      default:
>> @@ -3910,6 +3923,24 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>>          head.type = RDMA_CONTROL_RAM_BLOCKS_REQUEST;
>>          trace_qemu_rdma_registration_stop_ram();
>>  
>> +        if (migrate_use_multifd()) {
>> +            /*
>> +             * Inform the multifd channels to register memory
>> +             */
>> +            int i;
>> +            int thread_count = migrate_multifd_channels();
>> +            MultiFDSendParams *multifd_send_param = NULL;
>> +            for (i = 0; i < thread_count; i++) {
>> +                ret = get_multifd_send_param(i, &multifd_send_param);
>> +                if (ret) {
>> +                    error_report("rdma: error getting multifd(%d)", i);
>> +                    return ret;
>> +                }
>> +
>> +                qemu_sem_post(&multifd_send_param->sem_sync);
>> +            }
>> +        }
>> +
>>          /*
>>           * Make sure that we parallelize the pinning on both sides.
>>           * For very large guests, doing this serially takes a really
>> @@ -3968,6 +3999,21 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>>                      rdma->dest_blocks[i].remote_host_addr;
>>              local->block[i].remote_rkey = rdma->dest_blocks[i].remote_rkey;
>>          }
>> +        /* Wait for all multifd channels to complete registration */
>> +        if (migrate_use_multifd()) {
>> +            int i;
>> +            int thread_count = migrate_multifd_channels();
>> +            MultiFDSendParams *multifd_send_param = NULL;
>> +            for (i = 0; i < thread_count; i++) {
>> +                ret = get_multifd_send_param(i, &multifd_send_param);
>> +                if (ret) {
>> +                    error_report("rdma: error getting multifd(%d)", i);
>> +                    return ret;
>> +                }
>> +
>> +                qemu_sem_wait(&multifd_send_param->sem);
>> +            }
>> +        }
>>      }
>>  
>>      trace_qemu_rdma_registration_stop(flags);
>> @@ -3979,6 +4025,24 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>>          goto err;
>>      }
>>  
>> +    if (migrate_use_multifd()) {
>> +        /*
>> +         * Inform src send_thread to send FINISHED signal.
>> +         * Wait for multifd RDMA send threads to poll the CQE.
>> +         */
>> +        int i;
>> +        int thread_count = migrate_multifd_channels();
>> +        MultiFDSendParams *multifd_send_param = NULL;
>> +        for (i = 0; i < thread_count; i++) {
>> +            ret = get_multifd_send_param(i, &multifd_send_param);
>> +            if (ret < 0) {
>> +                goto err;
>> +            }
>> +
>> +            qemu_sem_post(&multifd_send_param->sem_sync);
>> +        }
>> +    }
>> +
>>      return 0;
>>  err:
>>      rdma->error_state = ret;
>> @@ -4355,19 +4419,37 @@ static void *multifd_rdma_send_thread(void *opaque)
>>  {
>>      MultiFDSendParams *p = opaque;
>>      Error *local_err = NULL;
>> +    int ret = 0;
>> +    RDMAControlHeader head = { .len = 0, .repeat = 1 };
>>  
>>      trace_multifd_send_thread_start(p->id);
>>      if (multifd_send_initial_packet(p, &local_err) < 0) {
>>          goto out;
>>      }
>>  
>> +    /* wait for semaphore notification to register memory */
>> +    qemu_sem_wait(&p->sem_sync);
>> +    if (qemu_rdma_registration(p->rdma) < 0) {
>> +        goto out;
>> +    }
>> +    /*
>> +     * Inform the main RDMA thread to run when multifd
>> +     * RDMA thread have completed registration.
>> +     */
>> +    qemu_sem_post(&p->sem);
>>      while (true) {
>> +        qemu_sem_wait(&p->sem_sync);
>>          WITH_QEMU_LOCK_GUARD(&p->mutex) {
>>              if (p->quit) {
>>                  break;
>>              }
>>          }
>> -        qemu_sem_wait(&p->sem);
> 
> Is this something where you put the line in just a few patches earlier -
> if so, put it in the right place in the original patch.
> 
> Dave
> 
My mistake. this is wrong place in patch-004, will correct it. Thanks.

>> +        /* Send FINISHED to the destination */
>> +        head.type = RDMA_CONTROL_REGISTER_FINISHED;
>> +        ret = qemu_rdma_exchange_send(p->rdma, &head, NULL, NULL, NULL, NULL);
>> +        if (ret < 0) {
>> +            return NULL;
>> +        }
>>      }
>>  
>>  out:
>> @@ -4406,14 +4488,20 @@ static void multifd_rdma_send_channel_setup(MultiFDSendParams *p)
>>  static void *multifd_rdma_recv_thread(void *opaque)
>>  {
>>      MultiFDRecvParams *p = opaque;
>> +    int ret = 0;
>>  
>>      while (true) {
>> +        qemu_sem_wait(&p->sem_sync);
>>          WITH_QEMU_LOCK_GUARD(&p->mutex) {
>>              if (p->quit) {
>>                  break;
>>              }
>>          }
>> -        qemu_sem_wait(&p->sem_sync);
>> +        ret = qemu_rdma_registration_handle(p->file, p->c);
>> +        if (ret < 0) {
>> +            qemu_file_set_error(p->file, ret);
>> +            break;
>> +        }
>>      }
>>  
>>      WITH_QEMU_LOCK_GUARD(&p->mutex) {
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 17/18] migration/rdma: send data for both rdma-pin-all and NOT rdma-pin-all mode

[Zheng Chuan]

On 2021/2/4 18:18, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>>  migration/rdma.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 61 insertions(+), 4 deletions(-)
>>
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index 2097839..c19a91f 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -2002,6 +2002,20 @@ static int qemu_rdma_write_one(QEMUFile *f, RDMAContext *rdma,
>>                                 .repeat = 1,
>>                               };
>>  
>> +    /* use multifd to send data */
>> +    if (migrate_use_multifd()) {
>> +        int channel = get_multifd_RDMA_channel();
>> +        int ret = 0;
>> +        MultiFDSendParams *multifd_send_param = NULL;
>> +        ret = get_multifd_send_param(channel, &multifd_send_param);
>> +        if (ret) {
>> +            error_report("rdma: error getting multifd_send_param(%d)", channel);
>> +            return -EINVAL;
>> +        }
>> +        rdma = (RDMAContext *)multifd_send_param->rdma;
>> +        block = &(rdma->local_ram_blocks.block[current_index]);
>> +    }
>> +
>>  retry:
>>      sge.addr = (uintptr_t)(block->local_host_addr +
>>                              (current_addr - block->offset));
>> @@ -2197,6 +2211,27 @@ retry:
>>      return 0;
>>  }
>>  
>> +static int multifd_rdma_write_flush(void)
>> +{
>> +    /* The multifd RDMA threads send data */
>> +    MultiFDSendParams *multifd_send_param = NULL;
>> +    RDMAContext *rdma = NULL;
>> +    MigrationState *s = migrate_get_current();
>> +    int ret = 0;
>> +
>> +    ret = get_multifd_send_param(s->rdma_channel,
>> +                                 &multifd_send_param);
>> +    if (ret) {
>> +        error_report("rdma: error getting multifd_send_param(%d)",
>> +                     s->rdma_channel);
> 
> Do we need these error_report's for get_multifd_send_param calls - how
> can they fail in practice?
> 
Maybe we do not need it.
The s->rdma_channel should not exceed the migrate_multifd_channels and should not negative.

>> +        return ret;
>> +    }
>> +    rdma = (RDMAContext *)(multifd_send_param->rdma);
>> +    rdma->nb_sent++;
>> +
>> +    return ret;
> 
> But this doesn't actually 'flush' anything?
> 
Yes, it just use to increase the nb_sent. we need to choose a more suitable function name.

>> +}
>> +
>>  /*
>>   * Push out any unwritten RDMA operations.
>>   *
>> @@ -2219,8 +2254,15 @@ static int qemu_rdma_write_flush(QEMUFile *f, RDMAContext *rdma)
>>      }
>>  
>>      if (ret == 0) {
>> -        rdma->nb_sent++;
>> -        trace_qemu_rdma_write_flush(rdma->nb_sent);
>> +        if (migrate_use_multifd()) {
>> +            ret = multifd_rdma_write_flush();
>> +            if (ret) {
>> +                return ret;
>> +            }
>> +        } else {
>> +            rdma->nb_sent++;
>> +            trace_qemu_rdma_write_flush(rdma->nb_sent);
>> +        }
>>      }
>>  
>>      rdma->current_length = 0;
>> @@ -4062,6 +4104,7 @@ wait_reg_complete:
>>              }
>>  
>>              qemu_sem_post(&multifd_send_param->sem_sync);
>> +            qemu_sem_wait(&multifd_send_param->sem);
> 
> why?
> 
The multifd send thread would post sem signal after finishing sending data.
The main thread need wait for multifd RDMA send threads to poll the CQE.
>>          }
>>      }
>>  
>> @@ -4443,6 +4486,7 @@ static void *multifd_rdma_send_thread(void *opaque)
>>      Error *local_err = NULL;
>>      int ret = 0;
>>      RDMAControlHeader head = { .len = 0, .repeat = 1 };
>> +    RDMAContext *rdma = p->rdma;
>>  
>>      trace_multifd_send_thread_start(p->id);
>>      if (multifd_send_initial_packet(p, &local_err) < 0) {
>> @@ -4451,7 +4495,7 @@ static void *multifd_rdma_send_thread(void *opaque)
>>  
>>      /* wait for semaphore notification to register memory */
>>      qemu_sem_wait(&p->sem_sync);
>> -    if (qemu_rdma_registration(p->rdma) < 0) {
>> +    if (qemu_rdma_registration(rdma) < 0) {
>>          goto out;
>>      }
>>      /*
>> @@ -4466,12 +4510,25 @@ static void *multifd_rdma_send_thread(void *opaque)
>>                  break;
>>              }
>>          }
>> +        /* To complete polling(CQE) */
>> +        while (rdma->nb_sent) {
> 
> Where is nb_sent decremented?
> 
the nb_sent is decreased in qemu_rdma_poll which is called by qemu_rdma_block_for_wrid.

>> +            ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
>> +            if (ret < 0) {
>> +                error_report("multifd RDMA migration: "
>> +                             "complete polling error!");
>> +                return NULL;
>> +            }
>> +        }
>>          /* Send FINISHED to the destination */
>>          head.type = RDMA_CONTROL_REGISTER_FINISHED;
>> -        ret = qemu_rdma_exchange_send(p->rdma, &head, NULL, NULL, NULL, NULL);
>> +        ret = qemu_rdma_exchange_send(rdma, &head, NULL, NULL, NULL, NULL);
>>          if (ret < 0) {
>> +            error_report("multifd RDMA migration: "
>> +                         "sending remote error!");
>>              return NULL;
>>          }
>> +        /* sync main thread */
>> +        qemu_sem_post(&p->sem);
>>      }
>>  
>>  out:
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan

Re: [PATCH v4 18/18] migration/rdma: RDMA cleanup for multifd migration

[Zheng Chuan]

On 2021/2/4 18:32, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>>  migration/multifd.c |  6 ++++++
>>  migration/multifd.h |  1 +
>>  migration/rdma.c    | 16 +++++++++++++++-
>>  3 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index 1186246..4031648 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -577,6 +577,9 @@ void multifd_save_cleanup(void)
>>          p->packet_len = 0;
>>          g_free(p->packet);
>>          p->packet = NULL;
>> +#ifdef CONFIG_RDMA
>> +        multifd_rdma_cleanup(p->rdma);
>> +#endif
> 
> You may find it easier to add an entry into stubs/ for
> multifd_rdma_cleanup; it then avoids the need for the ifdef.
> 
OK. I will do that in V5.

>>          multifd_send_state->ops->send_cleanup(p, &local_err);
>>          if (local_err) {
>>              migrate_set_error(migrate_get_current(), local_err);
>> @@ -1039,6 +1042,9 @@ int multifd_load_cleanup(Error **errp)
>>          p->packet_len = 0;
>>          g_free(p->packet);
>>          p->packet = NULL;
>> +#ifdef CONFIG_RDMA
>> +        multifd_rdma_cleanup(p->rdma);
>> +#endif
>>          multifd_recv_state->ops->recv_cleanup(p);
>>      }
>>      qemu_sem_destroy(&multifd_recv_state->sem_sync);
>> diff --git a/migration/multifd.h b/migration/multifd.h
>> index 26d4489..0ecec5e 100644
>> --- a/migration/multifd.h
>> +++ b/migration/multifd.h
>> @@ -183,6 +183,7 @@ typedef struct {
>>  
>>  #ifdef CONFIG_RDMA
>>  extern MultiFDSetup multifd_rdma_ops;
>> +void multifd_rdma_cleanup(void *opaque);
>>  #endif
>>  void multifd_send_terminate_threads(Error *err);
>>  int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp);
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index c19a91f..f14357f 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -2369,7 +2369,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
>>  {
>>      int idx;
>>  
>> -    if (rdma->cm_id && rdma->connected) {
>> +    if (rdma->channel && rdma->cm_id && rdma->connected) {
>>          if ((rdma->error_state ||
>>               migrate_get_current()->state == MIGRATION_STATUS_CANCELLING) &&
>>              !rdma->received_error) {
>> @@ -4599,6 +4599,20 @@ static void multifd_rdma_recv_channel_setup(QIOChannel *ioc,
>>      return;
>>  }
>>  
>> +void multifd_rdma_cleanup(void *opaque)
> 
> I think you need to make it clear that this is only to cleanup one
> channel, rather than the whole multifd-rdma connection;
> multifd_load_cleanup for example cleans up all the channels, where as I
> think this is only doing one?
> 
Yes, the multifd_load_cleanup cleans up all the channels with the iteration of migrate_multifd_channels.
Now, we just put multifd_rdma_cleanup into that iteration of multifd_load_cleanup to clear it one by one.
do you mean the name of multifd_rdma_cleanup is misleading and should changed it in order to distinguish with  multifd_load_cleanup?

> Don't use a 'void *opaque' except for something that's called via
> a registration/callback scheme that's designed to be generic
> (e.g. multifd_send_thread does it because it's called from
> qemu_thread_create that doesn't know the type).  Where you know
> the type, use it!
> 
I agree with you.
I will do that in V5 with typedefs.h.
>> +{
>> +    RDMAContext *rdma = (RDMAContext *)opaque;
>> +
>> +    if (!migrate_use_rdma()) {
>> +        return;
>> +    }
>> +
>> +    rdma->listen_id = NULL;
>> +    rdma->channel = NULL;
>> +    qemu_rdma_cleanup(rdma);
>> +    g_free(rdma);
>> +}
>> +
>>  MultiFDSetup multifd_rdma_ops = {
>>      .send_thread = multifd_rdma_send_thread,
>>      .recv_thread = multifd_rdma_recv_thread,
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan