[RFC v2 0/3] migration: reduce time of loading non-iterable vmstate

[RFC v2 0/3] migration: reduce time of loading non-iterable vmstate

[Chuang Xu]

Hi!

In this version:

- rebase to latest upstream.
- add sanity check to address_space_to_flatview().
- postpone the init of the vring cache until migration's loading completes. 

Please review, Chuang.

[v1]

The duration of loading non-iterable vmstate accounts for a significant
portion of downtime (starting with the timestamp of source qemu stop and
ending with the timestamp of target qemu start). Most of the time is spent
committing memory region changes repeatedly.

This patch packs all the changes to memory region during the period of
loading non-iterable vmstate in a single memory transaction. With the
increase of devices, this patch will greatly improve the performance.

Here are the test results:
test vm info:
- 32 CPUs 128GB RAM
- 8 16-queue vhost-net device
- 16 4-queue vhost-user-blk device.

	time of loading non-iterable vmstate
before		about 210 ms
after		about 40 ms

[RFC v2 1/3] memory: add depth assert in address_space_to_flatview

[Chuang Xu]

Before using any flatview, sanity check we're not during a memory
region transaction or the map can be invalid.

Signed-off-by: Chuang Xu <xuchuangxclwt@bytedance.com>
---
 include/exec/memory.h | 9 +++++++++
 softmmu/memory.c      | 1 -
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 91f8a2395a..b43cd46084 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1069,8 +1069,17 @@ struct FlatView {
     MemoryRegion *root;
 };
 
+static unsigned memory_region_transaction_depth;
+
 static inline FlatView *address_space_to_flatview(AddressSpace *as)
 {
+    /*
+     * Before using any flatview, sanity check we're not during a memory
+     * region transaction or the map can be invalid.  Note that this can
+     * also be called during commit phase of memory transaction, but that
+     * should also only happen when the depth decreases to 0 first.
+     */
+    assert(memory_region_transaction_depth == 0);
     return qatomic_rcu_read(&as->current_map);
 }
 
diff --git a/softmmu/memory.c b/softmmu/memory.c
index bc0be3f62c..f177c40cd8 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -37,7 +37,6 @@
 
 //#define DEBUG_UNASSIGNED
 
-static unsigned memory_region_transaction_depth;
 static bool memory_region_update_pending;
 static bool ioeventfd_update_pending;
 unsigned int global_dirty_tracking;
-- 
2.20.1

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

[Chuang Xu]

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(-)

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

[RFC v2 3/3] migration: reduce time of loading non-iterable vmstate

[Chuang Xu]

The duration of loading non-iterable vmstate accounts for a significant
portion of downtime (starting with the timestamp of source qemu stop and
ending with the timestamp of target qemu start). Most of the time is spent
committing memory region changes repeatedly.

This patch packs all the changes to memory region during the period of
loading non-iterable vmstate in a single memory transaction. With the
increase of devices, this patch will greatly improve the performance.

Here are the test results:
test vm info:
- 32 CPUs 128GB RAM
- 8 16-queue vhost-net device
- 16 4-queue vhost-user-blk device.

	time of loading non-iterable vmstate
before		about 210 ms
after		about 40 ms

Signed-off-by: Chuang Xu <xuchuangxclwt@bytedance.com>
---
 migration/savevm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index a0cdb714f7..68a7a99b79 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2617,6 +2617,9 @@ int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
     uint8_t section_type;
     int ret = 0;
 
+    /* call memory_region_transaction_begin() before loading vmstate */
+    memory_region_transaction_begin();
+
 retry:
     while (true) {
         section_type = qemu_get_byte(f);
@@ -2684,6 +2687,16 @@ out:
             goto retry;
         }
     }
+
+    /*
+     * call memory_region_transaction_commit() after loading non-iterable
+     * vmstate, make sure the migration_enable_load_check_delay flag is
+     * true during commit.
+     */
+    migration_enable_load_check_delay = true;
+    memory_region_transaction_commit();
+    migration_enable_load_check_delay = false;
+
     return ret;
 }
 
-- 
2.20.1

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

[Peter Xu]

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.

-- 
Peter Xu

Re: [RFC v2 0/3] migration: reduce time of loading non-iterable vmstate

[Peter Xu]

On Tue, Dec 13, 2022 at 12:49:39AM +0800, Chuang Xu wrote:
> 
> Hi!

Chuang,

> 
> In this version:
> 
> - rebase to latest upstream.
> - add sanity check to address_space_to_flatview().
> - postpone the init of the vring cache until migration's loading completes. 

Since there'll be other changes besides migration, please consider also
copy the relevant maintainers too on either memory and virtio in your next
post:

$ ./scripts/get_maintainer.pl -f softmmu/memory.c -f hw/virtio/virtio.c
Paolo Bonzini <pbonzini@redhat.com> (supporter:Memory API)
Peter Xu <peterx@redhat.com> (supporter:Memory API)
David Hildenbrand <david@redhat.com> (supporter:Memory API)
"Philippe Mathieu-Daudé" <philmd@linaro.org> (reviewer:Memory API)
"Michael S. Tsirkin" <mst@redhat.com> (supporter:virtio)
qemu-devel@nongnu.org (open list:All patches CC here)

-- 
Peter Xu

Re: [RFC v2 0/3] migration: reduce time of loading non-iterable vmstate

[Chuang Xu]

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]

On 2022/12/13 上午4:23, Peter Xu wrote:

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

Hi!

Chuang,


In this version:

- rebase to latest upstream.
- add sanity check to address_space_to_flatview().
- postpone the init of the vring cache until migration's loading completes.

Since there'll be other changes besides migration, please consider also
copy the relevant maintainers too on either memory and virtio in your next
post:

$ ./scripts/get_maintainer.pl -f softmmu/memory.c -f hw/virtio/virtio.c
Paolo Bonzini <pbonzini@redhat.com> <pbonzini@redhat.com> (supporter:Memory API)
Peter Xu <peterx@redhat.com> <peterx@redhat.com> (supporter:Memory API)
David Hildenbrand <david@redhat.com> <david@redhat.com> (supporter:Memory API)
"Philippe Mathieu-Daudé" <philmd@linaro.org> <philmd@linaro.org>
(reviewer:Memory API)
"Michael S. Tsirkin" <mst@redhat.com> <mst@redhat.com>
(supporter:virtio)qemu-devel@nongnu.org (open list:All patches CC
here)



Sorry I forgot to update the cc list..

Thanks for your reminder!

[-- Attachment #2: Type: text/html, Size: 2164 bytes --]

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

[Chuang Xu]

[-- Attachment #1: Type: text/plain, Size: 1445 bytes --]

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..😭

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.

That's a good idea!

But I don't think it's necessary to register a new vm state change handler.
Maybe we just need to add a delay_check flag to VirtIODevice and do those
delayed checks in virtio_vmstate_change() when delay_check is true.

Later I'll upload the v3 patches.

Thanks!

[-- Attachment #2: Type: text/html, Size: 2175 bytes --]