Re: [PATCH RFC 09/15] migration: Add postcopy_thread_create()

[Peter Xu]

On Thu, Feb 03, 2022 at 03:19:48PM +0000, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > Postcopy create threads. A common manner is we init a sem and use it to sync
> > with the thread.  Namely, we have fault_thread_sem and listen_thread_sem and
> > they're only used for this.
> > 
> > Make it a shared infrastructure so it's easier to create yet another thread.
> > 
> 
> It might be worth a note saying you now share that sem, so you can't
> start two threads in parallel.

I'll squash this into the patch:

---8<---
diff --git a/migration/migration.h b/migration/migration.h
index 845be3463c..2a311fd8d6 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -72,7 +72,10 @@ struct MigrationIncomingState {
     /* A hook to allow cleanup at the end of incoming migration */
     void *transport_data;
     void (*transport_cleanup)(void *data);
-    /* Used to sync thread creations */
+    /*
+     * Used to sync thread creations.  Note that we can't create threads in
+     * parallel with this sem.
+     */
     QemuSemaphore  thread_sync_sem;
     /*
      * Free at the start of the main state load, set as the main thread 
finishes
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 099d8ed478..1a3ba1db84 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -79,6 +79,10 @@ int postcopy_notify(enum PostcopyNotifyReason reason, Error 
**errp)
                                             &pnd);
 }
 
+/*
+ * NOTE: this routine is not thread safe, we can't call it concurrently. But it
+ * should be good enough for migration's purposes.
+ */
 void postcopy_thread_create(MigrationIncomingState *mis,
                             QemuThread *thread, const char *name,
                             void *(*fn)(void *), int joinable)
---8<---

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

Thanks,

-- 
Peter Xu