diff mbox series

[v3,04/18] migration/rdma: add multifd_setup_ops for rdma

Message ID 1602908748-43335-5-git-send-email-zhengchuan@huawei.com
State New
Headers show
Series Support Multifd for RDMA migration | expand

Commit Message

Zheng Chuan Oct. 17, 2020, 4:25 a.m. UTC 
Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/multifd.c |  6 ++++
 migration/multifd.h |  4 +++
 migration/rdma.c    | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+)

Comments

Dr. David Alan Gilbert Nov. 10, 2020, 12:30 p.m. UTC | #1 
* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>

> ---

>  migration/multifd.c |  6 ++++

>  migration/multifd.h |  4 +++

>  migration/rdma.c    | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++

>  3 files changed, 92 insertions(+)

> 

> diff --git a/migration/multifd.c b/migration/multifd.c

> index 1f82307..0d494df 100644

> --- a/migration/multifd.c

> +++ b/migration/multifd.c

> @@ -1210,6 +1210,12 @@ MultiFDSetup *multifd_setup_ops_init(void)

>  {

>      MultiFDSetup *ops;

>  

> +#ifdef CONFIG_RDMA

> +    if (migrate_use_rdma()) {

> +        ops = multifd_rdma_setup();

> +        return ops;

> +    }

> +#endif

>      ops = &multifd_socket_ops;

>      return ops;

>  }

> diff --git a/migration/multifd.h b/migration/multifd.h

> index 446315b..62a0b2a 100644

> --- a/migration/multifd.h

> +++ b/migration/multifd.h

> @@ -173,6 +173,10 @@ typedef struct {

>      void (*recv_channel_setup)(QIOChannel *ioc, MultiFDRecvParams *p);

>  } MultiFDSetup;

>  

> +#ifdef CONFIG_RDMA

> +MultiFDSetup *multifd_rdma_setup(void);

> +#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 0340841..ad4e4ba 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"

> @@ -4138,3 +4139,84 @@ err:

>      g_free(rdma);

>      g_free(rdma_return_path);

>  }

> +

> +static void *multifd_rdma_send_thread(void *opaque)

> +{

> +    MultiFDSendParams *p = opaque;

> +

> +    while (true) {

> +        qemu_mutex_lock(&p->mutex);

> +        if (p->quit) {

> +            qemu_mutex_unlock(&p->mutex);

> +            break;

> +        }

> +        qemu_mutex_unlock(&p->mutex);

> +        qemu_sem_wait(&p->sem);

> +    }

> +

> +    qemu_mutex_lock(&p->mutex);

> +    p->running = false;

> +    qemu_mutex_unlock(&p->mutex);

> +

> +    return NULL;

> +}


You might like to consider using WITH_QEMU_LOCK_GUARD, I think that
would become:

  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;
  }

> +

> +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) {

> +        qemu_mutex_lock(&p->mutex);

> +        if (p->quit) {

> +            qemu_mutex_unlock(&p->mutex);

> +            break;

> +        }

> +        qemu_mutex_unlock(&p->mutex);

> +        qemu_sem_wait(&p->sem_sync);

> +    }

> +

> +    qemu_mutex_lock(&p->mutex);

> +    p->running = false;

> +    qemu_mutex_unlock(&p->mutex);

> +

> +    return NULL;

> +}

> +

> +static void multifd_rdma_recv_channel_setup(QIOChannel *ioc,

> +                                            MultiFDRecvParams *p)

> +{

> +    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);

> +

> +    p->file = rioc->file;

> +    return;

> +}

> +

> +static MultiFDSetup multifd_rdma_ops = {

> +    .send_thread_setup = multifd_rdma_send_thread,

> +    .recv_thread_setup = multifd_rdma_recv_thread,

> +    .send_channel_setup = multifd_rdma_send_channel_setup,

> +    .recv_channel_setup = multifd_rdma_recv_channel_setup

> +};

> +

> +MultiFDSetup *multifd_rdma_setup(void)

> +{

> +    MultiFDSetup *rdma_ops;

> +

> +    rdma_ops = &multifd_rdma_ops;

> +

> +    return rdma_ops;


Why bother making this a function - just export multifd_rdma_ops ?

Dave

> +}

> -- 

> 1.8.3.1

> 

-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Zheng Chuan Nov. 11, 2020, 7:56 a.m. UTC | #2 
On 2020/11/10 20:30, 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 |  4 +++

>>  migration/rdma.c    | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++

>>  3 files changed, 92 insertions(+)

>>

>> diff --git a/migration/multifd.c b/migration/multifd.c

>> index 1f82307..0d494df 100644

>> --- a/migration/multifd.c

>> +++ b/migration/multifd.c

>> @@ -1210,6 +1210,12 @@ MultiFDSetup *multifd_setup_ops_init(void)

>>  {

>>      MultiFDSetup *ops;

>>  

>> +#ifdef CONFIG_RDMA

>> +    if (migrate_use_rdma()) {

>> +        ops = multifd_rdma_setup();

>> +        return ops;

>> +    }

>> +#endif

>>      ops = &multifd_socket_ops;

>>      return ops;

>>  }

>> diff --git a/migration/multifd.h b/migration/multifd.h

>> index 446315b..62a0b2a 100644

>> --- a/migration/multifd.h

>> +++ b/migration/multifd.h

>> @@ -173,6 +173,10 @@ typedef struct {

>>      void (*recv_channel_setup)(QIOChannel *ioc, MultiFDRecvParams *p);

>>  } MultiFDSetup;

>>  

>> +#ifdef CONFIG_RDMA

>> +MultiFDSetup *multifd_rdma_setup(void);

>> +#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 0340841..ad4e4ba 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"

>> @@ -4138,3 +4139,84 @@ err:

>>      g_free(rdma);

>>      g_free(rdma_return_path);

>>  }

>> +

>> +static void *multifd_rdma_send_thread(void *opaque)

>> +{

>> +    MultiFDSendParams *p = opaque;

>> +

>> +    while (true) {

>> +        qemu_mutex_lock(&p->mutex);

>> +        if (p->quit) {

>> +            qemu_mutex_unlock(&p->mutex);

>> +            break;

>> +        }

>> +        qemu_mutex_unlock(&p->mutex);

>> +        qemu_sem_wait(&p->sem);

>> +    }

>> +

>> +    qemu_mutex_lock(&p->mutex);

>> +    p->running = false;

>> +    qemu_mutex_unlock(&p->mutex);

>> +

>> +    return NULL;

>> +}

> 

> You might like to consider using WITH_QEMU_LOCK_GUARD, I think that

> would become:

> 

>   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;

>   }

> 

OK. and this remind me now we keep qemu_mutex_lock(&p->mutex); in our multifd code, it that should also optimized?
>> +

>> +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) {

>> +        qemu_mutex_lock(&p->mutex);

>> +        if (p->quit) {

>> +            qemu_mutex_unlock(&p->mutex);

>> +            break;

>> +        }

>> +        qemu_mutex_unlock(&p->mutex);

>> +        qemu_sem_wait(&p->sem_sync);

>> +    }

>> +

>> +    qemu_mutex_lock(&p->mutex);

>> +    p->running = false;

>> +    qemu_mutex_unlock(&p->mutex);

>> +

>> +    return NULL;

>> +}

>> +

>> +static void multifd_rdma_recv_channel_setup(QIOChannel *ioc,

>> +                                            MultiFDRecvParams *p)

>> +{

>> +    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);

>> +

>> +    p->file = rioc->file;

>> +    return;

>> +}

>> +

>> +static MultiFDSetup multifd_rdma_ops = {

>> +    .send_thread_setup = multifd_rdma_send_thread,

>> +    .recv_thread_setup = multifd_rdma_recv_thread,

>> +    .send_channel_setup = multifd_rdma_send_channel_setup,

>> +    .recv_channel_setup = multifd_rdma_recv_channel_setup

>> +};

>> +

>> +MultiFDSetup *multifd_rdma_setup(void)

>> +{

>> +    MultiFDSetup *rdma_ops;

>> +

>> +    rdma_ops = &multifd_rdma_ops;

>> +

>> +    return rdma_ops;

> 

> Why bother making this a function - just export multifd_rdma_ops ?

> 

> Dave

> 

OK, will consider in that way.

>> +}

>> -- 

>> 1.8.3.1

>>


-- 
Regards.
Chuan
diff mbox series

Patch

diff --git a/migration/multifd.c b/migration/multifd.c
index 1f82307..0d494df 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -1210,6 +1210,12 @@  MultiFDSetup *multifd_setup_ops_init(void)
 {
     MultiFDSetup *ops;
 
+#ifdef CONFIG_RDMA
+    if (migrate_use_rdma()) {
+        ops = multifd_rdma_setup();
+        return ops;
+    }
+#endif
     ops = &multifd_socket_ops;
     return ops;
 }
diff --git a/migration/multifd.h b/migration/multifd.h
index 446315b..62a0b2a 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -173,6 +173,10 @@  typedef struct {
     void (*recv_channel_setup)(QIOChannel *ioc, MultiFDRecvParams *p);
 } MultiFDSetup;
 
+#ifdef CONFIG_RDMA
+MultiFDSetup *multifd_rdma_setup(void);
+#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 0340841..ad4e4ba 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"
@@ -4138,3 +4139,84 @@  err:
     g_free(rdma);
     g_free(rdma_return_path);
 }
+
+static void *multifd_rdma_send_thread(void *opaque)
+{
+    MultiFDSendParams *p = opaque;
+
+    while (true) {
+        qemu_mutex_lock(&p->mutex);
+        if (p->quit) {
+            qemu_mutex_unlock(&p->mutex);
+            break;
+        }
+        qemu_mutex_unlock(&p->mutex);
+        qemu_sem_wait(&p->sem);
+    }
+
+    qemu_mutex_lock(&p->mutex);
+    p->running = false;
+    qemu_mutex_unlock(&p->mutex);
+
+    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) {
+        qemu_mutex_lock(&p->mutex);
+        if (p->quit) {
+            qemu_mutex_unlock(&p->mutex);
+            break;
+        }
+        qemu_mutex_unlock(&p->mutex);
+        qemu_sem_wait(&p->sem_sync);
+    }
+
+    qemu_mutex_lock(&p->mutex);
+    p->running = false;
+    qemu_mutex_unlock(&p->mutex);
+
+    return NULL;
+}
+
+static void multifd_rdma_recv_channel_setup(QIOChannel *ioc,
+                                            MultiFDRecvParams *p)
+{
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
+
+    p->file = rioc->file;
+    return;
+}
+
+static MultiFDSetup multifd_rdma_ops = {
+    .send_thread_setup = multifd_rdma_send_thread,
+    .recv_thread_setup = multifd_rdma_recv_thread,
+    .send_channel_setup = multifd_rdma_send_channel_setup,
+    .recv_channel_setup = multifd_rdma_recv_channel_setup
+};
+
+MultiFDSetup *multifd_rdma_setup(void)
+{
+    MultiFDSetup *rdma_ops;
+
+    rdma_ops = &multifd_rdma_ops;
+
+    return rdma_ops;
+}