diff mbox series

[RFC,v2,2/3] virtio: support delay of checks in virtio_load()

Message ID 20221212164942.3614611-3-xuchuangxclwt@bytedance.com (mailing list archive)
State New, archived
Headers show
Series migration: reduce time of loading non-iterable vmstate | expand

Commit Message

Chuang Xu Dec. 12, 2022, 4:49 p.m. UTC
Delay checks in virtio_load() to avoid possible address_space_to_flatview() call
during memory region's begin/commit.

Signed-off-by: Chuang Xu <xuchuangxclwt@bytedance.com>
---
 hw/virtio/virtio.c      | 33 ++++++++++++++++++++++-----------
 include/sysemu/sysemu.h |  1 +
 softmmu/globals.c       |  3 +++
 3 files changed, 26 insertions(+), 11 deletions(-)

Comments

Peter Xu Dec. 12, 2022, 8:18 p.m. UTC | #1
On Tue, Dec 13, 2022 at 12:49:41AM +0800, Chuang Xu wrote:
> +bool migration_enable_load_check_delay;

I'm just afraid this is still too hacky.

One thing is because this variable itself to be only set at specific phase
during migration to cover that commit().  The other thing is I'm not sure
we can always rely on the commit() being happen 100% - what if there's no
memory layout changes throughout the whole process of vm load?  That'll be
skipped if memory_region_update_pending==false as I said.

So far the best I can come up with is we allow each virtio device to
register a vm state change handler (during virtio_load) to do the rest,
then in the handler it unregisters itself so it only runs once right before
the VM starts.  But I'm not sure whether the virtio developers will be
happy with it.  Maybe worth a try.

Feel free to have a look at like kvmvapic_vm_state_change() if you think
that idea worth exploring.
Chuang Xu Dec. 13, 2022, 12:21 p.m. UTC | #2
On 2022/12/13 上午4:18, Peter Xu wrote:

On Tue, Dec 13, 2022 at 12:49:41AM +0800, Chuang Xu wrote:

+bool migration_enable_load_check_delay;

I'm just afraid this is still too hacky.

One thing is because this variable itself to be only set at specific phase
during migration to cover that commit().  The other thing is I'm not sure
we can always rely on the commit() being happen 100% - what if there's no
memory layout changes throughout the whole process of vm load?  That'll be
skipped if memory_region_update_pending==false as I said.

Yes, you're right. I wanted to set memory_region_update_pending to true at
the beginning of qemu_loadvm_state_main(), but somehow I forgot this detail..
diff mbox series

Patch

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index eb6347ab5d..3e3fa2a89d 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -33,6 +33,7 @@ 
 #include "hw/virtio/virtio-access.h"
 #include "sysemu/dma.h"
 #include "sysemu/runstate.h"
+#include "sysemu/sysemu.h"
 #include "standard-headers/linux/virtio_ids.h"
 #include "standard-headers/linux/vhost_types.h"
 #include "standard-headers/linux/virtio_blk.h"
@@ -3642,8 +3643,20 @@  int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
         vdev->start_on_kick = true;
     }
 
+    if (vdc->post_load) {
+        ret = vdc->post_load(vdev);
+        if (ret) {
+            return ret;
+        }
+    }
+
+    return 0;
+}
+
+static void virtio_load_check_delay(VirtIODevice *vdev)
+{
     RCU_READ_LOCK_GUARD();
-    for (i = 0; i < num; i++) {
+    for (int i = 0; i < VIRTIO_QUEUE_MAX; i++) {
         if (vdev->vq[i].vring.desc) {
             uint16_t nheads;
 
@@ -3696,19 +3709,12 @@  int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
                              i, vdev->vq[i].vring.num,
                              vdev->vq[i].last_avail_idx,
                              vdev->vq[i].used_idx);
-                return -1;
+                abort();
             }
         }
     }
 
-    if (vdc->post_load) {
-        ret = vdc->post_load(vdev);
-        if (ret) {
-            return ret;
-        }
-    }
-
-    return 0;
+    return;
 }
 
 void virtio_cleanup(VirtIODevice *vdev)
@@ -4158,7 +4164,12 @@  static void virtio_memory_listener_commit(MemoryListener *listener)
         if (vdev->vq[i].vring.num == 0) {
             break;
         }
-        virtio_init_region_cache(vdev, i);
+
+        if (migration_enable_load_check_delay) {
+            virtio_load_check_delay(vdev);
+        } else {
+            virtio_init_region_cache(vdev, i);
+        }
     }
 }
 
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 6a7a31e64d..0523091445 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -12,6 +12,7 @@  extern int only_migratable;
 extern const char *qemu_name;
 extern QemuUUID qemu_uuid;
 extern bool qemu_uuid_set;
+extern bool migration_enable_load_check_delay;
 
 const char *qemu_get_vm_name(void);
 
diff --git a/softmmu/globals.c b/softmmu/globals.c
index 527edbefdd..1bd8f6c978 100644
--- a/softmmu/globals.c
+++ b/softmmu/globals.c
@@ -65,3 +65,6 @@  bool qemu_uuid_set;
 uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_EMULATE;
 bool xen_domid_restrict;
+
+bool migration_enable_load_check_delay;
+