[PATCH 2/3] migration/postcopy: Factor out uffd_setup_incoming() from postcopy setup

Takeru Hayasaka posted 3 patches 2 days, 6 hours ago
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>
[PATCH 2/3] migration/postcopy: Factor out uffd_setup_incoming() from postcopy setup
Posted by Takeru Hayasaka 2 days, 6 hours ago
Extract the generic userfaultfd initialization from
postcopy_ram_incoming_setup() into a new uffd_setup_incoming() function
that can be shared between postcopy live migration and fast snapshot
load.

uffd_setup_incoming() handles:
- Opening the userfaultfd and API handshake
- Creating the eventfd for quit notification
- Setting the page fault handler callback
- Starting the fault handler thread
- Registering all RAM blocks with UFFDIO_REGISTER
- Allocating temporary page buffers for UFFDIO_COPY

postcopy_ram_incoming_setup() becomes a thin wrapper that adds
postcopy-specific logic: blocktime tracking and the preemption thread.

Also move the PostcopyPageHandler typedef from migration.h to
postcopy-ram.h where it logically belongs and is needed for the
uffd_setup_incoming() declaration.

The channels parameter of postcopy_temp_pages_setup() is now passed
by the caller instead of being determined internally, so
uffd_setup_incoming() callers can control how many temporary page
buffers are allocated.

No functional change for postcopy migration.

Signed-off-by: Takeru Hayasaka <hayatake396@gmail.com>
---
 migration/migration.h    | 11 --------
 migration/postcopy-ram.c | 66 +++++++++++++++++++++++++++++++++---------------
 migration/postcopy-ram.h | 27 ++++++++++++++++++++
 3 files changed, 73 insertions(+), 31 deletions(-)

diff --git a/migration/migration.h b/migration/migration.h
index 33525402922d..43ea32c07ba7 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -89,17 +89,6 @@ typedef enum {
     PREEMPT_THREAD_QUIT,
 } PreemptThreadStatus;
 
-/*
- * Callback to handle a page fault from the userfaultfd fault thread.
- * Implementations resolve the fault by supplying the requested page,
- * e.g., by requesting it from the migration source (postcopy) or by
- * reading it from a snapshot file (fast snapshot load).
- */
-typedef int (*PostcopyPageHandler)(MigrationIncomingState *mis,
-                                   RAMBlock *rb,
-                                   ram_addr_t offset,
-                                   void *fault_address);
-
 /* State for the incoming migration */
 struct MigrationIncomingState {
     QEMUFile *from_src_file;
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 8dcd8ff35e85..7af207d6ead5 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1478,22 +1478,15 @@ retry:
     return NULL;
 }
 
-static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
+static int postcopy_temp_pages_setup(MigrationIncomingState *mis,
+                                     unsigned int channels)
 {
     PostcopyTmpPage *tmp_page;
     int err;
-    unsigned i, channels;
+    unsigned i;
     void *temp_page;
 
-    if (migrate_postcopy_preempt()) {
-        /* If preemption enabled, need extra channel for urgent requests */
-        mis->postcopy_channels = RAM_CHANNEL_MAX;
-    } else {
-        /* Both precopy/postcopy on the same channel */
-        mis->postcopy_channels = 1;
-    }
-
-    channels = mis->postcopy_channels;
+    mis->postcopy_channels = channels;
     mis->postcopy_tmp_pages = g_new0(PostcopyTmpPage, channels);
 
     for (i = 0; i < channels; i++) {
@@ -1531,7 +1524,19 @@ static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
     return 0;
 }
 
-int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
+/*
+ * Generic userfaultfd setup for incoming migration.
+ *
+ * Opens the userfaultfd, performs the API handshake, registers all RAM
+ * blocks for missing-page notifications, allocates temporary page buffers,
+ * and starts the fault handler thread using the given callback.
+ *
+ * Both postcopy live migration and fast snapshot load use this function;
+ * the caller supplies the appropriate page_fault_handler.
+ */
+int uffd_setup_incoming(MigrationIncomingState *mis,
+                        PostcopyPageHandler handler,
+                        unsigned int channels)
 {
     Error *local_err = NULL;
 
@@ -1549,15 +1554,11 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
      */
     if (!ufd_check_and_apply(mis->userfault_fd, mis, &local_err)) {
         error_report_err(local_err);
+        close(mis->userfault_fd);
         return -1;
     }
 
-    if (migrate_postcopy_blocktime()) {
-        assert(mis->blocktime_ctx == NULL);
-        mis->blocktime_ctx = blocktime_context_new();
-    }
-
-    /* Now an eventfd we use to tell the fault-thread to quit */
+    /* An eventfd we use to tell the fault-thread to quit */
     mis->userfault_event_fd = eventfd(0, EFD_CLOEXEC);
     if (mis->userfault_event_fd == -1) {
         error_report("%s: Opening userfault_event_fd: %s", __func__,
@@ -1566,7 +1567,7 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
         return -1;
     }
 
-    mis->page_fault_handler = postcopy_page_fault_handler;
+    mis->page_fault_handler = handler;
 
     postcopy_thread_create(mis, &mis->fault_thread,
                            MIGRATION_THREAD_DST_FAULT,
@@ -1579,11 +1580,29 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
         return -1;
     }
 
-    if (postcopy_temp_pages_setup(mis)) {
+    if (postcopy_temp_pages_setup(mis, channels)) {
         /* Error dumped in the sub-function */
         return -1;
     }
 
+    return 0;
+}
+
+int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
+{
+    unsigned int channels;
+
+    if (migrate_postcopy_blocktime()) {
+        assert(mis->blocktime_ctx == NULL);
+        mis->blocktime_ctx = blocktime_context_new();
+    }
+
+    channels = migrate_postcopy_preempt() ? RAM_CHANNEL_MAX : 1;
+
+    if (uffd_setup_incoming(mis, postcopy_page_fault_handler, channels)) {
+        return -1;
+    }
+
     if (migrate_postcopy_preempt()) {
         /*
          * This thread needs to be created after the temp pages because
@@ -1745,6 +1764,13 @@ int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb,
     g_assert_not_reached();
 }
 
+int uffd_setup_incoming(MigrationIncomingState *mis,
+                        PostcopyPageHandler handler,
+                        unsigned int channels)
+{
+    g_assert_not_reached();
+}
+
 int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
 {
     g_assert_not_reached();
diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
index a080dd65a77a..3bbd1ed7e323 100644
--- a/migration/postcopy-ram.h
+++ b/migration/postcopy-ram.h
@@ -15,10 +15,37 @@
 
 #include "qapi/qapi-types-migration.h"
 
+/*
+ * Callback to handle a page fault from the userfaultfd fault thread.
+ * Implementations resolve the fault by supplying the requested page,
+ * e.g., by requesting it from the migration source (postcopy) or by
+ * reading it from a snapshot file (fast snapshot load).
+ */
+typedef int (*PostcopyPageHandler)(MigrationIncomingState *mis,
+                                   RAMBlock *rb,
+                                   ram_addr_t offset,
+                                   void *fault_address);
+
 /* Return true if the host supports everything we need to do postcopy-ram */
 bool postcopy_ram_supported_by_host(MigrationIncomingState *mis,
                                     Error **errp);
 
+/*
+ * Generic userfaultfd setup for incoming migration.
+ *
+ * Opens userfaultfd, registers all RAM blocks, allocates temporary
+ * page buffers and starts the fault handler thread.  The caller
+ * supplies the page_fault_handler callback appropriate for the use
+ * case (postcopy live migration or fast snapshot load).
+ *
+ * @channels: number of temporary page channels to allocate
+ *            (1 for most cases, RAM_CHANNEL_MAX when postcopy preempt
+ *            is enabled).
+ */
+int uffd_setup_incoming(MigrationIncomingState *mis,
+                        PostcopyPageHandler handler,
+                        unsigned int channels);
+
 /*
  * Make all of RAM sensitive to accesses to areas that haven't yet been written
  * and wire up anything necessary to deal with it.

-- 
2.43.0