- * [PATCH v4 01/18] migration/rdma: add the 'migrate_rdma_pin_all' function
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03  8:01 ` [PATCH v4 02/18] migration/rdma: judge whether or not the RDMA is used for migration Chuan Zheng
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * [PATCH v4 02/18] migration/rdma: judge whether or not the RDMA is used for migration
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
  2021-02-03  8:01 ` [PATCH v4 01/18] migration/rdma: add the 'migrate_rdma_pin_all' function Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 17:49   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 03/18] migration/rdma: create multifd_setup_ops for Tx/Rx thread Chuan Zheng
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 02/18] migration/rdma: judge whether or not the RDMA is used for migration
  2021-02-03  8:01 ` [PATCH v4 02/18] migration/rdma: judge whether or not the RDMA is used for migration Chuan Zheng
@ 2021-02-03 17:49   ` Dr. David Alan Gilbert
  2021-03-01 12:25     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 17:49 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 02/18] migration/rdma: judge whether or not the RDMA is used for migration
  2021-02-03 17:49   ` Dr. David Alan Gilbert
@ 2021-03-01 12:25     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-01 12:25 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 03/18] migration/rdma: create multifd_setup_ops for Tx/Rx thread
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
  2021-02-03  8:01 ` [PATCH v4 01/18] migration/rdma: add the 'migrate_rdma_pin_all' function Chuan Zheng
  2021-02-03  8:01 ` [PATCH v4 02/18] migration/rdma: judge whether or not the RDMA is used for migration Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03  8:01 ` [PATCH v4 04/18] migration/rdma: add multifd_setup_ops for rdma Chuan Zheng
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * [PATCH v4 04/18] migration/rdma: add multifd_setup_ops for rdma
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (2 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 03/18] migration/rdma: create multifd_setup_ops for Tx/Rx thread Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 17:58   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 05/18] migration/rdma: do not need sync main " Chuan Zheng
                   ` (13 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 04/18] migration/rdma: add multifd_setup_ops for rdma
  2021-02-03  8:01 ` [PATCH v4 04/18] migration/rdma: add multifd_setup_ops for rdma Chuan Zheng
@ 2021-02-03 17:58   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 17:58 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
- * [PATCH v4 05/18] migration/rdma: do not need sync main for rdma
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (3 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 04/18] migration/rdma: add multifd_setup_ops for rdma Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 18:10   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 06/18] migration/rdma: export MultiFDSendParams/MultiFDRecvParams Chuan Zheng
                   ` (12 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 05/18] migration/rdma: do not need sync main for rdma
  2021-02-03  8:01 ` [PATCH v4 05/18] migration/rdma: do not need sync main " Chuan Zheng
@ 2021-02-03 18:10   ` Dr. David Alan Gilbert
  2021-03-06  8:45     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 18:10 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 05/18] migration/rdma: do not need sync main for rdma
  2021-02-03 18:10   ` Dr. David Alan Gilbert
@ 2021-03-06  8:45     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-06  8:45 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 06/18] migration/rdma: export MultiFDSendParams/MultiFDRecvParams
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (4 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 05/18] migration/rdma: do not need sync main " Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 18:23   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 07/18] migration/rdma: add rdma field into multifd send/recv param Chuan Zheng
                   ` (11 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 06/18] migration/rdma: export MultiFDSendParams/MultiFDRecvParams
  2021-02-03  8:01 ` [PATCH v4 06/18] migration/rdma: export MultiFDSendParams/MultiFDRecvParams Chuan Zheng
@ 2021-02-03 18:23   ` Dr. David Alan Gilbert
  2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 18:23 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 06/18] migration/rdma: export MultiFDSendParams/MultiFDRecvParams
  2021-02-03 18:23   ` Dr. David Alan Gilbert
@ 2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-01 12:26 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 07/18] migration/rdma: add rdma field into multifd send/recv param
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (5 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 06/18] migration/rdma: export MultiFDSendParams/MultiFDRecvParams Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 18:32   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 08/18] migration/rdma: export getQIOChannel to get QIOchannel in rdma Chuan Zheng
                   ` (10 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 07/18] migration/rdma: add rdma field into multifd send/recv param
  2021-02-03  8:01 ` [PATCH v4 07/18] migration/rdma: add rdma field into multifd send/recv param Chuan Zheng
@ 2021-02-03 18:32   ` Dr. David Alan Gilbert
  2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 18:32 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 07/18] migration/rdma: add rdma field into multifd send/recv param
  2021-02-03 18:32   ` Dr. David Alan Gilbert
@ 2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-01 12:26 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 08/18] migration/rdma: export getQIOChannel to get QIOchannel in rdma
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (6 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 07/18] migration/rdma: add rdma field into multifd send/recv param Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 18:49   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 09/18] migration/rdma: add multifd_rdma_load_setup() to setup multifd rdma Chuan Zheng
                   ` (9 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 08/18] migration/rdma: export getQIOChannel to get QIOchannel in rdma
  2021-02-03  8:01 ` [PATCH v4 08/18] migration/rdma: export getQIOChannel to get QIOchannel in rdma Chuan Zheng
@ 2021-02-03 18:49   ` Dr. David Alan Gilbert
  2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 18:49 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 08/18] migration/rdma: export getQIOChannel to get QIOchannel in rdma
  2021-02-03 18:49   ` Dr. David Alan Gilbert
@ 2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-01 12:26 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 09/18] migration/rdma: add multifd_rdma_load_setup() to setup multifd rdma
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (7 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 08/18] migration/rdma: export getQIOChannel to get QIOchannel in rdma Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03  8:01 ` [PATCH v4 10/18] migration/rdma: Create the multifd recv channels for RDMA Chuan Zheng
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * [PATCH v4 10/18] migration/rdma: Create the multifd recv channels for RDMA
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (8 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 09/18] migration/rdma: add multifd_rdma_load_setup() to setup multifd rdma Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 18:59   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 11/18] migration/rdma: record host_port for multifd RDMA Chuan Zheng
                   ` (7 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 10/18] migration/rdma: Create the multifd recv channels for RDMA
  2021-02-03  8:01 ` [PATCH v4 10/18] migration/rdma: Create the multifd recv channels for RDMA Chuan Zheng
@ 2021-02-03 18:59   ` Dr. David Alan Gilbert
  2021-03-06  8:45     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 18:59 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 10/18] migration/rdma: Create the multifd recv channels for RDMA
  2021-02-03 18:59   ` Dr. David Alan Gilbert
@ 2021-03-06  8:45     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-06  8:45 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 11/18] migration/rdma: record host_port for multifd RDMA
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (9 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 10/18] migration/rdma: Create the multifd recv channels for RDMA Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 19:04   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 12/18] migration/rdma: Create the multifd send channels for RDMA Chuan Zheng
                   ` (6 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 11/18] migration/rdma: record host_port for multifd RDMA
  2021-02-03  8:01 ` [PATCH v4 11/18] migration/rdma: record host_port for multifd RDMA Chuan Zheng
@ 2021-02-03 19:04   ` Dr. David Alan Gilbert
  2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 19:04 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 11/18] migration/rdma: record host_port for multifd RDMA
  2021-02-03 19:04   ` Dr. David Alan Gilbert
@ 2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-01 12:26 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 12/18] migration/rdma: Create the multifd send channels for RDMA
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (10 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 11/18] migration/rdma: record host_port for multifd RDMA Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 19:52   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 13/18] migration/rdma: Add the function for dynamic page registration Chuan Zheng
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 12/18] migration/rdma: Create the multifd send channels for RDMA
  2021-02-03  8:01 ` [PATCH v4 12/18] migration/rdma: Create the multifd send channels for RDMA Chuan Zheng
@ 2021-02-03 19:52   ` Dr. David Alan Gilbert
  2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 19:52 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 12/18] migration/rdma: Create the multifd send channels for RDMA
  2021-02-03 19:52   ` Dr. David Alan Gilbert
@ 2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-01 12:26 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 13/18] migration/rdma: Add the function for dynamic page registration
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (11 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 12/18] migration/rdma: Create the multifd send channels for RDMA Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 20:06   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 14/18] migration/rdma: register memory for multifd RDMA channels Chuan Zheng
                   ` (4 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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,
+            ®_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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 13/18] migration/rdma: Add the function for dynamic page registration
  2021-02-03  8:01 ` [PATCH v4 13/18] migration/rdma: Add the function for dynamic page registration Chuan Zheng
@ 2021-02-03 20:06   ` Dr. David Alan Gilbert
  2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 20:06 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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,
> +            ®_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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 13/18] migration/rdma: Add the function for dynamic page registration
  2021-02-03 20:06   ` Dr. David Alan Gilbert
@ 2021-03-01 12:26     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-01 12:26 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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,
>> +            ®_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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 14/18] migration/rdma: register memory for multifd RDMA channels
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (12 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 13/18] migration/rdma: Add the function for dynamic page registration Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 20:12   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 15/18] migration/rdma: only register the memory for multifd channels Chuan Zheng
                   ` (3 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 14/18] migration/rdma: register memory for multifd RDMA channels
  2021-02-03  8:01 ` [PATCH v4 14/18] migration/rdma: register memory for multifd RDMA channels Chuan Zheng
@ 2021-02-03 20:12   ` Dr. David Alan Gilbert
  2021-03-06  8:45     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 20:12 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 14/18] migration/rdma: register memory for multifd RDMA channels
  2021-02-03 20:12   ` Dr. David Alan Gilbert
@ 2021-03-06  8:45     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-06  8:45 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 15/18] migration/rdma: only register the memory for multifd channels
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (13 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 14/18] migration/rdma: register memory for multifd RDMA channels Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-04 10:09   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 16/18] migration/rdma: add rdma_channel into Migrationstate field Chuan Zheng
                   ` (2 subsequent siblings)
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 15/18] migration/rdma: only register the memory for multifd channels
  2021-02-03  8:01 ` [PATCH v4 15/18] migration/rdma: only register the memory for multifd channels Chuan Zheng
@ 2021-02-04 10:09   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-04 10:09 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
- * [PATCH v4 16/18] migration/rdma: add rdma_channel into Migrationstate field
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (14 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 15/18] migration/rdma: only register the memory for multifd channels Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-03 20:19   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 17/18] migration/rdma: send data for both rdma-pin-all and NOT rdma-pin-all mode Chuan Zheng
  2021-02-03  8:01 ` [PATCH v4 18/18] migration/rdma: RDMA cleanup for multifd migration Chuan Zheng
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 16/18] migration/rdma: add rdma_channel into Migrationstate field
  2021-02-03  8:01 ` [PATCH v4 16/18] migration/rdma: add rdma_channel into Migrationstate field Chuan Zheng
@ 2021-02-03 20:19   ` Dr. David Alan Gilbert
  2021-03-01 12:27     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-03 20:19 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 16/18] migration/rdma: add rdma_channel into Migrationstate field
  2021-02-03 20:19   ` Dr. David Alan Gilbert
@ 2021-03-01 12:27     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-01 12:27 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 17/18] migration/rdma: send data for both rdma-pin-all and NOT rdma-pin-all mode
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (15 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 16/18] migration/rdma: add rdma_channel into Migrationstate field Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-04 10:18   ` Dr. David Alan Gilbert
  2021-02-03  8:01 ` [PATCH v4 18/18] migration/rdma: RDMA cleanup for multifd migration Chuan Zheng
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 17/18] migration/rdma: send data for both rdma-pin-all and NOT rdma-pin-all mode
  2021-02-03  8:01 ` [PATCH v4 17/18] migration/rdma: send data for both rdma-pin-all and NOT rdma-pin-all mode Chuan Zheng
@ 2021-02-04 10:18   ` Dr. David Alan Gilbert
  2021-03-06  8:45     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-04 10:18 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 17/18] migration/rdma: send data for both rdma-pin-all and NOT rdma-pin-all mode
  2021-02-04 10:18   ` Dr. David Alan Gilbert
@ 2021-03-06  8:45     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-06  8:45 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread
 
 
- * [PATCH v4 18/18] migration/rdma: RDMA cleanup for multifd migration
  2021-02-03  8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
                   ` (16 preceding siblings ...)
  2021-02-03  8:01 ` [PATCH v4 17/18] migration/rdma: send data for both rdma-pin-all and NOT rdma-pin-all mode Chuan Zheng
@ 2021-02-03  8:01 ` Chuan Zheng
  2021-02-04 10:32   ` Dr. David Alan Gilbert
  17 siblings, 1 reply; 47+ messages in thread
From: Chuan Zheng @ 2021-02-03  8:01 UTC (permalink / raw)
  To: quintela, dgilbert, berrange
  Cc: yubihong, zhang.zhanghailiang, qemu-devel, xiexiangyou, alex.chen,
	wanghao232
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
^ permalink raw reply related	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 18/18] migration/rdma: RDMA cleanup for multifd migration
  2021-02-03  8:01 ` [PATCH v4 18/18] migration/rdma: RDMA cleanup for multifd migration Chuan Zheng
@ 2021-02-04 10:32   ` Dr. David Alan Gilbert
  2021-03-06  8:45     ` Zheng Chuan
  0 siblings, 1 reply; 47+ messages in thread
From: Dr. David Alan Gilbert @ 2021-02-04 10:32 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
* 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
^ permalink raw reply	[flat|nested] 47+ messages in thread
- * Re: [PATCH v4 18/18] migration/rdma: RDMA cleanup for multifd migration
  2021-02-04 10:32   ` Dr. David Alan Gilbert
@ 2021-03-06  8:45     ` Zheng Chuan
  0 siblings, 0 replies; 47+ messages in thread
From: Zheng Chuan @ 2021-03-06  8:45 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: yubihong, berrange, zhang.zhanghailiang, quintela, qemu-devel,
	xiexiangyou, alex.chen, wanghao232
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
^ permalink raw reply	[flat|nested] 47+ messages in thread