Re: [PATCH v2 10/17] migration/multifd: Convert multifd_send()::next_channel to atomic

[Peter Xu <peterx@redhat.com>]

On Tue, Aug 27, 2024 at 07:54:29PM +0200, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> 
> This is necessary for multifd_send() to be able to be called
> from multiple threads.
> 
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>

Would it be much simpler to just use a mutex for enqueue?

Something like:

===8<===
diff --git a/migration/multifd.c b/migration/multifd.c
index 9b200f4ad9..979c9748b5 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -69,6 +69,8 @@ struct {
     QemuSemaphore channels_created;
     /* send channels ready */
     QemuSemaphore channels_ready;
+    /* Mutex to serialize multifd enqueues */
+    QemuMutex multifd_send_mutex;
     /*
      * Have we already run terminate threads.  There is a race when it
      * happens that we got one error while we are exiting.
@@ -305,6 +307,8 @@ bool multifd_send(MultiFDSendData **send_data)
     MultiFDSendParams *p = NULL; /* make happy gcc */
     MultiFDSendData *tmp;
 
+    QEMU_LOCK_GUARD(&multifd_send_mutex);
+
     if (multifd_send_should_exit()) {
         return false;
     }
@@ -824,6 +828,7 @@ bool multifd_send_setup(void)
     multifd_send_state->params = g_new0(MultiFDSendParams, thread_count);
     qemu_sem_init(&multifd_send_state->channels_created, 0);
     qemu_sem_init(&multifd_send_state->channels_ready, 0);
+    qemu_mutex_init(&multifd_send_state->multifd_send_mutex);
     qatomic_set(&multifd_send_state->exiting, 0);
     multifd_send_state->ops = multifd_ops[migrate_multifd_compression()];
===8<===

Then all the details doesn't need change (meanwhile the perf should be
similar)?

-- 
Peter Xu