* [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support
@ 2018-11-15 10:07 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:07 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
This is the deivce part implementation to add a new feature,
VIRTIO_BALLOON_F_FREE_PAGE_HINT to the virtio-balloon device. The device
receives the guest free page hints from the driver and clears the
corresponding bits in the dirty bitmap, so that those free pages are
not sent by the migration thread to the destination.
*Tests
1 Test Environment
Host: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
Migration setup: migrate_set_speed 100G, migrate_set_downtime 400ms
2 Test Results (results are averaged over several repeated runs)
2.1 Guest setup: 8G RAM, 4 vCPU
2.1.1 Idle guest live migration time
Optimization v.s. Legacy = 620ms vs 2970ms
--> ~79% reduction
2.1.2 Guest live migration with Linux compilation workload
(i.e. make bzImage -j4) running
1) Live Migration Time:
Optimization v.s. Legacy = 2273ms v.s. 4502ms
--> ~50% reduction
2) Linux Compilation Time:
Optimization v.s. Legacy = 8min42s v.s. 8min43s
--> no obvious difference
2.2 Guest setup: 128G RAM, 4 vCPU
2.2.1 Idle guest live migration time
Optimization v.s. Legacy = 5294ms vs 41651ms
--> ~87% reduction
2.2.2 Guest live migration with Linux compilation workload
1) Live Migration Time:
Optimization v.s. Legacy = 8816ms v.s. 54201ms
--> 84% reduction
2) Linux Compilation Time:
Optimization v.s. Legacy = 8min30s v.s. 8min36s
--> no obvious difference
ChangeLog:
v8->v9:
bitmap:
- fix bitmap_count_one to handle the nbits=0 case
migration:
- replace the ram save notifier chain with a more general precopy
notifier chain, which is similar to the postcopy notifier chain.
- Avoid exposing the RAMState struct, and add a function,
precopy_disable_bulk_stage, to let the virtio-balloon notifier
callback to disable the bulk stage flag.
virtio-balloon:
- Remove VIRTIO_BALLOON_F_PAGE_POISON from this series as it is not
a limit to the free page optimization now. Plan to add the device
support for VIRTIO_BALLOON_F_PAGE_POISON in another patch later.
Previous changelog:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg01908.html
Wei Wang (8):
bitmap: fix bitmap_count_one
bitmap: bitmap_count_one_with_offset
migration: use bitmap_mutex in migration_bitmap_clear_dirty
migration: API to clear bits of guest free pages from the dirty bitmap
migration/ram.c: add a notifier chain for precopy
migration/ram.c: add a function to disable the bulk stage
migration: move migrate_postcopy() to include/migration/misc.h
virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
hw/virtio/virtio-balloon.c | 255 ++++++++++++++++++++++++
include/hw/virtio/virtio-balloon.h | 28 ++-
include/migration/misc.h | 16 ++
include/qemu/bitmap.h | 17 ++
include/standard-headers/linux/virtio_balloon.h | 5 +
migration/migration.h | 2 -
migration/ram.c | 91 +++++++++
vl.c | 1 +
8 files changed, 412 insertions(+), 3 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* [virtio-dev] [PATCH v9 0/8] virtio-balloon: free page hint support
@ 2018-11-15 10:07 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:07 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
This is the deivce part implementation to add a new feature,
VIRTIO_BALLOON_F_FREE_PAGE_HINT to the virtio-balloon device. The device
receives the guest free page hints from the driver and clears the
corresponding bits in the dirty bitmap, so that those free pages are
not sent by the migration thread to the destination.
*Tests
1 Test Environment
Host: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
Migration setup: migrate_set_speed 100G, migrate_set_downtime 400ms
2 Test Results (results are averaged over several repeated runs)
2.1 Guest setup: 8G RAM, 4 vCPU
2.1.1 Idle guest live migration time
Optimization v.s. Legacy = 620ms vs 2970ms
--> ~79% reduction
2.1.2 Guest live migration with Linux compilation workload
(i.e. make bzImage -j4) running
1) Live Migration Time:
Optimization v.s. Legacy = 2273ms v.s. 4502ms
--> ~50% reduction
2) Linux Compilation Time:
Optimization v.s. Legacy = 8min42s v.s. 8min43s
--> no obvious difference
2.2 Guest setup: 128G RAM, 4 vCPU
2.2.1 Idle guest live migration time
Optimization v.s. Legacy = 5294ms vs 41651ms
--> ~87% reduction
2.2.2 Guest live migration with Linux compilation workload
1) Live Migration Time:
Optimization v.s. Legacy = 8816ms v.s. 54201ms
--> 84% reduction
2) Linux Compilation Time:
Optimization v.s. Legacy = 8min30s v.s. 8min36s
--> no obvious difference
ChangeLog:
v8->v9:
bitmap:
- fix bitmap_count_one to handle the nbits=0 case
migration:
- replace the ram save notifier chain with a more general precopy
notifier chain, which is similar to the postcopy notifier chain.
- Avoid exposing the RAMState struct, and add a function,
precopy_disable_bulk_stage, to let the virtio-balloon notifier
callback to disable the bulk stage flag.
virtio-balloon:
- Remove VIRTIO_BALLOON_F_PAGE_POISON from this series as it is not
a limit to the free page optimization now. Plan to add the device
support for VIRTIO_BALLOON_F_PAGE_POISON in another patch later.
Previous changelog:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg01908.html
Wei Wang (8):
bitmap: fix bitmap_count_one
bitmap: bitmap_count_one_with_offset
migration: use bitmap_mutex in migration_bitmap_clear_dirty
migration: API to clear bits of guest free pages from the dirty bitmap
migration/ram.c: add a notifier chain for precopy
migration/ram.c: add a function to disable the bulk stage
migration: move migrate_postcopy() to include/migration/misc.h
virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
hw/virtio/virtio-balloon.c | 255 ++++++++++++++++++++++++
include/hw/virtio/virtio-balloon.h | 28 ++-
include/migration/misc.h | 16 ++
include/qemu/bitmap.h | 17 ++
include/standard-headers/linux/virtio_balloon.h | 5 +
migration/migration.h | 2 -
migration/ram.c | 91 +++++++++
vl.c | 1 +
8 files changed, 412 insertions(+), 3 deletions(-)
--
1.8.3.1
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v9 1/8] bitmap: fix bitmap_count_one
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
@ 2018-11-15 10:07 ` Wei Wang
-1 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:07 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
BITMAP_LAST_WORD_MASK(nbits) returns 0xffffffff when "nbits=0", which
makes bitmap_count_one fail to handle the "nbits=0" case. It appears to be
preferred to remain BITMAP_LAST_WORD_MASK identical to the kernel
implementation that it is ported from.
So this patch fixes bitmap_count_one to handle the nbits=0 case.
Inital Discussion Link:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg554316.html
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
include/qemu/bitmap.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 509eedd..679f1bd 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -221,6 +221,10 @@ static inline int bitmap_intersects(const unsigned long *src1,
static inline long bitmap_count_one(const unsigned long *bitmap, long nbits)
{
+ if (unlikely(!nbits)) {
+ return 0;
+ }
+
if (small_nbits(nbits)) {
return ctpopl(*bitmap & BITMAP_LAST_WORD_MASK(nbits));
} else {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [virtio-dev] [PATCH v9 1/8] bitmap: fix bitmap_count_one
@ 2018-11-15 10:07 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:07 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
BITMAP_LAST_WORD_MASK(nbits) returns 0xffffffff when "nbits=0", which
makes bitmap_count_one fail to handle the "nbits=0" case. It appears to be
preferred to remain BITMAP_LAST_WORD_MASK identical to the kernel
implementation that it is ported from.
So this patch fixes bitmap_count_one to handle the nbits=0 case.
Inital Discussion Link:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg554316.html
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
include/qemu/bitmap.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 509eedd..679f1bd 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -221,6 +221,10 @@ static inline int bitmap_intersects(const unsigned long *src1,
static inline long bitmap_count_one(const unsigned long *bitmap, long nbits)
{
+ if (unlikely(!nbits)) {
+ return 0;
+ }
+
if (small_nbits(nbits)) {
return ctpopl(*bitmap & BITMAP_LAST_WORD_MASK(nbits));
} else {
--
1.8.3.1
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v9 2/8] bitmap: bitmap_count_one_with_offset
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
@ 2018-11-15 10:07 ` Wei Wang
-1 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:07 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
Count the number of 1s in a bitmap starting from an offset.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
include/qemu/bitmap.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 679f1bd..5c31334 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -232,6 +232,19 @@ static inline long bitmap_count_one(const unsigned long *bitmap, long nbits)
}
}
+static inline long bitmap_count_one_with_offset(const unsigned long *bitmap,
+ long offset, long nbits)
+{
+ long aligned_offset = QEMU_ALIGN_DOWN(offset, BITS_PER_LONG);
+ long redundant_bits = offset - aligned_offset;
+ long bits_to_count = nbits + redundant_bits;
+ const unsigned long *bitmap_start = bitmap +
+ aligned_offset / BITS_PER_LONG;
+
+ return bitmap_count_one(bitmap_start, bits_to_count) -
+ bitmap_count_one(bitmap_start, redundant_bits);
+}
+
void bitmap_set(unsigned long *map, long i, long len);
void bitmap_set_atomic(unsigned long *map, long i, long len);
void bitmap_clear(unsigned long *map, long start, long nr);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [virtio-dev] [PATCH v9 2/8] bitmap: bitmap_count_one_with_offset
@ 2018-11-15 10:07 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:07 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
Count the number of 1s in a bitmap starting from an offset.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
include/qemu/bitmap.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 679f1bd..5c31334 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -232,6 +232,19 @@ static inline long bitmap_count_one(const unsigned long *bitmap, long nbits)
}
}
+static inline long bitmap_count_one_with_offset(const unsigned long *bitmap,
+ long offset, long nbits)
+{
+ long aligned_offset = QEMU_ALIGN_DOWN(offset, BITS_PER_LONG);
+ long redundant_bits = offset - aligned_offset;
+ long bits_to_count = nbits + redundant_bits;
+ const unsigned long *bitmap_start = bitmap +
+ aligned_offset / BITS_PER_LONG;
+
+ return bitmap_count_one(bitmap_start, bits_to_count) -
+ bitmap_count_one(bitmap_start, redundant_bits);
+}
+
void bitmap_set(unsigned long *map, long i, long len);
void bitmap_set_atomic(unsigned long *map, long i, long len);
void bitmap_clear(unsigned long *map, long start, long nr);
--
1.8.3.1
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
@ 2018-11-15 10:07 ` Wei Wang
-1 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:07 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
The bitmap mutex is used to synchronize threads to update the dirty
bitmap and the migration_dirty_pages counter. For example, the free
page optimization clears bits of free pages from the bitmap in an
iothread context. This patch makes migration_bitmap_clear_dirty update
the bitmap and counter under the mutex.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 7e7deec..ef69dbe 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1556,11 +1556,14 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
{
bool ret;
+ qemu_mutex_lock(&rs->bitmap_mutex);
ret = test_and_clear_bit(page, rb->bmap);
if (ret) {
rs->migration_dirty_pages--;
}
+ qemu_mutex_unlock(&rs->bitmap_mutex);
+
return ret;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [virtio-dev] [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty
@ 2018-11-15 10:07 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:07 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
The bitmap mutex is used to synchronize threads to update the dirty
bitmap and the migration_dirty_pages counter. For example, the free
page optimization clears bits of free pages from the bitmap in an
iothread context. This patch makes migration_bitmap_clear_dirty update
the bitmap and counter under the mutex.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 7e7deec..ef69dbe 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1556,11 +1556,14 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
{
bool ret;
+ qemu_mutex_lock(&rs->bitmap_mutex);
ret = test_and_clear_bit(page, rb->bmap);
if (ret) {
rs->migration_dirty_pages--;
}
+ qemu_mutex_unlock(&rs->bitmap_mutex);
+
return ret;
}
--
1.8.3.1
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
@ 2018-11-15 10:08 ` Wei Wang
-1 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:08 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
This patch adds an API to clear bits corresponding to guest free pages
from the dirty bitmap. Spilt the free page block if it crosses the QEMU
RAMBlock boundary.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
include/migration/misc.h | 2 ++
migration/ram.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 4ebf24c..113320e 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -14,11 +14,13 @@
#ifndef MIGRATION_MISC_H
#define MIGRATION_MISC_H
+#include "exec/cpu-common.h"
#include "qemu/notify.h"
/* migration/ram.c */
void ram_mig_init(void);
+void qemu_guest_free_page_hint(void *addr, size_t len);
/* migration/block.c */
diff --git a/migration/ram.c b/migration/ram.c
index ef69dbe..229b791 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3131,6 +3131,54 @@ static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out)
}
/*
+ * This function clears bits of the free pages reported by the caller from the
+ * migration dirty bitmap. @addr is the host address corresponding to the
+ * start of the continuous guest free pages, and @len is the total bytes of
+ * those pages.
+ */
+void qemu_guest_free_page_hint(void *addr, size_t len)
+{
+ RAMBlock *block;
+ ram_addr_t offset;
+ size_t used_len, start, npages;
+ MigrationState *s = migrate_get_current();
+
+ /* This function is currently expected to be used during live migration */
+ if (!migration_is_setup_or_active(s->state)) {
+ return;
+ }
+
+ for (; len > 0; len -= used_len) {
+ block = qemu_ram_block_from_host(addr, false, &offset);
+ assert(block);
+
+ /*
+ * This handles the case that the RAMBlock is resized after the free
+ * page hint is reported.
+ */
+ if (unlikely(offset > block->used_length)) {
+ return;
+ }
+
+ if (len <= block->used_length - offset) {
+ used_len = len;
+ } else {
+ used_len = block->used_length - offset;
+ addr += used_len;
+ }
+
+ start = offset >> TARGET_PAGE_BITS;
+ npages = used_len >> TARGET_PAGE_BITS;
+
+ qemu_mutex_lock(&ram_state->bitmap_mutex);
+ ram_state->migration_dirty_pages -=
+ bitmap_count_one_with_offset(block->bmap, start, npages);
+ bitmap_clear(block->bmap, start, npages);
+ qemu_mutex_unlock(&ram_state->bitmap_mutex);
+ }
+}
+
+/*
* Each of ram_save_setup, ram_save_iterate and ram_save_complete has
* long-running RCU critical section. When rcu-reclaims in the code
* start to become numerous it will be necessary to reduce the
--
1.8.3.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [virtio-dev] [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap
@ 2018-11-15 10:08 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:08 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
This patch adds an API to clear bits corresponding to guest free pages
from the dirty bitmap. Spilt the free page block if it crosses the QEMU
RAMBlock boundary.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
include/migration/misc.h | 2 ++
migration/ram.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 4ebf24c..113320e 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -14,11 +14,13 @@
#ifndef MIGRATION_MISC_H
#define MIGRATION_MISC_H
+#include "exec/cpu-common.h"
#include "qemu/notify.h"
/* migration/ram.c */
void ram_mig_init(void);
+void qemu_guest_free_page_hint(void *addr, size_t len);
/* migration/block.c */
diff --git a/migration/ram.c b/migration/ram.c
index ef69dbe..229b791 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3131,6 +3131,54 @@ static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out)
}
/*
+ * This function clears bits of the free pages reported by the caller from the
+ * migration dirty bitmap. @addr is the host address corresponding to the
+ * start of the continuous guest free pages, and @len is the total bytes of
+ * those pages.
+ */
+void qemu_guest_free_page_hint(void *addr, size_t len)
+{
+ RAMBlock *block;
+ ram_addr_t offset;
+ size_t used_len, start, npages;
+ MigrationState *s = migrate_get_current();
+
+ /* This function is currently expected to be used during live migration */
+ if (!migration_is_setup_or_active(s->state)) {
+ return;
+ }
+
+ for (; len > 0; len -= used_len) {
+ block = qemu_ram_block_from_host(addr, false, &offset);
+ assert(block);
+
+ /*
+ * This handles the case that the RAMBlock is resized after the free
+ * page hint is reported.
+ */
+ if (unlikely(offset > block->used_length)) {
+ return;
+ }
+
+ if (len <= block->used_length - offset) {
+ used_len = len;
+ } else {
+ used_len = block->used_length - offset;
+ addr += used_len;
+ }
+
+ start = offset >> TARGET_PAGE_BITS;
+ npages = used_len >> TARGET_PAGE_BITS;
+
+ qemu_mutex_lock(&ram_state->bitmap_mutex);
+ ram_state->migration_dirty_pages -=
+ bitmap_count_one_with_offset(block->bmap, start, npages);
+ bitmap_clear(block->bmap, start, npages);
+ qemu_mutex_unlock(&ram_state->bitmap_mutex);
+ }
+}
+
+/*
* Each of ram_save_setup, ram_save_iterate and ram_save_complete has
* long-running RCU critical section. When rcu-reclaims in the code
* start to become numerous it will be necessary to reduce the
--
1.8.3.1
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
@ 2018-11-15 10:08 ` Wei Wang
-1 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:08 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
This patch adds a notifier chain for the memory precopy. This enables various
precopy optimizations to be invoked at specific places.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
include/migration/misc.h | 12 ++++++++++++
migration/ram.c | 31 +++++++++++++++++++++++++++++++
vl.c | 1 +
3 files changed, 44 insertions(+)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 113320e..0bac623 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -19,6 +19,18 @@
/* migration/ram.c */
+typedef enum PrecopyNotifyReason {
+ PRECOPY_NOTIFY_ERR = 0,
+ PRECOPY_NOTIFY_START_ITERATION = 1,
+ PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
+ PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
+ PRECOPY_NOTIFY_MAX = 4,
+} PrecopyNotifyReason;
+
+void precopy_infrastructure_init(void);
+void precopy_add_notifier(Notifier *n);
+void precopy_remove_notifier(Notifier *n);
+
void ram_mig_init(void);
void qemu_guest_free_page_hint(void *addr, size_t len);
diff --git a/migration/ram.c b/migration/ram.c
index 229b791..65b1223 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -292,6 +292,8 @@ struct RAMState {
bool ram_bulk_stage;
/* How many times we have dirty too many pages */
int dirty_rate_high_cnt;
+ /* ram save states used for notifiers */
+ int ram_save_state;
/* these variables are used for bitmap sync */
/* last time we did a full bitmap_sync */
int64_t time_last_bitmap_sync;
@@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
static RAMState *ram_state;
+static NotifierList precopy_notifier_list;
+
+void precopy_infrastructure_init(void)
+{
+ notifier_list_init(&precopy_notifier_list);
+}
+
+void precopy_add_notifier(Notifier *n)
+{
+ notifier_list_add(&precopy_notifier_list, n);
+}
+
+void precopy_remove_notifier(Notifier *n)
+{
+ notifier_remove(n);
+}
+
+static void precopy_notify(PrecopyNotifyReason reason)
+{
+ notifier_list_notify(&precopy_notifier_list, &reason);
+}
+
uint64_t ram_bytes_remaining(void)
{
return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
@@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
int64_t end_time;
uint64_t bytes_xfer_now;
+ precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
+
ram_counters.dirty_sync_count++;
if (!rs->time_last_bitmap_sync) {
@@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
if (migrate_use_events()) {
qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
}
+
+ precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
}
/**
@@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
rs->last_page = 0;
rs->last_version = ram_list.version;
rs->ram_bulk_stage = true;
+
+ precopy_notify(PRECOPY_NOTIFY_START_ITERATION);
}
#define MAX_WAIT 50 /* ms, half buffered_file limit */
@@ -3324,6 +3354,7 @@ out:
ret = qemu_file_get_error(f);
if (ret < 0) {
+ precopy_notify(PRECOPY_NOTIFY_ERR);
return ret;
}
diff --git a/vl.c b/vl.c
index fa25d1a..48ae0e8 100644
--- a/vl.c
+++ b/vl.c
@@ -3057,6 +3057,7 @@ int main(int argc, char **argv, char **envp)
module_call_init(MODULE_INIT_OPTS);
runstate_init();
+ precopy_infrastructure_init();
postcopy_infrastructure_init();
monitor_init_globals();
--
1.8.3.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [virtio-dev] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
@ 2018-11-15 10:08 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:08 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
This patch adds a notifier chain for the memory precopy. This enables various
precopy optimizations to be invoked at specific places.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
include/migration/misc.h | 12 ++++++++++++
migration/ram.c | 31 +++++++++++++++++++++++++++++++
vl.c | 1 +
3 files changed, 44 insertions(+)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 113320e..0bac623 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -19,6 +19,18 @@
/* migration/ram.c */
+typedef enum PrecopyNotifyReason {
+ PRECOPY_NOTIFY_ERR = 0,
+ PRECOPY_NOTIFY_START_ITERATION = 1,
+ PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
+ PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
+ PRECOPY_NOTIFY_MAX = 4,
+} PrecopyNotifyReason;
+
+void precopy_infrastructure_init(void);
+void precopy_add_notifier(Notifier *n);
+void precopy_remove_notifier(Notifier *n);
+
void ram_mig_init(void);
void qemu_guest_free_page_hint(void *addr, size_t len);
diff --git a/migration/ram.c b/migration/ram.c
index 229b791..65b1223 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -292,6 +292,8 @@ struct RAMState {
bool ram_bulk_stage;
/* How many times we have dirty too many pages */
int dirty_rate_high_cnt;
+ /* ram save states used for notifiers */
+ int ram_save_state;
/* these variables are used for bitmap sync */
/* last time we did a full bitmap_sync */
int64_t time_last_bitmap_sync;
@@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
static RAMState *ram_state;
+static NotifierList precopy_notifier_list;
+
+void precopy_infrastructure_init(void)
+{
+ notifier_list_init(&precopy_notifier_list);
+}
+
+void precopy_add_notifier(Notifier *n)
+{
+ notifier_list_add(&precopy_notifier_list, n);
+}
+
+void precopy_remove_notifier(Notifier *n)
+{
+ notifier_remove(n);
+}
+
+static void precopy_notify(PrecopyNotifyReason reason)
+{
+ notifier_list_notify(&precopy_notifier_list, &reason);
+}
+
uint64_t ram_bytes_remaining(void)
{
return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
@@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
int64_t end_time;
uint64_t bytes_xfer_now;
+ precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
+
ram_counters.dirty_sync_count++;
if (!rs->time_last_bitmap_sync) {
@@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
if (migrate_use_events()) {
qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
}
+
+ precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
}
/**
@@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
rs->last_page = 0;
rs->last_version = ram_list.version;
rs->ram_bulk_stage = true;
+
+ precopy_notify(PRECOPY_NOTIFY_START_ITERATION);
}
#define MAX_WAIT 50 /* ms, half buffered_file limit */
@@ -3324,6 +3354,7 @@ out:
ret = qemu_file_get_error(f);
if (ret < 0) {
+ precopy_notify(PRECOPY_NOTIFY_ERR);
return ret;
}
diff --git a/vl.c b/vl.c
index fa25d1a..48ae0e8 100644
--- a/vl.c
+++ b/vl.c
@@ -3057,6 +3057,7 @@ int main(int argc, char **argv, char **envp)
module_call_init(MODULE_INIT_OPTS);
runstate_init();
+ precopy_infrastructure_init();
postcopy_infrastructure_init();
monitor_init_globals();
--
1.8.3.1
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v9 6/8] migration/ram.c: add a function to disable the bulk stage
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
@ 2018-11-15 10:08 ` Wei Wang
-1 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:08 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
This patch adds a function to enable a precopy notifier callback outside
the migration subsystem to disable the bulk stage flag. This is needed by
the free page optimization offered by virtio-balloon.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
include/migration/misc.h | 1 +
migration/ram.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 0bac623..67cb275 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -30,6 +30,7 @@ typedef enum PrecopyNotifyReason {
void precopy_infrastructure_init(void);
void precopy_add_notifier(Notifier *n);
void precopy_remove_notifier(Notifier *n);
+void precopy_disable_bulk_stage(void);
void ram_mig_init(void);
void qemu_guest_free_page_hint(void *addr, size_t len);
diff --git a/migration/ram.c b/migration/ram.c
index 65b1223..8745ca3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -352,6 +352,15 @@ static void precopy_notify(PrecopyNotifyReason reason)
notifier_list_notify(&precopy_notifier_list, &reason);
}
+void precopy_disable_bulk_stage(void)
+{
+ if (!ram_state) {
+ return;
+ }
+
+ ram_state->ram_bulk_stage = false;
+}
+
uint64_t ram_bytes_remaining(void)
{
return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
--
1.8.3.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [virtio-dev] [PATCH v9 6/8] migration/ram.c: add a function to disable the bulk stage
@ 2018-11-15 10:08 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:08 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
This patch adds a function to enable a precopy notifier callback outside
the migration subsystem to disable the bulk stage flag. This is needed by
the free page optimization offered by virtio-balloon.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
include/migration/misc.h | 1 +
migration/ram.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 0bac623..67cb275 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -30,6 +30,7 @@ typedef enum PrecopyNotifyReason {
void precopy_infrastructure_init(void);
void precopy_add_notifier(Notifier *n);
void precopy_remove_notifier(Notifier *n);
+void precopy_disable_bulk_stage(void);
void ram_mig_init(void);
void qemu_guest_free_page_hint(void *addr, size_t len);
diff --git a/migration/ram.c b/migration/ram.c
index 65b1223..8745ca3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -352,6 +352,15 @@ static void precopy_notify(PrecopyNotifyReason reason)
notifier_list_notify(&precopy_notifier_list, &reason);
}
+void precopy_disable_bulk_stage(void)
+{
+ if (!ram_state) {
+ return;
+ }
+
+ ram_state->ram_bulk_stage = false;
+}
+
uint64_t ram_bytes_remaining(void)
{
return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
--
1.8.3.1
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v9 7/8] migration: move migrate_postcopy() to include/migration/misc.h
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
@ 2018-11-15 10:08 ` Wei Wang
-1 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:08 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
The ram save state notifier callback, for example the free page
optimization offerred by virtio-balloon, may need to check if
postcopy is in use, so move migrate_postcopy() to the outside header.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
include/migration/misc.h | 1 +
migration/migration.h | 2 --
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 67cb275..06b816f 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -70,6 +70,7 @@ bool migration_has_failed(MigrationState *);
/* ...and after the device transmission */
bool migration_in_postcopy_after_devices(MigrationState *);
void migration_global_dump(Monitor *mon);
+bool migrate_postcopy(void);
/* migration/block-dirty-bitmap.c */
void dirty_bitmap_mig_init(void);
diff --git a/migration/migration.h b/migration/migration.h
index e413d4d..e1e92ff 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -249,8 +249,6 @@ bool migration_is_blocked(Error **errp);
bool migration_in_postcopy(void);
MigrationState *migrate_get_current(void);
-bool migrate_postcopy(void);
-
bool migrate_release_ram(void);
bool migrate_postcopy_ram(void);
bool migrate_zero_blocks(void);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [virtio-dev] [PATCH v9 7/8] migration: move migrate_postcopy() to include/migration/misc.h
@ 2018-11-15 10:08 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:08 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
The ram save state notifier callback, for example the free page
optimization offerred by virtio-balloon, may need to check if
postcopy is in use, so move migrate_postcopy() to the outside header.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Peter Xu <peterx@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
include/migration/misc.h | 1 +
migration/migration.h | 2 --
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 67cb275..06b816f 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -70,6 +70,7 @@ bool migration_has_failed(MigrationState *);
/* ...and after the device transmission */
bool migration_in_postcopy_after_devices(MigrationState *);
void migration_global_dump(Monitor *mon);
+bool migrate_postcopy(void);
/* migration/block-dirty-bitmap.c */
void dirty_bitmap_mig_init(void);
diff --git a/migration/migration.h b/migration/migration.h
index e413d4d..e1e92ff 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -249,8 +249,6 @@ bool migration_is_blocked(Error **errp);
bool migration_in_postcopy(void);
MigrationState *migrate_get_current(void);
-bool migrate_postcopy(void);
-
bool migrate_release_ram(void);
bool migrate_postcopy_ram(void);
bool migrate_zero_blocks(void);
--
1.8.3.1
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v9 8/8] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
@ 2018-11-15 10:08 ` Wei Wang
-1 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:08 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
The new feature enables the virtio-balloon device to receive hints of
guest free pages from the free page vq.
A notifier is registered to the migration precopy notifier chain. The
notifier calls free_page_start after the migration thread syncs the dirty
bitmap, so that the free page optimization starts to clear bits of free
pages from the bitmap. It calls the free_page_stop before the migration
thread syncs the bitmap, which is the end of the current round of ram
save. The free_page_stop is also called to stop the optimization in the
case when there is an error occurred in the process of ram saving.
Note: balloon will report pages which were free at the time of this call.
As the reporting happens asynchronously, dirty bit logging must be
enabled before this free_page_start call is made. Guest reporting must be
disabled before the migration dirty bitmap is synchronized.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
hw/virtio/virtio-balloon.c | 255 ++++++++++++++++++++++++
include/hw/virtio/virtio-balloon.h | 28 ++-
include/standard-headers/linux/virtio_balloon.h | 5 +
3 files changed, 287 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 1728e4f..4a3eb30 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -27,6 +27,7 @@
#include "qapi/visitor.h"
#include "trace.h"
#include "qemu/error-report.h"
+#include "migration/misc.h"
#include "hw/virtio/virtio-bus.h"
#include "hw/virtio/virtio-access.h"
@@ -308,6 +309,176 @@ out:
}
}
+static void virtio_balloon_handle_free_page_vq(VirtIODevice *vdev,
+ VirtQueue *vq)
+{
+ VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+ qemu_bh_schedule(s->free_page_bh);
+}
+
+static bool get_free_page_hints(VirtIOBalloon *dev)
+{
+ VirtQueueElement *elem;
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VirtQueue *vq = dev->free_page_vq;
+
+ while (dev->block_iothread) {
+ qemu_cond_wait(&dev->free_page_cond, &dev->free_page_lock);
+ }
+
+ elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+ if (!elem) {
+ return false;
+ }
+
+ if (elem->out_num) {
+ uint32_t id;
+ size_t size = iov_to_buf(elem->out_sg, elem->out_num, 0,
+ &id, sizeof(id));
+ virtqueue_push(vq, elem, size);
+ g_free(elem);
+
+ virtio_tswap32s(vdev, &id);
+ if (unlikely(size != sizeof(id))) {
+ virtio_error(vdev, "received an incorrect cmd id");
+ return false;
+ }
+ if (id == dev->free_page_report_cmd_id) {
+ dev->free_page_report_status = FREE_PAGE_REPORT_S_START;
+ } else {
+ /*
+ * Stop the optimization only when it has started. This
+ * avoids a stale stop sign for the previous command.
+ */
+ if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
+ dev->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+ }
+ }
+ }
+
+ if (elem->in_num) {
+ if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
+ qemu_guest_free_page_hint(elem->in_sg[0].iov_base,
+ elem->in_sg[0].iov_len);
+ }
+ virtqueue_push(vq, elem, 1);
+ g_free(elem);
+ }
+
+ return true;
+}
+
+static void virtio_ballloon_get_free_page_hints(void *opaque)
+{
+ VirtIOBalloon *dev = opaque;
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VirtQueue *vq = dev->free_page_vq;
+ bool continue_to_get_hints;
+
+ do {
+ qemu_mutex_lock(&dev->free_page_lock);
+ virtio_queue_set_notification(vq, 0);
+ continue_to_get_hints = get_free_page_hints(dev);
+ qemu_mutex_unlock(&dev->free_page_lock);
+ virtio_notify(vdev, vq);
+ /*
+ * Start to poll the vq once the reporting started. Otherwise, continue
+ * only when there are entries on the vq, which need to be given back.
+ */
+ } while (continue_to_get_hints ||
+ dev->free_page_report_status == FREE_PAGE_REPORT_S_START);
+ virtio_queue_set_notification(vq, 1);
+}
+
+static bool virtio_balloon_free_page_support(void *opaque)
+{
+ VirtIOBalloon *s = opaque;
+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+ return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT);
+}
+
+static void virtio_balloon_free_page_start(VirtIOBalloon *s)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+ /* For the stop and copy phase, we don't need to start the optimization */
+ if (!vdev->vm_running) {
+ return;
+ }
+
+ if (s->free_page_report_cmd_id == UINT_MAX) {
+ s->free_page_report_cmd_id =
+ VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN;
+ } else {
+ s->free_page_report_cmd_id++;
+ }
+
+ s->free_page_report_status = FREE_PAGE_REPORT_S_REQUESTED;
+ virtio_notify_config(vdev);
+}
+
+static void virtio_balloon_free_page_stop(VirtIOBalloon *s)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+ if (s->free_page_report_status != FREE_PAGE_REPORT_S_STOP) {
+ /*
+ * The lock also guarantees us that the
+ * virtio_ballloon_get_free_page_hints exits after the
+ * free_page_report_status is set to S_STOP.
+ */
+ qemu_mutex_lock(&s->free_page_lock);
+ /*
+ * The guest hasn't done the reporting, so host sends a notification
+ * to the guest to actively stop the reporting.
+ */
+ s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+ qemu_mutex_unlock(&s->free_page_lock);
+ virtio_notify_config(vdev);
+ }
+}
+
+static void virtio_balloon_free_page_done(VirtIOBalloon *s)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+ s->free_page_report_status = FREE_PAGE_REPORT_S_DONE;
+ virtio_notify_config(vdev);
+}
+
+static void virtio_balloon_free_page_report_notify(Notifier *notifier,
+ void *data)
+{
+ VirtIOBalloon *dev = container_of(notifier, VirtIOBalloon,
+ free_page_report_notify);
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ PrecopyNotifyReason reason = *(PrecopyNotifyReason *)data;
+
+ if (!virtio_balloon_free_page_support(dev) || migrate_postcopy()) {
+ return;
+ }
+
+ switch (reason) {
+ case PRECOPY_NOTIFY_START_ITERATION:
+ precopy_disable_bulk_stage();
+ break;
+ case PRECOPY_NOTIFY_ERR:
+ case PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP:
+ virtio_balloon_free_page_stop(dev);
+ break;
+ case PRECOPY_NOTIFY_AFTER_SYNC_BITMAP:
+ if (vdev->vm_running) {
+ virtio_balloon_free_page_start(dev);
+ } else {
+ virtio_balloon_free_page_done(dev);
+ }
+ break;
+ default:
+ virtio_error(vdev, "%s: %d reason unknown", __func__, reason);
+ }
+}
+
static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
{
VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
@@ -316,6 +487,17 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
config.num_pages = cpu_to_le32(dev->num_pages);
config.actual = cpu_to_le32(dev->actual);
+ if (dev->free_page_report_status == FREE_PAGE_REPORT_S_REQUESTED) {
+ config.free_page_report_cmd_id =
+ cpu_to_le32(dev->free_page_report_cmd_id);
+ } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_STOP) {
+ config.free_page_report_cmd_id =
+ cpu_to_le32(VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID);
+ } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_DONE) {
+ config.free_page_report_cmd_id =
+ cpu_to_le32(VIRTIO_BALLOON_FREE_PAGE_REPORT_DONE_ID);
+ }
+
trace_virtio_balloon_get_config(config.num_pages, config.actual);
memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
}
@@ -376,6 +558,7 @@ static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f,
VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
f |= dev->host_features;
virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ);
+
return f;
}
@@ -412,6 +595,18 @@ static int virtio_balloon_post_load_device(void *opaque, int version_id)
return 0;
}
+static const VMStateDescription vmstate_virtio_balloon_free_page_report = {
+ .name = "virtio-balloon-device/free-page-report",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = virtio_balloon_free_page_support,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(free_page_report_cmd_id, VirtIOBalloon),
+ VMSTATE_UINT32(free_page_report_status, VirtIOBalloon),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_virtio_balloon_device = {
.name = "virtio-balloon-device",
.version_id = 1,
@@ -422,6 +617,10 @@ static const VMStateDescription vmstate_virtio_balloon_device = {
VMSTATE_UINT32(actual, VirtIOBalloon),
VMSTATE_END_OF_LIST()
},
+ .subsections = (const VMStateDescription * []) {
+ &vmstate_virtio_balloon_free_page_report,
+ NULL
+ }
};
static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
@@ -446,6 +645,29 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
s->svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
+ if (virtio_has_feature(s->host_features,
+ VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+ s->free_page_vq = virtio_add_queue(vdev, VIRTQUEUE_MAX_SIZE,
+ virtio_balloon_handle_free_page_vq);
+ s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+ s->free_page_report_cmd_id =
+ VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN;
+ s->free_page_report_notify.notify =
+ virtio_balloon_free_page_report_notify;
+ precopy_add_notifier(&s->free_page_report_notify);
+ if (s->iothread) {
+ object_ref(OBJECT(s->iothread));
+ s->free_page_bh = aio_bh_new(iothread_get_aio_context(s->iothread),
+ virtio_ballloon_get_free_page_hints, s);
+ qemu_mutex_init(&s->free_page_lock);
+ qemu_cond_init(&s->free_page_cond);
+ s->block_iothread = false;
+ } else {
+ /* Simply disable this feature if the iothread wasn't created. */
+ s->host_features &= ~(1 << VIRTIO_BALLOON_F_FREE_PAGE_HINT);
+ virtio_error(vdev, "iothread is missing");
+ }
+ }
reset_stats(s);
}
@@ -454,6 +676,11 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOBalloon *s = VIRTIO_BALLOON(dev);
+ if (virtio_balloon_free_page_support(s)) {
+ qemu_bh_delete(s->free_page_bh);
+ virtio_balloon_free_page_stop(s);
+ precopy_remove_notifier(&s->free_page_report_notify);
+ }
balloon_stats_destroy_timer(s);
qemu_remove_balloon_handler(s);
virtio_cleanup(vdev);
@@ -463,6 +690,10 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
{
VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+ if (virtio_balloon_free_page_support(s)) {
+ virtio_balloon_free_page_stop(s);
+ }
+
if (s->stats_vq_elem != NULL) {
virtqueue_unpop(s->svq, s->stats_vq_elem, 0);
g_free(s->stats_vq_elem);
@@ -480,6 +711,26 @@ static void virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
* was stopped */
virtio_balloon_receive_stats(vdev, s->svq);
}
+
+ if (virtio_balloon_free_page_support(s)) {
+ /*
+ * The VM is woken up and the iothread was blocked, so signal it to
+ * continue.
+ */
+ if (vdev->vm_running && s->block_iothread) {
+ qemu_mutex_lock(&s->free_page_lock);
+ s->block_iothread = false;
+ qemu_cond_signal(&s->free_page_cond);
+ qemu_mutex_unlock(&s->free_page_lock);
+ }
+
+ /* The VM is stopped, block the iothread. */
+ if (!vdev->vm_running) {
+ qemu_mutex_lock(&s->free_page_lock);
+ s->block_iothread = true;
+ qemu_mutex_unlock(&s->free_page_lock);
+ }
+ }
}
static void virtio_balloon_instance_init(Object *obj)
@@ -508,6 +759,10 @@ static const VMStateDescription vmstate_virtio_balloon = {
static Property virtio_balloon_properties[] = {
DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
+ DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
+ VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
+ DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
+ IOThread *),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index e0df352..df1d632 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -17,11 +17,14 @@
#include "standard-headers/linux/virtio_balloon.h"
#include "hw/virtio/virtio.h"
+#include "sysemu/iothread.h"
#define TYPE_VIRTIO_BALLOON "virtio-balloon-device"
#define VIRTIO_BALLOON(obj) \
OBJECT_CHECK(VirtIOBalloon, (obj), TYPE_VIRTIO_BALLOON)
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN 0x80000000
+
typedef struct virtio_balloon_stat VirtIOBalloonStat;
typedef struct virtio_balloon_stat_modern {
@@ -30,15 +33,38 @@ typedef struct virtio_balloon_stat_modern {
uint64_t val;
} VirtIOBalloonStatModern;
+enum virtio_balloon_free_page_report_status {
+ FREE_PAGE_REPORT_S_STOP = 0,
+ FREE_PAGE_REPORT_S_REQUESTED = 1,
+ FREE_PAGE_REPORT_S_START = 2,
+ FREE_PAGE_REPORT_S_DONE = 3,
+};
+
typedef struct VirtIOBalloon {
VirtIODevice parent_obj;
- VirtQueue *ivq, *dvq, *svq;
+ VirtQueue *ivq, *dvq, *svq, *free_page_vq;
+ uint32_t free_page_report_status;
uint32_t num_pages;
uint32_t actual;
+ uint32_t free_page_report_cmd_id;
uint64_t stats[VIRTIO_BALLOON_S_NR];
VirtQueueElement *stats_vq_elem;
size_t stats_vq_offset;
QEMUTimer *stats_timer;
+ IOThread *iothread;
+ QEMUBH *free_page_bh;
+ /*
+ * Lock to synchronize threads to access the free page reporting related
+ * fields (e.g. free_page_report_status).
+ */
+ QemuMutex free_page_lock;
+ QemuCond free_page_cond;
+ /*
+ * Set to block iothread to continue reading free page hints as the VM is
+ * stopped.
+ */
+ bool block_iothread;
+ Notifier free_page_report_notify;
int64_t stats_last_update;
int64_t stats_poll_interval;
uint32_t host_features;
diff --git a/include/standard-headers/linux/virtio_balloon.h b/include/standard-headers/linux/virtio_balloon.h
index 4dbb7dc..9eee1c6 100644
--- a/include/standard-headers/linux/virtio_balloon.h
+++ b/include/standard-headers/linux/virtio_balloon.h
@@ -34,15 +34,20 @@
#define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
#define VIRTIO_BALLOON_F_STATS_VQ 1 /* Memory Stats virtqueue */
#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
+#define VIRTIO_BALLOON_F_FREE_PAGE_HINT 3 /* VQ to report free pages */
/* Size of a PFN in the balloon interface. */
#define VIRTIO_BALLOON_PFN_SHIFT 12
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID 0
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_DONE_ID 1
struct virtio_balloon_config {
/* Number of pages host wants Guest to give up. */
uint32_t num_pages;
/* Number of pages we've actually got in balloon. */
uint32_t actual;
+ /* Free page report command id, readonly by guest */
+ uint32_t free_page_report_cmd_id;
};
#define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [virtio-dev] [PATCH v9 8/8] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
@ 2018-11-15 10:08 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-15 10:08 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel, wei.w.wang
The new feature enables the virtio-balloon device to receive hints of
guest free pages from the free page vq.
A notifier is registered to the migration precopy notifier chain. The
notifier calls free_page_start after the migration thread syncs the dirty
bitmap, so that the free page optimization starts to clear bits of free
pages from the bitmap. It calls the free_page_stop before the migration
thread syncs the bitmap, which is the end of the current round of ram
save. The free_page_stop is also called to stop the optimization in the
case when there is an error occurred in the process of ram saving.
Note: balloon will report pages which were free at the time of this call.
As the reporting happens asynchronously, dirty bit logging must be
enabled before this free_page_start call is made. Guest reporting must be
disabled before the migration dirty bitmap is synchronized.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Peter Xu <peterx@redhat.com>
---
hw/virtio/virtio-balloon.c | 255 ++++++++++++++++++++++++
include/hw/virtio/virtio-balloon.h | 28 ++-
include/standard-headers/linux/virtio_balloon.h | 5 +
3 files changed, 287 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 1728e4f..4a3eb30 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -27,6 +27,7 @@
#include "qapi/visitor.h"
#include "trace.h"
#include "qemu/error-report.h"
+#include "migration/misc.h"
#include "hw/virtio/virtio-bus.h"
#include "hw/virtio/virtio-access.h"
@@ -308,6 +309,176 @@ out:
}
}
+static void virtio_balloon_handle_free_page_vq(VirtIODevice *vdev,
+ VirtQueue *vq)
+{
+ VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+ qemu_bh_schedule(s->free_page_bh);
+}
+
+static bool get_free_page_hints(VirtIOBalloon *dev)
+{
+ VirtQueueElement *elem;
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VirtQueue *vq = dev->free_page_vq;
+
+ while (dev->block_iothread) {
+ qemu_cond_wait(&dev->free_page_cond, &dev->free_page_lock);
+ }
+
+ elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+ if (!elem) {
+ return false;
+ }
+
+ if (elem->out_num) {
+ uint32_t id;
+ size_t size = iov_to_buf(elem->out_sg, elem->out_num, 0,
+ &id, sizeof(id));
+ virtqueue_push(vq, elem, size);
+ g_free(elem);
+
+ virtio_tswap32s(vdev, &id);
+ if (unlikely(size != sizeof(id))) {
+ virtio_error(vdev, "received an incorrect cmd id");
+ return false;
+ }
+ if (id == dev->free_page_report_cmd_id) {
+ dev->free_page_report_status = FREE_PAGE_REPORT_S_START;
+ } else {
+ /*
+ * Stop the optimization only when it has started. This
+ * avoids a stale stop sign for the previous command.
+ */
+ if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
+ dev->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+ }
+ }
+ }
+
+ if (elem->in_num) {
+ if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
+ qemu_guest_free_page_hint(elem->in_sg[0].iov_base,
+ elem->in_sg[0].iov_len);
+ }
+ virtqueue_push(vq, elem, 1);
+ g_free(elem);
+ }
+
+ return true;
+}
+
+static void virtio_ballloon_get_free_page_hints(void *opaque)
+{
+ VirtIOBalloon *dev = opaque;
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VirtQueue *vq = dev->free_page_vq;
+ bool continue_to_get_hints;
+
+ do {
+ qemu_mutex_lock(&dev->free_page_lock);
+ virtio_queue_set_notification(vq, 0);
+ continue_to_get_hints = get_free_page_hints(dev);
+ qemu_mutex_unlock(&dev->free_page_lock);
+ virtio_notify(vdev, vq);
+ /*
+ * Start to poll the vq once the reporting started. Otherwise, continue
+ * only when there are entries on the vq, which need to be given back.
+ */
+ } while (continue_to_get_hints ||
+ dev->free_page_report_status == FREE_PAGE_REPORT_S_START);
+ virtio_queue_set_notification(vq, 1);
+}
+
+static bool virtio_balloon_free_page_support(void *opaque)
+{
+ VirtIOBalloon *s = opaque;
+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+ return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT);
+}
+
+static void virtio_balloon_free_page_start(VirtIOBalloon *s)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+ /* For the stop and copy phase, we don't need to start the optimization */
+ if (!vdev->vm_running) {
+ return;
+ }
+
+ if (s->free_page_report_cmd_id == UINT_MAX) {
+ s->free_page_report_cmd_id =
+ VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN;
+ } else {
+ s->free_page_report_cmd_id++;
+ }
+
+ s->free_page_report_status = FREE_PAGE_REPORT_S_REQUESTED;
+ virtio_notify_config(vdev);
+}
+
+static void virtio_balloon_free_page_stop(VirtIOBalloon *s)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+ if (s->free_page_report_status != FREE_PAGE_REPORT_S_STOP) {
+ /*
+ * The lock also guarantees us that the
+ * virtio_ballloon_get_free_page_hints exits after the
+ * free_page_report_status is set to S_STOP.
+ */
+ qemu_mutex_lock(&s->free_page_lock);
+ /*
+ * The guest hasn't done the reporting, so host sends a notification
+ * to the guest to actively stop the reporting.
+ */
+ s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+ qemu_mutex_unlock(&s->free_page_lock);
+ virtio_notify_config(vdev);
+ }
+}
+
+static void virtio_balloon_free_page_done(VirtIOBalloon *s)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+ s->free_page_report_status = FREE_PAGE_REPORT_S_DONE;
+ virtio_notify_config(vdev);
+}
+
+static void virtio_balloon_free_page_report_notify(Notifier *notifier,
+ void *data)
+{
+ VirtIOBalloon *dev = container_of(notifier, VirtIOBalloon,
+ free_page_report_notify);
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ PrecopyNotifyReason reason = *(PrecopyNotifyReason *)data;
+
+ if (!virtio_balloon_free_page_support(dev) || migrate_postcopy()) {
+ return;
+ }
+
+ switch (reason) {
+ case PRECOPY_NOTIFY_START_ITERATION:
+ precopy_disable_bulk_stage();
+ break;
+ case PRECOPY_NOTIFY_ERR:
+ case PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP:
+ virtio_balloon_free_page_stop(dev);
+ break;
+ case PRECOPY_NOTIFY_AFTER_SYNC_BITMAP:
+ if (vdev->vm_running) {
+ virtio_balloon_free_page_start(dev);
+ } else {
+ virtio_balloon_free_page_done(dev);
+ }
+ break;
+ default:
+ virtio_error(vdev, "%s: %d reason unknown", __func__, reason);
+ }
+}
+
static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
{
VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
@@ -316,6 +487,17 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
config.num_pages = cpu_to_le32(dev->num_pages);
config.actual = cpu_to_le32(dev->actual);
+ if (dev->free_page_report_status == FREE_PAGE_REPORT_S_REQUESTED) {
+ config.free_page_report_cmd_id =
+ cpu_to_le32(dev->free_page_report_cmd_id);
+ } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_STOP) {
+ config.free_page_report_cmd_id =
+ cpu_to_le32(VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID);
+ } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_DONE) {
+ config.free_page_report_cmd_id =
+ cpu_to_le32(VIRTIO_BALLOON_FREE_PAGE_REPORT_DONE_ID);
+ }
+
trace_virtio_balloon_get_config(config.num_pages, config.actual);
memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
}
@@ -376,6 +558,7 @@ static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f,
VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
f |= dev->host_features;
virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ);
+
return f;
}
@@ -412,6 +595,18 @@ static int virtio_balloon_post_load_device(void *opaque, int version_id)
return 0;
}
+static const VMStateDescription vmstate_virtio_balloon_free_page_report = {
+ .name = "virtio-balloon-device/free-page-report",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = virtio_balloon_free_page_support,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(free_page_report_cmd_id, VirtIOBalloon),
+ VMSTATE_UINT32(free_page_report_status, VirtIOBalloon),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_virtio_balloon_device = {
.name = "virtio-balloon-device",
.version_id = 1,
@@ -422,6 +617,10 @@ static const VMStateDescription vmstate_virtio_balloon_device = {
VMSTATE_UINT32(actual, VirtIOBalloon),
VMSTATE_END_OF_LIST()
},
+ .subsections = (const VMStateDescription * []) {
+ &vmstate_virtio_balloon_free_page_report,
+ NULL
+ }
};
static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
@@ -446,6 +645,29 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
s->svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
+ if (virtio_has_feature(s->host_features,
+ VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+ s->free_page_vq = virtio_add_queue(vdev, VIRTQUEUE_MAX_SIZE,
+ virtio_balloon_handle_free_page_vq);
+ s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+ s->free_page_report_cmd_id =
+ VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN;
+ s->free_page_report_notify.notify =
+ virtio_balloon_free_page_report_notify;
+ precopy_add_notifier(&s->free_page_report_notify);
+ if (s->iothread) {
+ object_ref(OBJECT(s->iothread));
+ s->free_page_bh = aio_bh_new(iothread_get_aio_context(s->iothread),
+ virtio_ballloon_get_free_page_hints, s);
+ qemu_mutex_init(&s->free_page_lock);
+ qemu_cond_init(&s->free_page_cond);
+ s->block_iothread = false;
+ } else {
+ /* Simply disable this feature if the iothread wasn't created. */
+ s->host_features &= ~(1 << VIRTIO_BALLOON_F_FREE_PAGE_HINT);
+ virtio_error(vdev, "iothread is missing");
+ }
+ }
reset_stats(s);
}
@@ -454,6 +676,11 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOBalloon *s = VIRTIO_BALLOON(dev);
+ if (virtio_balloon_free_page_support(s)) {
+ qemu_bh_delete(s->free_page_bh);
+ virtio_balloon_free_page_stop(s);
+ precopy_remove_notifier(&s->free_page_report_notify);
+ }
balloon_stats_destroy_timer(s);
qemu_remove_balloon_handler(s);
virtio_cleanup(vdev);
@@ -463,6 +690,10 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
{
VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+ if (virtio_balloon_free_page_support(s)) {
+ virtio_balloon_free_page_stop(s);
+ }
+
if (s->stats_vq_elem != NULL) {
virtqueue_unpop(s->svq, s->stats_vq_elem, 0);
g_free(s->stats_vq_elem);
@@ -480,6 +711,26 @@ static void virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
* was stopped */
virtio_balloon_receive_stats(vdev, s->svq);
}
+
+ if (virtio_balloon_free_page_support(s)) {
+ /*
+ * The VM is woken up and the iothread was blocked, so signal it to
+ * continue.
+ */
+ if (vdev->vm_running && s->block_iothread) {
+ qemu_mutex_lock(&s->free_page_lock);
+ s->block_iothread = false;
+ qemu_cond_signal(&s->free_page_cond);
+ qemu_mutex_unlock(&s->free_page_lock);
+ }
+
+ /* The VM is stopped, block the iothread. */
+ if (!vdev->vm_running) {
+ qemu_mutex_lock(&s->free_page_lock);
+ s->block_iothread = true;
+ qemu_mutex_unlock(&s->free_page_lock);
+ }
+ }
}
static void virtio_balloon_instance_init(Object *obj)
@@ -508,6 +759,10 @@ static const VMStateDescription vmstate_virtio_balloon = {
static Property virtio_balloon_properties[] = {
DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
+ DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
+ VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
+ DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
+ IOThread *),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index e0df352..df1d632 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -17,11 +17,14 @@
#include "standard-headers/linux/virtio_balloon.h"
#include "hw/virtio/virtio.h"
+#include "sysemu/iothread.h"
#define TYPE_VIRTIO_BALLOON "virtio-balloon-device"
#define VIRTIO_BALLOON(obj) \
OBJECT_CHECK(VirtIOBalloon, (obj), TYPE_VIRTIO_BALLOON)
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN 0x80000000
+
typedef struct virtio_balloon_stat VirtIOBalloonStat;
typedef struct virtio_balloon_stat_modern {
@@ -30,15 +33,38 @@ typedef struct virtio_balloon_stat_modern {
uint64_t val;
} VirtIOBalloonStatModern;
+enum virtio_balloon_free_page_report_status {
+ FREE_PAGE_REPORT_S_STOP = 0,
+ FREE_PAGE_REPORT_S_REQUESTED = 1,
+ FREE_PAGE_REPORT_S_START = 2,
+ FREE_PAGE_REPORT_S_DONE = 3,
+};
+
typedef struct VirtIOBalloon {
VirtIODevice parent_obj;
- VirtQueue *ivq, *dvq, *svq;
+ VirtQueue *ivq, *dvq, *svq, *free_page_vq;
+ uint32_t free_page_report_status;
uint32_t num_pages;
uint32_t actual;
+ uint32_t free_page_report_cmd_id;
uint64_t stats[VIRTIO_BALLOON_S_NR];
VirtQueueElement *stats_vq_elem;
size_t stats_vq_offset;
QEMUTimer *stats_timer;
+ IOThread *iothread;
+ QEMUBH *free_page_bh;
+ /*
+ * Lock to synchronize threads to access the free page reporting related
+ * fields (e.g. free_page_report_status).
+ */
+ QemuMutex free_page_lock;
+ QemuCond free_page_cond;
+ /*
+ * Set to block iothread to continue reading free page hints as the VM is
+ * stopped.
+ */
+ bool block_iothread;
+ Notifier free_page_report_notify;
int64_t stats_last_update;
int64_t stats_poll_interval;
uint32_t host_features;
diff --git a/include/standard-headers/linux/virtio_balloon.h b/include/standard-headers/linux/virtio_balloon.h
index 4dbb7dc..9eee1c6 100644
--- a/include/standard-headers/linux/virtio_balloon.h
+++ b/include/standard-headers/linux/virtio_balloon.h
@@ -34,15 +34,20 @@
#define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
#define VIRTIO_BALLOON_F_STATS_VQ 1 /* Memory Stats virtqueue */
#define VIRTIO_BALLOON_F_DEFLATE_ON_OOM 2 /* Deflate balloon on OOM */
+#define VIRTIO_BALLOON_F_FREE_PAGE_HINT 3 /* VQ to report free pages */
/* Size of a PFN in the balloon interface. */
#define VIRTIO_BALLOON_PFN_SHIFT 12
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID 0
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_DONE_ID 1
struct virtio_balloon_config {
/* Number of pages host wants Guest to give up. */
uint32_t num_pages;
/* Number of pages we've actually got in balloon. */
uint32_t actual;
+ /* Free page report command id, readonly by guest */
+ uint32_t free_page_report_cmd_id;
};
#define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */
--
1.8.3.1
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
` (8 preceding siblings ...)
(?)
@ 2018-11-15 18:50 ` no-reply
2018-11-16 1:38 ` [virtio-dev] " Wei Wang
-1 siblings, 1 reply; 53+ messages in thread
From: no-reply @ 2018-11-15 18:50 UTC (permalink / raw)
To: wei.w.wang
Cc: famz, qemu-devel, virtio-dev, mst, quintela, dgilbert,
liliang.opensource, peterx, pbonzini, nilal
Hi,
This series failed docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
Message-id: 1542276484-25508-1-git-send-email-wei.w.wang@intel.com
Type: series
Subject: [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support
=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-quick@centos7 SHOW_ENV=1 J=8
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20181115183408.1240762-1-eblake@redhat.com -> patchew/20181115183408.1240762-1-eblake@redhat.com
Switched to a new branch 'test'
f36f77c virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
2ed0c1a migration: move migrate_postcopy() to include/migration/misc.h
4261bba migration/ram.c: add a function to disable the bulk stage
56a8004 migration/ram.c: add a notifier chain for precopy
fd731fe migration: API to clear bits of guest free pages from the dirty bitmap
f928ac4 migration: use bitmap_mutex in migration_bitmap_clear_dirty
7671c7e bitmap: bitmap_count_one_with_offset
dd2f05b bitmap: fix bitmap_count_one
=== OUTPUT BEGIN ===
BUILD centos7
make[1]: Entering directory `/var/tmp/patchew-tester-tmp-5w43kt65/src'
GEN /var/tmp/patchew-tester-tmp-5w43kt65/src/docker-src.2018-11-15-13.47.54.26143/qemu.tar
Cloning into '/var/tmp/patchew-tester-tmp-5w43kt65/src/docker-src.2018-11-15-13.47.54.26143/qemu.tar.vroot'...
done.
Checking out files: 7% (467/6455)
Checking out files: 8% (517/6455)
Checking out files: 9% (581/6455)
Checking out files: 10% (646/6455)
Checking out files: 11% (711/6455)
Checking out files: 12% (775/6455)
Checking out files: 13% (840/6455)
Checking out files: 14% (904/6455)
Checking out files: 15% (969/6455)
Checking out files: 16% (1033/6455)
Checking out files: 17% (1098/6455)
Checking out files: 18% (1162/6455)
Checking out files: 19% (1227/6455)
Checking out files: 20% (1291/6455)
Checking out files: 21% (1356/6455)
Checking out files: 22% (1421/6455)
Checking out files: 23% (1485/6455)
Checking out files: 24% (1550/6455)
Checking out files: 25% (1614/6455)
Checking out files: 26% (1679/6455)
Checking out files: 27% (1743/6455)
Checking out files: 28% (1808/6455)
Checking out files: 29% (1872/6455)
Checking out files: 30% (1937/6455)
Checking out files: 31% (2002/6455)
Checking out files: 32% (2066/6455)
Checking out files: 33% (2131/6455)
Checking out files: 34% (2195/6455)
Checking out files: 35% (2260/6455)
Checking out files: 36% (2324/6455)
Checking out files: 37% (2389/6455)
Checking out files: 38% (2453/6455)
Checking out files: 39% (2518/6455)
Checking out files: 40% (2582/6455)
Checking out files: 41% (2647/6455)
Checking out files: 42% (2712/6455)
Checking out files: 43% (2776/6455)
Checking out files: 44% (2841/6455)
Checking out files: 45% (2905/6455)
Checking out files: 46% (2970/6455)
Checking out files: 47% (3034/6455)
Checking out files: 48% (3099/6455)
Checking out files: 49% (3163/6455)
Checking out files: 50% (3228/6455)
Checking out files: 51% (3293/6455)
Checking out files: 52% (3357/6455)
Checking out files: 53% (3422/6455)
Checking out files: 54% (3486/6455)
Checking out files: 55% (3551/6455)
Checking out files: 56% (3615/6455)
Checking out files: 57% (3680/6455)
Checking out files: 58% (3744/6455)
Checking out files: 59% (3809/6455)
Checking out files: 60% (3873/6455)
Checking out files: 61% (3938/6455)
Checking out files: 62% (4003/6455)
Checking out files: 63% (4067/6455)
Checking out files: 63% (4082/6455)
Checking out files: 64% (4132/6455)
Checking out files: 65% (4196/6455)
Checking out files: 66% (4261/6455)
Checking out files: 67% (4325/6455)
Checking out files: 68% (4390/6455)
Checking out files: 69% (4454/6455)
Checking out files: 70% (4519/6455)
Checking out files: 71% (4584/6455)
Checking out files: 72% (4648/6455)
Checking out files: 73% (4713/6455)
Checking out files: 74% (4777/6455)
Checking out files: 75% (4842/6455)
Checking out files: 76% (4906/6455)
Checking out files: 77% (4971/6455)
Checking out files: 78% (5035/6455)
Checking out files: 79% (5100/6455)
Checking out files: 80% (5164/6455)
Checking out files: 81% (5229/6455)
Checking out files: 82% (5294/6455)
Checking out files: 83% (5358/6455)
Checking out files: 84% (5423/6455)
Checking out files: 85% (5487/6455)
Checking out files: 86% (5552/6455)
Checking out files: 87% (5616/6455)
Checking out files: 88% (5681/6455)
Checking out files: 89% (5745/6455)
Checking out files: 90% (5810/6455)
Checking out files: 91% (5875/6455)
Checking out files: 92% (5939/6455)
Checking out files: 93% (6004/6455)
Checking out files: 94% (6068/6455)
Checking out files: 95% (6133/6455)
Checking out files: 96% (6197/6455)
Checking out files: 97% (6262/6455)
Checking out files: 98% (6326/6455)
Checking out files: 99% (6391/6455)
Checking out files: 100% (6455/6455)
Checking out files: 100% (6455/6455), done.
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
COPY RUNNER
RUN test-quick in qemu:centos7
Packages installed:
SDL-devel-1.2.15-14.el7.x86_64
bison-3.0.4-1.el7.x86_64
bzip2-1.0.6-13.el7.x86_64
bzip2-devel-1.0.6-13.el7.x86_64
ccache-3.3.4-1.el7.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el7.x86_64
flex-2.5.37-3.el7.x86_64
gcc-4.8.5-28.el7_5.1.x86_64
gettext-0.19.8.1-2.el7.x86_64
git-1.8.3.1-14.el7_5.x86_64
glib2-devel-2.54.2-2.el7.x86_64
libaio-devel-0.3.109-13.el7.x86_64
libepoxy-devel-1.3.1-2.el7_5.x86_64
libfdt-devel-1.4.6-1.el7.x86_64
lzo-devel-2.06-8.el7.x86_64
make-3.82-23.el7.x86_64
mesa-libEGL-devel-17.2.3-8.20171019.el7.x86_64
mesa-libgbm-devel-17.2.3-8.20171019.el7.x86_64
nettle-devel-2.7.1-8.el7.x86_64
package g++ is not installed
package librdmacm-devel is not installed
pixman-devel-0.34.0-1.el7.x86_64
spice-glib-devel-0.34-3.el7_5.2.x86_64
spice-server-devel-0.14.0-2.el7_5.5.x86_64
tar-1.26-34.el7.x86_64
vte-devel-0.28.2-10.el7.x86_64
xen-devel-4.8.4.43.ge52ec4b787-1.el7.x86_64
zlib-devel-1.2.7-17.el7.x86_64
Environment variables:
PACKAGES=bison bzip2 bzip2-devel ccache csnappy-devel flex g++ gcc gettext git glib2-devel libaio-devel libepoxy-devel libfdt-devel librdmacm-devel lzo-devel make mesa-libEGL-devel mesa-libgbm-devel nettle-devel pixman-devel SDL-devel spice-glib-devel spice-server-devel tar vte-devel xen-devel zlib-devel
HOSTNAME=e408575c0370
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/home/patchew
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env
Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install
No C++ compiler available; disabling C++ specific optional code
Install prefix /tmp/qemu-test/install
BIOS directory /tmp/qemu-test/install/share/qemu
firmware path /tmp/qemu-test/install/share/qemu-firmware
binary directory /tmp/qemu-test/install/bin
library directory /tmp/qemu-test/install/lib
module directory /tmp/qemu-test/install/lib/qemu
libexec directory /tmp/qemu-test/install/libexec
include directory /tmp/qemu-test/install/include
config directory /tmp/qemu-test/install/etc
local state directory /tmp/qemu-test/install/var
Manual directory /tmp/qemu-test/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path /tmp/qemu-test/src
GIT binary git
GIT submodules
C compiler cc
Host C compiler cc
C++ compiler
Objective-C compiler cc
ARFLAGS rv
CFLAGS -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g
QEMU_CFLAGS -I/usr/include/pixman-1 -Werror -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-strong -Wno-missing-braces -I/usr/include/libpng15 -pthread -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/spice-1
LDFLAGS -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g
QEMU_LDFLAGS
make make
install install
python python -B
smbd /usr/sbin/smbd
module support no
host CPU x86_64
host big endian no
target list x86_64-softmmu aarch64-softmmu
gprof enabled no
sparse enabled no
strip binaries yes
profiler no
static build no
SDL support yes (1.2.15)
GTK support no
GTK GL support no
VTE support no
TLS priority NORMAL
GNUTLS support no
libgcrypt no
nettle yes (2.7.1)
libtasn1 no
curses support yes
virgl support no
curl support no
mingw32 support no
Audio drivers oss
Block whitelist (rw)
Block whitelist (ro)
VirtFS support no
Multipath support no
VNC support yes
VNC SASL support no
VNC JPEG support no
VNC PNG support yes
xen support yes
xen ctrl version 40800
pv dom build no
brlapi support no
bluez support no
Documentation no
PIE yes
vde support no
netmap support no
Linux AIO support yes
ATTR/XATTR support yes
Install blobs yes
KVM support yes
HAX support no
HVF support no
WHPX support no
TCG support yes
TCG debug enabled no
TCG interpreter no
malloc trim support yes
RDMA support yes
PVRDMA support yes
fdt support system
membarrier no
preadv support yes
fdatasync yes
madvise yes
posix_madvise yes
posix_memalign yes
libcap-ng support no
vhost-net support yes
vhost-crypto support yes
vhost-scsi support yes
vhost-vsock support yes
vhost-user support yes
Trace backends log
spice support yes (0.12.13/0.14.0)
rbd support no
xfsctl support no
smartcard support yes
libusb no
usb net redir no
OpenGL support yes
OpenGL dmabufs yes
libiscsi support no
libnfs support no
build guest agent yes
QGA VSS support no
QGA w32 disk info no
QGA MSI support no
seccomp support no
coroutine backend ucontext
coroutine pool yes
debug stack usage no
mutex debugging no
crypto afalg no
GlusterFS support no
gcov gcov
gcov enabled no
TPM support yes
libssh2 support no
TPM passthrough yes
TPM emulator yes
QOM debugging yes
Live block migration yes
lzo support yes
snappy support no
bzip2 support yes
NUMA host support no
libxml2 no
tcmalloc support no
jemalloc support no
avx2 optimization yes
replication support yes
VxHS block device no
bochs support yes
cloop support yes
dmg support yes
qcow v1 support yes
vdi support yes
vvfat support yes
qed support yes
parallels support yes
sheepdog support yes
capstone no
docker no
libpmem support no
libudev no
WARNING: Use of SDL 1.2 is deprecated and will be removed in
WARNING: future releases. Please switch to using SDL 2.0
NOTE: cross-compilers enabled: 'cc'
GEN x86_64-softmmu/config-devices.mak.tmp
GEN aarch64-softmmu/config-devices.mak.tmp
GEN config-host.h
GEN qemu-options.def
GEN qapi-gen
GEN trace/generated-tcg-tracers.h
GEN trace/generated-helpers-wrappers.h
GEN trace/generated-helpers.h
GEN aarch64-softmmu/config-devices.mak
GEN x86_64-softmmu/config-devices.mak
GEN trace/generated-helpers.c
GEN module_block.h
GEN ui/input-keymap-atset1-to-qcode.c
GEN ui/input-keymap-linux-to-qcode.c
GEN ui/input-keymap-qcode-to-atset1.c
GEN ui/input-keymap-qcode-to-atset2.c
GEN ui/input-keymap-qcode-to-atset3.c
GEN ui/input-keymap-qcode-to-linux.c
GEN ui/input-keymap-qcode-to-qnum.c
GEN ui/input-keymap-qcode-to-sun.c
GEN ui/input-keymap-qnum-to-qcode.c
GEN ui/input-keymap-usb-to-qcode.c
GEN ui/input-keymap-win32-to-qcode.c
GEN ui/input-keymap-x11-to-qcode.c
GEN ui/input-keymap-xorgevdev-to-qcode.c
GEN ui/input-keymap-xorgkbd-to-qcode.c
GEN ui/input-keymap-xorgxquartz-to-qcode.c
GEN ui/input-keymap-xorgxwin-to-qcode.c
GEN ui/input-keymap-osx-to-qcode.c
GEN tests/test-qapi-gen
GEN trace-root.h
GEN accel/kvm/trace.h
GEN accel/tcg/trace.h
GEN audio/trace.h
GEN block/trace.h
GEN chardev/trace.h
GEN crypto/trace.h
GEN hw/9pfs/trace.h
GEN hw/acpi/trace.h
GEN hw/alpha/trace.h
GEN hw/arm/trace.h
GEN hw/audio/trace.h
GEN hw/block/trace.h
GEN hw/block/dataplane/trace.h
GEN hw/char/trace.h
GEN hw/display/trace.h
GEN hw/dma/trace.h
GEN hw/hppa/trace.h
GEN hw/i2c/trace.h
GEN hw/i386/trace.h
GEN hw/i386/xen/trace.h
GEN hw/ide/trace.h
GEN hw/input/trace.h
GEN hw/intc/trace.h
GEN hw/isa/trace.h
GEN hw/mem/trace.h
GEN hw/misc/trace.h
GEN hw/misc/macio/trace.h
GEN hw/net/trace.h
GEN hw/nvram/trace.h
GEN hw/pci/trace.h
GEN hw/pci-host/trace.h
GEN hw/ppc/trace.h
GEN hw/rdma/trace.h
GEN hw/rdma/vmw/trace.h
GEN hw/s390x/trace.h
GEN hw/scsi/trace.h
GEN hw/sd/trace.h
GEN hw/sparc/trace.h
GEN hw/sparc64/trace.h
GEN hw/timer/trace.h
GEN hw/tpm/trace.h
GEN hw/usb/trace.h
GEN hw/vfio/trace.h
GEN hw/virtio/trace.h
GEN hw/watchdog/trace.h
GEN hw/xen/trace.h
GEN io/trace.h
GEN linux-user/trace.h
GEN migration/trace.h
GEN nbd/trace.h
GEN net/trace.h
GEN qapi/trace.h
GEN qom/trace.h
GEN scsi/trace.h
GEN target/arm/trace.h
GEN target/i386/trace.h
GEN target/mips/trace.h
GEN target/ppc/trace.h
GEN target/s390x/trace.h
GEN target/sparc/trace.h
GEN ui/trace.h
GEN util/trace.h
GEN trace-root.c
GEN accel/kvm/trace.c
GEN accel/tcg/trace.c
GEN audio/trace.c
GEN block/trace.c
GEN chardev/trace.c
GEN crypto/trace.c
GEN hw/9pfs/trace.c
GEN hw/acpi/trace.c
GEN hw/alpha/trace.c
GEN hw/arm/trace.c
GEN hw/audio/trace.c
GEN hw/block/trace.c
GEN hw/block/dataplane/trace.c
GEN hw/char/trace.c
GEN hw/display/trace.c
GEN hw/dma/trace.c
GEN hw/hppa/trace.c
GEN hw/i2c/trace.c
GEN hw/i386/trace.c
GEN hw/i386/xen/trace.c
GEN hw/ide/trace.c
GEN hw/input/trace.c
GEN hw/intc/trace.c
GEN hw/isa/trace.c
GEN hw/mem/trace.c
GEN hw/misc/trace.c
GEN hw/misc/macio/trace.c
GEN hw/net/trace.c
GEN hw/nvram/trace.c
GEN hw/pci/trace.c
GEN hw/pci-host/trace.c
GEN hw/ppc/trace.c
GEN hw/rdma/trace.c
GEN hw/rdma/vmw/trace.c
GEN hw/s390x/trace.c
GEN hw/scsi/trace.c
GEN hw/sd/trace.c
GEN hw/sparc/trace.c
GEN hw/sparc64/trace.c
GEN hw/timer/trace.c
GEN hw/tpm/trace.c
GEN hw/usb/trace.c
GEN hw/vfio/trace.c
GEN hw/virtio/trace.c
GEN hw/watchdog/trace.c
GEN hw/xen/trace.c
GEN io/trace.c
GEN linux-user/trace.c
GEN migration/trace.c
GEN nbd/trace.c
GEN net/trace.c
GEN qapi/trace.c
GEN qom/trace.c
GEN scsi/trace.c
GEN target/arm/trace.c
GEN target/i386/trace.c
GEN target/mips/trace.c
GEN target/ppc/trace.c
GEN target/s390x/trace.c
GEN target/sparc/trace.c
GEN ui/trace.c
GEN util/trace.c
GEN config-all-devices.mak
CC tests/qemu-iotests/socket_scm_helper.o
GEN qga/qapi-generated/qapi-gen
CC qapi/qapi-builtin-types.o
CC qapi/qapi-types.o
CC qapi/qapi-types-block-core.o
CC qapi/qapi-types-block.o
CC qapi/qapi-types-char.o
CC qapi/qapi-types-common.o
CC qapi/qapi-types-crypto.o
CC qapi/qapi-types-introspect.o
CC qapi/qapi-types-job.o
CC qapi/qapi-types-migration.o
CC qapi/qapi-types-misc.o
CC qapi/qapi-types-net.o
CC qapi/qapi-types-rocker.o
CC qapi/qapi-types-run-state.o
CC qapi/qapi-types-sockets.o
CC qapi/qapi-types-tpm.o
CC qapi/qapi-types-trace.o
CC qapi/qapi-types-transaction.o
CC qapi/qapi-types-ui.o
CC qapi/qapi-builtin-visit.o
CC qapi/qapi-visit.o
CC qapi/qapi-visit-block-core.o
CC qapi/qapi-visit-block.o
CC qapi/qapi-visit-char.o
CC qapi/qapi-visit-common.o
CC qapi/qapi-visit-crypto.o
CC qapi/qapi-visit-introspect.o
CC qapi/qapi-visit-job.o
CC qapi/qapi-visit-migration.o
CC qapi/qapi-visit-misc.o
CC qapi/qapi-visit-rocker.o
CC qapi/qapi-visit-net.o
CC qapi/qapi-visit-run-state.o
CC qapi/qapi-visit-sockets.o
CC qapi/qapi-visit-tpm.o
CC qapi/qapi-visit-trace.o
CC qapi/qapi-visit-transaction.o
CC qapi/qapi-visit-ui.o
CC qapi/qapi-events.o
CC qapi/qapi-events-block-core.o
CC qapi/qapi-events-block.o
CC qapi/qapi-events-char.o
CC qapi/qapi-events-common.o
CC qapi/qapi-events-crypto.o
CC qapi/qapi-events-introspect.o
CC qapi/qapi-events-job.o
CC qapi/qapi-events-migration.o
CC qapi/qapi-events-misc.o
CC qapi/qapi-events-net.o
CC qapi/qapi-events-rocker.o
CC qapi/qapi-events-run-state.o
CC qapi/qapi-events-sockets.o
CC qapi/qapi-events-tpm.o
CC qapi/qapi-events-transaction.o
CC qapi/qapi-events-trace.o
CC qapi/qapi-events-ui.o
CC qapi/qapi-introspect.o
CC qapi/qapi-visit-core.o
CC qapi/qapi-dealloc-visitor.o
CC qapi/qobject-input-visitor.o
CC qapi/qobject-output-visitor.o
CC qapi/qmp-registry.o
CC qapi/qmp-dispatch.o
CC qapi/string-input-visitor.o
CC qapi/string-output-visitor.o
CC qapi/opts-visitor.o
CC qapi/qapi-clone-visitor.o
CC qapi/qmp-event.o
CC qapi/qapi-util.o
CC qobject/qnull.o
CC qobject/qnum.o
CC qobject/qstring.o
CC qobject/qdict.o
CC qobject/qlist.o
CC qobject/qbool.o
CC qobject/qlit.o
CC qobject/qjson.o
CC qobject/qobject.o
CC qobject/json-lexer.o
CC qobject/json-streamer.o
CC qobject/json-parser.o
CC qobject/block-qdict.o
CC trace/control.o
CC trace/qmp.o
CC util/osdep.o
CC util/cutils.o
CC util/unicode.o
CC util/qemu-timer-common.o
CC util/bufferiszero.o
CC util/lockcnt.o
CC util/aiocb.o
CC util/async.o
CC util/aio-wait.o
CC util/thread-pool.o
CC util/qemu-timer.o
CC util/main-loop.o
CC util/iohandler.o
CC util/aio-posix.o
CC util/compatfd.o
CC util/event_notifier-posix.o
CC util/mmap-alloc.o
CC util/oslib-posix.o
CC util/qemu-openpty.o
CC util/qemu-thread-posix.o
CC util/memfd.o
CC util/envlist.o
CC util/path.o
CC util/module.o
CC util/host-utils.o
CC util/bitmap.o
CC util/bitops.o
CC util/hbitmap.o
CC util/fifo8.o
CC util/acl.o
CC util/cacheinfo.o
CC util/error.o
CC util/qemu-error.o
CC util/id.o
CC util/iov.o
CC util/qemu-config.o
CC util/qemu-sockets.o
CC util/uri.o
CC util/notify.o
CC util/qemu-option.o
CC util/qemu-progress.o
CC util/keyval.o
CC util/hexdump.o
CC util/crc32c.o
CC util/uuid.o
CC util/throttle.o
CC util/getauxval.o
CC util/readline.o
CC util/rcu.o
CC util/qemu-coroutine.o
CC util/qemu-coroutine-lock.o
CC util/qemu-coroutine-io.o
CC util/qemu-coroutine-sleep.o
CC util/coroutine-ucontext.o
CC util/buffer.o
CC util/timed-average.o
CC util/base64.o
CC util/log.o
CC util/pagesize.o
CC util/qdist.o
CC util/qht.o
CC util/qsp.o
CC util/range.o
CC util/stats64.o
CC util/systemd.o
CC util/iova-tree.o
CC util/vfio-helpers.o
CC util/drm.o
CC trace-root.o
CC accel/kvm/trace.o
CC accel/tcg/trace.o
CC audio/trace.o
CC block/trace.o
CC chardev/trace.o
CC crypto/trace.o
CC hw/9pfs/trace.o
CC hw/acpi/trace.o
CC hw/alpha/trace.o
CC hw/arm/trace.o
CC hw/audio/trace.o
CC hw/block/trace.o
CC hw/block/dataplane/trace.o
CC hw/char/trace.o
CC hw/display/trace.o
CC hw/dma/trace.o
CC hw/hppa/trace.o
CC hw/i2c/trace.o
CC hw/i386/trace.o
CC hw/i386/xen/trace.o
CC hw/ide/trace.o
CC hw/input/trace.o
CC hw/intc/trace.o
CC hw/isa/trace.o
CC hw/mem/trace.o
CC hw/misc/trace.o
CC hw/misc/macio/trace.o
CC hw/net/trace.o
CC hw/nvram/trace.o
CC hw/pci/trace.o
CC hw/pci-host/trace.o
CC hw/ppc/trace.o
CC hw/rdma/trace.o
CC hw/rdma/vmw/trace.o
CC hw/s390x/trace.o
CC hw/scsi/trace.o
CC hw/sd/trace.o
CC hw/sparc/trace.o
CC hw/sparc64/trace.o
CC hw/timer/trace.o
CC hw/tpm/trace.o
CC hw/usb/trace.o
CC hw/vfio/trace.o
CC hw/virtio/trace.o
CC hw/watchdog/trace.o
CC hw/xen/trace.o
CC io/trace.o
CC linux-user/trace.o
CC migration/trace.o
CC nbd/trace.o
CC net/trace.o
CC qapi/trace.o
CC qom/trace.o
CC scsi/trace.o
CC target/arm/trace.o
CC target/i386/trace.o
CC target/mips/trace.o
CC target/ppc/trace.o
CC target/s390x/trace.o
CC target/sparc/trace.o
CC ui/trace.o
CC util/trace.o
CC crypto/pbkdf-stub.o
CC stubs/arch-query-cpu-def.o
CC stubs/arch-query-cpu-model-expansion.o
CC stubs/arch-query-cpu-model-comparison.o
CC stubs/arch-query-cpu-model-baseline.o
CC stubs/bdrv-next-monitor-owned.o
CC stubs/blk-commit-all.o
CC stubs/blockdev-close-all-bdrv-states.o
CC stubs/clock-warp.o
CC stubs/cpu-get-clock.o
CC stubs/cpu-get-icount.o
CC stubs/dump.o
CC stubs/error-printf.o
CC stubs/fdset.o
CC stubs/gdbstub.o
CC stubs/get-vm-name.o
CC stubs/iothread.o
CC stubs/iothread-lock.o
CC stubs/is-daemonized.o
CC stubs/linux-aio.o
CC stubs/migr-blocker.o
CC stubs/machine-init-done.o
CC stubs/change-state-handler.o
CC stubs/monitor.o
CC stubs/notify-event.o
CC stubs/qtest.o
CC stubs/replay.o
CC stubs/runstate-check.o
CC stubs/set-fd-handler.o
CC stubs/slirp.o
CC stubs/sysbus.o
CC stubs/tpm.o
CC stubs/trace-control.o
CC stubs/uuid.o
CC stubs/vm-stop.o
CC stubs/vmstate.o
CC stubs/qmp_memory_device.o
CC stubs/target-monitor-defs.o
CC stubs/target-get-monitor-def.o
CC stubs/pc_madt_cpu_entry.o
CC stubs/vmgenid.o
CC stubs/xen-common.o
CC stubs/xen-hvm.o
CC stubs/pci-host-piix.o
CC stubs/ram-block.o
CC stubs/ramfb.o
CC contrib/ivshmem-client/ivshmem-client.o
CC contrib/ivshmem-client/main.o
CC contrib/ivshmem-server/main.o
CC contrib/ivshmem-server/ivshmem-server.o
CC qemu-nbd.o
CC block.o
CC blockjob.o
CC job.o
CC qemu-io-cmds.o
CC replication.o
CC block/raw-format.o
CC block/vmdk.o
CC block/vpc.o
CC block/qcow.o
CC block/vdi.o
CC block/cloop.o
CC block/bochs.o
CC block/vvfat.o
CC block/qcow2.o
CC block/dmg.o
CC block/qcow2-refcount.o
CC block/qcow2-cluster.o
CC block/qcow2-snapshot.o
CC block/qcow2-cache.o
CC block/qcow2-bitmap.o
CC block/qed.o
CC block/qed-l2-cache.o
CC block/qed-table.o
CC block/qed-cluster.o
CC block/qed-check.o
CC block/vhdx.o
CC block/vhdx-endian.o
CC block/vhdx-log.o
CC block/quorum.o
CC block/blkdebug.o
CC block/blkverify.o
CC block/blkreplay.o
CC block/parallels.o
CC block/blklogwrites.o
CC block/block-backend.o
CC block/snapshot.o
CC block/qapi.o
CC block/file-posix.o
CC block/linux-aio.o
CC block/null.o
CC block/mirror.o
CC block/commit.o
CC block/io.o
CC block/create.o
CC block/throttle-groups.o
CC block/nvme.o
CC block/nbd.o
CC block/nbd-client.o
CC block/sheepdog.o
CC block/accounting.o
CC block/dirty-bitmap.o
CC block/write-threshold.o
CC block/backup.o
CC block/replication.o
CC block/throttle.o
CC block/copy-on-read.o
CC block/crypto.o
CC nbd/server.o
CC nbd/client.o
CC nbd/common.o
CC scsi/utils.o
CC scsi/pr-manager.o
CC scsi/pr-manager-helper.o
CC block/dmg-bz2.o
CC crypto/init.o
CC crypto/hash.o
CC crypto/hash-nettle.o
CC crypto/hmac.o
CC crypto/hmac-nettle.o
CC crypto/aes.o
CC crypto/desrfb.o
CC crypto/cipher.o
CC crypto/tlscreds.o
CC crypto/tlscredsanon.o
CC crypto/tlscredspsk.o
CC crypto/tlscredsx509.o
CC crypto/tlssession.o
CC crypto/secret.o
CC crypto/random-platform.o
CC crypto/pbkdf.o
CC crypto/pbkdf-nettle.o
CC crypto/ivgen.o
CC crypto/ivgen-essiv.o
CC crypto/ivgen-plain.o
CC crypto/ivgen-plain64.o
CC crypto/afsplit.o
CC crypto/xts.o
CC crypto/block.o
CC crypto/block-qcow.o
CC crypto/block-luks.o
CC io/channel.o
CC io/channel-buffer.o
CC io/channel-command.o
CC io/channel-file.o
CC io/channel-socket.o
CC io/channel-tls.o
CC io/channel-watch.o
CC io/channel-websock.o
CC io/channel-util.o
CC io/dns-resolver.o
CC io/net-listener.o
CC io/task.o
CC qom/object.o
CC qom/container.o
CC qom/qom-qobject.o
CC qom/object_interfaces.o
GEN qemu-img-cmds.h
CC qemu-io.o
CC qemu-edid.o
CC hw/display/edid-generate.o
CC scsi/qemu-pr-helper.o
CC blockdev.o
CC qemu-bridge-helper.o
CC blockdev-nbd.o
CC bootdevice.o
CC iothread.o
CC job-qmp.o
CC qdev-monitor.o
CC device-hotplug.o
CC os-posix.o
CC bt-host.o
CC bt-vhci.o
CC dma-helpers.o
CC vl.o
CC tpm.o
CC device_tree.o
CC qapi/qapi-commands.o
CC qapi/qapi-commands-block-core.o
CC qapi/qapi-commands-block.o
CC qapi/qapi-commands-char.o
CC qapi/qapi-commands-common.o
CC qapi/qapi-commands-crypto.o
CC qapi/qapi-commands-introspect.o
CC qapi/qapi-commands-job.o
CC qapi/qapi-commands-migration.o
CC qapi/qapi-commands-misc.o
CC qapi/qapi-commands-net.o
CC qapi/qapi-commands-rocker.o
CC qapi/qapi-commands-run-state.o
CC qapi/qapi-commands-sockets.o
CC qapi/qapi-commands-tpm.o
CC qapi/qapi-commands-trace.o
CC qapi/qapi-commands-transaction.o
CC qapi/qapi-commands-ui.o
CC qmp.o
CC hmp.o
CC cpus-common.o
CC audio/audio.o
CC audio/noaudio.o
CC audio/wavaudio.o
CC audio/mixeng.o
CC audio/spiceaudio.o
CC audio/wavcapture.o
CC backends/rng.o
CC backends/rng-egd.o
CC backends/rng-random.o
CC backends/tpm.o
CC backends/hostmem.o
CC backends/hostmem-ram.o
CC backends/hostmem-file.o
CC backends/cryptodev.o
CC backends/cryptodev-builtin.o
CC backends/cryptodev-vhost.o
CC backends/cryptodev-vhost-user.o
CC backends/hostmem-memfd.o
CC block/stream.o
CC chardev/msmouse.o
CC chardev/wctablet.o
CC chardev/testdev.o
CC chardev/spice.o
CC disas/arm.o
CC disas/i386.o
CC fsdev/qemu-fsdev-dummy.o
CC fsdev/qemu-fsdev-opts.o
CC fsdev/qemu-fsdev-throttle.o
CC hw/acpi/core.o
CC hw/acpi/piix4.o
CC hw/acpi/pcihp.o
CC hw/acpi/ich9.o
CC hw/acpi/tco.o
CC hw/acpi/cpu_hotplug.o
CC hw/acpi/memory_hotplug.o
CC hw/acpi/cpu.o
CC hw/acpi/nvdimm.o
CC hw/acpi/vmgenid.o
CC hw/acpi/acpi_interface.o
CC hw/acpi/bios-linker-loader.o
CC hw/acpi/aml-build.o
CC hw/acpi/ipmi.o
CC hw/acpi/acpi-stub.o
CC hw/acpi/ipmi-stub.o
CC hw/audio/sb16.o
CC hw/audio/es1370.o
CC hw/audio/ac97.o
CC hw/audio/fmopl.o
CC hw/audio/adlib.o
CC hw/audio/gus.o
CC hw/audio/gusemu_hal.o
CC hw/audio/gusemu_mixer.o
CC hw/audio/cs4231a.o
CC hw/audio/intel-hda.o
CC hw/audio/hda-codec.o
CC hw/audio/pcspk.o
CC hw/audio/wm8750.o
CC hw/audio/pl041.o
CC hw/audio/lm4549.o
CC hw/audio/marvell_88w8618.o
CC hw/audio/soundhw.o
CC hw/block/block.o
CC hw/block/cdrom.o
CC hw/block/hd-geometry.o
CC hw/block/fdc.o
CC hw/block/m25p80.o
CC hw/block/nand.o
CC hw/block/pflash_cfi01.o
CC hw/block/pflash_cfi02.o
CC hw/block/xen_disk.o
CC hw/block/ecc.o
CC hw/block/onenand.o
CC hw/block/nvme.o
CC hw/bt/core.o
CC hw/bt/l2cap.o
CC hw/bt/sdp.o
CC hw/bt/hci.o
CC hw/bt/hid.o
CC hw/bt/hci-csr.o
CC hw/char/ipoctal232.o
CC hw/char/nrf51_uart.o
CC hw/char/parallel.o
CC hw/char/parallel-isa.o
CC hw/char/pl011.o
CC hw/char/serial.o
CC hw/char/serial-isa.o
CC hw/char/serial-pci.o
CC hw/char/virtio-console.o
CC hw/char/xen_console.o
CC hw/char/cadence_uart.o
CC hw/char/cmsdk-apb-uart.o
CC hw/char/debugcon.o
CC hw/char/imx_serial.o
CC hw/core/qdev.o
CC hw/core/qdev-properties.o
CC hw/core/bus.o
CC hw/core/reset.o
CC hw/core/qdev-fw.o
CC hw/core/fw-path-provider.o
CC hw/core/irq.o
CC hw/core/hotplug.o
CC hw/core/nmi.o
CC hw/core/stream.o
CC hw/core/ptimer.o
CC hw/core/sysbus.o
CC hw/core/machine.o
CC hw/core/loader.o
CC hw/core/qdev-properties-system.o
CC hw/core/register.o
CC hw/core/or-irq.o
CC hw/core/split-irq.o
CC hw/core/platform-bus.o
CC hw/core/generic-loader.o
CC hw/core/null-machine.o
CC hw/cpu/core.o
CC hw/display/ramfb.o
CC hw/display/ramfb-standalone.o
CC hw/display/ads7846.o
CC hw/display/cirrus_vga.o
CC hw/display/cirrus_vga_isa.o
CC hw/display/pl110.o
CC hw/display/sii9022.o
CC hw/display/ssd0303.o
CC hw/display/ssd0323.o
CC hw/display/xenfb.o
CC hw/display/vga-pci.o
CC hw/display/edid-region.o
CC hw/display/vga-isa.o
CC hw/display/vmware_vga.o
CC hw/display/bochs-display.o
CC hw/display/blizzard.o
CC hw/display/exynos4210_fimd.o
CC hw/display/framebuffer.o
CC hw/display/tc6393xb.o
CC hw/display/qxl.o
CC hw/display/qxl-logger.o
CC hw/display/qxl-render.o
CC hw/dma/pl080.o
CC hw/dma/pl330.o
CC hw/dma/i8257.o
CC hw/dma/xilinx_axidma.o
CC hw/dma/xlnx-zynq-devcfg.o
CC hw/dma/xlnx-zdma.o
CC hw/gpio/max7310.o
CC hw/gpio/pl061.o
CC hw/gpio/zaurus.o
CC hw/gpio/gpio_key.o
CC hw/i2c/core.o
CC hw/i2c/smbus.o
CC hw/i2c/smbus_eeprom.o
CC hw/i2c/i2c-ddc.o
CC hw/i2c/versatile_i2c.o
CC hw/i2c/smbus_ich9.o
CC hw/i2c/pm_smbus.o
CC hw/i2c/bitbang_i2c.o
CC hw/i2c/exynos4210_i2c.o
CC hw/i2c/imx_i2c.o
CC hw/i2c/aspeed_i2c.o
CC hw/ide/core.o
CC hw/ide/atapi.o
CC hw/ide/qdev.o
CC hw/ide/pci.o
CC hw/ide/isa.o
CC hw/ide/piix.o
CC hw/ide/microdrive.o
CC hw/ide/ahci.o
CC hw/ide/ich.o
CC hw/ide/ahci-allwinner.o
CC hw/input/hid.o
CC hw/input/lm832x.o
CC hw/input/pckbd.o
CC hw/input/pl050.o
CC hw/input/ps2.o
CC hw/input/stellaris_input.o
CC hw/input/tsc2005.o
CC hw/input/virtio-input.o
CC hw/input/virtio-input-hid.o
CC hw/input/virtio-input-host.o
CC hw/intc/i8259_common.o
CC hw/intc/i8259.o
CC hw/intc/pl190.o
CC hw/intc/xlnx-pmu-iomod-intc.o
CC hw/intc/xlnx-zynqmp-ipi.o
CC hw/intc/imx_avic.o
CC hw/intc/imx_gpcv2.o
CC hw/intc/realview_gic.o
CC hw/intc/ioapic_common.o
CC hw/intc/arm_gic_common.o
CC hw/intc/arm_gic.o
CC hw/intc/arm_gicv2m.o
CC hw/intc/arm_gicv3_common.o
CC hw/intc/arm_gicv3.o
CC hw/intc/arm_gicv3_dist.o
CC hw/intc/arm_gicv3_redist.o
CC hw/intc/arm_gicv3_its_common.o
CC hw/intc/intc.o
CC hw/ipack/ipack.o
CC hw/ipack/tpci200.o
CC hw/ipmi/ipmi.o
CC hw/ipmi/ipmi_bmc_sim.o
CC hw/ipmi/ipmi_bmc_extern.o
CC hw/ipmi/isa_ipmi_kcs.o
CC hw/ipmi/isa_ipmi_bt.o
CC hw/isa/isa-bus.o
CC hw/isa/isa-superio.o
CC hw/isa/apm.o
CC hw/mem/pc-dimm.o
CC hw/mem/memory-device.o
CC hw/mem/nvdimm.o
CC hw/misc/applesmc.o
CC hw/misc/max111x.o
CC hw/misc/tmp105.o
CC hw/misc/tmp421.o
CC hw/misc/debugexit.o
CC hw/misc/sga.o
CC hw/misc/pc-testdev.o
CC hw/misc/pci-testdev.o
CC hw/misc/edu.o
CC hw/misc/pca9552.o
CC hw/misc/unimp.o
CC hw/misc/vmcoreinfo.o
CC hw/misc/arm_l2x0.o
CC hw/misc/arm_integrator_debug.o
CC hw/misc/a9scu.o
CC hw/misc/arm11scu.o
CC hw/net/xen_nic.o
CC hw/net/ne2000.o
CC hw/net/eepro100.o
CC hw/net/pcnet-pci.o
CC hw/net/pcnet.o
CC hw/net/e1000.o
CC hw/net/e1000x_common.o
CC hw/net/net_tx_pkt.o
CC hw/net/net_rx_pkt.o
CC hw/net/e1000e.o
CC hw/net/e1000e_core.o
CC hw/net/rtl8139.o
CC hw/net/vmxnet3.o
CC hw/net/smc91c111.o
CC hw/net/lan9118.o
CC hw/net/ne2000-isa.o
CC hw/net/xgmac.o
CC hw/net/xilinx_axienet.o
CC hw/net/allwinner_emac.o
CC hw/net/imx_fec.o
CC hw/net/cadence_gem.o
CC hw/net/stellaris_enet.o
CC hw/net/ftgmac100.o
CC hw/net/rocker/rocker.o
CC hw/net/rocker/rocker_fp.o
CC hw/net/rocker/rocker_desc.o
CC hw/net/rocker/rocker_world.o
CC hw/net/rocker/rocker_of_dpa.o
CC hw/net/can/can_sja1000.o
CC hw/net/can/can_kvaser_pci.o
CC hw/net/can/can_pcm3680_pci.o
CC hw/net/can/can_mioe3680_pci.o
CC hw/nvram/eeprom93xx.o
CC hw/nvram/fw_cfg.o
CC hw/nvram/chrp_nvram.o
CC hw/pci-bridge/pci_bridge_dev.o
CC hw/pci-bridge/pcie_root_port.o
CC hw/pci-bridge/pcie_pci_bridge.o
CC hw/pci-bridge/gen_pcie_root_port.o
CC hw/pci-bridge/pci_expander_bridge.o
CC hw/pci-bridge/xio3130_upstream.o
CC hw/pci-bridge/xio3130_downstream.o
CC hw/pci-bridge/ioh3420.o
CC hw/pci-bridge/i82801b11.o
CC hw/pci-host/pam.o
CC hw/pci-host/versatile.o
CC hw/pci-host/piix.o
CC hw/pci-host/q35.o
CC hw/pci-host/gpex.o
CC hw/pci-host/designware.o
CC hw/pci/pci.o
CC hw/pci/pci_bridge.o
CC hw/pci/msix.o
CC hw/pci/msi.o
CC hw/pci/shpc.o
CC hw/pci/slotid_cap.o
CC hw/pci/pci_host.o
CC hw/pci/pcie_host.o
CC hw/pci/pcie.o
CC hw/pci/pcie_aer.o
CC hw/pci/pcie_port.o
CC hw/pci/pci-stub.o
CC hw/pcmcia/pcmcia.o
CC hw/scsi/scsi-disk.o
CC hw/scsi/emulation.o
CC hw/scsi/scsi-generic.o
CC hw/scsi/scsi-bus.o
CC hw/scsi/lsi53c895a.o
CC hw/scsi/mptsas.o
CC hw/scsi/mptconfig.o
CC hw/scsi/mptendian.o
CC hw/scsi/megasas.o
CC hw/scsi/vmw_pvscsi.o
CC hw/scsi/esp.o
CC hw/scsi/esp-pci.o
CC hw/sd/pl181.o
CC hw/sd/ssi-sd.o
CC hw/sd/sd.o
CC hw/sd/core.o
CC hw/sd/sdmmc-internal.o
CC hw/sd/sdhci.o
CC hw/smbios/smbios.o
CC hw/smbios/smbios_type_38.o
CC hw/smbios/smbios-stub.o
CC hw/smbios/smbios_type_38-stub.o
CC hw/ssi/pl022.o
CC hw/ssi/ssi.o
CC hw/ssi/xilinx_spips.o
CC hw/ssi/aspeed_smc.o
CC hw/ssi/stm32f2xx_spi.o
CC hw/ssi/mss-spi.o
CC hw/timer/arm_timer.o
CC hw/timer/arm_mptimer.o
CC hw/timer/armv7m_systick.o
CC hw/timer/a9gtimer.o
CC hw/timer/ds1338.o
CC hw/timer/cadence_ttc.o
CC hw/timer/hpet.o
CC hw/timer/i8254_common.o
CC hw/timer/i8254.o
CC hw/timer/pl031.o
CC hw/timer/twl92230.o
CC hw/timer/imx_epit.o
CC hw/timer/imx_gpt.o
CC hw/timer/xlnx-zynqmp-rtc.o
CC hw/timer/stm32f2xx_timer.o
CC hw/timer/aspeed_timer.o
CC hw/timer/cmsdk-apb-timer.o
CC hw/timer/cmsdk-apb-dualtimer.o
CC hw/timer/mss-timer.o
CC hw/tpm/tpm_util.o
CC hw/tpm/tpm_tis.o
CC hw/tpm/tpm_crb.o
CC hw/tpm/tpm_passthrough.o
CC hw/tpm/tpm_emulator.o
CC hw/usb/core.o
CC hw/usb/combined-packet.o
CC hw/usb/bus.o
CC hw/usb/libhw.o
CC hw/usb/desc.o
CC hw/usb/desc-msos.o
CC hw/usb/hcd-uhci.o
CC hw/usb/hcd-ohci.o
CC hw/usb/hcd-ehci.o
CC hw/usb/hcd-ehci-pci.o
CC hw/usb/hcd-ehci-sysbus.o
CC hw/usb/hcd-xhci.o
CC hw/usb/hcd-xhci-nec.o
CC hw/usb/hcd-musb.o
CC hw/usb/dev-hub.o
CC hw/usb/dev-hid.o
CC hw/usb/dev-wacom.o
CC hw/usb/dev-storage.o
CC hw/usb/dev-uas.o
CC hw/usb/dev-audio.o
CC hw/usb/dev-serial.o
CC hw/usb/dev-network.o
CC hw/usb/dev-bluetooth.o
CC hw/usb/dev-smartcard-reader.o
CC hw/usb/ccid-card-passthru.o
CC hw/usb/ccid-card-emulated.o
CC hw/usb/dev-mtp.o
CC hw/usb/host-stub.o
CC hw/virtio/virtio-bus.o
CC hw/virtio/virtio-rng.o
CC hw/virtio/virtio-pci.o
CC hw/virtio/virtio-mmio.o
CC hw/virtio/vhost-stub.o
CC hw/watchdog/watchdog.o
CC hw/watchdog/cmsdk-apb-watchdog.o
CC hw/watchdog/wdt_i6300esb.o
CC hw/watchdog/wdt_ib700.o
CC hw/watchdog/wdt_aspeed.o
CC hw/xen/xen_backend.o
CC hw/xen/xen_devconfig.o
CC hw/xen/xen_pvdev.o
CC hw/xen/xen-common.o
CC migration/migration.o
CC migration/socket.o
CC migration/fd.o
CC migration/exec.o
CC migration/tls.o
CC migration/channel.o
CC migration/savevm.o
CC migration/colo.o
CC migration/colo-failover.o
CC migration/vmstate.o
CC migration/vmstate-types.o
CC migration/qemu-file.o
CC migration/page_cache.o
CC migration/global_state.o
CC migration/qemu-file-channel.o
CC migration/xbzrle.o
CC migration/postcopy-ram.o
CC migration/qjson.o
CC migration/block-dirty-bitmap.o
CC migration/rdma.o
CC migration/block.o
CC net/net.o
CC net/queue.o
CC net/checksum.o
CC net/util.o
CC net/hub.o
CC net/socket.o
CC net/dump.o
CC net/eth.o
CC net/l2tpv3.o
CC net/vhost-user.o
CC net/slirp.o
CC net/filter.o
CC net/filter-buffer.o
CC net/filter-mirror.o
CC net/colo-compare.o
/tmp/qemu-test/src/migration/rdma.c: In function 'qemu_rdma_accept':
/tmp/qemu-test/src/migration/rdma.c:3353:5: error: implicit declaration of function 'migrate_postcopy' [-Werror=implicit-function-declaration]
if (migrate_postcopy() && !rdma->is_return_path) {
^
/tmp/qemu-test/src/migration/rdma.c:3353:5: error: nested extern declaration of 'migrate_postcopy' [-Werror=nested-externs]
cc1: all warnings being treated as errors
make: *** [migration/rdma.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
File "./tests/docker/docker.py", line 563, in <module>
sys.exit(main())
File "./tests/docker/docker.py", line 560, in main
return args.cmdobj.run(args, argv)
File "./tests/docker/docker.py", line 306, in run
return Docker().run(argv, args.keep, quiet=args.quiet)
File "./tests/docker/docker.py", line 274, in run
quiet=quiet)
File "./tests/docker/docker.py", line 181, in _do_check
return subprocess.check_call(self._command + cmd, **kwargs)
File "/usr/lib64/python2.7/subprocess.py", line 542, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=033cedace90711e8ba3b68b59973b7d0', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '--net=none', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-5w43kt65/src/docker-src.2018-11-15-13.47.54.26143:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-5w43kt65/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 2m26.991s
user 0m17.224s
sys 0m3.671s
=== OUTPUT END ===
Test command exited with code: 2
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support
2018-11-15 18:50 ` [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support no-reply
@ 2018-11-16 1:38 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-16 1:38 UTC (permalink / raw)
To: qemu-devel
Cc: famz, virtio-dev, mst, quintela, dgilbert, liliang.opensource,
peterx, pbonzini, nilal
On 11/16/2018 02:50 AM, no-reply@patchew.org wrote:
> Hi,
>
> This series failed docker-quick@centos7 build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
>
> CC net/filter.o
> CC net/filter-buffer.o
> CC net/filter-mirror.o
> CC net/colo-compare.o
> /tmp/qemu-test/src/migration/rdma.c: In function 'qemu_rdma_accept':
> /tmp/qemu-test/src/migration/rdma.c:3353:5: error: implicit declaration of function 'migrate_postcopy' [-Werror=implicit-function-declaration]
> if (migrate_postcopy() && !rdma->is_return_path) {
> ^
> /tmp/qemu-test/src/migration/rdma.c:3353:5: error: nested extern declaration of 'migrate_postcopy' [-Werror=nested-externs]
> cc1: all warnings being treated as errors
> make: *** [migration/rdma.o] Error 1
> make: *** Waiting for unfinished jobs....
This is caused by missing "migration/misc.h" in rdma.c, since we moved
the migrate_postcopy() declaration there. I'll add it.
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* [virtio-dev] Re: [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support
@ 2018-11-16 1:38 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-16 1:38 UTC (permalink / raw)
To: qemu-devel
Cc: famz, virtio-dev, mst, quintela, dgilbert, liliang.opensource,
peterx, pbonzini, nilal
On 11/16/2018 02:50 AM, no-reply@patchew.org wrote:
> Hi,
>
> This series failed docker-quick@centos7 build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
>
> CC net/filter.o
> CC net/filter-buffer.o
> CC net/filter-mirror.o
> CC net/colo-compare.o
> /tmp/qemu-test/src/migration/rdma.c: In function 'qemu_rdma_accept':
> /tmp/qemu-test/src/migration/rdma.c:3353:5: error: implicit declaration of function 'migrate_postcopy' [-Werror=implicit-function-declaration]
> if (migrate_postcopy() && !rdma->is_return_path) {
> ^
> /tmp/qemu-test/src/migration/rdma.c:3353:5: error: nested extern declaration of 'migrate_postcopy' [-Werror=nested-externs]
> cc1: all warnings being treated as errors
> make: *** [migration/rdma.o] Error 1
> make: *** Waiting for unfinished jobs....
This is caused by missing "migration/misc.h" in rdma.c, since we moved
the migrate_postcopy() declaration there. I'll add it.
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
@ 2018-11-27 3:11 ` Wei Wang
-1 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 3:11 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel
On 11/15/2018 06:07 PM, Wei Wang wrote:
> This is the deivce part implementation to add a new feature,
> VIRTIO_BALLOON_F_FREE_PAGE_HINT to the virtio-balloon device. The device
> receives the guest free page hints from the driver and clears the
> corresponding bits in the dirty bitmap, so that those free pages are
> not sent by the migration thread to the destination.
>
> *Tests
> 1 Test Environment
> Host: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
> Migration setup: migrate_set_speed 100G, migrate_set_downtime 400ms
>
> 2 Test Results (results are averaged over several repeated runs)
> 2.1 Guest setup: 8G RAM, 4 vCPU
> 2.1.1 Idle guest live migration time
> Optimization v.s. Legacy = 620ms vs 2970ms
> --> ~79% reduction
> 2.1.2 Guest live migration with Linux compilation workload
> (i.e. make bzImage -j4) running
> 1) Live Migration Time:
> Optimization v.s. Legacy = 2273ms v.s. 4502ms
> --> ~50% reduction
> 2) Linux Compilation Time:
> Optimization v.s. Legacy = 8min42s v.s. 8min43s
> --> no obvious difference
>
> 2.2 Guest setup: 128G RAM, 4 vCPU
> 2.2.1 Idle guest live migration time
> Optimization v.s. Legacy = 5294ms vs 41651ms
> --> ~87% reduction
> 2.2.2 Guest live migration with Linux compilation workload
> 1) Live Migration Time:
> Optimization v.s. Legacy = 8816ms v.s. 54201ms
> --> 84% reduction
> 2) Linux Compilation Time:
> Optimization v.s. Legacy = 8min30s v.s. 8min36s
> --> no obvious difference
>
> ChangeLog:
> v8->v9:
> bitmap:
> - fix bitmap_count_one to handle the nbits=0 case
> migration:
> - replace the ram save notifier chain with a more general precopy
> notifier chain, which is similar to the postcopy notifier chain.
> - Avoid exposing the RAMState struct, and add a function,
> precopy_disable_bulk_stage, to let the virtio-balloon notifier
> callback to disable the bulk stage flag.
Hi Dave and Peter,
Could you continue to review the patches? Thanks!
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* [virtio-dev] Re: [PATCH v9 0/8] virtio-balloon: free page hint support
@ 2018-11-27 3:11 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 3:11 UTC (permalink / raw)
To: qemu-devel, virtio-dev, mst, quintela, dgilbert
Cc: peterx, pbonzini, liliang.opensource, nilal, riel
On 11/15/2018 06:07 PM, Wei Wang wrote:
> This is the deivce part implementation to add a new feature,
> VIRTIO_BALLOON_F_FREE_PAGE_HINT to the virtio-balloon device. The device
> receives the guest free page hints from the driver and clears the
> corresponding bits in the dirty bitmap, so that those free pages are
> not sent by the migration thread to the destination.
>
> *Tests
> 1 Test Environment
> Host: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
> Migration setup: migrate_set_speed 100G, migrate_set_downtime 400ms
>
> 2 Test Results (results are averaged over several repeated runs)
> 2.1 Guest setup: 8G RAM, 4 vCPU
> 2.1.1 Idle guest live migration time
> Optimization v.s. Legacy = 620ms vs 2970ms
> --> ~79% reduction
> 2.1.2 Guest live migration with Linux compilation workload
> (i.e. make bzImage -j4) running
> 1) Live Migration Time:
> Optimization v.s. Legacy = 2273ms v.s. 4502ms
> --> ~50% reduction
> 2) Linux Compilation Time:
> Optimization v.s. Legacy = 8min42s v.s. 8min43s
> --> no obvious difference
>
> 2.2 Guest setup: 128G RAM, 4 vCPU
> 2.2.1 Idle guest live migration time
> Optimization v.s. Legacy = 5294ms vs 41651ms
> --> ~87% reduction
> 2.2.2 Guest live migration with Linux compilation workload
> 1) Live Migration Time:
> Optimization v.s. Legacy = 8816ms v.s. 54201ms
> --> 84% reduction
> 2) Linux Compilation Time:
> Optimization v.s. Legacy = 8min30s v.s. 8min36s
> --> no obvious difference
>
> ChangeLog:
> v8->v9:
> bitmap:
> - fix bitmap_count_one to handle the nbits=0 case
> migration:
> - replace the ram save notifier chain with a more general precopy
> notifier chain, which is similar to the postcopy notifier chain.
> - Avoid exposing the RAMState struct, and add a function,
> precopy_disable_bulk_stage, to let the virtio-balloon notifier
> callback to disable the bulk stage flag.
Hi Dave and Peter,
Could you continue to review the patches? Thanks!
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
(?)
@ 2018-11-27 5:40 ` Peter Xu
2018-11-27 6:02 ` [virtio-dev] " Wei Wang
-1 siblings, 1 reply; 53+ messages in thread
From: Peter Xu @ 2018-11-27 5:40 UTC (permalink / raw)
To: Wei Wang
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
> The bitmap mutex is used to synchronize threads to update the dirty
> bitmap and the migration_dirty_pages counter. For example, the free
> page optimization clears bits of free pages from the bitmap in an
> iothread context. This patch makes migration_bitmap_clear_dirty update
> the bitmap and counter under the mutex.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Peter Xu <peterx@redhat.com>
> ---
> migration/ram.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 7e7deec..ef69dbe 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1556,11 +1556,14 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
> {
> bool ret;
>
> + qemu_mutex_lock(&rs->bitmap_mutex);
> ret = test_and_clear_bit(page, rb->bmap);
>
> if (ret) {
> rs->migration_dirty_pages--;
> }
> + qemu_mutex_unlock(&rs->bitmap_mutex);
> +
> return ret;
> }
It seems fine to me, but have you thought about
test_and_clear_bit_atomic()? Note that we just had
test_and_set_bit_atomic() a few months ago.
And not related to this patch: I'm unclear on why we have had
bitmap_mutex before, since it seems unnecessary.
Regards,
--
Peter Xu
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty
2018-11-27 5:40 ` [Qemu-devel] " Peter Xu
@ 2018-11-27 6:02 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 6:02 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/27/2018 01:40 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
>> The bitmap mutex is used to synchronize threads to update the dirty
>> bitmap and the migration_dirty_pages counter. For example, the free
>> page optimization clears bits of free pages from the bitmap in an
>> iothread context. This patch makes migration_bitmap_clear_dirty update
>> the bitmap and counter under the mutex.
>>
>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> CC: Juan Quintela <quintela@redhat.com>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> CC: Peter Xu <peterx@redhat.com>
>> ---
>> migration/ram.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 7e7deec..ef69dbe 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -1556,11 +1556,14 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
>> {
>> bool ret;
>>
>> + qemu_mutex_lock(&rs->bitmap_mutex);
>> ret = test_and_clear_bit(page, rb->bmap);
>>
>> if (ret) {
>> rs->migration_dirty_pages--;
>> }
>> + qemu_mutex_unlock(&rs->bitmap_mutex);
>> +
>> return ret;
>> }
> It seems fine to me, but have you thought about
> test_and_clear_bit_atomic()? Note that we just had
> test_and_set_bit_atomic() a few months ago.
Thanks for sharing. I think we might also need to
mutex migration_dirty_pages.
>
> And not related to this patch: I'm unclear on why we have had
> bitmap_mutex before, since it seems unnecessary.
OK. This is because with the optimization we have a thread
which clears bits (of free pages) from the bitmap and updates
migration_dirty_pages. So we need to synchronization between
the migration thread and the optimization thread.
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* [virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty
@ 2018-11-27 6:02 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 6:02 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/27/2018 01:40 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
>> The bitmap mutex is used to synchronize threads to update the dirty
>> bitmap and the migration_dirty_pages counter. For example, the free
>> page optimization clears bits of free pages from the bitmap in an
>> iothread context. This patch makes migration_bitmap_clear_dirty update
>> the bitmap and counter under the mutex.
>>
>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> CC: Juan Quintela <quintela@redhat.com>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> CC: Peter Xu <peterx@redhat.com>
>> ---
>> migration/ram.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 7e7deec..ef69dbe 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -1556,11 +1556,14 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
>> {
>> bool ret;
>>
>> + qemu_mutex_lock(&rs->bitmap_mutex);
>> ret = test_and_clear_bit(page, rb->bmap);
>>
>> if (ret) {
>> rs->migration_dirty_pages--;
>> }
>> + qemu_mutex_unlock(&rs->bitmap_mutex);
>> +
>> return ret;
>> }
> It seems fine to me, but have you thought about
> test_and_clear_bit_atomic()? Note that we just had
> test_and_set_bit_atomic() a few months ago.
Thanks for sharing. I think we might also need to
mutex migration_dirty_pages.
>
> And not related to this patch: I'm unclear on why we have had
> bitmap_mutex before, since it seems unnecessary.
OK. This is because with the optimization we have a thread
which clears bits (of free pages) from the bitmap and updates
migration_dirty_pages. So we need to synchronization between
the migration thread and the optimization thread.
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap
2018-11-15 10:08 ` [virtio-dev] " Wei Wang
(?)
@ 2018-11-27 6:06 ` Peter Xu
2018-11-27 6:52 ` [virtio-dev] " Wei Wang
-1 siblings, 1 reply; 53+ messages in thread
From: Peter Xu @ 2018-11-27 6:06 UTC (permalink / raw)
To: Wei Wang
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang wrote:
> This patch adds an API to clear bits corresponding to guest free pages
> from the dirty bitmap. Spilt the free page block if it crosses the QEMU
> RAMBlock boundary.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Peter Xu <peterx@redhat.com>
> ---
> include/migration/misc.h | 2 ++
> migration/ram.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index 4ebf24c..113320e 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -14,11 +14,13 @@
> #ifndef MIGRATION_MISC_H
> #define MIGRATION_MISC_H
>
> +#include "exec/cpu-common.h"
> #include "qemu/notify.h"
>
> /* migration/ram.c */
>
> void ram_mig_init(void);
> +void qemu_guest_free_page_hint(void *addr, size_t len);
>
> /* migration/block.c */
>
> diff --git a/migration/ram.c b/migration/ram.c
> index ef69dbe..229b791 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3131,6 +3131,54 @@ static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out)
> }
>
> /*
> + * This function clears bits of the free pages reported by the caller from the
> + * migration dirty bitmap. @addr is the host address corresponding to the
> + * start of the continuous guest free pages, and @len is the total bytes of
> + * those pages.
> + */
> +void qemu_guest_free_page_hint(void *addr, size_t len)
> +{
> + RAMBlock *block;
> + ram_addr_t offset;
> + size_t used_len, start, npages;
> + MigrationState *s = migrate_get_current();
> +
> + /* This function is currently expected to be used during live migration */
> + if (!migration_is_setup_or_active(s->state)) {
> + return;
> + }
> +
> + for (; len > 0; len -= used_len) {
> + block = qemu_ram_block_from_host(addr, false, &offset);
> + assert(block);
> +
> + /*
> + * This handles the case that the RAMBlock is resized after the free
> + * page hint is reported.
> + */
> + if (unlikely(offset > block->used_length)) {
> + return;
> + }
Again, is it possible to resize during migration?
So I think the check is fine, but uncertain about the comment.
And shall we print something if that happened? We can use
error_report_once(), and squashing the above assert:
if (!block || offset > block->used_length) {
/* should never happen, but if it happens we ignore the hints and warn */
error_report_once("...");
return;
}
What do you think?
> +
> + if (len <= block->used_length - offset) {
> + used_len = len;
> + } else {
> + used_len = block->used_length - offset;
> + addr += used_len;
Maybe moving this line into the for() could be a bit better?
for (; len > 0; len -= used_len, addr += used_len) {
> + }
> +
> + start = offset >> TARGET_PAGE_BITS;
> + npages = used_len >> TARGET_PAGE_BITS;
> +
> + qemu_mutex_lock(&ram_state->bitmap_mutex);
> + ram_state->migration_dirty_pages -=
> + bitmap_count_one_with_offset(block->bmap, start, npages);
> + bitmap_clear(block->bmap, start, npages);
> + qemu_mutex_unlock(&ram_state->bitmap_mutex);
> + }
> +}
> +
> +/*
> * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
> * long-running RCU critical section. When rcu-reclaims in the code
> * start to become numerous it will be necessary to reduce the
> --
> 1.8.3.1
>
Regards,
--
Peter Xu
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty
2018-11-27 6:02 ` [virtio-dev] " Wei Wang
@ 2018-11-27 6:12 ` Wei Wang
-1 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 6:12 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/27/2018 02:02 PM, Wei Wang wrote:
> On 11/27/2018 01:40 PM, Peter Xu wrote:
>> On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
>>> The bitmap mutex is used to synchronize threads to update the dirty
>>> bitmap and the migration_dirty_pages counter. For example, the free
>>> page optimization clears bits of free pages from the bitmap in an
>>> iothread context. This patch makes migration_bitmap_clear_dirty update
>>> the bitmap and counter under the mutex.
>>>
>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>>> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> CC: Juan Quintela <quintela@redhat.com>
>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>> CC: Peter Xu <peterx@redhat.com>
>>> ---
>>> migration/ram.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/migration/ram.c b/migration/ram.c
>>> index 7e7deec..ef69dbe 100644
>>> --- a/migration/ram.c
>>> +++ b/migration/ram.c
>>> @@ -1556,11 +1556,14 @@ static inline bool
>>> migration_bitmap_clear_dirty(RAMState *rs,
>>> {
>>> bool ret;
>>> + qemu_mutex_lock(&rs->bitmap_mutex);
>>> ret = test_and_clear_bit(page, rb->bmap);
>>> if (ret) {
>>> rs->migration_dirty_pages--;
>>> }
>>> + qemu_mutex_unlock(&rs->bitmap_mutex);
>>> +
>>> return ret;
>>> }
>> It seems fine to me, but have you thought about
>> test_and_clear_bit_atomic()? Note that we just had
>> test_and_set_bit_atomic() a few months ago.
>
> Thanks for sharing. I think we might also need to
> mutex migration_dirty_pages.
>
>>
>> And not related to this patch: I'm unclear on why we have had
>> bitmap_mutex before, since it seems unnecessary.
>
> OK. This is because with the optimization we have a thread
> which clears bits (of free pages) from the bitmap and updates
> migration_dirty_pages. So we need to synchronization between
> the migration thread and the optimization thread.
>
And before this feature, I think yes, that bitmap_mutex is not needed.
It was left there due to some historical reasons.
I remember Dave previous said he was about to remove it. But the new
feature will need it again.
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty
@ 2018-11-27 6:12 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 6:12 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/27/2018 02:02 PM, Wei Wang wrote:
> On 11/27/2018 01:40 PM, Peter Xu wrote:
>> On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
>>> The bitmap mutex is used to synchronize threads to update the dirty
>>> bitmap and the migration_dirty_pages counter. For example, the free
>>> page optimization clears bits of free pages from the bitmap in an
>>> iothread context. This patch makes migration_bitmap_clear_dirty update
>>> the bitmap and counter under the mutex.
>>>
>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>>> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> CC: Juan Quintela <quintela@redhat.com>
>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>> CC: Peter Xu <peterx@redhat.com>
>>> ---
>>> migration/ram.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/migration/ram.c b/migration/ram.c
>>> index 7e7deec..ef69dbe 100644
>>> --- a/migration/ram.c
>>> +++ b/migration/ram.c
>>> @@ -1556,11 +1556,14 @@ static inline bool
>>> migration_bitmap_clear_dirty(RAMState *rs,
>>> {
>>> bool ret;
>>> + qemu_mutex_lock(&rs->bitmap_mutex);
>>> ret = test_and_clear_bit(page, rb->bmap);
>>> if (ret) {
>>> rs->migration_dirty_pages--;
>>> }
>>> + qemu_mutex_unlock(&rs->bitmap_mutex);
>>> +
>>> return ret;
>>> }
>> It seems fine to me, but have you thought about
>> test_and_clear_bit_atomic()? Note that we just had
>> test_and_set_bit_atomic() a few months ago.
>
> Thanks for sharing. I think we might also need to
> mutex migration_dirty_pages.
>
>>
>> And not related to this patch: I'm unclear on why we have had
>> bitmap_mutex before, since it seems unnecessary.
>
> OK. This is because with the optimization we have a thread
> which clears bits (of free pages) from the bitmap and updates
> migration_dirty_pages. So we need to synchronization between
> the migration thread and the optimization thread.
>
And before this feature, I think yes, that bitmap_mutex is not needed.
It was left there due to some historical reasons.
I remember Dave previous said he was about to remove it. But the new
feature will need it again.
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap
2018-11-27 6:06 ` [Qemu-devel] " Peter Xu
@ 2018-11-27 6:52 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 6:52 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/27/2018 02:06 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang wrote:
> Again, is it possible to resize during migration?
>
> So I think the check is fine, but uncertain about the comment.
Yes, resize would not happen with the current implementation.
But heard it could just be a temporal implementation. Probably
we could improve the comment like this:
"
Though the implementation might not support ram resize currently,
this could happen in theory with future updates. So the check here
handles the case that RAMBLOCK is resized after the free page hint is
reported.
"
>
> And shall we print something if that happened? We can use
> error_report_once(), and squashing the above assert:
>
> if (!block || offset > block->used_length) {
> /* should never happen, but if it happens we ignore the hints and warn */
> error_report_once("...");
> return;
> }
>
> What do you think?
Sounds good.
>
>> +
>> + if (len <= block->used_length - offset) {
>> + used_len = len;
>> + } else {
>> + used_len = block->used_length - offset;
>> + addr += used_len;
> Maybe moving this line into the for() could be a bit better?
>
> for (; len > 0; len -= used_len, addr += used_len) {
>
Yes, I think it looks better, thanks.
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* [virtio-dev] Re: [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap
@ 2018-11-27 6:52 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 6:52 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/27/2018 02:06 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang wrote:
> Again, is it possible to resize during migration?
>
> So I think the check is fine, but uncertain about the comment.
Yes, resize would not happen with the current implementation.
But heard it could just be a temporal implementation. Probably
we could improve the comment like this:
"
Though the implementation might not support ram resize currently,
this could happen in theory with future updates. So the check here
handles the case that RAMBLOCK is resized after the free page hint is
reported.
"
>
> And shall we print something if that happened? We can use
> error_report_once(), and squashing the above assert:
>
> if (!block || offset > block->used_length) {
> /* should never happen, but if it happens we ignore the hints and warn */
> error_report_once("...");
> return;
> }
>
> What do you think?
Sounds good.
>
>> +
>> + if (len <= block->used_length - offset) {
>> + used_len = len;
>> + } else {
>> + used_len = block->used_length - offset;
>> + addr += used_len;
> Maybe moving this line into the for() could be a bit better?
>
> for (; len > 0; len -= used_len, addr += used_len) {
>
Yes, I think it looks better, thanks.
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-15 10:08 ` [virtio-dev] " Wei Wang
(?)
@ 2018-11-27 7:38 ` Peter Xu
2018-11-27 10:25 ` [virtio-dev] " Wei Wang
-1 siblings, 1 reply; 53+ messages in thread
From: Peter Xu @ 2018-11-27 7:38 UTC (permalink / raw)
To: Wei Wang
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On Thu, Nov 15, 2018 at 06:08:01PM +0800, Wei Wang wrote:
> This patch adds a notifier chain for the memory precopy. This enables various
> precopy optimizations to be invoked at specific places.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Peter Xu <peterx@redhat.com>
> ---
> include/migration/misc.h | 12 ++++++++++++
> migration/ram.c | 31 +++++++++++++++++++++++++++++++
> vl.c | 1 +
> 3 files changed, 44 insertions(+)
>
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index 113320e..0bac623 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -19,6 +19,18 @@
>
> /* migration/ram.c */
>
> +typedef enum PrecopyNotifyReason {
> + PRECOPY_NOTIFY_ERR = 0,
> + PRECOPY_NOTIFY_START_ITERATION = 1,
> + PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
> + PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
> + PRECOPY_NOTIFY_MAX = 4,
It would be nice to add some comments for each of the notify reason.
E.g., from the name PRECOPY_NOTIFY_START_ITERATION seems more like a
hook at the start of each iteration but according to [1] it should be
at the start of migration rather than each iteration (or when
migration restarts, though I'm not sure whether we really have this
yet).
> +} PrecopyNotifyReason;
> +
> +void precopy_infrastructure_init(void);
> +void precopy_add_notifier(Notifier *n);
> +void precopy_remove_notifier(Notifier *n);
> +
> void ram_mig_init(void);
> void qemu_guest_free_page_hint(void *addr, size_t len);
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 229b791..65b1223 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -292,6 +292,8 @@ struct RAMState {
> bool ram_bulk_stage;
> /* How many times we have dirty too many pages */
> int dirty_rate_high_cnt;
> + /* ram save states used for notifiers */
> + int ram_save_state;
This can be removed?
> /* these variables are used for bitmap sync */
> /* last time we did a full bitmap_sync */
> int64_t time_last_bitmap_sync;
> @@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
>
> static RAMState *ram_state;
>
> +static NotifierList precopy_notifier_list;
> +
> +void precopy_infrastructure_init(void)
> +{
> + notifier_list_init(&precopy_notifier_list);
> +}
> +
> +void precopy_add_notifier(Notifier *n)
> +{
> + notifier_list_add(&precopy_notifier_list, n);
> +}
> +
> +void precopy_remove_notifier(Notifier *n)
> +{
> + notifier_remove(n);
> +}
> +
> +static void precopy_notify(PrecopyNotifyReason reason)
> +{
> + notifier_list_notify(&precopy_notifier_list, &reason);
> +}
> +
> uint64_t ram_bytes_remaining(void)
> {
> return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
> @@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
> int64_t end_time;
> uint64_t bytes_xfer_now;
>
> + precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
> +
> ram_counters.dirty_sync_count++;
>
> if (!rs->time_last_bitmap_sync) {
> @@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
> if (migrate_use_events()) {
> qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
> }
> +
> + precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
> }
>
> /**
> @@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
> rs->last_page = 0;
> rs->last_version = ram_list.version;
> rs->ram_bulk_stage = true;
> +
> + precopy_notify(PRECOPY_NOTIFY_START_ITERATION);
[1]
> }
>
> #define MAX_WAIT 50 /* ms, half buffered_file limit */
> @@ -3324,6 +3354,7 @@ out:
>
> ret = qemu_file_get_error(f);
> if (ret < 0) {
> + precopy_notify(PRECOPY_NOTIFY_ERR);
Could you show me which function is this line in?
Line 3324 here is ram_save_complete(), but I cannot find this exact
place.
Another thing to mention about the "reasons" (though I see it more
like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
It might help in some cases:
- then you don't need to trickily export the migrate_postcopy()
since you'll notify that before postcopy starts
- you'll have a solid point that you'll 100% guarantee that we'll
stop the free page hinting and don't need to worry about whether
there is chance the hinting will be running without an end [2].
Regarding [2] above: now the series only stops the hinting when
PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP is notified. Could there be a case
that it's missing? E.g., what if we cancel/fail a migration during
precopy? Have you tried it?
Regards,
--
Peter Xu
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty
2018-11-27 6:12 ` Wei Wang
(?)
@ 2018-11-27 7:41 ` Peter Xu
2018-11-27 10:17 ` Wei Wang
-1 siblings, 1 reply; 53+ messages in thread
From: Peter Xu @ 2018-11-27 7:41 UTC (permalink / raw)
To: Wei Wang
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On Tue, Nov 27, 2018 at 02:12:34PM +0800, Wei Wang wrote:
> On 11/27/2018 02:02 PM, Wei Wang wrote:
> > On 11/27/2018 01:40 PM, Peter Xu wrote:
> > > On Thu, Nov 15, 2018 at 06:07:59PM +0800, Wei Wang wrote:
> > > > The bitmap mutex is used to synchronize threads to update the dirty
> > > > bitmap and the migration_dirty_pages counter. For example, the free
> > > > page optimization clears bits of free pages from the bitmap in an
> > > > iothread context. This patch makes migration_bitmap_clear_dirty update
> > > > the bitmap and counter under the mutex.
> > > >
> > > > Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> > > > CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > > CC: Juan Quintela <quintela@redhat.com>
> > > > CC: Michael S. Tsirkin <mst@redhat.com>
> > > > CC: Peter Xu <peterx@redhat.com>
> > > > ---
> > > > migration/ram.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/migration/ram.c b/migration/ram.c
> > > > index 7e7deec..ef69dbe 100644
> > > > --- a/migration/ram.c
> > > > +++ b/migration/ram.c
> > > > @@ -1556,11 +1556,14 @@ static inline bool
> > > > migration_bitmap_clear_dirty(RAMState *rs,
> > > > {
> > > > bool ret;
> > > > + qemu_mutex_lock(&rs->bitmap_mutex);
> > > > ret = test_and_clear_bit(page, rb->bmap);
> > > > if (ret) {
> > > > rs->migration_dirty_pages--;
> > > > }
> > > > + qemu_mutex_unlock(&rs->bitmap_mutex);
> > > > +
> > > > return ret;
> > > > }
> > > It seems fine to me, but have you thought about
> > > test_and_clear_bit_atomic()? Note that we just had
> > > test_and_set_bit_atomic() a few months ago.
> >
> > Thanks for sharing. I think we might also need to
> > mutex migration_dirty_pages.
> >
> > >
> > > And not related to this patch: I'm unclear on why we have had
> > > bitmap_mutex before, since it seems unnecessary.
> >
> > OK. This is because with the optimization we have a thread
> > which clears bits (of free pages) from the bitmap and updates
> > migration_dirty_pages. So we need to synchronization between
> > the migration thread and the optimization thread.
> >
>
> And before this feature, I think yes, that bitmap_mutex is not needed.
> It was left there due to some historical reasons.
> I remember Dave previous said he was about to remove it. But the new
> feature will need it again.
Ok then I'm fine with it. Though you could update the comments too if
you like:
/* protects modification of the bitmap and migration_dirty_pages */
QemuMutex bitmap_mutex;
And it's tricky that sometimes we don't take the lock when reading
this variable "migration_dirty_pages". I don't see obvious issue so
far, hope it's true (at least I skipped the colo ones...).
ram_bytes_remaining[333] return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
migration_bitmap_clear_dirty[1562] rs->migration_dirty_pages--;
migration_bitmap_sync_range[1570] rs->migration_dirty_pages +=
postcopy_chunk_hostpages_pass[2809] rs->migration_dirty_pages += !test_and_set_bit(page, bitmap);
ram_state_init[3037] (*rsp)->migration_dirty_pages = ram_bytes_total() >> TARGET_PAGE_BITS;
ram_state_resume_prepare[3112] rs->migration_dirty_pages = pages;
ram_save_pending[3344] remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE;
ram_save_pending[3353] remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE;
colo_cache_from_block_offset[3468] ram_state->migration_dirty_pages++;
colo_init_ram_cache[3716] ram_state->migration_dirty_pages = 0;
colo_flush_ram_cache[3997] trace_colo_flush_ram_cache_begin(ram_state->migration_dirty_pages);
Regards,
--
Peter Xu
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap
2018-11-27 6:52 ` [virtio-dev] " Wei Wang
(?)
@ 2018-11-27 7:43 ` Peter Xu
-1 siblings, 0 replies; 53+ messages in thread
From: Peter Xu @ 2018-11-27 7:43 UTC (permalink / raw)
To: Wei Wang
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On Tue, Nov 27, 2018 at 02:52:35PM +0800, Wei Wang wrote:
> On 11/27/2018 02:06 PM, Peter Xu wrote:
> > On Thu, Nov 15, 2018 at 06:08:00PM +0800, Wei Wang wrote:
> > Again, is it possible to resize during migration?
> >
> > So I think the check is fine, but uncertain about the comment.
>
> Yes, resize would not happen with the current implementation.
> But heard it could just be a temporal implementation. Probably
> we could improve the comment like this:
>
> "
> Though the implementation might not support ram resize currently,
> this could happen in theory with future updates. So the check here
> handles the case that RAMBLOCK is resized after the free page hint is
> reported.
> "
I'm not familiar with that part, but this seems ok to me.
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty
2018-11-27 7:41 ` [Qemu-devel] " Peter Xu
@ 2018-11-27 10:17 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 10:17 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/27/2018 03:41 PM, Peter Xu wrote:
>
> Ok then I'm fine with it. Though you could update the comments too if
> you like:
>
> /* protects modification of the bitmap and migration_dirty_pages */
> QemuMutex bitmap_mutex;
>
> And it's tricky that sometimes we don't take the lock when reading
> this variable "migration_dirty_pages". I don't see obvious issue so
> far, hope it's true (at least I skipped the colo ones...).
The caller reads the value just to estimate the remaining_size, and
it seems fine without a lock, because whether it reads a
value before the update or after the update seem not causing
an issue.
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [virtio-dev] Re: [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty
@ 2018-11-27 10:17 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 10:17 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/27/2018 03:41 PM, Peter Xu wrote:
>
> Ok then I'm fine with it. Though you could update the comments too if
> you like:
>
> /* protects modification of the bitmap and migration_dirty_pages */
> QemuMutex bitmap_mutex;
>
> And it's tricky that sometimes we don't take the lock when reading
> this variable "migration_dirty_pages". I don't see obvious issue so
> far, hope it's true (at least I skipped the colo ones...).
The caller reads the value just to estimate the remaining_size, and
it seems fine without a lock, because whether it reads a
value before the update or after the update seem not causing
an issue.
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-27 7:38 ` [Qemu-devel] " Peter Xu
@ 2018-11-27 10:25 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 10:25 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/27/2018 03:38 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:08:01PM +0800, Wei Wang wrote:
>>
>> +typedef enum PrecopyNotifyReason {
>> + PRECOPY_NOTIFY_ERR = 0,
>> + PRECOPY_NOTIFY_START_ITERATION = 1,
>> + PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
>> + PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
>> + PRECOPY_NOTIFY_MAX = 4,
> It would be nice to add some comments for each of the notify reason.
> E.g., from the name PRECOPY_NOTIFY_START_ITERATION seems more like a
> hook at the start of each iteration but according to [1] it should be
> at the start of migration rather than each iteration (or when
> migration restarts, though I'm not sure whether we really have this
> yet).
OK. I think It would be better if the name itself could be straightforward.
Probably we could change PRECOPY_NOTIFY_START_ITERATION to
PRECOPY_NOTIFY_START_MIGRATION.
>> +} PrecopyNotifyReason;
>> +
>> +void precopy_infrastructure_init(void);
>> +void precopy_add_notifier(Notifier *n);
>> +void precopy_remove_notifier(Notifier *n);
>> +
>> void ram_mig_init(void);
>> void qemu_guest_free_page_hint(void *addr, size_t len);
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 229b791..65b1223 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -292,6 +292,8 @@ struct RAMState {
>> bool ram_bulk_stage;
>> /* How many times we have dirty too many pages */
>> int dirty_rate_high_cnt;
>> + /* ram save states used for notifiers */
>> + int ram_save_state;
> This can be removed?
Yes, thanks.
>
>> /* these variables are used for bitmap sync */
>> /* last time we did a full bitmap_sync */
>> int64_t time_last_bitmap_sync;
>> @@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
>>
>> static RAMState *ram_state;
>>
>> +static NotifierList precopy_notifier_list;
>> +
>> +void precopy_infrastructure_init(void)
>> +{
>> + notifier_list_init(&precopy_notifier_list);
>> +}
>> +
>> +void precopy_add_notifier(Notifier *n)
>> +{
>> + notifier_list_add(&precopy_notifier_list, n);
>> +}
>> +
>> +void precopy_remove_notifier(Notifier *n)
>> +{
>> + notifier_remove(n);
>> +}
>> +
>> +static void precopy_notify(PrecopyNotifyReason reason)
>> +{
>> + notifier_list_notify(&precopy_notifier_list, &reason);
>> +}
>> +
>> uint64_t ram_bytes_remaining(void)
>> {
>> return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
>> @@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
>> int64_t end_time;
>> uint64_t bytes_xfer_now;
>>
>> + precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
>> +
>> ram_counters.dirty_sync_count++;
>>
>> if (!rs->time_last_bitmap_sync) {
>> @@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
>> if (migrate_use_events()) {
>> qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
>> }
>> +
>> + precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
>> }
>>
>> /**
>> @@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
>> rs->last_page = 0;
>> rs->last_version = ram_list.version;
>> rs->ram_bulk_stage = true;
>> +
>> + precopy_notify(PRECOPY_NOTIFY_START_ITERATION);
> [1]
>
>> }
>>
>> #define MAX_WAIT 50 /* ms, half buffered_file limit */
>> @@ -3324,6 +3354,7 @@ out:
>>
>> ret = qemu_file_get_error(f);
>> if (ret < 0) {
>> + precopy_notify(PRECOPY_NOTIFY_ERR);
> Could you show me which function is this line in?
>
> Line 3324 here is ram_save_complete(), but I cannot find this exact
> place.
Sure, it's in ram_save_iterate():
...
out:
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
qemu_fflush(f);
ram_counters.transferred += 8;
ret = qemu_file_get_error(f);
if (ret < 0) {
+ precopy_notify(PRECOPY_NOTIFY_ERR);
return ret;
}
return done;
}
>
> Another thing to mention about the "reasons" (though I see it more
> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> It might help in some cases:
>
> - then you don't need to trickily export the migrate_postcopy()
> since you'll notify that before postcopy starts
I'm thinking probably we don't need to export migrate_postcopy even now.
It's more like a sanity check, and not needed because now we have the
notifier registered to the precopy specific callchain, which has ensured
that
it is invoked via precopy.
> - you'll have a solid point that you'll 100% guarantee that we'll
> stop the free page hinting and don't need to worry about whether
> there is chance the hinting will be running without an end [2].
Thanks, I think it makes sense. Plan to add PRECOPY_NOTIFY_END in
ram_save_complete.
>
> Regarding [2] above: now the series only stops the hinting when
> PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP is notified. Could there be a case
> that it's missing? E.g., what if we cancel/fail a migration during
> precopy? Have you tried it?
>
I think it has been handled by the above PRECOPY_NOTIFY_ERR
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* [virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
@ 2018-11-27 10:25 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-27 10:25 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/27/2018 03:38 PM, Peter Xu wrote:
> On Thu, Nov 15, 2018 at 06:08:01PM +0800, Wei Wang wrote:
>>
>> +typedef enum PrecopyNotifyReason {
>> + PRECOPY_NOTIFY_ERR = 0,
>> + PRECOPY_NOTIFY_START_ITERATION = 1,
>> + PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
>> + PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
>> + PRECOPY_NOTIFY_MAX = 4,
> It would be nice to add some comments for each of the notify reason.
> E.g., from the name PRECOPY_NOTIFY_START_ITERATION seems more like a
> hook at the start of each iteration but according to [1] it should be
> at the start of migration rather than each iteration (or when
> migration restarts, though I'm not sure whether we really have this
> yet).
OK. I think It would be better if the name itself could be straightforward.
Probably we could change PRECOPY_NOTIFY_START_ITERATION to
PRECOPY_NOTIFY_START_MIGRATION.
>> +} PrecopyNotifyReason;
>> +
>> +void precopy_infrastructure_init(void);
>> +void precopy_add_notifier(Notifier *n);
>> +void precopy_remove_notifier(Notifier *n);
>> +
>> void ram_mig_init(void);
>> void qemu_guest_free_page_hint(void *addr, size_t len);
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 229b791..65b1223 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -292,6 +292,8 @@ struct RAMState {
>> bool ram_bulk_stage;
>> /* How many times we have dirty too many pages */
>> int dirty_rate_high_cnt;
>> + /* ram save states used for notifiers */
>> + int ram_save_state;
> This can be removed?
Yes, thanks.
>
>> /* these variables are used for bitmap sync */
>> /* last time we did a full bitmap_sync */
>> int64_t time_last_bitmap_sync;
>> @@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
>>
>> static RAMState *ram_state;
>>
>> +static NotifierList precopy_notifier_list;
>> +
>> +void precopy_infrastructure_init(void)
>> +{
>> + notifier_list_init(&precopy_notifier_list);
>> +}
>> +
>> +void precopy_add_notifier(Notifier *n)
>> +{
>> + notifier_list_add(&precopy_notifier_list, n);
>> +}
>> +
>> +void precopy_remove_notifier(Notifier *n)
>> +{
>> + notifier_remove(n);
>> +}
>> +
>> +static void precopy_notify(PrecopyNotifyReason reason)
>> +{
>> + notifier_list_notify(&precopy_notifier_list, &reason);
>> +}
>> +
>> uint64_t ram_bytes_remaining(void)
>> {
>> return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
>> @@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
>> int64_t end_time;
>> uint64_t bytes_xfer_now;
>>
>> + precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
>> +
>> ram_counters.dirty_sync_count++;
>>
>> if (!rs->time_last_bitmap_sync) {
>> @@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
>> if (migrate_use_events()) {
>> qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
>> }
>> +
>> + precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
>> }
>>
>> /**
>> @@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
>> rs->last_page = 0;
>> rs->last_version = ram_list.version;
>> rs->ram_bulk_stage = true;
>> +
>> + precopy_notify(PRECOPY_NOTIFY_START_ITERATION);
> [1]
>
>> }
>>
>> #define MAX_WAIT 50 /* ms, half buffered_file limit */
>> @@ -3324,6 +3354,7 @@ out:
>>
>> ret = qemu_file_get_error(f);
>> if (ret < 0) {
>> + precopy_notify(PRECOPY_NOTIFY_ERR);
> Could you show me which function is this line in?
>
> Line 3324 here is ram_save_complete(), but I cannot find this exact
> place.
Sure, it's in ram_save_iterate():
...
out:
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
qemu_fflush(f);
ram_counters.transferred += 8;
ret = qemu_file_get_error(f);
if (ret < 0) {
+ precopy_notify(PRECOPY_NOTIFY_ERR);
return ret;
}
return done;
}
>
> Another thing to mention about the "reasons" (though I see it more
> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> It might help in some cases:
>
> - then you don't need to trickily export the migrate_postcopy()
> since you'll notify that before postcopy starts
I'm thinking probably we don't need to export migrate_postcopy even now.
It's more like a sanity check, and not needed because now we have the
notifier registered to the precopy specific callchain, which has ensured
that
it is invoked via precopy.
> - you'll have a solid point that you'll 100% guarantee that we'll
> stop the free page hinting and don't need to worry about whether
> there is chance the hinting will be running without an end [2].
Thanks, I think it makes sense. Plan to add PRECOPY_NOTIFY_END in
ram_save_complete.
>
> Regarding [2] above: now the series only stops the hinting when
> PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP is notified. Could there be a case
> that it's missing? E.g., what if we cancel/fail a migration during
> precopy? Have you tried it?
>
I think it has been handled by the above PRECOPY_NOTIFY_ERR
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-27 10:25 ` [virtio-dev] " Wei Wang
(?)
@ 2018-11-28 5:26 ` Peter Xu
2018-11-28 9:01 ` [virtio-dev] " Wei Wang
-1 siblings, 1 reply; 53+ messages in thread
From: Peter Xu @ 2018-11-28 5:26 UTC (permalink / raw)
To: Wei Wang
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On Tue, Nov 27, 2018 at 06:25:40PM +0800, Wei Wang wrote:
> On 11/27/2018 03:38 PM, Peter Xu wrote:
> > On Thu, Nov 15, 2018 at 06:08:01PM +0800, Wei Wang wrote:
> > > +typedef enum PrecopyNotifyReason {
> > > + PRECOPY_NOTIFY_ERR = 0,
> > > + PRECOPY_NOTIFY_START_ITERATION = 1,
> > > + PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP = 2,
> > > + PRECOPY_NOTIFY_AFTER_SYNC_BITMAP = 3,
> > > + PRECOPY_NOTIFY_MAX = 4,
> > It would be nice to add some comments for each of the notify reason.
> > E.g., from the name PRECOPY_NOTIFY_START_ITERATION seems more like a
> > hook at the start of each iteration but according to [1] it should be
> > at the start of migration rather than each iteration (or when
> > migration restarts, though I'm not sure whether we really have this
> > yet).
>
> OK. I think It would be better if the name itself could be straightforward.
> Probably we could change PRECOPY_NOTIFY_START_ITERATION to
> PRECOPY_NOTIFY_START_MIGRATION.
Sounds good.
>
>
> > > +} PrecopyNotifyReason;
> > > +
> > > +void precopy_infrastructure_init(void);
> > > +void precopy_add_notifier(Notifier *n);
> > > +void precopy_remove_notifier(Notifier *n);
> > > +
> > > void ram_mig_init(void);
> > > void qemu_guest_free_page_hint(void *addr, size_t len);
> > > diff --git a/migration/ram.c b/migration/ram.c
> > > index 229b791..65b1223 100644
> > > --- a/migration/ram.c
> > > +++ b/migration/ram.c
> > > @@ -292,6 +292,8 @@ struct RAMState {
> > > bool ram_bulk_stage;
> > > /* How many times we have dirty too many pages */
> > > int dirty_rate_high_cnt;
> > > + /* ram save states used for notifiers */
> > > + int ram_save_state;
> > This can be removed?
>
> Yes, thanks.
>
> >
> > > /* these variables are used for bitmap sync */
> > > /* last time we did a full bitmap_sync */
> > > int64_t time_last_bitmap_sync;
> > > @@ -328,6 +330,28 @@ typedef struct RAMState RAMState;
> > > static RAMState *ram_state;
> > > +static NotifierList precopy_notifier_list;
> > > +
> > > +void precopy_infrastructure_init(void)
> > > +{
> > > + notifier_list_init(&precopy_notifier_list);
> > > +}
> > > +
> > > +void precopy_add_notifier(Notifier *n)
> > > +{
> > > + notifier_list_add(&precopy_notifier_list, n);
> > > +}
> > > +
> > > +void precopy_remove_notifier(Notifier *n)
> > > +{
> > > + notifier_remove(n);
> > > +}
> > > +
> > > +static void precopy_notify(PrecopyNotifyReason reason)
> > > +{
> > > + notifier_list_notify(&precopy_notifier_list, &reason);
> > > +}
> > > +
> > > uint64_t ram_bytes_remaining(void)
> > > {
> > > return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
> > > @@ -1642,6 +1666,8 @@ static void migration_bitmap_sync(RAMState *rs)
> > > int64_t end_time;
> > > uint64_t bytes_xfer_now;
> > > + precopy_notify(PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP);
> > > +
> > > ram_counters.dirty_sync_count++;
> > > if (!rs->time_last_bitmap_sync) {
> > > @@ -1699,6 +1725,8 @@ static void migration_bitmap_sync(RAMState *rs)
> > > if (migrate_use_events()) {
> > > qapi_event_send_migration_pass(ram_counters.dirty_sync_count);
> > > }
> > > +
> > > + precopy_notify(PRECOPY_NOTIFY_AFTER_SYNC_BITMAP);
> > > }
> > > /**
> > > @@ -2555,6 +2583,8 @@ static void ram_state_reset(RAMState *rs)
> > > rs->last_page = 0;
> > > rs->last_version = ram_list.version;
> > > rs->ram_bulk_stage = true;
> > > +
> > > + precopy_notify(PRECOPY_NOTIFY_START_ITERATION);
> > [1]
> >
> > > }
> > > #define MAX_WAIT 50 /* ms, half buffered_file limit */
> > > @@ -3324,6 +3354,7 @@ out:
> > > ret = qemu_file_get_error(f);
> > > if (ret < 0) {
> > > + precopy_notify(PRECOPY_NOTIFY_ERR);
> > Could you show me which function is this line in?
> >
> > Line 3324 here is ram_save_complete(), but I cannot find this exact
> > place.
>
> Sure, it's in ram_save_iterate():
> ...
> out:
> qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
> qemu_fflush(f);
> ram_counters.transferred += 8;
>
> ret = qemu_file_get_error(f);
> if (ret < 0) {
> + precopy_notify(PRECOPY_NOTIFY_ERR);
> return ret;
> }
>
> return done;
> }
Ok thanks. Please just make sure you will capture all the error
cases, e.g., I also see path like this (a few lines below):
if (pages < 0) {
qemu_file_set_error(f, pages);
break;
}
It seems that you missed that one.
I would even suggest that you capture the error with higher level.
E.g., in migration_iteration_run() after qemu_savevm_state_iterate().
Or we can just check the return value of qemu_savevm_state_iterate(),
which we have had ignored so far.
[1]
>
>
> >
> > Another thing to mention about the "reasons" (though I see it more
> > like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> > It might help in some cases:
> >
> > - then you don't need to trickily export the migrate_postcopy()
> > since you'll notify that before postcopy starts
>
> I'm thinking probably we don't need to export migrate_postcopy even now.
> It's more like a sanity check, and not needed because now we have the
> notifier registered to the precopy specific callchain, which has ensured
> that
> it is invoked via precopy.
But postcopy will always start with precopy, no?
>
> > - you'll have a solid point that you'll 100% guarantee that we'll
> > stop the free page hinting and don't need to worry about whether
> > there is chance the hinting will be running without an end [2].
>
> Thanks, I think it makes sense. Plan to add PRECOPY_NOTIFY_END in
> ram_save_complete.
Yeah you can.
Btw, if you're mostly adding the notifies only in RAM-only codes, then
you can consider add the "RAM" into the names of events too to be
clear.
My suggestion at [1] is precopy general, but you can still capture it
at the end of ram_save_iterate, then they are RAM-only again. Please
feel free to choose what fits more...
>
>
> >
> > Regarding [2] above: now the series only stops the hinting when
> > PRECOPY_NOTIFY_BEFORE_SYNC_BITMAP is notified. Could there be a case
> > that it's missing? E.g., what if we cancel/fail a migration during
> > precopy? Have you tried it?
> >
>
> I think it has been handled by the above PRECOPY_NOTIFY_ERR
Indeed it should, as long as you're covering all the error cases.
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-28 5:26 ` [Qemu-devel] " Peter Xu
@ 2018-11-28 9:01 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-28 9:01 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/28/2018 01:26 PM, Peter Xu wrote:
>
> Ok thanks. Please just make sure you will capture all the error
> cases, e.g., I also see path like this (a few lines below):
>
> if (pages < 0) {
> qemu_file_set_error(f, pages);
> break;
> }
>
> It seems that you missed that one.
I think that one should be fine. This notification is actually put at the
bottom of ram_save_iterate. All the above error will bail out to the "out:"
path and then go to call precopy_notify(PRECOPY_NOTIFY_ERR).
>
> I would even suggest that you capture the error with higher level.
> E.g., in migration_iteration_run() after qemu_savevm_state_iterate().
> Or we can just check the return value of qemu_savevm_state_iterate(),
> which we have had ignored so far.
Not very sure about the higher level, because other SaveStateEntry may cause
errors that this feature don't need to care, I think we may only need it
in ram_save.
> [1]
>
>>
>>> Another thing to mention about the "reasons" (though I see it more
>>> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
>>> It might help in some cases:
>>>
>>> - then you don't need to trickily export the migrate_postcopy()
>>> since you'll notify that before postcopy starts
>> I'm thinking probably we don't need to export migrate_postcopy even now.
>> It's more like a sanity check, and not needed because now we have the
>> notifier registered to the precopy specific callchain, which has ensured
>> that
>> it is invoked via precopy.
> But postcopy will always start with precopy, no?
Yes, but I think we could add the check in precopy_notify()
>
>>> - you'll have a solid point that you'll 100% guarantee that we'll
>>> stop the free page hinting and don't need to worry about whether
>>> there is chance the hinting will be running without an end [2].
>> Thanks, I think it makes sense. Plan to add PRECOPY_NOTIFY_END in
>> ram_save_complete.
> Yeah you can.
>
> Btw, if you're mostly adding the notifies only in RAM-only codes, then
> you can consider add the "RAM" into the names of events too to be
> clear.
Sounds good, will try and see if the name would become too long :)
>
> My suggestion at [1] is precopy general, but you can still capture it
> at the end of ram_save_iterate, then they are RAM-only again. Please
> feel free to choose what fits more...
OK, thanks.
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* [virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
@ 2018-11-28 9:01 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-28 9:01 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/28/2018 01:26 PM, Peter Xu wrote:
>
> Ok thanks. Please just make sure you will capture all the error
> cases, e.g., I also see path like this (a few lines below):
>
> if (pages < 0) {
> qemu_file_set_error(f, pages);
> break;
> }
>
> It seems that you missed that one.
I think that one should be fine. This notification is actually put at the
bottom of ram_save_iterate. All the above error will bail out to the "out:"
path and then go to call precopy_notify(PRECOPY_NOTIFY_ERR).
>
> I would even suggest that you capture the error with higher level.
> E.g., in migration_iteration_run() after qemu_savevm_state_iterate().
> Or we can just check the return value of qemu_savevm_state_iterate(),
> which we have had ignored so far.
Not very sure about the higher level, because other SaveStateEntry may cause
errors that this feature don't need to care, I think we may only need it
in ram_save.
> [1]
>
>>
>>> Another thing to mention about the "reasons" (though I see it more
>>> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
>>> It might help in some cases:
>>>
>>> - then you don't need to trickily export the migrate_postcopy()
>>> since you'll notify that before postcopy starts
>> I'm thinking probably we don't need to export migrate_postcopy even now.
>> It's more like a sanity check, and not needed because now we have the
>> notifier registered to the precopy specific callchain, which has ensured
>> that
>> it is invoked via precopy.
> But postcopy will always start with precopy, no?
Yes, but I think we could add the check in precopy_notify()
>
>>> - you'll have a solid point that you'll 100% guarantee that we'll
>>> stop the free page hinting and don't need to worry about whether
>>> there is chance the hinting will be running without an end [2].
>> Thanks, I think it makes sense. Plan to add PRECOPY_NOTIFY_END in
>> ram_save_complete.
> Yeah you can.
>
> Btw, if you're mostly adding the notifies only in RAM-only codes, then
> you can consider add the "RAM" into the names of events too to be
> clear.
Sounds good, will try and see if the name would become too long :)
>
> My suggestion at [1] is precopy general, but you can still capture it
> at the end of ram_save_iterate, then they are RAM-only again. Please
> feel free to choose what fits more...
OK, thanks.
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-28 9:01 ` [virtio-dev] " Wei Wang
(?)
@ 2018-11-28 9:32 ` Peter Xu
2018-11-29 3:40 ` [virtio-dev] " Wei Wang
-1 siblings, 1 reply; 53+ messages in thread
From: Peter Xu @ 2018-11-28 9:32 UTC (permalink / raw)
To: Wei Wang
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On Wed, Nov 28, 2018 at 05:01:31PM +0800, Wei Wang wrote:
> On 11/28/2018 01:26 PM, Peter Xu wrote:
> >
> > Ok thanks. Please just make sure you will capture all the error
> > cases, e.g., I also see path like this (a few lines below):
> >
> > if (pages < 0) {
> > qemu_file_set_error(f, pages);
> > break;
> > }
> >
> > It seems that you missed that one.
>
> I think that one should be fine. This notification is actually put at the
> bottom of ram_save_iterate. All the above error will bail out to the "out:"
> path and then go to call precopy_notify(PRECOPY_NOTIFY_ERR).
Ok, maybe I was pointing to a wrong one. :)
>
> >
> > I would even suggest that you capture the error with higher level.
> > E.g., in migration_iteration_run() after qemu_savevm_state_iterate().
> > Or we can just check the return value of qemu_savevm_state_iterate(),
> > which we have had ignored so far.
>
> Not very sure about the higher level, because other SaveStateEntry may cause
> errors that this feature don't need to care, I think we may only need it in
> ram_save.
So what I am worrying here are corner cases where we might forget to
stop the hinting. I'm fabricating one example sequence of events:
(start migration)
START_MIGRATION
BEFORE_SYNC
AFTER_SYNC
...
BEFORE_SYNC
AFTER_SYNC
(some SaveStateEntry failed rather than RAM, then
migration_detect_error returned MIG_THR_ERR_FATAL so we need to
fail the migration, however when running the previous
ram_save_iterate for RAM's specific SaveStateEntry we didn't see
any error so no ERROR event detected)
Then it seems the hinting will last forever. Considering that now I'm
not sure whether this can be done ram-only, since even if you capture
ram_save_complete() and at the same time you introduce PRECOPY_END you
may still miss the PRECOPY_END event since AFAIU ram_save_complete()
won't be called at all in this case.
Could this happen?
>
>
> > [1]
> >
> > >
> > > > Another thing to mention about the "reasons" (though I see it more
> > > > like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> > > > It might help in some cases:
> > > >
> > > > - then you don't need to trickily export the migrate_postcopy()
> > > > since you'll notify that before postcopy starts
> > > I'm thinking probably we don't need to export migrate_postcopy even now.
> > > It's more like a sanity check, and not needed because now we have the
> > > notifier registered to the precopy specific callchain, which has ensured
> > > that
> > > it is invoked via precopy.
> > But postcopy will always start with precopy, no?
>
> Yes, but I think we could add the check in precopy_notify()
I'm not sure that's good. If the notifier could potentially have
other user, they might still work with postcopy, and they might expect
e.g. BEFORE_SYNC to be called for every sync, even if it's at the
precopy stage of a postcopy. In that sense I still feel the
PRECOPY_END is better (so contantly call it at the end of precopy, no
matter whether there's another postcopy afterwards). It sounds like a
cleaner interface.
Or you can check it in the balloon specific callback and ignore the
event if postcopy is on, but then we're going backward to need to
export the API so it seems meaningless.
Regards,
--
Peter Xu
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-28 9:32 ` [Qemu-devel] " Peter Xu
@ 2018-11-29 3:40 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-29 3:40 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/28/2018 05:32 PM, Peter Xu wrote:
>
> So what I am worrying here are corner cases where we might forget to
> stop the hinting. I'm fabricating one example sequence of events:
>
> (start migration)
> START_MIGRATION
> BEFORE_SYNC
> AFTER_SYNC
> ...
> BEFORE_SYNC
> AFTER_SYNC
> (some SaveStateEntry failed rather than RAM, then
> migration_detect_error returned MIG_THR_ERR_FATAL so we need to
> fail the migration, however when running the previous
> ram_save_iterate for RAM's specific SaveStateEntry we didn't see
> any error so no ERROR event detected)
>
> Then it seems the hinting will last forever. Considering that now I'm
> not sure whether this can be done ram-only, since even if you capture
> ram_save_complete() and at the same time you introduce PRECOPY_END you
> may still miss the PRECOPY_END event since AFAIU ram_save_complete()
> won't be called at all in this case.
>
> Could this happen?
Thanks, indeed this case could happen if we add PRECOPY_END in
ram_save_complete.
How about putting PRECOPY_END in ram_save_cleanup?
I think it would be called in any case.
I'm also thinking probably we don't need PRECOPY_ERR when we have
PRECOPY_END,
and what do you think of the notifier names below:
+typedef enum PrecopyNotifyReason {
+ PRECOPY_NOTIFY_RAM_SAVE_END = 0,
+ PRECOPY_NOTIFY_RAM_SAVE_START = 1,
+ PRECOPY_NOTIFY_RAM_SAVE_BEFORE_SYNC_BITMAP = 2,
+ PRECOPY_NOTIFY_RAM_SAVE_AFTER_SYNC_BITMAP = 3,
+ PRECOPY_NOTIFY_RAM_SAVE_MAX = 4,
+} PrecopyNotifyReason;
>
>>
>>> [1]
>>>
>>>>> Another thing to mention about the "reasons" (though I see it more
>>>>> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
>>>>> It might help in some cases:
>>>>>
>>>>> - then you don't need to trickily export the migrate_postcopy()
>>>>> since you'll notify that before postcopy starts
>>>> I'm thinking probably we don't need to export migrate_postcopy even now.
>>>> It's more like a sanity check, and not needed because now we have the
>>>> notifier registered to the precopy specific callchain, which has ensured
>>>> that
>>>> it is invoked via precopy.
>>> But postcopy will always start with precopy, no?
>> Yes, but I think we could add the check in precopy_notify()
> I'm not sure that's good. If the notifier could potentially have
> other user, they might still work with postcopy, and they might expect
> e.g. BEFORE_SYNC to be called for every sync, even if it's at the
> precopy stage of a postcopy.
I think this precopy notifier callchain is expected to be used only for
the precopy mode. Postcopy has its dedicated notifier callchain that
users could use.
How about changing the migrate_postcopy() check to "ms->start_postcopy":
bool migration_postcopy_start(void)
{
MigrationState *s;
s = migrate_get_current();
return atomic_read(&s->start_postcopy);
}
static void precopy_notify(PrecopyNotifyReason reason)
{
if (migration_postcopy_start())
return;
notifier_list_notify(&precopy_notifier_list, &reason);
}
If postcopy started with precopy, the precopy optimization feature
could still be used until it switches to the postcopy mode.
> In that sense I still feel the
> PRECOPY_END is better (so contantly call it at the end of precopy, no
> matter whether there's another postcopy afterwards). It sounds like a
> cleaner interface.
Probably I still haven't got the point how PRECOPY_END could help above yet.
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* [virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
@ 2018-11-29 3:40 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-29 3:40 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/28/2018 05:32 PM, Peter Xu wrote:
>
> So what I am worrying here are corner cases where we might forget to
> stop the hinting. I'm fabricating one example sequence of events:
>
> (start migration)
> START_MIGRATION
> BEFORE_SYNC
> AFTER_SYNC
> ...
> BEFORE_SYNC
> AFTER_SYNC
> (some SaveStateEntry failed rather than RAM, then
> migration_detect_error returned MIG_THR_ERR_FATAL so we need to
> fail the migration, however when running the previous
> ram_save_iterate for RAM's specific SaveStateEntry we didn't see
> any error so no ERROR event detected)
>
> Then it seems the hinting will last forever. Considering that now I'm
> not sure whether this can be done ram-only, since even if you capture
> ram_save_complete() and at the same time you introduce PRECOPY_END you
> may still miss the PRECOPY_END event since AFAIU ram_save_complete()
> won't be called at all in this case.
>
> Could this happen?
Thanks, indeed this case could happen if we add PRECOPY_END in
ram_save_complete.
How about putting PRECOPY_END in ram_save_cleanup?
I think it would be called in any case.
I'm also thinking probably we don't need PRECOPY_ERR when we have
PRECOPY_END,
and what do you think of the notifier names below:
+typedef enum PrecopyNotifyReason {
+ PRECOPY_NOTIFY_RAM_SAVE_END = 0,
+ PRECOPY_NOTIFY_RAM_SAVE_START = 1,
+ PRECOPY_NOTIFY_RAM_SAVE_BEFORE_SYNC_BITMAP = 2,
+ PRECOPY_NOTIFY_RAM_SAVE_AFTER_SYNC_BITMAP = 3,
+ PRECOPY_NOTIFY_RAM_SAVE_MAX = 4,
+} PrecopyNotifyReason;
>
>>
>>> [1]
>>>
>>>>> Another thing to mention about the "reasons" (though I see it more
>>>>> like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
>>>>> It might help in some cases:
>>>>>
>>>>> - then you don't need to trickily export the migrate_postcopy()
>>>>> since you'll notify that before postcopy starts
>>>> I'm thinking probably we don't need to export migrate_postcopy even now.
>>>> It's more like a sanity check, and not needed because now we have the
>>>> notifier registered to the precopy specific callchain, which has ensured
>>>> that
>>>> it is invoked via precopy.
>>> But postcopy will always start with precopy, no?
>> Yes, but I think we could add the check in precopy_notify()
> I'm not sure that's good. If the notifier could potentially have
> other user, they might still work with postcopy, and they might expect
> e.g. BEFORE_SYNC to be called for every sync, even if it's at the
> precopy stage of a postcopy.
I think this precopy notifier callchain is expected to be used only for
the precopy mode. Postcopy has its dedicated notifier callchain that
users could use.
How about changing the migrate_postcopy() check to "ms->start_postcopy":
bool migration_postcopy_start(void)
{
MigrationState *s;
s = migrate_get_current();
return atomic_read(&s->start_postcopy);
}
static void precopy_notify(PrecopyNotifyReason reason)
{
if (migration_postcopy_start())
return;
notifier_list_notify(&precopy_notifier_list, &reason);
}
If postcopy started with precopy, the precopy optimization feature
could still be used until it switches to the postcopy mode.
> In that sense I still feel the
> PRECOPY_END is better (so contantly call it at the end of precopy, no
> matter whether there's another postcopy afterwards). It sounds like a
> cleaner interface.
Probably I still haven't got the point how PRECOPY_END could help above yet.
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-29 3:40 ` [virtio-dev] " Wei Wang
(?)
@ 2018-11-29 5:10 ` Peter Xu
2018-11-29 5:47 ` Peter Xu
` (2 more replies)
-1 siblings, 3 replies; 53+ messages in thread
From: Peter Xu @ 2018-11-29 5:10 UTC (permalink / raw)
To: Wei Wang
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
> On 11/28/2018 05:32 PM, Peter Xu wrote:
> >
> > So what I am worrying here are corner cases where we might forget to
> > stop the hinting. I'm fabricating one example sequence of events:
> >
> > (start migration)
> > START_MIGRATION
> > BEFORE_SYNC
> > AFTER_SYNC
> > ...
> > BEFORE_SYNC
> > AFTER_SYNC
> > (some SaveStateEntry failed rather than RAM, then
> > migration_detect_error returned MIG_THR_ERR_FATAL so we need to
> > fail the migration, however when running the previous
> > ram_save_iterate for RAM's specific SaveStateEntry we didn't see
> > any error so no ERROR event detected)
> >
> > Then it seems the hinting will last forever. Considering that now I'm
> > not sure whether this can be done ram-only, since even if you capture
> > ram_save_complete() and at the same time you introduce PRECOPY_END you
> > may still miss the PRECOPY_END event since AFAIU ram_save_complete()
> > won't be called at all in this case.
> >
> > Could this happen?
>
> Thanks, indeed this case could happen if we add PRECOPY_END in
> ram_save_complete.
>
> How about putting PRECOPY_END in ram_save_cleanup?
> I think it would be called in any case.
Sounds good.
>
> I'm also thinking probably we don't need PRECOPY_ERR when we have
> PRECOPY_END,
> and what do you think of the notifier names below:
>
> +typedef enum PrecopyNotifyReason {
> + PRECOPY_NOTIFY_RAM_SAVE_END = 0,
> + PRECOPY_NOTIFY_RAM_SAVE_START = 1,
> + PRECOPY_NOTIFY_RAM_SAVE_BEFORE_SYNC_BITMAP = 2,
> + PRECOPY_NOTIFY_RAM_SAVE_AFTER_SYNC_BITMAP = 3,
> + PRECOPY_NOTIFY_RAM_SAVE_MAX = 4,
> +} PrecopyNotifyReason;
(please see below [1]...)
>
>
> >
> > >
> > > > [1]
> > > >
> > > > > > Another thing to mention about the "reasons" (though I see it more
> > > > > > like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> > > > > > It might help in some cases:
> > > > > >
> > > > > > - then you don't need to trickily export the migrate_postcopy()
> > > > > > since you'll notify that before postcopy starts
> > > > > I'm thinking probably we don't need to export migrate_postcopy even now.
> > > > > It's more like a sanity check, and not needed because now we have the
> > > > > notifier registered to the precopy specific callchain, which has ensured
> > > > > that
> > > > > it is invoked via precopy.
> > > > But postcopy will always start with precopy, no?
> > > Yes, but I think we could add the check in precopy_notify()
> > I'm not sure that's good. If the notifier could potentially have
> > other user, they might still work with postcopy, and they might expect
> > e.g. BEFORE_SYNC to be called for every sync, even if it's at the
> > precopy stage of a postcopy.
>
> I think this precopy notifier callchain is expected to be used only for
> the precopy mode. Postcopy has its dedicated notifier callchain that
> users could use.
>
> How about changing the migrate_postcopy() check to "ms->start_postcopy":
>
> bool migration_postcopy_start(void)
> {
> MigrationState *s;
>
> s = migrate_get_current();
>
> return atomic_read(&s->start_postcopy);
> }
>
>
> static void precopy_notify(PrecopyNotifyReason reason)
> {
> if (migration_postcopy_start())
> return;
>
> notifier_list_notify(&precopy_notifier_list, &reason);
> }
>
> If postcopy started with precopy, the precopy optimization feature
> could still be used until it switches to the postcopy mode.
I'm not sure we can use start_postcopy. It's a variable being set in
the QMP handler but it does not mean postcopy has started. I'm afraid
there can be race where it's still precopy but the variable is set so
event could be missed...
IMHO the problem is not that complicated. How about this proposal:
[1]
typedef enum PrecopyNotifyReason {
PRECOPY_NOTIFY_RAM_START,
PRECOPY_NOTIFY_RAM_BEFORE_SYNC,
PRECOPY_NOTIFY_RAM_AFTER_SYNC,
PRECOPY_NOTIFY_COMPLETE,
PRECOPY_NOTIFY_RAM_CLEANUP,
};
The first three keep the same as your old ones. Notify RAM_CLEANUP in
ram_save_cleanup() to make sure it'll always be cleaned up (the same
as PRECOPY_END, just another name). Notify COMPLETE in
qemu_savevm_state_complete_precopy() to show that precopy is
completed. Meanwhile on balloon side you should stop the hinting for
either RAM_CLEANUP or COMPLETE event. Then either:
- precopy is switching to postcopy, or
- precopy completed, or
- precopy failed/cancelled
You should always get at least a notification to stop the balloon.
Though you could also get one RAM_CLEANUP after one COMPLETE, but
the balloon should easily handle it (stop the hinting twice).
Here maybe you can even remove the "RAM_" in both RAM_START and
RAM_CLEANUP if we're going to have COMPLETE since after all it'll be
not only limited to RAM.
Another suggestion is that you can add an Error into the notify hooks,
please refer to the postcopy one:
int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);
So the hook functions have a way to even stop the migration (though
for balloon hinting it'll be always optional so no error should be
reported...), then the two interfaces are matched.
>
>
>
> > In that sense I still feel the
> > PRECOPY_END is better (so contantly call it at the end of precopy, no
> > matter whether there's another postcopy afterwards). It sounds like a
> > cleaner interface.
>
> Probably I still haven't got the point how PRECOPY_END could help above yet.
Please have a look at above proposal. Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-29 5:10 ` [Qemu-devel] " Peter Xu
@ 2018-11-29 5:47 ` Peter Xu
2018-11-29 6:30 ` [virtio-dev] " Wei Wang
2018-11-30 5:05 ` [virtio-dev] " Wei Wang
2 siblings, 0 replies; 53+ messages in thread
From: Peter Xu @ 2018-11-29 5:47 UTC (permalink / raw)
To: Wei Wang
Cc: virtio-dev, quintela, liliang.opensource, mst, qemu-devel,
dgilbert, pbonzini, nilal
On Thu, Nov 29, 2018 at 01:10:14PM +0800, Peter Xu wrote:
> On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
> > On 11/28/2018 05:32 PM, Peter Xu wrote:
> > >
> > > So what I am worrying here are corner cases where we might forget to
> > > stop the hinting. I'm fabricating one example sequence of events:
> > >
> > > (start migration)
> > > START_MIGRATION
> > > BEFORE_SYNC
> > > AFTER_SYNC
> > > ...
> > > BEFORE_SYNC
> > > AFTER_SYNC
> > > (some SaveStateEntry failed rather than RAM, then
> > > migration_detect_error returned MIG_THR_ERR_FATAL so we need to
> > > fail the migration, however when running the previous
> > > ram_save_iterate for RAM's specific SaveStateEntry we didn't see
> > > any error so no ERROR event detected)
> > >
> > > Then it seems the hinting will last forever. Considering that now I'm
> > > not sure whether this can be done ram-only, since even if you capture
> > > ram_save_complete() and at the same time you introduce PRECOPY_END you
> > > may still miss the PRECOPY_END event since AFAIU ram_save_complete()
> > > won't be called at all in this case.
> > >
> > > Could this happen?
> >
> > Thanks, indeed this case could happen if we add PRECOPY_END in
> > ram_save_complete.
> >
> > How about putting PRECOPY_END in ram_save_cleanup?
> > I think it would be called in any case.
>
> Sounds good.
>
> >
> > I'm also thinking probably we don't need PRECOPY_ERR when we have
> > PRECOPY_END,
> > and what do you think of the notifier names below:
> >
> > +typedef enum PrecopyNotifyReason {
> > + PRECOPY_NOTIFY_RAM_SAVE_END = 0,
> > + PRECOPY_NOTIFY_RAM_SAVE_START = 1,
> > + PRECOPY_NOTIFY_RAM_SAVE_BEFORE_SYNC_BITMAP = 2,
> > + PRECOPY_NOTIFY_RAM_SAVE_AFTER_SYNC_BITMAP = 3,
> > + PRECOPY_NOTIFY_RAM_SAVE_MAX = 4,
> > +} PrecopyNotifyReason;
>
> (please see below [1]...)
>
> >
> >
> > >
> > > >
> > > > > [1]
> > > > >
> > > > > > > Another thing to mention about the "reasons" (though I see it more
> > > > > > > like "events"): have you thought about adding a PRECOPY_NOTIFY_END?
> > > > > > > It might help in some cases:
> > > > > > >
> > > > > > > - then you don't need to trickily export the migrate_postcopy()
> > > > > > > since you'll notify that before postcopy starts
> > > > > > I'm thinking probably we don't need to export migrate_postcopy even now.
> > > > > > It's more like a sanity check, and not needed because now we have the
> > > > > > notifier registered to the precopy specific callchain, which has ensured
> > > > > > that
> > > > > > it is invoked via precopy.
> > > > > But postcopy will always start with precopy, no?
> > > > Yes, but I think we could add the check in precopy_notify()
> > > I'm not sure that's good. If the notifier could potentially have
> > > other user, they might still work with postcopy, and they might expect
> > > e.g. BEFORE_SYNC to be called for every sync, even if it's at the
> > > precopy stage of a postcopy.
> >
> > I think this precopy notifier callchain is expected to be used only for
> > the precopy mode. Postcopy has its dedicated notifier callchain that
> > users could use.
> >
> > How about changing the migrate_postcopy() check to "ms->start_postcopy":
> >
> > bool migration_postcopy_start(void)
> > {
> > MigrationState *s;
> >
> > s = migrate_get_current();
> >
> > return atomic_read(&s->start_postcopy);
> > }
> >
> >
> > static void precopy_notify(PrecopyNotifyReason reason)
> > {
> > if (migration_postcopy_start())
> > return;
> >
> > notifier_list_notify(&precopy_notifier_list, &reason);
> > }
> >
> > If postcopy started with precopy, the precopy optimization feature
> > could still be used until it switches to the postcopy mode.
>
> I'm not sure we can use start_postcopy. It's a variable being set in
> the QMP handler but it does not mean postcopy has started. I'm afraid
> there can be race where it's still precopy but the variable is set so
> event could be missed...
>
> IMHO the problem is not that complicated. How about this proposal:
>
> [1]
>
> typedef enum PrecopyNotifyReason {
> PRECOPY_NOTIFY_RAM_START,
> PRECOPY_NOTIFY_RAM_BEFORE_SYNC,
> PRECOPY_NOTIFY_RAM_AFTER_SYNC,
> PRECOPY_NOTIFY_COMPLETE,
> PRECOPY_NOTIFY_RAM_CLEANUP,
> };
>
> The first three keep the same as your old ones. Notify RAM_CLEANUP in
> ram_save_cleanup() to make sure it'll always be cleaned up (the same
> as PRECOPY_END, just another name). Notify COMPLETE in
> qemu_savevm_state_complete_precopy() to show that precopy is
> completed. Meanwhile on balloon side you should stop the hinting for
> either RAM_CLEANUP or COMPLETE event. Then either:
>
> - precopy is switching to postcopy, or
> - precopy completed, or
> - precopy failed/cancelled
>
> You should always get at least a notification to stop the balloon.
> Though you could also get one RAM_CLEANUP after one COMPLETE, but
> the balloon should easily handle it (stop the hinting twice).
>
> Here maybe you can even remove the "RAM_" in both RAM_START and
> RAM_CLEANUP if we're going to have COMPLETE since after all it'll be
> not only limited to RAM.
Oh maybe we can remove all the RAM_ prefix to make it precopy
general...
typedef enum PrecopyNotifyReason {
PRECOPY_NOTIFY_SETUP,
PRECOPY_NOTIFY_BEFORE_SYNC,
PRECOPY_NOTIFY_AFTER_SYNC,
PRECOPY_NOTIFY_COMPLETE,
PRECOPY_NOTIFY_CLEANUP,
};
Then we can just hook everything with the corresponding names:
SETUP: hooks with qemu_savevm_state_setup
COMPLETE: hooks with qemu_savevm_state_complete_precopy
CLEANUP: hooks with qemu_savevm_state_cleanup
I'm not sure whether you'll need another hook in ram_state_reset in
the future but for now I don't see it necessary since I don't thnk
ram_list.version would change during migration for now, so
ram_state_reset should only be called during setup.
>
> Another suggestion is that you can add an Error into the notify hooks,
> please refer to the postcopy one:
>
> int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);
>
> So the hook functions have a way to even stop the migration (though
> for balloon hinting it'll be always optional so no error should be
> reported...), then the two interfaces are matched.
>
> >
> >
> >
> > > In that sense I still feel the
> > > PRECOPY_END is better (so contantly call it at the end of precopy, no
> > > matter whether there's another postcopy afterwards). It sounds like a
> > > cleaner interface.
> >
> > Probably I still haven't got the point how PRECOPY_END could help above yet.
>
> Please have a look at above proposal. Thanks,
>
> --
> Peter Xu
>
Regards,
--
Peter Xu
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-29 5:10 ` [Qemu-devel] " Peter Xu
@ 2018-11-29 6:30 ` Wei Wang
2018-11-29 6:30 ` [virtio-dev] " Wei Wang
2018-11-30 5:05 ` [virtio-dev] " Wei Wang
2 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-29 6:30 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/29/2018 01:10 PM, Peter Xu wrote:
> On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
>> On 11/28/2018 05:32 PM, Peter Xu wrote:
> I'm not sure we can use start_postcopy. It's a variable being set in
> the QMP handler but it does not mean postcopy has started. I'm afraid
> there can be race where it's still precopy but the variable is set so
> event could be missed...
>
> IMHO the problem is not that complicated. How about this proposal:
>
> [1]
>
> typedef enum PrecopyNotifyReason {
> PRECOPY_NOTIFY_RAM_START,
> PRECOPY_NOTIFY_RAM_BEFORE_SYNC,
> PRECOPY_NOTIFY_RAM_AFTER_SYNC,
> PRECOPY_NOTIFY_COMPLETE,
> PRECOPY_NOTIFY_RAM_CLEANUP,
> };
>
> The first three keep the same as your old ones. Notify RAM_CLEANUP in
> ram_save_cleanup() to make sure it'll always be cleaned up (the same
> as PRECOPY_END, just another name). Notify COMPLETE in
> qemu_savevm_state_complete_precopy() to show that precopy is
> completed. Meanwhile on balloon side you should stop the hinting for
> either RAM_CLEANUP or COMPLETE event. Then either:
>
> - precopy is switching to postcopy, or
> - precopy completed, or
> - precopy failed/cancelled
>
> You should always get at least a notification to stop the balloon.
> Though you could also get one RAM_CLEANUP after one COMPLETE, but
> the balloon should easily handle it (stop the hinting twice).
Sounds better, thanks.
>
> Here maybe you can even remove the "RAM_" in both RAM_START and
> RAM_CLEANUP if we're going to have COMPLETE since after all it'll be
> not only limited to RAM.
>
> Another suggestion is that you can add an Error into the notify hooks,
> please refer to the postcopy one:
>
> int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);
>
> So the hook functions have a way to even stop the migration (though
> for balloon hinting it'll be always optional so no error should be
> reported...), then the two interfaces are matched.
Yes, I removed that "errp" as it's not needed by the balloon usage.
But no problem, I can add it back if no objections from others.
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* [virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
@ 2018-11-29 6:30 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-29 6:30 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/29/2018 01:10 PM, Peter Xu wrote:
> On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
>> On 11/28/2018 05:32 PM, Peter Xu wrote:
> I'm not sure we can use start_postcopy. It's a variable being set in
> the QMP handler but it does not mean postcopy has started. I'm afraid
> there can be race where it's still precopy but the variable is set so
> event could be missed...
>
> IMHO the problem is not that complicated. How about this proposal:
>
> [1]
>
> typedef enum PrecopyNotifyReason {
> PRECOPY_NOTIFY_RAM_START,
> PRECOPY_NOTIFY_RAM_BEFORE_SYNC,
> PRECOPY_NOTIFY_RAM_AFTER_SYNC,
> PRECOPY_NOTIFY_COMPLETE,
> PRECOPY_NOTIFY_RAM_CLEANUP,
> };
>
> The first three keep the same as your old ones. Notify RAM_CLEANUP in
> ram_save_cleanup() to make sure it'll always be cleaned up (the same
> as PRECOPY_END, just another name). Notify COMPLETE in
> qemu_savevm_state_complete_precopy() to show that precopy is
> completed. Meanwhile on balloon side you should stop the hinting for
> either RAM_CLEANUP or COMPLETE event. Then either:
>
> - precopy is switching to postcopy, or
> - precopy completed, or
> - precopy failed/cancelled
>
> You should always get at least a notification to stop the balloon.
> Though you could also get one RAM_CLEANUP after one COMPLETE, but
> the balloon should easily handle it (stop the hinting twice).
Sounds better, thanks.
>
> Here maybe you can even remove the "RAM_" in both RAM_START and
> RAM_CLEANUP if we're going to have COMPLETE since after all it'll be
> not only limited to RAM.
>
> Another suggestion is that you can add an Error into the notify hooks,
> please refer to the postcopy one:
>
> int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);
>
> So the hook functions have a way to even stop the migration (though
> for balloon hinting it'll be always optional so no error should be
> reported...), then the two interfaces are matched.
Yes, I removed that "errp" as it's not needed by the balloon usage.
But no problem, I can add it back if no objections from others.
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-29 5:10 ` [Qemu-devel] " Peter Xu
@ 2018-11-30 5:05 ` Wei Wang
2018-11-29 6:30 ` [virtio-dev] " Wei Wang
2018-11-30 5:05 ` [virtio-dev] " Wei Wang
2 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-30 5:05 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/29/2018 01:10 PM, Peter Xu wrote:
> On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
> I think this precopy notifier callchain is expected to be used only for
> the precopy mode. Postcopy has its dedicated notifier callchain that
> users could use.
>
> How about changing the migrate_postcopy() check to "ms->start_postcopy":
>
> bool migration_postcopy_start(void)
> {
> MigrationState *s;
>
> s = migrate_get_current();
>
> return atomic_read(&s->start_postcopy);
> }
>
>
> static void precopy_notify(PrecopyNotifyReason reason)
> {
> if (migration_postcopy_start())
> return;
>
> notifier_list_notify(&precopy_notifier_list, &reason);
> }
>
> If postcopy started with precopy, the precopy optimization feature
> could still be used until it switches to the postcopy mode.
> I'm not sure we can use start_postcopy. It's a variable being set in
> the QMP handler but it does not mean postcopy has started. I'm afraid
> there can be race where it's still precopy but the variable is set so
> event could be missed...
Peter,
I just found that migration_bitmap_sync is also called by
ram_postcopy_send_discard_bitmap().
But we don't expect notifier_list_notify to be called in the postcopy mode.
So, probably we still need the start_postcopy check in
notifier_list_notify though we have
the COMPLETE notifier.
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* [virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
@ 2018-11-30 5:05 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-30 5:05 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/29/2018 01:10 PM, Peter Xu wrote:
> On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
> I think this precopy notifier callchain is expected to be used only for
> the precopy mode. Postcopy has its dedicated notifier callchain that
> users could use.
>
> How about changing the migrate_postcopy() check to "ms->start_postcopy":
>
> bool migration_postcopy_start(void)
> {
> MigrationState *s;
>
> s = migrate_get_current();
>
> return atomic_read(&s->start_postcopy);
> }
>
>
> static void precopy_notify(PrecopyNotifyReason reason)
> {
> if (migration_postcopy_start())
> return;
>
> notifier_list_notify(&precopy_notifier_list, &reason);
> }
>
> If postcopy started with precopy, the precopy optimization feature
> could still be used until it switches to the postcopy mode.
> I'm not sure we can use start_postcopy. It's a variable being set in
> the QMP handler but it does not mean postcopy has started. I'm afraid
> there can be race where it's still precopy but the variable is set so
> event could be missed...
Peter,
I just found that migration_bitmap_sync is also called by
ram_postcopy_send_discard_bitmap().
But we don't expect notifier_list_notify to be called in the postcopy mode.
So, probably we still need the start_postcopy check in
notifier_list_notify though we have
the COMPLETE notifier.
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-30 5:05 ` [virtio-dev] " Wei Wang
(?)
@ 2018-11-30 5:57 ` Peter Xu
2018-11-30 7:09 ` [virtio-dev] " Wei Wang
-1 siblings, 1 reply; 53+ messages in thread
From: Peter Xu @ 2018-11-30 5:57 UTC (permalink / raw)
To: Wei Wang
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On Fri, Nov 30, 2018 at 01:05:51PM +0800, Wei Wang wrote:
> On 11/29/2018 01:10 PM, Peter Xu wrote:
> > On Thu, Nov 29, 2018 at 11:40:57AM +0800, Wei Wang wrote:
> > I think this precopy notifier callchain is expected to be used only for
> > the precopy mode. Postcopy has its dedicated notifier callchain that
> > users could use.
> >
> > How about changing the migrate_postcopy() check to "ms->start_postcopy":
> >
> > bool migration_postcopy_start(void)
> > {
> > MigrationState *s;
> >
> > s = migrate_get_current();
> >
> > return atomic_read(&s->start_postcopy);
> > }
> >
> >
> > static void precopy_notify(PrecopyNotifyReason reason)
> > {
> > if (migration_postcopy_start())
> > return;
> >
> > notifier_list_notify(&precopy_notifier_list, &reason);
> > }
> >
> > If postcopy started with precopy, the precopy optimization feature
> > could still be used until it switches to the postcopy mode.
> > I'm not sure we can use start_postcopy. It's a variable being set in
> > the QMP handler but it does not mean postcopy has started. I'm afraid
> > there can be race where it's still precopy but the variable is set so
> > event could be missed...
>
> Peter,
> I just found that migration_bitmap_sync is also called by
> ram_postcopy_send_discard_bitmap().
> But we don't expect notifier_list_notify to be called in the postcopy mode.
>
> So, probably we still need the start_postcopy check in notifier_list_notify
> though we have
> the COMPLETE notifier.
I still don't think using start_postcopy is a good idea, as
explained in my previous reply.
Maybe move the notify()s outside migration_bitmap_sync() but only at
the call sites in precopy? Say these three:
ram_init_bitmaps[3508] migration_bitmap_sync(rs);
ram_save_complete[3731] migration_bitmap_sync(rs);
ram_save_pending[3776] migration_bitmap_sync(rs);
Or you can introduce migration_bitmap_sync_precopy() and let precopy
code call that one instead.
PS. I'm a bit unsure on why we need to sync bitmap in ram_init_bitmaps.
I feel like it can be removed...
Regards,
--
Peter Xu
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
2018-11-30 5:57 ` [Qemu-devel] " Peter Xu
@ 2018-11-30 7:09 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-30 7:09 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/30/2018 01:57 PM, Peter Xu wrote:
>>
>>
>> Or you can introduce migration_bitmap_sync_precopy() and let precopy
>> code call that one instead.
Agree, thanks.
> PS. I'm a bit unsure on why we need to sync bitmap in ram_init_bitmaps.
> I feel like it can be removed...
Seems it was added in early days to fix some bug:
https://git.virtualopensystems.com/dev/qemu/commit/79536f4f16934d6759a1d67f0342b4e7ceb66671?w=1
Hopefully, Juan could share some details about that.
Best,
Wei
^ permalink raw reply [flat|nested] 53+ messages in thread
* [virtio-dev] Re: [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy
@ 2018-11-30 7:09 ` Wei Wang
0 siblings, 0 replies; 53+ messages in thread
From: Wei Wang @ 2018-11-30 7:09 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, virtio-dev, mst, quintela, dgilbert, pbonzini,
liliang.opensource, nilal, riel
On 11/30/2018 01:57 PM, Peter Xu wrote:
>>
>>
>> Or you can introduce migration_bitmap_sync_precopy() and let precopy
>> code call that one instead.
Agree, thanks.
> PS. I'm a bit unsure on why we need to sync bitmap in ram_init_bitmaps.
> I feel like it can be removed...
Seems it was added in early days to fix some bug:
https://git.virtualopensystems.com/dev/qemu/commit/79536f4f16934d6759a1d67f0342b4e7ceb66671?w=1
Hopefully, Juan could share some details about that.
Best,
Wei
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 53+ messages in thread
end of thread, other threads:[~2018-11-30 7:06 UTC | newest]
Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-15 10:07 [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support Wei Wang
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
2018-11-15 10:07 ` [Qemu-devel] [PATCH v9 1/8] bitmap: fix bitmap_count_one Wei Wang
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
2018-11-15 10:07 ` [Qemu-devel] [PATCH v9 2/8] bitmap: bitmap_count_one_with_offset Wei Wang
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
2018-11-15 10:07 ` [Qemu-devel] [PATCH v9 3/8] migration: use bitmap_mutex in migration_bitmap_clear_dirty Wei Wang
2018-11-15 10:07 ` [virtio-dev] " Wei Wang
2018-11-27 5:40 ` [Qemu-devel] " Peter Xu
2018-11-27 6:02 ` Wei Wang
2018-11-27 6:02 ` [virtio-dev] " Wei Wang
2018-11-27 6:12 ` [Qemu-devel] " Wei Wang
2018-11-27 6:12 ` Wei Wang
2018-11-27 7:41 ` [Qemu-devel] " Peter Xu
2018-11-27 10:17 ` Wei Wang
2018-11-27 10:17 ` Wei Wang
2018-11-15 10:08 ` [Qemu-devel] [PATCH v9 4/8] migration: API to clear bits of guest free pages from the dirty bitmap Wei Wang
2018-11-15 10:08 ` [virtio-dev] " Wei Wang
2018-11-27 6:06 ` [Qemu-devel] " Peter Xu
2018-11-27 6:52 ` Wei Wang
2018-11-27 6:52 ` [virtio-dev] " Wei Wang
2018-11-27 7:43 ` [Qemu-devel] " Peter Xu
2018-11-15 10:08 ` [Qemu-devel] [PATCH v9 5/8] migration/ram.c: add a notifier chain for precopy Wei Wang
2018-11-15 10:08 ` [virtio-dev] " Wei Wang
2018-11-27 7:38 ` [Qemu-devel] " Peter Xu
2018-11-27 10:25 ` Wei Wang
2018-11-27 10:25 ` [virtio-dev] " Wei Wang
2018-11-28 5:26 ` [Qemu-devel] " Peter Xu
2018-11-28 9:01 ` Wei Wang
2018-11-28 9:01 ` [virtio-dev] " Wei Wang
2018-11-28 9:32 ` [Qemu-devel] " Peter Xu
2018-11-29 3:40 ` Wei Wang
2018-11-29 3:40 ` [virtio-dev] " Wei Wang
2018-11-29 5:10 ` [Qemu-devel] " Peter Xu
2018-11-29 5:47 ` Peter Xu
2018-11-29 6:30 ` Wei Wang
2018-11-29 6:30 ` [virtio-dev] " Wei Wang
2018-11-30 5:05 ` [Qemu-devel] " Wei Wang
2018-11-30 5:05 ` [virtio-dev] " Wei Wang
2018-11-30 5:57 ` [Qemu-devel] " Peter Xu
2018-11-30 7:09 ` Wei Wang
2018-11-30 7:09 ` [virtio-dev] " Wei Wang
2018-11-15 10:08 ` [Qemu-devel] [PATCH v9 6/8] migration/ram.c: add a function to disable the bulk stage Wei Wang
2018-11-15 10:08 ` [virtio-dev] " Wei Wang
2018-11-15 10:08 ` [Qemu-devel] [PATCH v9 7/8] migration: move migrate_postcopy() to include/migration/misc.h Wei Wang
2018-11-15 10:08 ` [virtio-dev] " Wei Wang
2018-11-15 10:08 ` [Qemu-devel] [PATCH v9 8/8] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT Wei Wang
2018-11-15 10:08 ` [virtio-dev] " Wei Wang
2018-11-15 18:50 ` [Qemu-devel] [PATCH v9 0/8] virtio-balloon: free page hint support no-reply
2018-11-16 1:38 ` Wei Wang
2018-11-16 1:38 ` [virtio-dev] " Wei Wang
2018-11-27 3:11 ` Wei Wang
2018-11-27 3:11 ` [virtio-dev] " Wei Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.