* [Qemu-devel] [PATCH v10 01/10] userfault: update kernel header for UFFD_FEATURE_*
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 00/10] calculate blocktime for postcopy live migration Alexey Perevalov
@ 2017-09-19 16:47 ` Alexey Perevalov
2017-09-20 18:43 ` Dr. David Alan Gilbert
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 02/10] migration: pass MigrationIncomingState* into migration check functions Alexey Perevalov
` (8 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-19 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Perevalov, i.maximets, peterx, eblake, dgilbert, quintela,
heetae82.ahn
This commit adds modification for UFFD_FEATURE_SIGBUS and
UFFD_FEATURE_THREAD_ID.
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
| 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
--git a/linux-headers/linux/userfaultfd.h b/linux-headers/linux/userfaultfd.h
index 9701772..b43cf0d 100644
--- a/linux-headers/linux/userfaultfd.h
+++ b/linux-headers/linux/userfaultfd.h
@@ -23,7 +23,9 @@
UFFD_FEATURE_EVENT_REMOVE | \
UFFD_FEATURE_EVENT_UNMAP | \
UFFD_FEATURE_MISSING_HUGETLBFS | \
- UFFD_FEATURE_MISSING_SHMEM)
+ UFFD_FEATURE_MISSING_SHMEM | \
+ UFFD_FEATURE_SIGBUS | \
+ UFFD_FEATURE_THREAD_ID)
#define UFFD_API_IOCTLS \
((__u64)1 << _UFFDIO_REGISTER | \
(__u64)1 << _UFFDIO_UNREGISTER | \
@@ -78,6 +80,9 @@ struct uffd_msg {
struct {
__u64 flags;
__u64 address;
+ union {
+ __u32 ptid;
+ } feat;
} pagefault;
struct {
@@ -153,6 +158,13 @@ struct uffdio_api {
* UFFD_FEATURE_MISSING_SHMEM works the same as
* UFFD_FEATURE_MISSING_HUGETLBFS, but it applies to shmem
* (i.e. tmpfs and other shmem based APIs).
+ *
+ * UFFD_FEATURE_SIGBUS feature means no page-fault
+ * (UFFD_EVENT_PAGEFAULT) event will be delivered, instead
+ * a SIGBUS signal will be sent to the faulting process.
+ *
+ * UFFD_FEATURE_THREAD_ID pid of the page faulted task_struct will
+ * be returned, if feature is not requested 0 will be returned.
*/
#define UFFD_FEATURE_PAGEFAULT_FLAG_WP (1<<0)
#define UFFD_FEATURE_EVENT_FORK (1<<1)
@@ -161,6 +173,8 @@ struct uffdio_api {
#define UFFD_FEATURE_MISSING_HUGETLBFS (1<<4)
#define UFFD_FEATURE_MISSING_SHMEM (1<<5)
#define UFFD_FEATURE_EVENT_UNMAP (1<<6)
+#define UFFD_FEATURE_SIGBUS (1<<7)
+#define UFFD_FEATURE_THREAD_ID (1<<8)
__u64 features;
__u64 ioctls;
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 01/10] userfault: update kernel header for UFFD_FEATURE_*
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 01/10] userfault: update kernel header for UFFD_FEATURE_* Alexey Perevalov
@ 2017-09-20 18:43 ` Dr. David Alan Gilbert
2017-09-21 7:33 ` Alexey Perevalov
0 siblings, 1 reply; 27+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-20 18:43 UTC (permalink / raw)
To: Alexey Perevalov
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> This commit adds modification for UFFD_FEATURE_SIGBUS and
> UFFD_FEATURE_THREAD_ID.
>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
This should be replaced with just running the
scripts/update-linux-headers.sh
against a 4.14-rc1 checkout.
That can be done as a separate patch or the first patch
of this series.
Dave
> ---
> linux-headers/linux/userfaultfd.h | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/linux-headers/linux/userfaultfd.h b/linux-headers/linux/userfaultfd.h
> index 9701772..b43cf0d 100644
> --- a/linux-headers/linux/userfaultfd.h
> +++ b/linux-headers/linux/userfaultfd.h
> @@ -23,7 +23,9 @@
> UFFD_FEATURE_EVENT_REMOVE | \
> UFFD_FEATURE_EVENT_UNMAP | \
> UFFD_FEATURE_MISSING_HUGETLBFS | \
> - UFFD_FEATURE_MISSING_SHMEM)
> + UFFD_FEATURE_MISSING_SHMEM | \
> + UFFD_FEATURE_SIGBUS | \
> + UFFD_FEATURE_THREAD_ID)
> #define UFFD_API_IOCTLS \
> ((__u64)1 << _UFFDIO_REGISTER | \
> (__u64)1 << _UFFDIO_UNREGISTER | \
> @@ -78,6 +80,9 @@ struct uffd_msg {
> struct {
> __u64 flags;
> __u64 address;
> + union {
> + __u32 ptid;
> + } feat;
> } pagefault;
>
> struct {
> @@ -153,6 +158,13 @@ struct uffdio_api {
> * UFFD_FEATURE_MISSING_SHMEM works the same as
> * UFFD_FEATURE_MISSING_HUGETLBFS, but it applies to shmem
> * (i.e. tmpfs and other shmem based APIs).
> + *
> + * UFFD_FEATURE_SIGBUS feature means no page-fault
> + * (UFFD_EVENT_PAGEFAULT) event will be delivered, instead
> + * a SIGBUS signal will be sent to the faulting process.
> + *
> + * UFFD_FEATURE_THREAD_ID pid of the page faulted task_struct will
> + * be returned, if feature is not requested 0 will be returned.
> */
> #define UFFD_FEATURE_PAGEFAULT_FLAG_WP (1<<0)
> #define UFFD_FEATURE_EVENT_FORK (1<<1)
> @@ -161,6 +173,8 @@ struct uffdio_api {
> #define UFFD_FEATURE_MISSING_HUGETLBFS (1<<4)
> #define UFFD_FEATURE_MISSING_SHMEM (1<<5)
> #define UFFD_FEATURE_EVENT_UNMAP (1<<6)
> +#define UFFD_FEATURE_SIGBUS (1<<7)
> +#define UFFD_FEATURE_THREAD_ID (1<<8)
> __u64 features;
>
> __u64 ioctls;
> --
> 1.9.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 01/10] userfault: update kernel header for UFFD_FEATURE_*
2017-09-20 18:43 ` Dr. David Alan Gilbert
@ 2017-09-21 7:33 ` Alexey Perevalov
0 siblings, 0 replies; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-21 7:33 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
On 09/20/2017 09:43 PM, Dr. David Alan Gilbert wrote:
> * Alexey Perevalov (a.perevalov@samsung.com) wrote:
>> This commit adds modification for UFFD_FEATURE_SIGBUS and
>> UFFD_FEATURE_THREAD_ID.
>>
>> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> This should be replaced with just running the
> scripts/update-linux-headers.sh
> against a 4.14-rc1 checkout.
>
> That can be done as a separate patch or the first patch
> of this series.
Ok, in case of separate patch it's reasonably to
send modification for all headers.
--
Best regards,
Alexey Perevalov
>
> Dave
>
>> ---
>> linux-headers/linux/userfaultfd.h | 16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-headers/linux/userfaultfd.h b/linux-headers/linux/userfaultfd.h
>> index 9701772..b43cf0d 100644
>> --- a/linux-headers/linux/userfaultfd.h
>> +++ b/linux-headers/linux/userfaultfd.h
>> @@ -23,7 +23,9 @@
>> UFFD_FEATURE_EVENT_REMOVE | \
>> UFFD_FEATURE_EVENT_UNMAP | \
>> UFFD_FEATURE_MISSING_HUGETLBFS | \
>> - UFFD_FEATURE_MISSING_SHMEM)
>> + UFFD_FEATURE_MISSING_SHMEM | \
>> + UFFD_FEATURE_SIGBUS | \
>> + UFFD_FEATURE_THREAD_ID)
>> #define UFFD_API_IOCTLS \
>> ((__u64)1 << _UFFDIO_REGISTER | \
>> (__u64)1 << _UFFDIO_UNREGISTER | \
>> @@ -78,6 +80,9 @@ struct uffd_msg {
>> struct {
>> __u64 flags;
>> __u64 address;
>> + union {
>> + __u32 ptid;
>> + } feat;
>> } pagefault;
>>
>> struct {
>> @@ -153,6 +158,13 @@ struct uffdio_api {
>> * UFFD_FEATURE_MISSING_SHMEM works the same as
>> * UFFD_FEATURE_MISSING_HUGETLBFS, but it applies to shmem
>> * (i.e. tmpfs and other shmem based APIs).
>> + *
>> + * UFFD_FEATURE_SIGBUS feature means no page-fault
>> + * (UFFD_EVENT_PAGEFAULT) event will be delivered, instead
>> + * a SIGBUS signal will be sent to the faulting process.
>> + *
>> + * UFFD_FEATURE_THREAD_ID pid of the page faulted task_struct will
>> + * be returned, if feature is not requested 0 will be returned.
>> */
>> #define UFFD_FEATURE_PAGEFAULT_FLAG_WP (1<<0)
>> #define UFFD_FEATURE_EVENT_FORK (1<<1)
>> @@ -161,6 +173,8 @@ struct uffdio_api {
>> #define UFFD_FEATURE_MISSING_HUGETLBFS (1<<4)
>> #define UFFD_FEATURE_MISSING_SHMEM (1<<5)
>> #define UFFD_FEATURE_EVENT_UNMAP (1<<6)
>> +#define UFFD_FEATURE_SIGBUS (1<<7)
>> +#define UFFD_FEATURE_THREAD_ID (1<<8)
>> __u64 features;
>>
>> __u64 ioctls;
>> --
>> 1.9.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v10 02/10] migration: pass MigrationIncomingState* into migration check functions
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 00/10] calculate blocktime for postcopy live migration Alexey Perevalov
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 01/10] userfault: update kernel header for UFFD_FEATURE_* Alexey Perevalov
@ 2017-09-19 16:47 ` Alexey Perevalov
2017-09-20 9:01 ` Juan Quintela
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 03/10] migration: fix hardcoded function name in error report Alexey Perevalov
` (7 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-19 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Perevalov, i.maximets, peterx, eblake, dgilbert, quintela,
heetae82.ahn
That tiny refactoring is necessary to be able to set
UFFD_FEATURE_THREAD_ID while requesting features, and then
to create downtime context in case when kernel supports it.
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
migration/migration.c | 3 ++-
migration/postcopy-ram.c | 10 +++++-----
migration/postcopy-ram.h | 2 +-
migration/savevm.c | 2 +-
4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 959e8ec..e820d47 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -603,6 +603,7 @@ static bool migrate_caps_check(bool *cap_list,
{
MigrationCapabilityStatusList *cap;
bool old_postcopy_cap;
+ MigrationIncomingState *mis = migration_incoming_get_current();
old_postcopy_cap = cap_list[MIGRATION_CAPABILITY_POSTCOPY_RAM];
@@ -636,7 +637,7 @@ static bool migrate_caps_check(bool *cap_list,
* special support.
*/
if (!old_postcopy_cap && runstate_check(RUN_STATE_INMIGRATE) &&
- !postcopy_ram_supported_by_host()) {
+ !postcopy_ram_supported_by_host(mis)) {
/* postcopy_ram_supported_by_host will have emitted a more
* detailed message
*/
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 7a414eb..4350dd0 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -61,7 +61,7 @@ struct PostcopyDiscardState {
#include <sys/eventfd.h>
#include <linux/userfaultfd.h>
-static bool ufd_version_check(int ufd)
+static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
{
struct uffdio_api api_struct;
uint64_t ioctl_mask;
@@ -124,7 +124,7 @@ static int test_ramblock_postcopiable(const char *block_name, void *host_addr,
* normally fine since if the postcopy succeeds it gets turned back on at the
* end.
*/
-bool postcopy_ram_supported_by_host(void)
+bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
{
long pagesize = getpagesize();
int ufd = -1;
@@ -147,7 +147,7 @@ bool postcopy_ram_supported_by_host(void)
}
/* Version and features check */
- if (!ufd_version_check(ufd)) {
+ if (!ufd_version_check(ufd, mis)) {
goto out;
}
@@ -523,7 +523,7 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
* Although the host check already tested the API, we need to
* do the check again as an ABI handshake on the new fd.
*/
- if (!ufd_version_check(mis->userfault_fd)) {
+ if (!ufd_version_check(mis->userfault_fd, mis)) {
return -1;
}
@@ -677,7 +677,7 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
#else
/* No target OS support, stubs just fail */
-bool postcopy_ram_supported_by_host(void)
+bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
{
error_report("%s: No OS support", __func__);
return false;
diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
index 78a3591..77ea0fd 100644
--- a/migration/postcopy-ram.h
+++ b/migration/postcopy-ram.h
@@ -14,7 +14,7 @@
#define QEMU_POSTCOPY_RAM_H
/* Return true if the host supports everything we need to do postcopy-ram */
-bool postcopy_ram_supported_by_host(void);
+bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
/*
* Make all of RAM sensitive to accesses to areas that haven't yet been written
diff --git a/migration/savevm.c b/migration/savevm.c
index 7a55023..6ed6d57 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1352,7 +1352,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
return -1;
}
- if (!postcopy_ram_supported_by_host()) {
+ if (!postcopy_ram_supported_by_host(mis)) {
postcopy_state_set(POSTCOPY_INCOMING_NONE);
return -1;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v10 03/10] migration: fix hardcoded function name in error report
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 00/10] calculate blocktime for postcopy live migration Alexey Perevalov
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 01/10] userfault: update kernel header for UFFD_FEATURE_* Alexey Perevalov
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 02/10] migration: pass MigrationIncomingState* into migration check functions Alexey Perevalov
@ 2017-09-19 16:47 ` Alexey Perevalov
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 04/10] migration: split ufd_version_check onto receive/request features part Alexey Perevalov
` (6 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-19 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Perevalov, i.maximets, peterx, eblake, dgilbert, quintela,
heetae82.ahn
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
migration/postcopy-ram.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 4350dd0..a0e74db 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -69,7 +69,7 @@ static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
api_struct.api = UFFD_API;
api_struct.features = 0;
if (ioctl(ufd, UFFDIO_API, &api_struct)) {
- error_report("postcopy_ram_supported_by_host: UFFDIO_API failed: %s",
+ error_report("%s: UFFDIO_API failed: %s", __func__,
strerror(errno));
return false;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v10 04/10] migration: split ufd_version_check onto receive/request features part
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 00/10] calculate blocktime for postcopy live migration Alexey Perevalov
` (2 preceding siblings ...)
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 03/10] migration: fix hardcoded function name in error report Alexey Perevalov
@ 2017-09-19 16:47 ` Alexey Perevalov
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 05/10] migration: introduce postcopy-blocktime capability Alexey Perevalov
` (5 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-19 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Perevalov, i.maximets, peterx, eblake, dgilbert, quintela,
heetae82.ahn
This modification is necessary for userfault fd features which are
required to be requested from userspace.
UFFD_FEATURE_THREAD_ID is a one of such "on demand" feature, which will
be introduced in the next patch.
QEMU have to use separate userfault file descriptor, due to
userfault context has internal state, and after first call of
ioctl UFFD_API it changes its state to UFFD_STATE_RUNNING (in case of
success), but kernel while handling ioctl UFFD_API expects UFFD_STATE_WAIT_API.
So only one ioctl with UFFD_API is possible per ufd.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
migration/postcopy-ram.c | 94 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 88 insertions(+), 6 deletions(-)
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index a0e74db..bec6c2c 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -61,16 +61,67 @@ struct PostcopyDiscardState {
#include <sys/eventfd.h>
#include <linux/userfaultfd.h>
-static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
+
+/**
+ * receive_ufd_features: check userfault fd features, to request only supported
+ * features in the future.
+ *
+ * Returns: true on success
+ *
+ * __NR_userfaultfd - should be checked before
+ * @features: out parameter will contain uffdio_api.features provided by kernel
+ * in case of success
+ */
+static bool receive_ufd_features(uint64_t *features)
{
- struct uffdio_api api_struct;
- uint64_t ioctl_mask;
+ struct uffdio_api api_struct = {0};
+ int ufd;
+ bool ret = true;
+
+ /* if we are here __NR_userfaultfd should exists */
+ ufd = syscall(__NR_userfaultfd, O_CLOEXEC);
+ if (ufd == -1) {
+ error_report("%s: syscall __NR_userfaultfd failed: %s", __func__,
+ strerror(errno));
+ return false;
+ }
+ /* ask features */
api_struct.api = UFFD_API;
api_struct.features = 0;
if (ioctl(ufd, UFFDIO_API, &api_struct)) {
error_report("%s: UFFDIO_API failed: %s", __func__,
strerror(errno));
+ ret = false;
+ goto release_ufd;
+ }
+
+ *features = api_struct.features;
+
+release_ufd:
+ close(ufd);
+ return ret;
+}
+
+/**
+ * request_ufd_features: this function should be called only once on a newly
+ * opened ufd, subsequent calls will lead to error.
+ *
+ * Returns: true on succes
+ *
+ * @ufd: fd obtained from userfaultfd syscall
+ * @features: bit mask see UFFD_API_FEATURES
+ */
+static bool request_ufd_features(int ufd, uint64_t features)
+{
+ struct uffdio_api api_struct = {0};
+ uint64_t ioctl_mask;
+
+ api_struct.api = UFFD_API;
+ api_struct.features = features;
+ if (ioctl(ufd, UFFDIO_API, &api_struct)) {
+ error_report("%s failed: UFFDIO_API failed: %s", __func__,
+ strerror(errno));
return false;
}
@@ -82,11 +133,42 @@ static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
return false;
}
+ return true;
+}
+
+static bool ufd_check_and_apply(int ufd, MigrationIncomingState *mis)
+{
+ uint64_t asked_features = 0;
+ static uint64_t supported_features;
+
+ /*
+ * it's not possible to
+ * request UFFD_API twice per one fd
+ * userfault fd features is persistent
+ */
+ if (!supported_features) {
+ if (!receive_ufd_features(&supported_features)) {
+ error_report("%s failed", __func__);
+ return false;
+ }
+ }
+
+ /*
+ * request features, even if asked_features is 0, due to
+ * kernel expects UFFD_API before UFFDIO_REGISTER, per
+ * userfault file descriptor
+ */
+ if (!request_ufd_features(ufd, asked_features)) {
+ error_report("%s failed: features %" PRIu64, __func__,
+ asked_features);
+ return false;
+ }
+
if (getpagesize() != ram_pagesize_summary()) {
bool have_hp = false;
/* We've got a huge page */
#ifdef UFFD_FEATURE_MISSING_HUGETLBFS
- have_hp = api_struct.features & UFFD_FEATURE_MISSING_HUGETLBFS;
+ have_hp = supported_features & UFFD_FEATURE_MISSING_HUGETLBFS;
#endif
if (!have_hp) {
error_report("Userfault on this host does not support huge pages");
@@ -147,7 +229,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
}
/* Version and features check */
- if (!ufd_version_check(ufd, mis)) {
+ if (!ufd_check_and_apply(ufd, mis)) {
goto out;
}
@@ -523,7 +605,7 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
* Although the host check already tested the API, we need to
* do the check again as an ABI handshake on the new fd.
*/
- if (!ufd_version_check(mis->userfault_fd, mis)) {
+ if (!ufd_check_and_apply(mis->userfault_fd, mis)) {
return -1;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v10 05/10] migration: introduce postcopy-blocktime capability
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 00/10] calculate blocktime for postcopy live migration Alexey Perevalov
` (3 preceding siblings ...)
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 04/10] migration: split ufd_version_check onto receive/request features part Alexey Perevalov
@ 2017-09-19 16:47 ` Alexey Perevalov
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 06/10] migration: add postcopy blocktime ctx into MigrationIncomingState Alexey Perevalov
` (4 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-19 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Perevalov, i.maximets, peterx, eblake, dgilbert, quintela,
heetae82.ahn
Right now it could be used on destination side to
enable vCPU blocktime calculation for postcopy live migration.
vCPU blocktime - it's time since vCPU thread was put into
interruptible sleep, till memory page was copied and thread awake.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
migration/migration.c | 9 +++++++++
migration/migration.h | 1 +
qapi/migration.json | 5 ++++-
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/migration/migration.c b/migration/migration.c
index e820d47..4f029e8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1380,6 +1380,15 @@ bool migrate_zero_blocks(void)
return s->enabled_capabilities[MIGRATION_CAPABILITY_ZERO_BLOCKS];
}
+bool migrate_postcopy_blocktime(void)
+{
+ MigrationState *s;
+
+ s = migrate_get_current();
+
+ return s->enabled_capabilities[MIGRATION_CAPABILITY_POSTCOPY_BLOCKTIME];
+}
+
bool migrate_use_compression(void)
{
MigrationState *s;
diff --git a/migration/migration.h b/migration/migration.h
index 148c9fa..56bf33c 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -184,6 +184,7 @@ int migrate_compress_level(void);
int migrate_compress_threads(void);
int migrate_decompress_threads(void);
bool migrate_use_events(void);
+bool migrate_postcopy_blocktime(void);
/* Sending on the return path - generic and then for each message type */
void migrate_send_rp_shut(MigrationIncomingState *mis,
diff --git a/qapi/migration.json b/qapi/migration.json
index ee2b3b8..2e4a15d 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -341,12 +341,15 @@
# @return-path: If enabled, migration will use the return path even
# for precopy. (since 2.10)
#
+# @postcopy-blocktime: Calculate downtime for postcopy live migration
+# (since 2.11)
+#
# Since: 1.2
##
{ 'enum': 'MigrationCapability',
'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
- 'block', 'return-path' ] }
+ 'block', 'return-path', 'postcopy-blocktime' ] }
##
# @MigrationCapabilityStatus:
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v10 06/10] migration: add postcopy blocktime ctx into MigrationIncomingState
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 00/10] calculate blocktime for postcopy live migration Alexey Perevalov
` (4 preceding siblings ...)
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 05/10] migration: introduce postcopy-blocktime capability Alexey Perevalov
@ 2017-09-19 16:48 ` Alexey Perevalov
2017-09-21 10:16 ` Dr. David Alan Gilbert
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 07/10] migration: calculate vCPU blocktime on dst side Alexey Perevalov
` (3 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-19 16:48 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Perevalov, i.maximets, peterx, eblake, dgilbert, quintela,
heetae82.ahn
This patch adds request to kernel space for UFFD_FEATURE_THREAD_ID,
in case when this feature is provided by kernel.
PostcopyBlocktimeContext is incapsulated inside postcopy-ram.c,
due to it's postcopy only feature.
Also it defines PostcopyBlocktimeContext's instance live time.
Information from PostcopyBlocktimeContext instance will be provided
much after postcopy migration end, instance of PostcopyBlocktimeContext
will live till QEMU exit, but part of it (vcpu_addr,
page_fault_vcpu_time) used only during calculation, will be released
when postcopy ended or failed.
To enable postcopy blocktime calculation on destination, need to request
proper capabiltiy (Patch for documentation will be at the tail of the patch
set).
As an example following command enable that capability, assume QEMU was
started with
-chardev socket,id=charmonitor,path=/var/lib/migrate-vm-monitor.sock
option to control it
[root@host]#printf "{\"execute\" : \"qmp_capabilities\"}\r\n \
{\"execute\": \"migrate-set-capabilities\" , \"arguments\": {
\"capabilities\": [ { \"capability\": \"postcopy-blocktime\", \"state\":
true } ] } }" | nc -U /var/lib/migrate-vm-monitor.sock
Or just with HMP
(qemu) migrate_set_capability postcopy-blocktime on
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
migration/migration.h | 8 ++++++
migration/postcopy-ram.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+)
diff --git a/migration/migration.h b/migration/migration.h
index 56bf33c..770466b 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -21,6 +21,8 @@
#include "qemu/coroutine_int.h"
#include "hw/qdev.h"
+struct PostcopyBlocktimeContext;
+
/* State for the incoming migration */
struct MigrationIncomingState {
QEMUFile *from_src_file;
@@ -58,6 +60,12 @@ struct MigrationIncomingState {
/* The coroutine we should enter (back) after failover */
Coroutine *migration_incoming_co;
QemuSemaphore colo_incoming_sem;
+
+ /*
+ * PostcopyBlocktimeContext to keep information for postcopy
+ * live migration, to calculate vCPU block time
+ * */
+ struct PostcopyBlocktimeContext *blocktime_ctx;
};
MigrationIncomingState *migration_incoming_get_current(void);
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index bec6c2c..cc78981 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -61,6 +61,58 @@ struct PostcopyDiscardState {
#include <sys/eventfd.h>
#include <linux/userfaultfd.h>
+typedef struct PostcopyBlocktimeContext {
+ /* time when page fault initiated per vCPU */
+ int64_t *page_fault_vcpu_time;
+ /* page address per vCPU */
+ uint64_t *vcpu_addr;
+ int64_t total_blocktime;
+ /* blocktime per vCPU */
+ int64_t *vcpu_blocktime;
+ /* point in time when last page fault was initiated */
+ int64_t last_begin;
+ /* number of vCPU are suspended */
+ int smp_cpus_down;
+
+ /*
+ * Handler for exit event, necessary for
+ * releasing whole blocktime_ctx
+ */
+ Notifier exit_notifier;
+ /*
+ * Handler for postcopy event, necessary for
+ * releasing unnecessary part of blocktime_ctx
+ */
+ Notifier postcopy_notifier;
+} PostcopyBlocktimeContext;
+
+static void destroy_blocktime_context(struct PostcopyBlocktimeContext *ctx)
+{
+ g_free(ctx->page_fault_vcpu_time);
+ g_free(ctx->vcpu_addr);
+ g_free(ctx->vcpu_blocktime);
+ g_free(ctx);
+}
+
+static void migration_exit_cb(Notifier *n, void *data)
+{
+ PostcopyBlocktimeContext *ctx = container_of(n, PostcopyBlocktimeContext,
+ exit_notifier);
+ destroy_blocktime_context(ctx);
+}
+
+static struct PostcopyBlocktimeContext *blocktime_context_new(void)
+{
+ PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1);
+ ctx->page_fault_vcpu_time = g_new0(int64_t, smp_cpus);
+ ctx->vcpu_addr = g_new0(uint64_t, smp_cpus);
+ ctx->vcpu_blocktime = g_new0(int64_t, smp_cpus);
+
+ ctx->exit_notifier.notify = migration_exit_cb;
+ qemu_add_exit_notifier(&ctx->exit_notifier);
+ add_migration_state_change_notifier(&ctx->postcopy_notifier);
+ return ctx;
+}
/**
* receive_ufd_features: check userfault fd features, to request only supported
@@ -153,6 +205,19 @@ static bool ufd_check_and_apply(int ufd, MigrationIncomingState *mis)
}
}
+#ifdef UFFD_FEATURE_THREAD_ID
+ if (migrate_postcopy_blocktime() && mis &&
+ UFFD_FEATURE_THREAD_ID & supported_features) {
+ /* kernel supports that feature */
+ /* don't create blocktime_context if it exists */
+ if (!mis->blocktime_ctx) {
+ mis->blocktime_ctx = blocktime_context_new();
+ }
+
+ asked_features |= UFFD_FEATURE_THREAD_ID;
+ }
+#endif
+
/*
* request features, even if asked_features is 0, due to
* kernel expects UFFD_API before UFFDIO_REGISTER, per
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 06/10] migration: add postcopy blocktime ctx into MigrationIncomingState
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 06/10] migration: add postcopy blocktime ctx into MigrationIncomingState Alexey Perevalov
@ 2017-09-21 10:16 ` Dr. David Alan Gilbert
2017-09-21 11:27 ` Alexey Perevalov
0 siblings, 1 reply; 27+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-21 10:16 UTC (permalink / raw)
To: Alexey Perevalov
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> This patch adds request to kernel space for UFFD_FEATURE_THREAD_ID,
> in case when this feature is provided by kernel.
>
> PostcopyBlocktimeContext is incapsulated inside postcopy-ram.c,
> due to it's postcopy only feature.
> Also it defines PostcopyBlocktimeContext's instance live time.
> Information from PostcopyBlocktimeContext instance will be provided
> much after postcopy migration end, instance of PostcopyBlocktimeContext
> will live till QEMU exit, but part of it (vcpu_addr,
> page_fault_vcpu_time) used only during calculation, will be released
> when postcopy ended or failed.
>
> To enable postcopy blocktime calculation on destination, need to request
> proper capabiltiy (Patch for documentation will be at the tail of the patch
> set).
>
> As an example following command enable that capability, assume QEMU was
> started with
> -chardev socket,id=charmonitor,path=/var/lib/migrate-vm-monitor.sock
> option to control it
>
> [root@host]#printf "{\"execute\" : \"qmp_capabilities\"}\r\n \
> {\"execute\": \"migrate-set-capabilities\" , \"arguments\": {
> \"capabilities\": [ { \"capability\": \"postcopy-blocktime\", \"state\":
> true } ] } }" | nc -U /var/lib/migrate-vm-monitor.sock
>
> Or just with HMP
> (qemu) migrate_set_capability postcopy-blocktime on
>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
> migration/migration.h | 8 ++++++
> migration/postcopy-ram.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 73 insertions(+)
>
> diff --git a/migration/migration.h b/migration/migration.h
> index 56bf33c..770466b 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -21,6 +21,8 @@
> #include "qemu/coroutine_int.h"
> #include "hw/qdev.h"
>
> +struct PostcopyBlocktimeContext;
> +
> /* State for the incoming migration */
> struct MigrationIncomingState {
> QEMUFile *from_src_file;
> @@ -58,6 +60,12 @@ struct MigrationIncomingState {
> /* The coroutine we should enter (back) after failover */
> Coroutine *migration_incoming_co;
> QemuSemaphore colo_incoming_sem;
> +
> + /*
> + * PostcopyBlocktimeContext to keep information for postcopy
> + * live migration, to calculate vCPU block time
> + * */
> + struct PostcopyBlocktimeContext *blocktime_ctx;
> };
>
> MigrationIncomingState *migration_incoming_get_current(void);
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index bec6c2c..cc78981 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -61,6 +61,58 @@ struct PostcopyDiscardState {
> #include <sys/eventfd.h>
> #include <linux/userfaultfd.h>
>
> +typedef struct PostcopyBlocktimeContext {
> + /* time when page fault initiated per vCPU */
> + int64_t *page_fault_vcpu_time;
> + /* page address per vCPU */
> + uint64_t *vcpu_addr;
> + int64_t total_blocktime;
> + /* blocktime per vCPU */
> + int64_t *vcpu_blocktime;
> + /* point in time when last page fault was initiated */
> + int64_t last_begin;
> + /* number of vCPU are suspended */
> + int smp_cpus_down;
> +
> + /*
> + * Handler for exit event, necessary for
> + * releasing whole blocktime_ctx
> + */
> + Notifier exit_notifier;
> + /*
> + * Handler for postcopy event, necessary for
> + * releasing unnecessary part of blocktime_ctx
> + */
> + Notifier postcopy_notifier;
Is this actually used? It's just that...
> +} PostcopyBlocktimeContext;
> +
> +static void destroy_blocktime_context(struct PostcopyBlocktimeContext *ctx)
> +{
> + g_free(ctx->page_fault_vcpu_time);
> + g_free(ctx->vcpu_addr);
> + g_free(ctx->vcpu_blocktime);
> + g_free(ctx);
> +}
> +
> +static void migration_exit_cb(Notifier *n, void *data)
> +{
> + PostcopyBlocktimeContext *ctx = container_of(n, PostcopyBlocktimeContext,
> + exit_notifier);
> + destroy_blocktime_context(ctx);
> +}
> +
> +static struct PostcopyBlocktimeContext *blocktime_context_new(void)
> +{
> + PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1);
> + ctx->page_fault_vcpu_time = g_new0(int64_t, smp_cpus);
> + ctx->vcpu_addr = g_new0(uint64_t, smp_cpus);
> + ctx->vcpu_blocktime = g_new0(int64_t, smp_cpus);
> +
> + ctx->exit_notifier.notify = migration_exit_cb;
> + qemu_add_exit_notifier(&ctx->exit_notifier);
> + add_migration_state_change_notifier(&ctx->postcopy_notifier);
Patch 7 removes that line, and I don't see what puts it back;
and this line doesn't actually set up ctx->postcopy_notifier.
Other than that, it looks OK.
Dave
> + return ctx;
> +}
>
> /**
> * receive_ufd_features: check userfault fd features, to request only supported
> @@ -153,6 +205,19 @@ static bool ufd_check_and_apply(int ufd, MigrationIncomingState *mis)
> }
> }
>
> +#ifdef UFFD_FEATURE_THREAD_ID
> + if (migrate_postcopy_blocktime() && mis &&
> + UFFD_FEATURE_THREAD_ID & supported_features) {
> + /* kernel supports that feature */
> + /* don't create blocktime_context if it exists */
> + if (!mis->blocktime_ctx) {
> + mis->blocktime_ctx = blocktime_context_new();
> + }
> +
> + asked_features |= UFFD_FEATURE_THREAD_ID;
> + }
> +#endif
> +
> /*
> * request features, even if asked_features is 0, due to
> * kernel expects UFFD_API before UFFDIO_REGISTER, per
> --
> 1.9.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 06/10] migration: add postcopy blocktime ctx into MigrationIncomingState
2017-09-21 10:16 ` Dr. David Alan Gilbert
@ 2017-09-21 11:27 ` Alexey Perevalov
0 siblings, 0 replies; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-21 11:27 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
On 09/21/2017 01:16 PM, Dr. David Alan Gilbert wrote:
> * Alexey Perevalov (a.perevalov@samsung.com) wrote:
>> This patch adds request to kernel space for UFFD_FEATURE_THREAD_ID,
>> in case when this feature is provided by kernel.
>>
>> PostcopyBlocktimeContext is incapsulated inside postcopy-ram.c,
>> due to it's postcopy only feature.
>> Also it defines PostcopyBlocktimeContext's instance live time.
>> Information from PostcopyBlocktimeContext instance will be provided
>> much after postcopy migration end, instance of PostcopyBlocktimeContext
>> will live till QEMU exit, but part of it (vcpu_addr,
>> page_fault_vcpu_time) used only during calculation, will be released
>> when postcopy ended or failed.
>>
>> To enable postcopy blocktime calculation on destination, need to request
>> proper capabiltiy (Patch for documentation will be at the tail of the patch
>> set).
>>
>> As an example following command enable that capability, assume QEMU was
>> started with
>> -chardev socket,id=charmonitor,path=/var/lib/migrate-vm-monitor.sock
>> option to control it
>>
>> [root@host]#printf "{\"execute\" : \"qmp_capabilities\"}\r\n \
>> {\"execute\": \"migrate-set-capabilities\" , \"arguments\": {
>> \"capabilities\": [ { \"capability\": \"postcopy-blocktime\", \"state\":
>> true } ] } }" | nc -U /var/lib/migrate-vm-monitor.sock
>>
>> Or just with HMP
>> (qemu) migrate_set_capability postcopy-blocktime on
>>
>> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
>> ---
>> migration/migration.h | 8 ++++++
>> migration/postcopy-ram.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 73 insertions(+)
>>
>> diff --git a/migration/migration.h b/migration/migration.h
>> index 56bf33c..770466b 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -21,6 +21,8 @@
>> #include "qemu/coroutine_int.h"
>> #include "hw/qdev.h"
>>
>> +struct PostcopyBlocktimeContext;
>> +
>> /* State for the incoming migration */
>> struct MigrationIncomingState {
>> QEMUFile *from_src_file;
>> @@ -58,6 +60,12 @@ struct MigrationIncomingState {
>> /* The coroutine we should enter (back) after failover */
>> Coroutine *migration_incoming_co;
>> QemuSemaphore colo_incoming_sem;
>> +
>> + /*
>> + * PostcopyBlocktimeContext to keep information for postcopy
>> + * live migration, to calculate vCPU block time
>> + * */
>> + struct PostcopyBlocktimeContext *blocktime_ctx;
>> };
>>
>> MigrationIncomingState *migration_incoming_get_current(void);
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index bec6c2c..cc78981 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -61,6 +61,58 @@ struct PostcopyDiscardState {
>> #include <sys/eventfd.h>
>> #include <linux/userfaultfd.h>
>>
>> +typedef struct PostcopyBlocktimeContext {
>> + /* time when page fault initiated per vCPU */
>> + int64_t *page_fault_vcpu_time;
>> + /* page address per vCPU */
>> + uint64_t *vcpu_addr;
>> + int64_t total_blocktime;
>> + /* blocktime per vCPU */
>> + int64_t *vcpu_blocktime;
>> + /* point in time when last page fault was initiated */
>> + int64_t last_begin;
>> + /* number of vCPU are suspended */
>> + int smp_cpus_down;
>> +
>> + /*
>> + * Handler for exit event, necessary for
>> + * releasing whole blocktime_ctx
>> + */
>> + Notifier exit_notifier;
>> + /*
>> + * Handler for postcopy event, necessary for
>> + * releasing unnecessary part of blocktime_ctx
>> + */
>> + Notifier postcopy_notifier;
> Is this actually used? It's just that...
>
>> +} PostcopyBlocktimeContext;
>> +
>> +static void destroy_blocktime_context(struct PostcopyBlocktimeContext *ctx)
>> +{
>> + g_free(ctx->page_fault_vcpu_time);
>> + g_free(ctx->vcpu_addr);
>> + g_free(ctx->vcpu_blocktime);
>> + g_free(ctx);
>> +}
>> +
>> +static void migration_exit_cb(Notifier *n, void *data)
>> +{
>> + PostcopyBlocktimeContext *ctx = container_of(n, PostcopyBlocktimeContext,
>> + exit_notifier);
>> + destroy_blocktime_context(ctx);
>> +}
>> +
>> +static struct PostcopyBlocktimeContext *blocktime_context_new(void)
>> +{
>> + PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1);
>> + ctx->page_fault_vcpu_time = g_new0(int64_t, smp_cpus);
>> + ctx->vcpu_addr = g_new0(uint64_t, smp_cpus);
>> + ctx->vcpu_blocktime = g_new0(int64_t, smp_cpus);
>> +
>> + ctx->exit_notifier.notify = migration_exit_cb;
>> + qemu_add_exit_notifier(&ctx->exit_notifier);
>> + add_migration_state_change_notifier(&ctx->postcopy_notifier);
> Patch 7 removes that line, and I don't see what puts it back;
> and this line doesn't actually set up ctx->postcopy_notifier.
>
> Other than that, it looks OK.
Thank you, I really changed my mind, and decided to keep
blocktime context (and all calculated values) till the
stop of VM, but not till the end of migration.
--
Best regards,
Alexey Perevalov
>
> Dave
>
>> + return ctx;
>> +}
>>
>> /**
>> * receive_ufd_features: check userfault fd features, to request only supported
>> @@ -153,6 +205,19 @@ static bool ufd_check_and_apply(int ufd, MigrationIncomingState *mis)
>> }
>> }
>>
>> +#ifdef UFFD_FEATURE_THREAD_ID
>> + if (migrate_postcopy_blocktime() && mis &&
>> + UFFD_FEATURE_THREAD_ID & supported_features) {
>> + /* kernel supports that feature */
>> + /* don't create blocktime_context if it exists */
>> + if (!mis->blocktime_ctx) {
>> + mis->blocktime_ctx = blocktime_context_new();
>> + }
>> +
>> + asked_features |= UFFD_FEATURE_THREAD_ID;
>> + }
>> +#endif
>> +
>> /*
>> * request features, even if asked_features is 0, due to
>> * kernel expects UFFD_API before UFFDIO_REGISTER, per
>> --
>> 1.9.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v10 07/10] migration: calculate vCPU blocktime on dst side
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 00/10] calculate blocktime for postcopy live migration Alexey Perevalov
` (5 preceding siblings ...)
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 06/10] migration: add postcopy blocktime ctx into MigrationIncomingState Alexey Perevalov
@ 2017-09-19 16:48 ` Alexey Perevalov
2017-09-21 11:57 ` Dr. David Alan Gilbert
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 08/10] migration: postcopy_blocktime documentation Alexey Perevalov
` (2 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-19 16:48 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Perevalov, i.maximets, peterx, eblake, dgilbert, quintela,
heetae82.ahn
This patch provides blocktime calculation per vCPU,
as a summary and as a overlapped value for all vCPUs.
This approach was suggested by Peter Xu, as an improvements of
previous approch where QEMU kept tree with faulted page address and cpus bitmask
in it. Now QEMU is keeping array with faulted page address as value and vCPU
as index. It helps to find proper vCPU at UFFD_COPY time. Also it keeps
list for blocktime per vCPU (could be traced with page_fault_addr)
Blocktime will not calculated if postcopy_blocktime field of
MigrationIncomingState wasn't initialized.
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
migration/postcopy-ram.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++-
migration/trace-events | 5 +-
2 files changed, 140 insertions(+), 3 deletions(-)
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index cc78981..9a5133f 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -110,7 +110,6 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
ctx->exit_notifier.notify = migration_exit_cb;
qemu_add_exit_notifier(&ctx->exit_notifier);
- add_migration_state_change_notifier(&ctx->postcopy_notifier);
return ctx;
}
@@ -559,6 +558,136 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,
return 0;
}
+static int get_mem_fault_cpu_index(uint32_t pid)
+{
+ CPUState *cpu_iter;
+
+ CPU_FOREACH(cpu_iter) {
+ if (cpu_iter->thread_id == pid) {
+ trace_get_mem_fault_cpu_index(cpu_iter->cpu_index, pid);
+ return cpu_iter->cpu_index;
+ }
+ }
+ trace_get_mem_fault_cpu_index(-1, pid);
+ return -1;
+}
+
+/*
+ * This function is being called when pagefault occurs. It
+ * tracks down vCPU blocking time.
+ *
+ * @addr: faulted host virtual address
+ * @ptid: faulted process thread id
+ * @rb: ramblock appropriate to addr
+ */
+static void mark_postcopy_blocktime_begin(uint64_t addr, uint32_t ptid,
+ RAMBlock *rb)
+{
+ int cpu, already_received;
+ MigrationIncomingState *mis = migration_incoming_get_current();
+ PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
+ int64_t now_ms;
+
+ if (!dc || ptid == 0) {
+ return;
+ }
+ cpu = get_mem_fault_cpu_index(ptid);
+ if (cpu < 0) {
+ return;
+ }
+
+ now_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+ if (dc->vcpu_addr[cpu] == 0) {
+ atomic_inc(&dc->smp_cpus_down);
+ }
+
+ atomic_xchg__nocheck(&dc->vcpu_addr[cpu], addr);
+ atomic_xchg__nocheck(&dc->last_begin, now_ms);
+ atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], now_ms);
+
+ already_received = ramblock_recv_bitmap_test(rb, (void *)addr);
+ if (already_received) {
+ atomic_xchg__nocheck(&dc->vcpu_addr[cpu], 0);
+ atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], 0);
+ atomic_sub(&dc->smp_cpus_down, 1);
+ }
+ trace_mark_postcopy_blocktime_begin(addr, dc, dc->page_fault_vcpu_time[cpu],
+ cpu, already_received);
+}
+
+/*
+ * This function just provide calculated blocktime per cpu and trace it.
+ * Total blocktime is calculated in mark_postcopy_blocktime_end.
+ *
+ *
+ * Assume we have 3 CPU
+ *
+ * S1 E1 S1 E1
+ * -----***********------------xxx***************------------------------> CPU1
+ *
+ * S2 E2
+ * ------------****************xxx---------------------------------------> CPU2
+ *
+ * S3 E3
+ * ------------------------****xxx********-------------------------------> CPU3
+ *
+ * We have sequence S1,S2,E1,S3,S1,E2,E3,E1
+ * S2,E1 - doesn't match condition due to sequence S1,S2,E1 doesn't include CPU3
+ * S3,S1,E2 - sequence includes all CPUs, in this case overlap will be S1,E2 -
+ * it's a part of total blocktime.
+ * S1 - here is last_begin
+ * Legend of the picture is following:
+ * * - means blocktime per vCPU
+ * x - means overlapped blocktime (total blocktime)
+ *
+ * @addr: host virtual address
+ */
+static void mark_postcopy_blocktime_end(uint64_t addr)
+{
+ MigrationIncomingState *mis = migration_incoming_get_current();
+ PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
+ int i, affected_cpu = 0;
+ int64_t now_ms;
+ bool vcpu_total_blocktime = false;
+
+ if (!dc) {
+ return;
+ }
+
+ now_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+
+ /* lookup cpu, to clear it,
+ * that algorithm looks straighforward, but it's not
+ * optimal, more optimal algorithm is keeping tree or hash
+ * where key is address value is a list of */
+ for (i = 0; i < smp_cpus; i++) {
+ uint64_t vcpu_blocktime = 0;
+ if (atomic_fetch_add(&dc->vcpu_addr[i], 0) != addr) {
+ continue;
+ }
+ atomic_xchg__nocheck(&dc->vcpu_addr[i], 0);
+ vcpu_blocktime = now_ms -
+ atomic_fetch_add(&dc->page_fault_vcpu_time[i], 0);
+ affected_cpu += 1;
+ /* we need to know is that mark_postcopy_end was due to
+ * faulted page, another possible case it's prefetched
+ * page and in that case we shouldn't be here */
+ if (!vcpu_total_blocktime &&
+ atomic_fetch_add(&dc->smp_cpus_down, 0) == smp_cpus) {
+ vcpu_total_blocktime = true;
+ }
+ /* continue cycle, due to one page could affect several vCPUs */
+ dc->vcpu_blocktime[i] += vcpu_blocktime;
+ }
+
+ atomic_sub(&dc->smp_cpus_down, affected_cpu);
+ if (vcpu_total_blocktime) {
+ dc->total_blocktime += now_ms - atomic_fetch_add(&dc->last_begin, 0);
+ }
+ trace_mark_postcopy_blocktime_end(addr, dc, dc->total_blocktime,
+ affected_cpu);
+}
+
/*
* Handle faults detected by the USERFAULT markings
*/
@@ -636,8 +765,11 @@ static void *postcopy_ram_fault_thread(void *opaque)
rb_offset &= ~(qemu_ram_pagesize(rb) - 1);
trace_postcopy_ram_fault_thread_request(msg.arg.pagefault.address,
qemu_ram_get_idstr(rb),
- rb_offset);
+ rb_offset,
+ msg.arg.pagefault.feat.ptid);
+ mark_postcopy_blocktime_begin((uintptr_t)(msg.arg.pagefault.address),
+ msg.arg.pagefault.feat.ptid, rb);
/*
* Send the request to the source - we want to request one
* of our host page sizes (which is >= TPS)
@@ -727,6 +859,8 @@ static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
if (!ret) {
ramblock_recv_bitmap_set_range(rb, host_addr,
pagesize / qemu_target_page_size());
+ mark_postcopy_blocktime_end((uint64_t)(uintptr_t)host_addr);
+
}
return ret;
}
diff --git a/migration/trace-events b/migration/trace-events
index d2910a6..01f30fe 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -114,6 +114,8 @@ process_incoming_migration_co_end(int ret, int ps) "ret=%d postcopy-state=%d"
process_incoming_migration_co_postcopy_end_main(void) ""
migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"
migration_set_outgoing_channel(void *ioc, const char *ioctype, const char *hostname) "ioc=%p ioctype=%s hostname=%s"
+mark_postcopy_blocktime_begin(uint64_t addr, void *dd, int64_t time, int cpu, int received) "addr: 0x%" PRIx64 ", dd: %p, time: %" PRId64 ", cpu: %d, already_received: %d"
+mark_postcopy_blocktime_end(uint64_t addr, void *dd, int64_t time, int affected_cpu) "addr: 0x%" PRIx64 ", dd: %p, time: %" PRId64 ", affected_cpu: %d"
# migration/rdma.c
qemu_rdma_accept_incoming_migration(void) ""
@@ -190,7 +192,7 @@ postcopy_ram_enable_notify(void) ""
postcopy_ram_fault_thread_entry(void) ""
postcopy_ram_fault_thread_exit(void) ""
postcopy_ram_fault_thread_quit(void) ""
-postcopy_ram_fault_thread_request(uint64_t hostaddr, const char *ramblock, size_t offset) "Request for HVA=0x%" PRIx64 " rb=%s offset=0x%zx"
+postcopy_ram_fault_thread_request(uint64_t hostaddr, const char *ramblock, size_t offset, uint32_t pid) "Request for HVA=0x%" PRIx64 " rb=%s offset=0x%zx pid=%u"
postcopy_ram_incoming_cleanup_closeuf(void) ""
postcopy_ram_incoming_cleanup_entry(void) ""
postcopy_ram_incoming_cleanup_exit(void) ""
@@ -199,6 +201,7 @@ save_xbzrle_page_skipping(void) ""
save_xbzrle_page_overflow(void) ""
ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations"
ram_load_complete(int ret, uint64_t seq_iter) "exit_code %d seq iteration %" PRIu64
+get_mem_fault_cpu_index(int cpu, uint32_t pid) "cpu: %d, pid: %u"
# migration/exec.c
migration_exec_outgoing(const char *cmd) "cmd=%s"
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 07/10] migration: calculate vCPU blocktime on dst side
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 07/10] migration: calculate vCPU blocktime on dst side Alexey Perevalov
@ 2017-09-21 11:57 ` Dr. David Alan Gilbert
2017-09-22 8:47 ` Alexey Perevalov
2017-09-28 8:01 ` Alexey Perevalov
0 siblings, 2 replies; 27+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-21 11:57 UTC (permalink / raw)
To: Alexey Perevalov
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> This patch provides blocktime calculation per vCPU,
> as a summary and as a overlapped value for all vCPUs.
>
> This approach was suggested by Peter Xu, as an improvements of
> previous approch where QEMU kept tree with faulted page address and cpus bitmask
> in it. Now QEMU is keeping array with faulted page address as value and vCPU
> as index. It helps to find proper vCPU at UFFD_COPY time. Also it keeps
> list for blocktime per vCPU (could be traced with page_fault_addr)
>
> Blocktime will not calculated if postcopy_blocktime field of
> MigrationIncomingState wasn't initialized.
>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
> migration/postcopy-ram.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++-
> migration/trace-events | 5 +-
> 2 files changed, 140 insertions(+), 3 deletions(-)
>
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index cc78981..9a5133f 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -110,7 +110,6 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
>
> ctx->exit_notifier.notify = migration_exit_cb;
> qemu_add_exit_notifier(&ctx->exit_notifier);
> - add_migration_state_change_notifier(&ctx->postcopy_notifier);
> return ctx;
> }
>
> @@ -559,6 +558,136 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,
> return 0;
> }
>
> +static int get_mem_fault_cpu_index(uint32_t pid)
> +{
> + CPUState *cpu_iter;
> +
> + CPU_FOREACH(cpu_iter) {
> + if (cpu_iter->thread_id == pid) {
> + trace_get_mem_fault_cpu_index(cpu_iter->cpu_index, pid);
> + return cpu_iter->cpu_index;
> + }
> + }
> + trace_get_mem_fault_cpu_index(-1, pid);
> + return -1;
> +}
> +
> +/*
> + * This function is being called when pagefault occurs. It
> + * tracks down vCPU blocking time.
> + *
> + * @addr: faulted host virtual address
> + * @ptid: faulted process thread id
> + * @rb: ramblock appropriate to addr
> + */
> +static void mark_postcopy_blocktime_begin(uint64_t addr, uint32_t ptid,
> + RAMBlock *rb)
> +{
> + int cpu, already_received;
> + MigrationIncomingState *mis = migration_incoming_get_current();
> + PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
> + int64_t now_ms;
> +
> + if (!dc || ptid == 0) {
> + return;
> + }
> + cpu = get_mem_fault_cpu_index(ptid);
> + if (cpu < 0) {
> + return;
> + }
> +
> + now_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> + if (dc->vcpu_addr[cpu] == 0) {
> + atomic_inc(&dc->smp_cpus_down);
> + }
> +
> + atomic_xchg__nocheck(&dc->vcpu_addr[cpu], addr);
> + atomic_xchg__nocheck(&dc->last_begin, now_ms);
> + atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], now_ms);
> +
> + already_received = ramblock_recv_bitmap_test(rb, (void *)addr);
> + if (already_received) {
> + atomic_xchg__nocheck(&dc->vcpu_addr[cpu], 0);
> + atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], 0);
> + atomic_sub(&dc->smp_cpus_down, 1);
> + }
> + trace_mark_postcopy_blocktime_begin(addr, dc, dc->page_fault_vcpu_time[cpu],
> + cpu, already_received);
> +}
> +
> +/*
> + * This function just provide calculated blocktime per cpu and trace it.
> + * Total blocktime is calculated in mark_postcopy_blocktime_end.
> + *
> + *
> + * Assume we have 3 CPU
> + *
> + * S1 E1 S1 E1
> + * -----***********------------xxx***************------------------------> CPU1
> + *
> + * S2 E2
> + * ------------****************xxx---------------------------------------> CPU2
> + *
> + * S3 E3
> + * ------------------------****xxx********-------------------------------> CPU3
> + *
> + * We have sequence S1,S2,E1,S3,S1,E2,E3,E1
> + * S2,E1 - doesn't match condition due to sequence S1,S2,E1 doesn't include CPU3
> + * S3,S1,E2 - sequence includes all CPUs, in this case overlap will be S1,E2 -
> + * it's a part of total blocktime.
> + * S1 - here is last_begin
> + * Legend of the picture is following:
> + * * - means blocktime per vCPU
> + * x - means overlapped blocktime (total blocktime)
> + *
> + * @addr: host virtual address
> + */
> +static void mark_postcopy_blocktime_end(uint64_t addr)
> +{
> + MigrationIncomingState *mis = migration_incoming_get_current();
> + PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
> + int i, affected_cpu = 0;
> + int64_t now_ms;
> + bool vcpu_total_blocktime = false;
> +
> + if (!dc) {
> + return;
> + }
> +
> + now_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +
> + /* lookup cpu, to clear it,
> + * that algorithm looks straighforward, but it's not
> + * optimal, more optimal algorithm is keeping tree or hash
> + * where key is address value is a list of */
> + for (i = 0; i < smp_cpus; i++) {
> + uint64_t vcpu_blocktime = 0;
> + if (atomic_fetch_add(&dc->vcpu_addr[i], 0) != addr) {
> + continue;
> + }
> + atomic_xchg__nocheck(&dc->vcpu_addr[i], 0);
> + vcpu_blocktime = now_ms -
> + atomic_fetch_add(&dc->page_fault_vcpu_time[i], 0);
> + affected_cpu += 1;
> + /* we need to know is that mark_postcopy_end was due to
> + * faulted page, another possible case it's prefetched
> + * page and in that case we shouldn't be here */
> + if (!vcpu_total_blocktime &&
> + atomic_fetch_add(&dc->smp_cpus_down, 0) == smp_cpus) {
> + vcpu_total_blocktime = true;
> + }
> + /* continue cycle, due to one page could affect several vCPUs */
> + dc->vcpu_blocktime[i] += vcpu_blocktime;
> + }
Unfortunately this still isn't thread safe; consider the code in
mark_postcopy_blocktime_begin is:
1 check vcpu_addr
2 write vcpu_addr
3 write last_begin
4 write vcpu_time
5 smp_cpus_down++
6 already_received:
7 write addr = 0
8 write vcpu_time = 0
9 smp_cpus_down--
and this code is:
a check vcpu_addr
b write vcpu_addr
c read vcpu_time
d read smp_cpus_down
e dec smp_cpus_down
if (a) happens after (2) but before (3), (c) and (d) can also
happen before (3), and so you end up reading a bogus
vcpu_time.
This is tricky to get right; if you changed the source to do:
1 check vcpu_addr
3 write last_begin
4 write vcpu_time
5 smp_cpus_down++
2 write vcpu_addr
6 already_received:
7 write addr = 0
8 write vcpu_time = 0
9 smp_cpus_down--
I think it's safer; if you read a good vcpu_addr you know
that the vcpu_time has already been written.
However, can this check (a) happen between the new (2) and (7) ?
It's slim but I think possibly; on the receiving side we've
just set the bitmap flag to say received - if a fault comes
in at about the same time then we could end up with
1,3,4,5,2 ab 6 7 8 9 cde
So again we end up reading a bogus vcpu_time and double decrement
smp_cpus_down.
So I think we have to have:
a' read vcpu_addr
b' read vcpu_time
c' if vcpu_addr == addr && vcpu_time != 0 ...
d' clear vcpu_addr
e' read/dec smp_cpus_down
You should comment to say where the order is important as well;
because we'll never remember this - it's hairy!
(Better suggestions welcome)
Dave
> + atomic_sub(&dc->smp_cpus_down, affected_cpu);
> + if (vcpu_total_blocktime) {
> + dc->total_blocktime += now_ms - atomic_fetch_add(&dc->last_begin, 0);
> + }
> + trace_mark_postcopy_blocktime_end(addr, dc, dc->total_blocktime,
> + affected_cpu);
> +}
> +
> /*
> * Handle faults detected by the USERFAULT markings
> */
> @@ -636,8 +765,11 @@ static void *postcopy_ram_fault_thread(void *opaque)
> rb_offset &= ~(qemu_ram_pagesize(rb) - 1);
> trace_postcopy_ram_fault_thread_request(msg.arg.pagefault.address,
> qemu_ram_get_idstr(rb),
> - rb_offset);
> + rb_offset,
> + msg.arg.pagefault.feat.ptid);
>
> + mark_postcopy_blocktime_begin((uintptr_t)(msg.arg.pagefault.address),
> + msg.arg.pagefault.feat.ptid, rb);
> /*
> * Send the request to the source - we want to request one
> * of our host page sizes (which is >= TPS)
> @@ -727,6 +859,8 @@ static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
> if (!ret) {
> ramblock_recv_bitmap_set_range(rb, host_addr,
> pagesize / qemu_target_page_size());
> + mark_postcopy_blocktime_end((uint64_t)(uintptr_t)host_addr);
> +
> }
> return ret;
> }
> diff --git a/migration/trace-events b/migration/trace-events
> index d2910a6..01f30fe 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -114,6 +114,8 @@ process_incoming_migration_co_end(int ret, int ps) "ret=%d postcopy-state=%d"
> process_incoming_migration_co_postcopy_end_main(void) ""
> migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"
> migration_set_outgoing_channel(void *ioc, const char *ioctype, const char *hostname) "ioc=%p ioctype=%s hostname=%s"
> +mark_postcopy_blocktime_begin(uint64_t addr, void *dd, int64_t time, int cpu, int received) "addr: 0x%" PRIx64 ", dd: %p, time: %" PRId64 ", cpu: %d, already_received: %d"
> +mark_postcopy_blocktime_end(uint64_t addr, void *dd, int64_t time, int affected_cpu) "addr: 0x%" PRIx64 ", dd: %p, time: %" PRId64 ", affected_cpu: %d"
>
> # migration/rdma.c
> qemu_rdma_accept_incoming_migration(void) ""
> @@ -190,7 +192,7 @@ postcopy_ram_enable_notify(void) ""
> postcopy_ram_fault_thread_entry(void) ""
> postcopy_ram_fault_thread_exit(void) ""
> postcopy_ram_fault_thread_quit(void) ""
> -postcopy_ram_fault_thread_request(uint64_t hostaddr, const char *ramblock, size_t offset) "Request for HVA=0x%" PRIx64 " rb=%s offset=0x%zx"
> +postcopy_ram_fault_thread_request(uint64_t hostaddr, const char *ramblock, size_t offset, uint32_t pid) "Request for HVA=0x%" PRIx64 " rb=%s offset=0x%zx pid=%u"
> postcopy_ram_incoming_cleanup_closeuf(void) ""
> postcopy_ram_incoming_cleanup_entry(void) ""
> postcopy_ram_incoming_cleanup_exit(void) ""
> @@ -199,6 +201,7 @@ save_xbzrle_page_skipping(void) ""
> save_xbzrle_page_overflow(void) ""
> ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations"
> ram_load_complete(int ret, uint64_t seq_iter) "exit_code %d seq iteration %" PRIu64
> +get_mem_fault_cpu_index(int cpu, uint32_t pid) "cpu: %d, pid: %u"
>
> # migration/exec.c
> migration_exec_outgoing(const char *cmd) "cmd=%s"
> --
> 1.9.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 07/10] migration: calculate vCPU blocktime on dst side
2017-09-21 11:57 ` Dr. David Alan Gilbert
@ 2017-09-22 8:47 ` Alexey Perevalov
2017-09-28 8:01 ` Alexey Perevalov
1 sibling, 0 replies; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-22 8:47 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
On 09/21/2017 02:57 PM, Dr. David Alan Gilbert wrote:
> * Alexey Perevalov (a.perevalov@samsung.com) wrote:
>> This patch provides blocktime calculation per vCPU,
>> as a summary and as a overlapped value for all vCPUs.
>>
>> This approach was suggested by Peter Xu, as an improvements of
>> previous approch where QEMU kept tree with faulted page address and cpus bitmask
>> in it. Now QEMU is keeping array with faulted page address as value and vCPU
>> as index. It helps to find proper vCPU at UFFD_COPY time. Also it keeps
>> list for blocktime per vCPU (could be traced with page_fault_addr)
>>
>> Blocktime will not calculated if postcopy_blocktime field of
>> MigrationIncomingState wasn't initialized.
>>
>> Signed-off-by: Alexey Perevalov<a.perevalov@samsung.com>
>> ---
>> migration/postcopy-ram.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++-
>> migration/trace-events | 5 +-
>> 2 files changed, 140 insertions(+), 3 deletions(-)
>>
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index cc78981..9a5133f 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -110,7 +110,6 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
>>
>> ctx->exit_notifier.notify = migration_exit_cb;
>> qemu_add_exit_notifier(&ctx->exit_notifier);
>> - add_migration_state_change_notifier(&ctx->postcopy_notifier);
>> return ctx;
>> }
>>
>> @@ -559,6 +558,136 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,
>> return 0;
>> }
>>
>> +static int get_mem_fault_cpu_index(uint32_t pid)
>> +{
>> + CPUState *cpu_iter;
>> +
>> + CPU_FOREACH(cpu_iter) {
>> + if (cpu_iter->thread_id == pid) {
>> + trace_get_mem_fault_cpu_index(cpu_iter->cpu_index, pid);
>> + return cpu_iter->cpu_index;
>> + }
>> + }
>> + trace_get_mem_fault_cpu_index(-1, pid);
>> + return -1;
>> +}
>> +
>> +/*
>> + * This function is being called when pagefault occurs. It
>> + * tracks down vCPU blocking time.
>> + *
>> + * @addr: faulted host virtual address
>> + * @ptid: faulted process thread id
>> + * @rb: ramblock appropriate to addr
>> + */
>> +static void mark_postcopy_blocktime_begin(uint64_t addr, uint32_t ptid,
>> + RAMBlock *rb)
>> +{
>> + int cpu, already_received;
>> + MigrationIncomingState *mis = migration_incoming_get_current();
>> + PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
>> + int64_t now_ms;
>> +
>> + if (!dc || ptid == 0) {
>> + return;
>> + }
>> + cpu = get_mem_fault_cpu_index(ptid);
>> + if (cpu < 0) {
>> + return;
>> + }
>> +
>> + now_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>> + if (dc->vcpu_addr[cpu] == 0) {
>> + atomic_inc(&dc->smp_cpus_down);
>> + }
>> +
>> + atomic_xchg__nocheck(&dc->vcpu_addr[cpu], addr);
>> + atomic_xchg__nocheck(&dc->last_begin, now_ms);
>> + atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], now_ms);
>> +
>> + already_received = ramblock_recv_bitmap_test(rb, (void *)addr);
>> + if (already_received) {
>> + atomic_xchg__nocheck(&dc->vcpu_addr[cpu], 0);
>> + atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], 0);
>> + atomic_sub(&dc->smp_cpus_down, 1);
>> + }
>> + trace_mark_postcopy_blocktime_begin(addr, dc, dc->page_fault_vcpu_time[cpu],
>> + cpu, already_received);
>> +}
>> +
>> +/*
>> + * This function just provide calculated blocktime per cpu and trace it.
>> + * Total blocktime is calculated in mark_postcopy_blocktime_end.
>> + *
>> + *
>> + * Assume we have 3 CPU
>> + *
>> + * S1 E1 S1 E1
>> + * -----***********------------xxx***************------------------------> CPU1
>> + *
>> + * S2 E2
>> + * ------------****************xxx---------------------------------------> CPU2
>> + *
>> + * S3 E3
>> + * ------------------------****xxx********-------------------------------> CPU3
>> + *
>> + * We have sequence S1,S2,E1,S3,S1,E2,E3,E1
>> + * S2,E1 - doesn't match condition due to sequence S1,S2,E1 doesn't include CPU3
>> + * S3,S1,E2 - sequence includes all CPUs, in this case overlap will be S1,E2 -
>> + * it's a part of total blocktime.
>> + * S1 - here is last_begin
>> + * Legend of the picture is following:
>> + * * - means blocktime per vCPU
>> + * x - means overlapped blocktime (total blocktime)
>> + *
>> + * @addr: host virtual address
>> + */
>> +static void mark_postcopy_blocktime_end(uint64_t addr)
>> +{
>> + MigrationIncomingState *mis = migration_incoming_get_current();
>> + PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
>> + int i, affected_cpu = 0;
>> + int64_t now_ms;
>> + bool vcpu_total_blocktime = false;
>> +
>> + if (!dc) {
>> + return;
>> + }
>> +
>> + now_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>> +
>> + /* lookup cpu, to clear it,
>> + * that algorithm looks straighforward, but it's not
>> + * optimal, more optimal algorithm is keeping tree or hash
>> + * where key is address value is a list of */
>> + for (i = 0; i < smp_cpus; i++) {
>> + uint64_t vcpu_blocktime = 0;
>> + if (atomic_fetch_add(&dc->vcpu_addr[i], 0) != addr) {
>> + continue;
>> + }
>> + atomic_xchg__nocheck(&dc->vcpu_addr[i], 0);
>> + vcpu_blocktime = now_ms -
>> + atomic_fetch_add(&dc->page_fault_vcpu_time[i], 0);
>> + affected_cpu += 1;
>> + /* we need to know is that mark_postcopy_end was due to
>> + * faulted page, another possible case it's prefetched
>> + * page and in that case we shouldn't be here */
>> + if (!vcpu_total_blocktime &&
>> + atomic_fetch_add(&dc->smp_cpus_down, 0) == smp_cpus) {
>> + vcpu_total_blocktime = true;
>> + }
>> + /* continue cycle, due to one page could affect several vCPUs */
>> + dc->vcpu_blocktime[i] += vcpu_blocktime;
>> + }
> Unfortunately this still isn't thread safe; consider the code in
> mark_postcopy_blocktime_begin is:
>
> 1 check vcpu_addr
> 2 write vcpu_addr
> 3 write last_begin
> 4 write vcpu_time
> 5 smp_cpus_down++
I think sequence is following:
1 check vcpu_addr
1.1 smp_cpus_down++
2 write vcpu_addr
3 write last_begin
4 write vcpu_time
>
> 6 already_received:
> 7 write addr = 0
> 8 write vcpu_time = 0
> 9 smp_cpus_down--
>
> and this code is:
> a check vcpu_addr
that check is part of lookup, you assume, lookup completed successfully,
but data for that lookup is not yet set.
> b write vcpu_addr
> c read vcpu_time
> d read smp_cpus_down
>
> e dec smp_cpus_down
>
> if (a) happens after (2) but before (3), (c) and (d) can also
> happen before (3), and so you end up reading a bogus
> vcpu_time.
yes in this case total_blocktime will be longer
>
> This is tricky to get right; if you changed the source to do:
> 1 check vcpu_addr
> 3 write last_begin
> 4 write vcpu_time
> 5 smp_cpus_down++
> 2 write vcpu_addr
>
> 6 already_received:
> 7 write addr = 0
> 8 write vcpu_time = 0
> 9 smp_cpus_down--
>
> I think it's safer; if you read a good vcpu_addr you know
> that the vcpu_time has already been written.
>
> However, can this check (a) happen between the new (2) and (7) ?
> It's slim but I think possibly; on the receiving side we've
> just set the bitmap flag to say received - if a fault comes
> in at about the same time then we could end up with
>
> 1,3,4,5,2 ab 6 7 8 9 cde
>
> So again we end up reading a bogus vcpu_time and double decrement
> smp_cpus_down.
mm, double decrement will break this feature till the end of
migration.
as I remember I offered second bitmap, in v6 maybe.
>
> So I think we have to have:
>
> a' read vcpu_addr
> b' read vcpu_time
> c' if vcpu_addr == addr && vcpu_time != 0 ...
> d' clear vcpu_addr
> e' read/dec smp_cpus_down
>
> You should comment to say where the order is important as well;
> because we'll never remember this - it's hairy!
> (Better suggestions welcome)
protect whole dc by mutex is not considered due to it's hot path.
--
Best regards,
Alexey Perevalov
> Dave
>
>> + atomic_sub(&dc->smp_cpus_down, affected_cpu);
>> + if (vcpu_total_blocktime) {
>> + dc->total_blocktime += now_ms - atomic_fetch_add(&dc->last_begin, 0);
>> + }
>> + trace_mark_postcopy_blocktime_end(addr, dc, dc->total_blocktime,
>> + affected_cpu);
>> +}
>> +
>> /*
>> * Handle faults detected by the USERFAULT markings
>> */
>> @@ -636,8 +765,11 @@ static void *postcopy_ram_fault_thread(void *opaque)
>> rb_offset &= ~(qemu_ram_pagesize(rb) - 1);
>> trace_postcopy_ram_fault_thread_request(msg.arg.pagefault.address,
>> qemu_ram_get_idstr(rb),
>> - rb_offset);
>> + rb_offset,
>> + msg.arg.pagefault.feat.ptid);
>>
>> + mark_postcopy_blocktime_begin((uintptr_t)(msg.arg.pagefault.address),
>> + msg.arg.pagefault.feat.ptid, rb);
>> /*
>> * Send the request to the source - we want to request one
>> * of our host page sizes (which is >= TPS)
>> @@ -727,6 +859,8 @@ static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
>> if (!ret) {
>> ramblock_recv_bitmap_set_range(rb, host_addr,
>> pagesize / qemu_target_page_size());
>> + mark_postcopy_blocktime_end((uint64_t)(uintptr_t)host_addr);
>> +
>> }
>> return ret;
>> }
>> diff --git a/migration/trace-events b/migration/trace-events
>> index d2910a6..01f30fe 100644
>> --- a/migration/trace-events
>> +++ b/migration/trace-events
>> @@ -114,6 +114,8 @@ process_incoming_migration_co_end(int ret, int ps) "ret=%d postcopy-state=%d"
>> process_incoming_migration_co_postcopy_end_main(void) ""
>> migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"
>> migration_set_outgoing_channel(void *ioc, const char *ioctype, const char *hostname) "ioc=%p ioctype=%s hostname=%s"
>> +mark_postcopy_blocktime_begin(uint64_t addr, void *dd, int64_t time, int cpu, int received) "addr: 0x%" PRIx64 ", dd: %p, time: %" PRId64 ", cpu: %d, already_received: %d"
>> +mark_postcopy_blocktime_end(uint64_t addr, void *dd, int64_t time, int affected_cpu) "addr: 0x%" PRIx64 ", dd: %p, time: %" PRId64 ", affected_cpu: %d"
>>
>> # migration/rdma.c
>> qemu_rdma_accept_incoming_migration(void) ""
>> @@ -190,7 +192,7 @@ postcopy_ram_enable_notify(void) ""
>> postcopy_ram_fault_thread_entry(void) ""
>> postcopy_ram_fault_thread_exit(void) ""
>> postcopy_ram_fault_thread_quit(void) ""
>> -postcopy_ram_fault_thread_request(uint64_t hostaddr, const char *ramblock, size_t offset) "Request for HVA=0x%" PRIx64 " rb=%s offset=0x%zx"
>> +postcopy_ram_fault_thread_request(uint64_t hostaddr, const char *ramblock, size_t offset, uint32_t pid) "Request for HVA=0x%" PRIx64 " rb=%s offset=0x%zx pid=%u"
>> postcopy_ram_incoming_cleanup_closeuf(void) ""
>> postcopy_ram_incoming_cleanup_entry(void) ""
>> postcopy_ram_incoming_cleanup_exit(void) ""
>> @@ -199,6 +201,7 @@ save_xbzrle_page_skipping(void) ""
>> save_xbzrle_page_overflow(void) ""
>> ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations"
>> ram_load_complete(int ret, uint64_t seq_iter) "exit_code %d seq iteration %" PRIu64
>> +get_mem_fault_cpu_index(int cpu, uint32_t pid) "cpu: %d, pid: %u"
>>
>> # migration/exec.c
>> migration_exec_outgoing(const char *cmd) "cmd=%s"
>> --
>> 1.9.1
>>
> --
> Dr. David Alan Gilbert /dgilbert@redhat.com / Manchester, UK
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 07/10] migration: calculate vCPU blocktime on dst side
2017-09-21 11:57 ` Dr. David Alan Gilbert
2017-09-22 8:47 ` Alexey Perevalov
@ 2017-09-28 8:01 ` Alexey Perevalov
1 sibling, 0 replies; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-28 8:01 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
On 09/21/2017 02:57 PM, Dr. David Alan Gilbert wrote:
> * Alexey Perevalov (a.perevalov@samsung.com) wrote:
>> This patch provides blocktime calculation per vCPU,
>> as a summary and as a overlapped value for all vCPUs.
>>
>> This approach was suggested by Peter Xu, as an improvements of
>> previous approch where QEMU kept tree with faulted page address and cpus bitmask
>> in it. Now QEMU is keeping array with faulted page address as value and vCPU
>> as index. It helps to find proper vCPU at UFFD_COPY time. Also it keeps
>> list for blocktime per vCPU (could be traced with page_fault_addr)
>>
>> Blocktime will not calculated if postcopy_blocktime field of
>> MigrationIncomingState wasn't initialized.
>>
>> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
>> ---
>> migration/postcopy-ram.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++-
>> migration/trace-events | 5 +-
>> 2 files changed, 140 insertions(+), 3 deletions(-)
>>
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index cc78981..9a5133f 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -110,7 +110,6 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
>>
>> ctx->exit_notifier.notify = migration_exit_cb;
>> qemu_add_exit_notifier(&ctx->exit_notifier);
>> - add_migration_state_change_notifier(&ctx->postcopy_notifier);
>> return ctx;
>> }
>>
>> @@ -559,6 +558,136 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,
>> return 0;
>> }
>>
>> +static int get_mem_fault_cpu_index(uint32_t pid)
>> +{
>> + CPUState *cpu_iter;
>> +
>> + CPU_FOREACH(cpu_iter) {
>> + if (cpu_iter->thread_id == pid) {
>> + trace_get_mem_fault_cpu_index(cpu_iter->cpu_index, pid);
>> + return cpu_iter->cpu_index;
>> + }
>> + }
>> + trace_get_mem_fault_cpu_index(-1, pid);
>> + return -1;
>> +}
>> +
>> +/*
>> + * This function is being called when pagefault occurs. It
>> + * tracks down vCPU blocking time.
>> + *
>> + * @addr: faulted host virtual address
>> + * @ptid: faulted process thread id
>> + * @rb: ramblock appropriate to addr
>> + */
>> +static void mark_postcopy_blocktime_begin(uint64_t addr, uint32_t ptid,
>> + RAMBlock *rb)
>> +{
>> + int cpu, already_received;
>> + MigrationIncomingState *mis = migration_incoming_get_current();
>> + PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
>> + int64_t now_ms;
>> +
>> + if (!dc || ptid == 0) {
>> + return;
>> + }
>> + cpu = get_mem_fault_cpu_index(ptid);
>> + if (cpu < 0) {
>> + return;
>> + }
>> +
>> + now_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>> + if (dc->vcpu_addr[cpu] == 0) {
>> + atomic_inc(&dc->smp_cpus_down);
>> + }
>> +
>> + atomic_xchg__nocheck(&dc->vcpu_addr[cpu], addr);
>> + atomic_xchg__nocheck(&dc->last_begin, now_ms);
>> + atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], now_ms);
>> +
>> + already_received = ramblock_recv_bitmap_test(rb, (void *)addr);
>> + if (already_received) {
>> + atomic_xchg__nocheck(&dc->vcpu_addr[cpu], 0);
>> + atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], 0);
>> + atomic_sub(&dc->smp_cpus_down, 1);
>> + }
>> + trace_mark_postcopy_blocktime_begin(addr, dc, dc->page_fault_vcpu_time[cpu],
>> + cpu, already_received);
>> +}
>> +
>> +/*
>> + * This function just provide calculated blocktime per cpu and trace it.
>> + * Total blocktime is calculated in mark_postcopy_blocktime_end.
>> + *
>> + *
>> + * Assume we have 3 CPU
>> + *
>> + * S1 E1 S1 E1
>> + * -----***********------------xxx***************------------------------> CPU1
>> + *
>> + * S2 E2
>> + * ------------****************xxx---------------------------------------> CPU2
>> + *
>> + * S3 E3
>> + * ------------------------****xxx********-------------------------------> CPU3
>> + *
>> + * We have sequence S1,S2,E1,S3,S1,E2,E3,E1
>> + * S2,E1 - doesn't match condition due to sequence S1,S2,E1 doesn't include CPU3
>> + * S3,S1,E2 - sequence includes all CPUs, in this case overlap will be S1,E2 -
>> + * it's a part of total blocktime.
>> + * S1 - here is last_begin
>> + * Legend of the picture is following:
>> + * * - means blocktime per vCPU
>> + * x - means overlapped blocktime (total blocktime)
>> + *
>> + * @addr: host virtual address
>> + */
>> +static void mark_postcopy_blocktime_end(uint64_t addr)
>> +{
>> + MigrationIncomingState *mis = migration_incoming_get_current();
>> + PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
>> + int i, affected_cpu = 0;
>> + int64_t now_ms;
>> + bool vcpu_total_blocktime = false;
>> +
>> + if (!dc) {
>> + return;
>> + }
>> +
>> + now_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>> +
>> + /* lookup cpu, to clear it,
>> + * that algorithm looks straighforward, but it's not
>> + * optimal, more optimal algorithm is keeping tree or hash
>> + * where key is address value is a list of */
>> + for (i = 0; i < smp_cpus; i++) {
>> + uint64_t vcpu_blocktime = 0;
>> + if (atomic_fetch_add(&dc->vcpu_addr[i], 0) != addr) {
>> + continue;
>> + }
>> + atomic_xchg__nocheck(&dc->vcpu_addr[i], 0);
>> + vcpu_blocktime = now_ms -
>> + atomic_fetch_add(&dc->page_fault_vcpu_time[i], 0);
>> + affected_cpu += 1;
>> + /* we need to know is that mark_postcopy_end was due to
>> + * faulted page, another possible case it's prefetched
>> + * page and in that case we shouldn't be here */
>> + if (!vcpu_total_blocktime &&
>> + atomic_fetch_add(&dc->smp_cpus_down, 0) == smp_cpus) {
>> + vcpu_total_blocktime = true;
>> + }
>> + /* continue cycle, due to one page could affect several vCPUs */
>> + dc->vcpu_blocktime[i] += vcpu_blocktime;
>> + }
> Unfortunately this still isn't thread safe; consider the code in
> mark_postcopy_blocktime_begin is:
>
> 1 check vcpu_addr
> 2 write vcpu_addr
> 3 write last_begin
> 4 write vcpu_time
> 5 smp_cpus_down++
>
> 6 already_received:
> 7 write addr = 0
> 8 write vcpu_time = 0
> 9 smp_cpus_down--
>
> and this code is:
> a check vcpu_addr
> b write vcpu_addr
> c read vcpu_time
> d read smp_cpus_down
>
> e dec smp_cpus_down
>
> if (a) happens after (2) but before (3), (c) and (d) can also
> happen before (3), and so you end up reading a bogus
> vcpu_time.
>
> This is tricky to get right; if you changed the source to do:
> 1 check vcpu_addr
> 3 write last_begin
> 4 write vcpu_time
> 5 smp_cpus_down++
> 2 write vcpu_addr
>
> 6 already_received:
> 7 write addr = 0
> 8 write vcpu_time = 0
> 9 smp_cpus_down--
>
> I think it's safer; if you read a good vcpu_addr you know
> that the vcpu_time has already been written.
>
> However, can this check (a) happen between the new (2) and (7) ?
> It's slim but I think possibly; on the receiving side we've
> just set the bitmap flag to say received - if a fault comes
> in at about the same time then we could end up with
>
> 1,3,4,5,2 ab 6 7 8 9 cde
>
> So again we end up reading a bogus vcpu_time and double decrement
> smp_cpus_down.
>
> So I think we have to have:
>
> a' read vcpu_addr
> b' read vcpu_time
> c' if vcpu_addr == addr && vcpu_time != 0 ...
> d' clear vcpu_addr
> e' read/dec smp_cpus_down
I think even in this sequence it's possible to have
lookup condition
if (atomic_fetch_add(&dc->vcpu_addr[i], 0) != addr ||
atomic_fetch_add(&dc->page_fault_vcpu_time[i], 0) == 0) {
continue;
}
in you terms it's (not c)
between
atomic_xchg__nocheck(&dc->vcpu_addr[cpu], addr); - (2)
and
atomic_xchg__nocheck(&dc->vcpu_addr[cpu], 0); - (7)
2c7
Probability is lesser, but it still exists.
Here comes up a second bitmap, but it will increase code complexity.
BTW, remind me why we don't protect body of these functions by mutex?
>
> You should comment to say where the order is important as well;
> because we'll never remember this - it's hairy!
> (Better suggestions welcome)
>
> Dave
>
>> + atomic_sub(&dc->smp_cpus_down, affected_cpu);
>> + if (vcpu_total_blocktime) {
>> + dc->total_blocktime += now_ms - atomic_fetch_add(&dc->last_begin, 0);
>> + }
>> + trace_mark_postcopy_blocktime_end(addr, dc, dc->total_blocktime,
>> + affected_cpu);
>> +}
>> +
>> /*
>> * Handle faults detected by the USERFAULT markings
>> */
>> @@ -636,8 +765,11 @@ static void *postcopy_ram_fault_thread(void *opaque)
>> rb_offset &= ~(qemu_ram_pagesize(rb) - 1);
>> trace_postcopy_ram_fault_thread_request(msg.arg.pagefault.address,
>> qemu_ram_get_idstr(rb),
>> - rb_offset);
>> + rb_offset,
>> + msg.arg.pagefault.feat.ptid);
>>
>> + mark_postcopy_blocktime_begin((uintptr_t)(msg.arg.pagefault.address),
>> + msg.arg.pagefault.feat.ptid, rb);
>> /*
>> * Send the request to the source - we want to request one
>> * of our host page sizes (which is >= TPS)
>> @@ -727,6 +859,8 @@ static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
>> if (!ret) {
>> ramblock_recv_bitmap_set_range(rb, host_addr,
>> pagesize / qemu_target_page_size());
>> + mark_postcopy_blocktime_end((uint64_t)(uintptr_t)host_addr);
>> +
>> }
>> return ret;
>> }
>> diff --git a/migration/trace-events b/migration/trace-events
>> index d2910a6..01f30fe 100644
>> --- a/migration/trace-events
>> +++ b/migration/trace-events
>> @@ -114,6 +114,8 @@ process_incoming_migration_co_end(int ret, int ps) "ret=%d postcopy-state=%d"
>> process_incoming_migration_co_postcopy_end_main(void) ""
>> migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"
>> migration_set_outgoing_channel(void *ioc, const char *ioctype, const char *hostname) "ioc=%p ioctype=%s hostname=%s"
>> +mark_postcopy_blocktime_begin(uint64_t addr, void *dd, int64_t time, int cpu, int received) "addr: 0x%" PRIx64 ", dd: %p, time: %" PRId64 ", cpu: %d, already_received: %d"
>> +mark_postcopy_blocktime_end(uint64_t addr, void *dd, int64_t time, int affected_cpu) "addr: 0x%" PRIx64 ", dd: %p, time: %" PRId64 ", affected_cpu: %d"
>>
>> # migration/rdma.c
>> qemu_rdma_accept_incoming_migration(void) ""
>> @@ -190,7 +192,7 @@ postcopy_ram_enable_notify(void) ""
>> postcopy_ram_fault_thread_entry(void) ""
>> postcopy_ram_fault_thread_exit(void) ""
>> postcopy_ram_fault_thread_quit(void) ""
>> -postcopy_ram_fault_thread_request(uint64_t hostaddr, const char *ramblock, size_t offset) "Request for HVA=0x%" PRIx64 " rb=%s offset=0x%zx"
>> +postcopy_ram_fault_thread_request(uint64_t hostaddr, const char *ramblock, size_t offset, uint32_t pid) "Request for HVA=0x%" PRIx64 " rb=%s offset=0x%zx pid=%u"
>> postcopy_ram_incoming_cleanup_closeuf(void) ""
>> postcopy_ram_incoming_cleanup_entry(void) ""
>> postcopy_ram_incoming_cleanup_exit(void) ""
>> @@ -199,6 +201,7 @@ save_xbzrle_page_skipping(void) ""
>> save_xbzrle_page_overflow(void) ""
>> ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations"
>> ram_load_complete(int ret, uint64_t seq_iter) "exit_code %d seq iteration %" PRIu64
>> +get_mem_fault_cpu_index(int cpu, uint32_t pid) "cpu: %d, pid: %u"
>>
>> # migration/exec.c
>> migration_exec_outgoing(const char *cmd) "cmd=%s"
>> --
>> 1.9.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
>
>
--
Best regards,
Alexey Perevalov
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v10 08/10] migration: postcopy_blocktime documentation
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 00/10] calculate blocktime for postcopy live migration Alexey Perevalov
` (6 preceding siblings ...)
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 07/10] migration: calculate vCPU blocktime on dst side Alexey Perevalov
@ 2017-09-19 16:48 ` Alexey Perevalov
2017-09-21 12:33 ` Dr. David Alan Gilbert
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 09/10] migration: add blocktime calculation into postcopy-test Alexey Perevalov
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 10/10] migration: add postcopy total blocktime into query-migrate Alexey Perevalov
9 siblings, 1 reply; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-19 16:48 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Perevalov, i.maximets, peterx, eblake, dgilbert, quintela,
heetae82.ahn
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
docs/devel/migration.txt | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/docs/devel/migration.txt b/docs/devel/migration.txt
index 1b940a8..4b625ca 100644
--- a/docs/devel/migration.txt
+++ b/docs/devel/migration.txt
@@ -402,6 +402,16 @@ will now cause the transition from precopy to postcopy.
It can be issued immediately after migration is started or any
time later on. Issuing it after the end of a migration is harmless.
+Blocktime is a postcopy live migration metric, intended to show
+how long the vCPU was in state of interruptable sleep due to pagefault.
+This value is calculated on destination side.
+To enable postcopy blocktime calculation, enter following command on destination
+monitor:
+
+migrate_set_capability postcopy-blocktime on
+
+Postcopy blocktime can be retrieved by query-migrate qmp command.
+
Note: During the postcopy phase, the bandwidth limits set using
migrate_set_speed is ignored (to avoid delaying requested pages that
the destination is waiting for).
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 08/10] migration: postcopy_blocktime documentation
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 08/10] migration: postcopy_blocktime documentation Alexey Perevalov
@ 2017-09-21 12:33 ` Dr. David Alan Gilbert
2017-09-21 13:26 ` Alexey Perevalov
0 siblings, 1 reply; 27+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-21 12:33 UTC (permalink / raw)
To: Alexey Perevalov
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
Although it does have my R-b it might be worth adding some clarification
that it's a measure of when *all* cpus are blocked and so isn't a
total measure of impact of postcopy (when blocking some of them).
Dave
> ---
> docs/devel/migration.txt | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/docs/devel/migration.txt b/docs/devel/migration.txt
> index 1b940a8..4b625ca 100644
> --- a/docs/devel/migration.txt
> +++ b/docs/devel/migration.txt
> @@ -402,6 +402,16 @@ will now cause the transition from precopy to postcopy.
> It can be issued immediately after migration is started or any
> time later on. Issuing it after the end of a migration is harmless.
>
> +Blocktime is a postcopy live migration metric, intended to show
> +how long the vCPU was in state of interruptable sleep due to pagefault.
> +This value is calculated on destination side.
> +To enable postcopy blocktime calculation, enter following command on destination
> +monitor:
> +
> +migrate_set_capability postcopy-blocktime on
> +
> +Postcopy blocktime can be retrieved by query-migrate qmp command.
> +
> Note: During the postcopy phase, the bandwidth limits set using
> migrate_set_speed is ignored (to avoid delaying requested pages that
> the destination is waiting for).
> --
> 1.9.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v10 08/10] migration: postcopy_blocktime documentation
2017-09-21 12:33 ` Dr. David Alan Gilbert
@ 2017-09-21 13:26 ` Alexey Perevalov
2017-09-21 14:40 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-21 13:26 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
On 09/21/2017 03:33 PM, Dr. David Alan Gilbert wrote:
> * Alexey Perevalov (a.perevalov@samsung.com) wrote:
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> Although it does have my R-b it might be worth adding some clarification
> that it's a measure of when *all* cpus are blocked and so isn't a
> total measure of impact of postcopy (when blocking some of them).
yes, maybe additional clarification is necessary.
now we have both values:
{"return": {"postcopy-blocktime": 5691, "status": "completed",
"postcopy-vcpu-blocktime": [7671, 6388]}}
where postcopy-blocktime is for *all* and postcopy-vcpu-blocktime is per
vCPU,
it's really worth to describe it, like:
Blocktime is a postcopy live migration metric, intended to show
how long the vCPU was in state of interruptible sleep due to pagefault.
That metric is calculated both for all vCPUs as overlapped value, and
separately for each vCPU. These values are calculated on destination side.
To enable postcopy blocktime calculation, enter following command on destination
monitor:
migrate_set_capability postcopy-blocktime on
Postcopy blocktime can be retrieved by query-migrate qmp command.
postcopy-blocktime value of qmp command will show overlapped blocking time for all vCPU,
postcopy-vcpu-blocktime will show list of blocking time per vCPU.
--
Best regards,
Alexey Perevalov
>
> Dave
>
>> ---
>> docs/devel/migration.txt | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/docs/devel/migration.txt b/docs/devel/migration.txt
>> index 1b940a8..4b625ca 100644
>> --- a/docs/devel/migration.txt
>> +++ b/docs/devel/migration.txt
>> @@ -402,6 +402,16 @@ will now cause the transition from precopy to postcopy.
>> It can be issued immediately after migration is started or any
>> time later on. Issuing it after the end of a migration is harmless.
>>
>> +Blocktime is a postcopy live migration metric, intended to show
>> +how long the vCPU was in state of interruptable sleep due to pagefault.
>> +This value is calculated on destination side.
>> +To enable postcopy blocktime calculation, enter following command on destination
>> +monitor:
>> +
>> +migrate_set_capability postcopy-blocktime on
>> +
>> +Postcopy blocktime can be retrieved by query-migrate qmp command.
>> +
>> Note: During the postcopy phase, the bandwidth limits set using
>> migrate_set_speed is ignored (to avoid delaying requested pages that
>> the destination is waiting for).
>> --
>> 1.9.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 08/10] migration: postcopy_blocktime documentation
2017-09-21 13:26 ` Alexey Perevalov
@ 2017-09-21 14:40 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 27+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-21 14:40 UTC (permalink / raw)
To: Alexey Perevalov
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> On 09/21/2017 03:33 PM, Dr. David Alan Gilbert wrote:
> > * Alexey Perevalov (a.perevalov@samsung.com) wrote:
> > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> > Although it does have my R-b it might be worth adding some clarification
> > that it's a measure of when *all* cpus are blocked and so isn't a
> > total measure of impact of postcopy (when blocking some of them).
> yes, maybe additional clarification is necessary.
> now we have both values:
> {"return": {"postcopy-blocktime": 5691, "status": "completed",
> "postcopy-vcpu-blocktime": [7671, 6388]}}
> where postcopy-blocktime is for *all* and postcopy-vcpu-blocktime is per
> vCPU,
> it's really worth to describe it, like:
>
> Blocktime is a postcopy live migration metric, intended to show
> how long the vCPU was in state of interruptible sleep due to pagefault.
> That metric is calculated both for all vCPUs as overlapped value, and
> separately for each vCPU. These values are calculated on destination side.
> To enable postcopy blocktime calculation, enter following command on destination
> monitor:
>
> migrate_set_capability postcopy-blocktime on
>
> Postcopy blocktime can be retrieved by query-migrate qmp command.
> postcopy-blocktime value of qmp command will show overlapped blocking time for all vCPU,
> postcopy-vcpu-blocktime will show list of blocking time per vCPU.
Yep that seems OK.
Dave
> --
> Best regards,
> Alexey Perevalov
>
> >
> > Dave
> >
> > > ---
> > > docs/devel/migration.txt | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/docs/devel/migration.txt b/docs/devel/migration.txt
> > > index 1b940a8..4b625ca 100644
> > > --- a/docs/devel/migration.txt
> > > +++ b/docs/devel/migration.txt
> > > @@ -402,6 +402,16 @@ will now cause the transition from precopy to postcopy.
> > > It can be issued immediately after migration is started or any
> > > time later on. Issuing it after the end of a migration is harmless.
> > > +Blocktime is a postcopy live migration metric, intended to show
> > > +how long the vCPU was in state of interruptable sleep due to pagefault.
> > > +This value is calculated on destination side.
> > > +To enable postcopy blocktime calculation, enter following command on destination
> > > +monitor:
> > > +
> > > +migrate_set_capability postcopy-blocktime on
> > > +
> > > +Postcopy blocktime can be retrieved by query-migrate qmp command.
> > > +
> > > Note: During the postcopy phase, the bandwidth limits set using
> > > migrate_set_speed is ignored (to avoid delaying requested pages that
> > > the destination is waiting for).
> > > --
> > > 1.9.1
> > >
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
> >
> >
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v10 09/10] migration: add blocktime calculation into postcopy-test
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 00/10] calculate blocktime for postcopy live migration Alexey Perevalov
` (7 preceding siblings ...)
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 08/10] migration: postcopy_blocktime documentation Alexey Perevalov
@ 2017-09-19 16:48 ` Alexey Perevalov
2017-09-21 12:39 ` Dr. David Alan Gilbert
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 10/10] migration: add postcopy total blocktime into query-migrate Alexey Perevalov
9 siblings, 1 reply; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-19 16:48 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Perevalov, i.maximets, peterx, eblake, dgilbert, quintela,
heetae82.ahn
This patch just requests blocktime calculation, but doesn't
add any facility to check or show it.
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
tests/postcopy-test.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
index 8142f2a..1db5359 100644
--- a/tests/postcopy-test.c
+++ b/tests/postcopy-test.c
@@ -425,6 +425,17 @@ static void test_migrate(void)
g_assert(qdict_haskey(rsp, "return"));
QDECREF(rsp);
+#ifdef UFFD_FEATURE_THREAD_ID
+ global_qtest = to;
+ rsp = qmp("{ 'execute': 'migrate-set-capabilities',"
+ "'arguments': { "
+ "'capabilities': [ {"
+ "'capability': 'postcopy-blocktime',"
+ "'state': true } ] } }");
+ g_assert(qdict_haskey(rsp, "return"));
+ QDECREF(rsp);
+#endif
+
/* We want to pick a speed slow enough that the test completes
* quickly, but that it doesn't complete precopy even on a slow
* machine, so also set the downtime.
@@ -441,7 +452,6 @@ static void test_migrate(void)
g_assert(qdict_haskey(rsp, "return"));
QDECREF(rsp);
-
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 09/10] migration: add blocktime calculation into postcopy-test
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 09/10] migration: add blocktime calculation into postcopy-test Alexey Perevalov
@ 2017-09-21 12:39 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 27+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-21 12:39 UTC (permalink / raw)
To: Alexey Perevalov
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> This patch just requests blocktime calculation, but doesn't
> add any facility to check or show it.
>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
> tests/postcopy-test.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
> index 8142f2a..1db5359 100644
> --- a/tests/postcopy-test.c
> +++ b/tests/postcopy-test.c
> @@ -425,6 +425,17 @@ static void test_migrate(void)
> g_assert(qdict_haskey(rsp, "return"));
> QDECREF(rsp);
>
> +#ifdef UFFD_FEATURE_THREAD_ID
> + global_qtest = to;
> + rsp = qmp("{ 'execute': 'migrate-set-capabilities',"
> + "'arguments': { "
> + "'capabilities': [ {"
> + "'capability': 'postcopy-blocktime',"
> + "'state': true } ] } }");
> + g_assert(qdict_haskey(rsp, "return"));
> + QDECREF(rsp);
> +#endif
> +
I think that'll break; once you sync the kernel headers up,
we'll always get that defined even when we're building or running
on an older kernel.
So I think you need to try and enable it but fall back if it's not
available somehow.
Dave
> /* We want to pick a speed slow enough that the test completes
> * quickly, but that it doesn't complete precopy even on a slow
> * machine, so also set the downtime.
> @@ -441,7 +452,6 @@ static void test_migrate(void)
> g_assert(qdict_haskey(rsp, "return"));
> QDECREF(rsp);
>
> -
> /* Wait for the first serial output from the source */
> wait_for_serial("src_serial");
>
> --
> 1.9.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v10 10/10] migration: add postcopy total blocktime into query-migrate
2017-09-19 16:47 ` [Qemu-devel] [PATCH v10 00/10] calculate blocktime for postcopy live migration Alexey Perevalov
` (8 preceding siblings ...)
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 09/10] migration: add blocktime calculation into postcopy-test Alexey Perevalov
@ 2017-09-19 16:48 ` Alexey Perevalov
2017-09-19 17:41 ` Eric Blake
2017-09-21 12:42 ` Dr. David Alan Gilbert
9 siblings, 2 replies; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-19 16:48 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Perevalov, i.maximets, peterx, eblake, dgilbert, quintela,
heetae82.ahn
Postcopy total blocktime is available on destination side only.
But query-migrate was possible only for source. This patch
adds ability to call query-migrate on destination.
To be able to see postcopy blocktime, need to request postcopy-blocktime
capability.
The query-migrate command will show following sample result:
{"return":
"postcopy-vcpu-blocktime": [115, 100],
"status": "completed",
"postcopy-blocktime": 100
}}
postcopy_vcpu_blocktime contains list, where the first item is the first
vCPU in QEMU.
This patch has a drawback, it combines states of incoming and
outgoing migration. Ongoing migration state will overwrite incoming
state. Looks like better to separate query-migrate for incoming and
outgoing migration or add parameter to indicate type of migration.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
hmp.c | 15 +++++++++++++
migration/migration.c | 42 +++++++++++++++++++++++++++++++----
migration/migration.h | 4 ++++
migration/postcopy-ram.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
migration/trace-events | 1 +
qapi/migration.json | 11 +++++++++-
6 files changed, 125 insertions(+), 5 deletions(-)
diff --git a/hmp.c b/hmp.c
index 0fb2bc7..142f76e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -264,6 +264,21 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
info->cpu_throttle_percentage);
}
+ if (info->has_postcopy_blocktime) {
+ monitor_printf(mon, "postcopy blocktime: %" PRId64 "\n",
+ info->postcopy_blocktime);
+ }
+
+ if (info->has_postcopy_vcpu_blocktime) {
+ Visitor *v;
+ char *str;
+ v = string_output_visitor_new(false, &str);
+ visit_type_int64List(v, NULL, &info->postcopy_vcpu_blocktime, NULL);
+ visit_complete(v, &str);
+ monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
+ g_free(str);
+ visit_free(v);
+ }
qapi_free_MigrationInfo(info);
qapi_free_MigrationCapabilityStatusList(caps);
}
diff --git a/migration/migration.c b/migration/migration.c
index 4f029e8..e1d3248 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -528,14 +528,15 @@ static void populate_disk_info(MigrationInfo *info)
}
}
-MigrationInfo *qmp_query_migrate(Error **errp)
+static void fill_source_migration_info(MigrationInfo *info)
{
- MigrationInfo *info = g_malloc0(sizeof(*info));
MigrationState *s = migrate_get_current();
switch (s->state) {
case MIGRATION_STATUS_NONE:
/* no migration has happened ever */
+ /* do not overwrite destination migration status */
+ return;
break;
case MIGRATION_STATUS_SETUP:
info->has_status = true;
@@ -584,8 +585,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
break;
}
info->status = s->state;
-
- return info;
}
/**
@@ -649,6 +648,41 @@ static bool migrate_caps_check(bool *cap_list,
return true;
}
+static void fill_destination_migration_info(MigrationInfo *info)
+{
+ MigrationIncomingState *mis = migration_incoming_get_current();
+
+ switch (mis->state) {
+ case MIGRATION_STATUS_NONE:
+ return;
+ break;
+ case MIGRATION_STATUS_SETUP:
+ case MIGRATION_STATUS_CANCELLING:
+ case MIGRATION_STATUS_CANCELLED:
+ case MIGRATION_STATUS_ACTIVE:
+ case MIGRATION_STATUS_POSTCOPY_ACTIVE:
+ case MIGRATION_STATUS_FAILED:
+ case MIGRATION_STATUS_COLO:
+ info->has_status = true;
+ break;
+ case MIGRATION_STATUS_COMPLETED:
+ info->has_status = true;
+ fill_destination_postcopy_migration_info(info);
+ break;
+ }
+ info->status = mis->state;
+}
+
+MigrationInfo *qmp_query_migrate(Error **errp)
+{
+ MigrationInfo *info = g_malloc0(sizeof(*info));
+
+ fill_destination_migration_info(info);
+ fill_source_migration_info(info);
+
+ return info;
+}
+
void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
Error **errp)
{
diff --git a/migration/migration.h b/migration/migration.h
index 770466b..882a59b 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -70,6 +70,10 @@ struct MigrationIncomingState {
MigrationIncomingState *migration_incoming_get_current(void);
void migration_incoming_state_destroy(void);
+/*
+ * Functions to work with blocktime context
+ */
+void fill_destination_postcopy_migration_info(MigrationInfo *info);
#define TYPE_MIGRATION "migration"
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 9a5133f..5fdbf1e 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -113,6 +113,55 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
return ctx;
}
+static int64List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
+{
+ int64List *list = NULL, *entry = NULL;
+ int i;
+
+ for (i = smp_cpus - 1; i >= 0; i--) {
+ entry = g_new0(int64List, 1);
+ entry->value = ctx->vcpu_blocktime[i];
+ entry->next = list;
+ list = entry;
+ }
+
+ return list;
+}
+
+/*
+ * This function just populates MigrationInfo from postcopy's
+ * blocktime context. It will not populate MigrationInfo,
+ * unless postcopy-blocktime capability was set.
+ *
+ * @info: pointer to MigrationInfo to populate
+ */
+void fill_destination_postcopy_migration_info(MigrationInfo *info)
+{
+ MigrationIncomingState *mis = migration_incoming_get_current();
+ PostcopyBlocktimeContext *bc = mis->blocktime_ctx;
+
+ if (!bc) {
+ return;
+ }
+
+ info->has_postcopy_blocktime = true;
+ info->postcopy_blocktime = bc->total_blocktime;
+ info->has_postcopy_vcpu_blocktime = true;
+ info->postcopy_vcpu_blocktime = get_vcpu_blocktime_list(bc);
+}
+
+static uint64_t get_postcopy_total_blocktime(void)
+{
+ MigrationIncomingState *mis = migration_incoming_get_current();
+ PostcopyBlocktimeContext *bc = mis->blocktime_ctx;
+
+ if (!bc) {
+ return 0;
+ }
+
+ return bc->total_blocktime;
+}
+
/**
* receive_ufd_features: check userfault fd features, to request only supported
* features in the future.
@@ -487,6 +536,9 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
munmap(mis->postcopy_tmp_zero_page, mis->largest_page_size);
mis->postcopy_tmp_zero_page = NULL;
}
+ trace_postcopy_ram_incoming_cleanup_blocktime(
+ get_postcopy_total_blocktime());
+
trace_postcopy_ram_incoming_cleanup_exit();
return 0;
}
@@ -958,6 +1010,11 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
#else
/* No target OS support, stubs just fail */
+void fill_destination_postcopy_migration_info(MigrationInfo *info)
+{
+ error_report("%s: No OS support", __func__);
+}
+
bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
{
error_report("%s: No OS support", __func__);
diff --git a/migration/trace-events b/migration/trace-events
index 01f30fe..741f2ae 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -197,6 +197,7 @@ postcopy_ram_incoming_cleanup_closeuf(void) ""
postcopy_ram_incoming_cleanup_entry(void) ""
postcopy_ram_incoming_cleanup_exit(void) ""
postcopy_ram_incoming_cleanup_join(void) ""
+postcopy_ram_incoming_cleanup_blocktime(uint64_t total) "total blocktime %" PRIu64
save_xbzrle_page_skipping(void) ""
save_xbzrle_page_overflow(void) ""
ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations"
diff --git a/qapi/migration.json b/qapi/migration.json
index 2e4a15d..55b055e 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -150,6 +150,13 @@
# @status is 'failed'. Clients should not attempt to parse the
# error strings. (Since 2.7)
#
+# @postcopy-blocktime: total time when all vCPU were blocked during postcopy
+# live migration (Since 2.11)
+#
+# @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU (Since 2.10)
+#
+
+#
# Since: 0.14.0
##
{ 'struct': 'MigrationInfo',
@@ -161,7 +168,9 @@
'*downtime': 'int',
'*setup-time': 'int',
'*cpu-throttle-percentage': 'int',
- '*error-desc': 'str'} }
+ '*error-desc': 'str',
+ '*postcopy-blocktime' : 'int64',
+ '*postcopy-vcpu-blocktime': ['int64']} }
##
# @query-migrate:
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 10/10] migration: add postcopy total blocktime into query-migrate
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 10/10] migration: add postcopy total blocktime into query-migrate Alexey Perevalov
@ 2017-09-19 17:41 ` Eric Blake
2017-09-21 12:42 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 27+ messages in thread
From: Eric Blake @ 2017-09-19 17:41 UTC (permalink / raw)
To: Alexey Perevalov, qemu-devel
Cc: i.maximets, peterx, dgilbert, quintela, heetae82.ahn
[-- Attachment #1: Type: text/plain, Size: 938 bytes --]
On 09/19/2017 11:48 AM, Alexey Perevalov wrote:
> Postcopy total blocktime is available on destination side only.
> But query-migrate was possible only for source. This patch
> adds ability to call query-migrate on destination.
> To be able to see postcopy blocktime, need to request postcopy-blocktime
> capability.
>
> +++ b/qapi/migration.json
> @@ -150,6 +150,13 @@
> # @status is 'failed'. Clients should not attempt to parse the
> # error strings. (Since 2.7)
> #
> +# @postcopy-blocktime: total time when all vCPU were blocked during postcopy
> +# live migration (Since 2.11)
> +#
You got this one right,
> +# @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU (Since 2.10)
but left this one at 2.10. Should be 2.11.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v10 10/10] migration: add postcopy total blocktime into query-migrate
2017-09-19 16:48 ` [Qemu-devel] [PATCH v10 10/10] migration: add postcopy total blocktime into query-migrate Alexey Perevalov
2017-09-19 17:41 ` Eric Blake
@ 2017-09-21 12:42 ` Dr. David Alan Gilbert
2017-09-21 15:24 ` Alexey Perevalov
1 sibling, 1 reply; 27+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-21 12:42 UTC (permalink / raw)
To: Alexey Perevalov
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> Postcopy total blocktime is available on destination side only.
> But query-migrate was possible only for source. This patch
> adds ability to call query-migrate on destination.
> To be able to see postcopy blocktime, need to request postcopy-blocktime
> capability.
>
> The query-migrate command will show following sample result:
> {"return":
> "postcopy-vcpu-blocktime": [115, 100],
> "status": "completed",
> "postcopy-blocktime": 100
> }}
>
> postcopy_vcpu_blocktime contains list, where the first item is the first
> vCPU in QEMU.
>
> This patch has a drawback, it combines states of incoming and
> outgoing migration. Ongoing migration state will overwrite incoming
> state. Looks like better to separate query-migrate for incoming and
> outgoing migration or add parameter to indicate type of migration.
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
> hmp.c | 15 +++++++++++++
> migration/migration.c | 42 +++++++++++++++++++++++++++++++----
> migration/migration.h | 4 ++++
> migration/postcopy-ram.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> migration/trace-events | 1 +
> qapi/migration.json | 11 +++++++++-
> 6 files changed, 125 insertions(+), 5 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 0fb2bc7..142f76e 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -264,6 +264,21 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
> info->cpu_throttle_percentage);
> }
>
> + if (info->has_postcopy_blocktime) {
> + monitor_printf(mon, "postcopy blocktime: %" PRId64 "\n",
> + info->postcopy_blocktime);
> + }
> +
> + if (info->has_postcopy_vcpu_blocktime) {
> + Visitor *v;
> + char *str;
> + v = string_output_visitor_new(false, &str);
> + visit_type_int64List(v, NULL, &info->postcopy_vcpu_blocktime, NULL);
> + visit_complete(v, &str);
> + monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
> + g_free(str);
> + visit_free(v);
> + }
> qapi_free_MigrationInfo(info);
> qapi_free_MigrationCapabilityStatusList(caps);
> }
> diff --git a/migration/migration.c b/migration/migration.c
> index 4f029e8..e1d3248 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -528,14 +528,15 @@ static void populate_disk_info(MigrationInfo *info)
> }
> }
>
> -MigrationInfo *qmp_query_migrate(Error **errp)
> +static void fill_source_migration_info(MigrationInfo *info)
> {
> - MigrationInfo *info = g_malloc0(sizeof(*info));
> MigrationState *s = migrate_get_current();
>
> switch (s->state) {
> case MIGRATION_STATUS_NONE:
> /* no migration has happened ever */
> + /* do not overwrite destination migration status */
> + return;
> break;
> case MIGRATION_STATUS_SETUP:
> info->has_status = true;
> @@ -584,8 +585,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> break;
> }
> info->status = s->state;
> -
> - return info;
> }
>
> /**
> @@ -649,6 +648,41 @@ static bool migrate_caps_check(bool *cap_list,
> return true;
> }
>
> +static void fill_destination_migration_info(MigrationInfo *info)
> +{
> + MigrationIncomingState *mis = migration_incoming_get_current();
> +
> + switch (mis->state) {
> + case MIGRATION_STATUS_NONE:
> + return;
> + break;
> + case MIGRATION_STATUS_SETUP:
> + case MIGRATION_STATUS_CANCELLING:
> + case MIGRATION_STATUS_CANCELLED:
> + case MIGRATION_STATUS_ACTIVE:
> + case MIGRATION_STATUS_POSTCOPY_ACTIVE:
> + case MIGRATION_STATUS_FAILED:
> + case MIGRATION_STATUS_COLO:
> + info->has_status = true;
> + break;
> + case MIGRATION_STATUS_COMPLETED:
> + info->has_status = true;
> + fill_destination_postcopy_migration_info(info);
> + break;
> + }
> + info->status = mis->state;
> +}
> +
> +MigrationInfo *qmp_query_migrate(Error **errp)
> +{
> + MigrationInfo *info = g_malloc0(sizeof(*info));
> +
> + fill_destination_migration_info(info);
> + fill_source_migration_info(info);
> +
> + return info;
> +}
> +
> void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> Error **errp)
> {
> diff --git a/migration/migration.h b/migration/migration.h
> index 770466b..882a59b 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -70,6 +70,10 @@ struct MigrationIncomingState {
>
> MigrationIncomingState *migration_incoming_get_current(void);
> void migration_incoming_state_destroy(void);
> +/*
> + * Functions to work with blocktime context
> + */
> +void fill_destination_postcopy_migration_info(MigrationInfo *info);
>
> #define TYPE_MIGRATION "migration"
>
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 9a5133f..5fdbf1e 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -113,6 +113,55 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
> return ctx;
> }
>
> +static int64List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
> +{
> + int64List *list = NULL, *entry = NULL;
> + int i;
> +
> + for (i = smp_cpus - 1; i >= 0; i--) {
> + entry = g_new0(int64List, 1);
> + entry->value = ctx->vcpu_blocktime[i];
> + entry->next = list;
> + list = entry;
> + }
> +
> + return list;
> +}
> +
> +/*
> + * This function just populates MigrationInfo from postcopy's
> + * blocktime context. It will not populate MigrationInfo,
> + * unless postcopy-blocktime capability was set.
> + *
> + * @info: pointer to MigrationInfo to populate
> + */
> +void fill_destination_postcopy_migration_info(MigrationInfo *info)
> +{
> + MigrationIncomingState *mis = migration_incoming_get_current();
> + PostcopyBlocktimeContext *bc = mis->blocktime_ctx;
> +
> + if (!bc) {
> + return;
> + }
> +
> + info->has_postcopy_blocktime = true;
> + info->postcopy_blocktime = bc->total_blocktime;
> + info->has_postcopy_vcpu_blocktime = true;
> + info->postcopy_vcpu_blocktime = get_vcpu_blocktime_list(bc);
> +}
> +
> +static uint64_t get_postcopy_total_blocktime(void)
> +{
> + MigrationIncomingState *mis = migration_incoming_get_current();
> + PostcopyBlocktimeContext *bc = mis->blocktime_ctx;
> +
> + if (!bc) {
> + return 0;
> + }
> +
> + return bc->total_blocktime;
> +}
> +
> /**
> * receive_ufd_features: check userfault fd features, to request only supported
> * features in the future.
> @@ -487,6 +536,9 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
> munmap(mis->postcopy_tmp_zero_page, mis->largest_page_size);
> mis->postcopy_tmp_zero_page = NULL;
> }
> + trace_postcopy_ram_incoming_cleanup_blocktime(
> + get_postcopy_total_blocktime());
> +
> trace_postcopy_ram_incoming_cleanup_exit();
> return 0;
> }
> @@ -958,6 +1010,11 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
>
> #else
> /* No target OS support, stubs just fail */
> +void fill_destination_postcopy_migration_info(MigrationInfo *info)
> +{
> + error_report("%s: No OS support", __func__);
> +}
> +
Do we want that error_report? info migrate shouldn't give an error on a
non-postcopy host.
Also, don't you fancy just checking for the presence of this new info in
the test?
Dave
> bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
> {
> error_report("%s: No OS support", __func__);
> diff --git a/migration/trace-events b/migration/trace-events
> index 01f30fe..741f2ae 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -197,6 +197,7 @@ postcopy_ram_incoming_cleanup_closeuf(void) ""
> postcopy_ram_incoming_cleanup_entry(void) ""
> postcopy_ram_incoming_cleanup_exit(void) ""
> postcopy_ram_incoming_cleanup_join(void) ""
> +postcopy_ram_incoming_cleanup_blocktime(uint64_t total) "total blocktime %" PRIu64
> save_xbzrle_page_skipping(void) ""
> save_xbzrle_page_overflow(void) ""
> ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations"
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 2e4a15d..55b055e 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -150,6 +150,13 @@
> # @status is 'failed'. Clients should not attempt to parse the
> # error strings. (Since 2.7)
> #
> +# @postcopy-blocktime: total time when all vCPU were blocked during postcopy
> +# live migration (Since 2.11)
> +#
> +# @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU (Since 2.10)
> +#
> +
> +#
> # Since: 0.14.0
> ##
> { 'struct': 'MigrationInfo',
> @@ -161,7 +168,9 @@
> '*downtime': 'int',
> '*setup-time': 'int',
> '*cpu-throttle-percentage': 'int',
> - '*error-desc': 'str'} }
> + '*error-desc': 'str',
> + '*postcopy-blocktime' : 'int64',
> + '*postcopy-vcpu-blocktime': ['int64']} }
>
> ##
> # @query-migrate:
> --
> 1.9.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 10/10] migration: add postcopy total blocktime into query-migrate
2017-09-21 12:42 ` Dr. David Alan Gilbert
@ 2017-09-21 15:24 ` Alexey Perevalov
2017-09-21 16:44 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 27+ messages in thread
From: Alexey Perevalov @ 2017-09-21 15:24 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
On 09/21/2017 03:42 PM, Dr. David Alan Gilbert wrote:
> * Alexey Perevalov (a.perevalov@samsung.com) wrote:
>> Postcopy total blocktime is available on destination side only.
>> But query-migrate was possible only for source. This patch
>> adds ability to call query-migrate on destination.
>> To be able to see postcopy blocktime, need to request postcopy-blocktime
>> capability.
>>
>> The query-migrate command will show following sample result:
>> {"return":
>> "postcopy-vcpu-blocktime": [115, 100],
>> "status": "completed",
>> "postcopy-blocktime": 100
>> }}
>>
>> postcopy_vcpu_blocktime contains list, where the first item is the first
>> vCPU in QEMU.
>>
>> This patch has a drawback, it combines states of incoming and
>> outgoing migration. Ongoing migration state will overwrite incoming
>> state. Looks like better to separate query-migrate for incoming and
>> outgoing migration or add parameter to indicate type of migration.
>>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
>> ---
>> hmp.c | 15 +++++++++++++
>> migration/migration.c | 42 +++++++++++++++++++++++++++++++----
>> migration/migration.h | 4 ++++
>> migration/postcopy-ram.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
>> migration/trace-events | 1 +
>> qapi/migration.json | 11 +++++++++-
>> 6 files changed, 125 insertions(+), 5 deletions(-)
>>
>> diff --git a/hmp.c b/hmp.c
>> index 0fb2bc7..142f76e 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -264,6 +264,21 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>> info->cpu_throttle_percentage);
>> }
>>
>> + if (info->has_postcopy_blocktime) {
>> + monitor_printf(mon, "postcopy blocktime: %" PRId64 "\n",
>> + info->postcopy_blocktime);
>> + }
>> +
>> + if (info->has_postcopy_vcpu_blocktime) {
>> + Visitor *v;
>> + char *str;
>> + v = string_output_visitor_new(false, &str);
>> + visit_type_int64List(v, NULL, &info->postcopy_vcpu_blocktime, NULL);
>> + visit_complete(v, &str);
>> + monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
>> + g_free(str);
>> + visit_free(v);
>> + }
>> qapi_free_MigrationInfo(info);
>> qapi_free_MigrationCapabilityStatusList(caps);
>> }
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 4f029e8..e1d3248 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -528,14 +528,15 @@ static void populate_disk_info(MigrationInfo *info)
>> }
>> }
>>
>> -MigrationInfo *qmp_query_migrate(Error **errp)
>> +static void fill_source_migration_info(MigrationInfo *info)
>> {
>> - MigrationInfo *info = g_malloc0(sizeof(*info));
>> MigrationState *s = migrate_get_current();
>>
>> switch (s->state) {
>> case MIGRATION_STATUS_NONE:
>> /* no migration has happened ever */
>> + /* do not overwrite destination migration status */
>> + return;
>> break;
>> case MIGRATION_STATUS_SETUP:
>> info->has_status = true;
>> @@ -584,8 +585,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>> break;
>> }
>> info->status = s->state;
>> -
>> - return info;
>> }
>>
>> /**
>> @@ -649,6 +648,41 @@ static bool migrate_caps_check(bool *cap_list,
>> return true;
>> }
>>
>> +static void fill_destination_migration_info(MigrationInfo *info)
>> +{
>> + MigrationIncomingState *mis = migration_incoming_get_current();
>> +
>> + switch (mis->state) {
>> + case MIGRATION_STATUS_NONE:
>> + return;
>> + break;
>> + case MIGRATION_STATUS_SETUP:
>> + case MIGRATION_STATUS_CANCELLING:
>> + case MIGRATION_STATUS_CANCELLED:
>> + case MIGRATION_STATUS_ACTIVE:
>> + case MIGRATION_STATUS_POSTCOPY_ACTIVE:
>> + case MIGRATION_STATUS_FAILED:
>> + case MIGRATION_STATUS_COLO:
>> + info->has_status = true;
>> + break;
>> + case MIGRATION_STATUS_COMPLETED:
>> + info->has_status = true;
>> + fill_destination_postcopy_migration_info(info);
>> + break;
>> + }
>> + info->status = mis->state;
>> +}
>> +
>> +MigrationInfo *qmp_query_migrate(Error **errp)
>> +{
>> + MigrationInfo *info = g_malloc0(sizeof(*info));
>> +
>> + fill_destination_migration_info(info);
>> + fill_source_migration_info(info);
>> +
>> + return info;
>> +}
>> +
>> void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
>> Error **errp)
>> {
>> diff --git a/migration/migration.h b/migration/migration.h
>> index 770466b..882a59b 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -70,6 +70,10 @@ struct MigrationIncomingState {
>>
>> MigrationIncomingState *migration_incoming_get_current(void);
>> void migration_incoming_state_destroy(void);
>> +/*
>> + * Functions to work with blocktime context
>> + */
>> +void fill_destination_postcopy_migration_info(MigrationInfo *info);
>>
>> #define TYPE_MIGRATION "migration"
>>
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index 9a5133f..5fdbf1e 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -113,6 +113,55 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
>> return ctx;
>> }
>>
>> +static int64List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
>> +{
>> + int64List *list = NULL, *entry = NULL;
>> + int i;
>> +
>> + for (i = smp_cpus - 1; i >= 0; i--) {
>> + entry = g_new0(int64List, 1);
>> + entry->value = ctx->vcpu_blocktime[i];
>> + entry->next = list;
>> + list = entry;
>> + }
>> +
>> + return list;
>> +}
>> +
>> +/*
>> + * This function just populates MigrationInfo from postcopy's
>> + * blocktime context. It will not populate MigrationInfo,
>> + * unless postcopy-blocktime capability was set.
>> + *
>> + * @info: pointer to MigrationInfo to populate
>> + */
>> +void fill_destination_postcopy_migration_info(MigrationInfo *info)
>> +{
>> + MigrationIncomingState *mis = migration_incoming_get_current();
>> + PostcopyBlocktimeContext *bc = mis->blocktime_ctx;
>> +
>> + if (!bc) {
>> + return;
>> + }
>> +
>> + info->has_postcopy_blocktime = true;
>> + info->postcopy_blocktime = bc->total_blocktime;
>> + info->has_postcopy_vcpu_blocktime = true;
>> + info->postcopy_vcpu_blocktime = get_vcpu_blocktime_list(bc);
>> +}
>> +
>> +static uint64_t get_postcopy_total_blocktime(void)
>> +{
>> + MigrationIncomingState *mis = migration_incoming_get_current();
>> + PostcopyBlocktimeContext *bc = mis->blocktime_ctx;
>> +
>> + if (!bc) {
>> + return 0;
>> + }
>> +
>> + return bc->total_blocktime;
>> +}
>> +
>> /**
>> * receive_ufd_features: check userfault fd features, to request only supported
>> * features in the future.
>> @@ -487,6 +536,9 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
>> munmap(mis->postcopy_tmp_zero_page, mis->largest_page_size);
>> mis->postcopy_tmp_zero_page = NULL;
>> }
>> + trace_postcopy_ram_incoming_cleanup_blocktime(
>> + get_postcopy_total_blocktime());
>> +
>> trace_postcopy_ram_incoming_cleanup_exit();
>> return 0;
>> }
>> @@ -958,6 +1010,11 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
>>
>> #else
>> /* No target OS support, stubs just fail */
>> +void fill_destination_postcopy_migration_info(MigrationInfo *info)
>> +{
>> + error_report("%s: No OS support", __func__);
>> +}
>> +
> Do we want that error_report? info migrate shouldn't give an error on a
> non-postcopy host.
>
> Also, don't you fancy just checking for the presence of this new info in
> the test?
I'll add assert, like that
g_assert(qdict_haskey(rsp_return, "postcopy-blocktime"));
when host supports UFFD_FEATURE_THREAD_ID
--
Best regards,
Alexey Perevalov
>
> Dave
>
>> bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
>> {
>> error_report("%s: No OS support", __func__);
>> diff --git a/migration/trace-events b/migration/trace-events
>> index 01f30fe..741f2ae 100644
>> --- a/migration/trace-events
>> +++ b/migration/trace-events
>> @@ -197,6 +197,7 @@ postcopy_ram_incoming_cleanup_closeuf(void) ""
>> postcopy_ram_incoming_cleanup_entry(void) ""
>> postcopy_ram_incoming_cleanup_exit(void) ""
>> postcopy_ram_incoming_cleanup_join(void) ""
>> +postcopy_ram_incoming_cleanup_blocktime(uint64_t total) "total blocktime %" PRIu64
>> save_xbzrle_page_skipping(void) ""
>> save_xbzrle_page_overflow(void) ""
>> ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations"
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 2e4a15d..55b055e 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -150,6 +150,13 @@
>> # @status is 'failed'. Clients should not attempt to parse the
>> # error strings. (Since 2.7)
>> #
>> +# @postcopy-blocktime: total time when all vCPU were blocked during postcopy
>> +# live migration (Since 2.11)
>> +#
>> +# @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU (Since 2.10)
>> +#
>> +
>> +#
>> # Since: 0.14.0
>> ##
>> { 'struct': 'MigrationInfo',
>> @@ -161,7 +168,9 @@
>> '*downtime': 'int',
>> '*setup-time': 'int',
>> '*cpu-throttle-percentage': 'int',
>> - '*error-desc': 'str'} }
>> + '*error-desc': 'str',
>> + '*postcopy-blocktime' : 'int64',
>> + '*postcopy-vcpu-blocktime': ['int64']} }
>>
>> ##
>> # @query-migrate:
>> --
>> 1.9.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v10 10/10] migration: add postcopy total blocktime into query-migrate
2017-09-21 15:24 ` Alexey Perevalov
@ 2017-09-21 16:44 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 27+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-21 16:44 UTC (permalink / raw)
To: Alexey Perevalov
Cc: qemu-devel, i.maximets, peterx, eblake, quintela, heetae82.ahn
* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> On 09/21/2017 03:42 PM, Dr. David Alan Gilbert wrote:
> > * Alexey Perevalov (a.perevalov@samsung.com) wrote:
> > > Postcopy total blocktime is available on destination side only.
> > > But query-migrate was possible only for source. This patch
> > > adds ability to call query-migrate on destination.
> > > To be able to see postcopy blocktime, need to request postcopy-blocktime
> > > capability.
> > >
> > > The query-migrate command will show following sample result:
> > > {"return":
> > > "postcopy-vcpu-blocktime": [115, 100],
> > > "status": "completed",
> > > "postcopy-blocktime": 100
> > > }}
> > >
> > > postcopy_vcpu_blocktime contains list, where the first item is the first
> > > vCPU in QEMU.
> > >
> > > This patch has a drawback, it combines states of incoming and
> > > outgoing migration. Ongoing migration state will overwrite incoming
> > > state. Looks like better to separate query-migrate for incoming and
> > > outgoing migration or add parameter to indicate type of migration.
> > >
> > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> > > ---
> > > hmp.c | 15 +++++++++++++
> > > migration/migration.c | 42 +++++++++++++++++++++++++++++++----
> > > migration/migration.h | 4 ++++
> > > migration/postcopy-ram.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > migration/trace-events | 1 +
> > > qapi/migration.json | 11 +++++++++-
> > > 6 files changed, 125 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/hmp.c b/hmp.c
> > > index 0fb2bc7..142f76e 100644
> > > --- a/hmp.c
> > > +++ b/hmp.c
> > > @@ -264,6 +264,21 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
> > > info->cpu_throttle_percentage);
> > > }
> > > + if (info->has_postcopy_blocktime) {
> > > + monitor_printf(mon, "postcopy blocktime: %" PRId64 "\n",
> > > + info->postcopy_blocktime);
> > > + }
> > > +
> > > + if (info->has_postcopy_vcpu_blocktime) {
> > > + Visitor *v;
> > > + char *str;
> > > + v = string_output_visitor_new(false, &str);
> > > + visit_type_int64List(v, NULL, &info->postcopy_vcpu_blocktime, NULL);
> > > + visit_complete(v, &str);
> > > + monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
> > > + g_free(str);
> > > + visit_free(v);
> > > + }
> > > qapi_free_MigrationInfo(info);
> > > qapi_free_MigrationCapabilityStatusList(caps);
> > > }
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index 4f029e8..e1d3248 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -528,14 +528,15 @@ static void populate_disk_info(MigrationInfo *info)
> > > }
> > > }
> > > -MigrationInfo *qmp_query_migrate(Error **errp)
> > > +static void fill_source_migration_info(MigrationInfo *info)
> > > {
> > > - MigrationInfo *info = g_malloc0(sizeof(*info));
> > > MigrationState *s = migrate_get_current();
> > > switch (s->state) {
> > > case MIGRATION_STATUS_NONE:
> > > /* no migration has happened ever */
> > > + /* do not overwrite destination migration status */
> > > + return;
> > > break;
> > > case MIGRATION_STATUS_SETUP:
> > > info->has_status = true;
> > > @@ -584,8 +585,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> > > break;
> > > }
> > > info->status = s->state;
> > > -
> > > - return info;
> > > }
> > > /**
> > > @@ -649,6 +648,41 @@ static bool migrate_caps_check(bool *cap_list,
> > > return true;
> > > }
> > > +static void fill_destination_migration_info(MigrationInfo *info)
> > > +{
> > > + MigrationIncomingState *mis = migration_incoming_get_current();
> > > +
> > > + switch (mis->state) {
> > > + case MIGRATION_STATUS_NONE:
> > > + return;
> > > + break;
> > > + case MIGRATION_STATUS_SETUP:
> > > + case MIGRATION_STATUS_CANCELLING:
> > > + case MIGRATION_STATUS_CANCELLED:
> > > + case MIGRATION_STATUS_ACTIVE:
> > > + case MIGRATION_STATUS_POSTCOPY_ACTIVE:
> > > + case MIGRATION_STATUS_FAILED:
> > > + case MIGRATION_STATUS_COLO:
> > > + info->has_status = true;
> > > + break;
> > > + case MIGRATION_STATUS_COMPLETED:
> > > + info->has_status = true;
> > > + fill_destination_postcopy_migration_info(info);
> > > + break;
> > > + }
> > > + info->status = mis->state;
> > > +}
> > > +
> > > +MigrationInfo *qmp_query_migrate(Error **errp)
> > > +{
> > > + MigrationInfo *info = g_malloc0(sizeof(*info));
> > > +
> > > + fill_destination_migration_info(info);
> > > + fill_source_migration_info(info);
> > > +
> > > + return info;
> > > +}
> > > +
> > > void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> > > Error **errp)
> > > {
> > > diff --git a/migration/migration.h b/migration/migration.h
> > > index 770466b..882a59b 100644
> > > --- a/migration/migration.h
> > > +++ b/migration/migration.h
> > > @@ -70,6 +70,10 @@ struct MigrationIncomingState {
> > > MigrationIncomingState *migration_incoming_get_current(void);
> > > void migration_incoming_state_destroy(void);
> > > +/*
> > > + * Functions to work with blocktime context
> > > + */
> > > +void fill_destination_postcopy_migration_info(MigrationInfo *info);
> > > #define TYPE_MIGRATION "migration"
> > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > > index 9a5133f..5fdbf1e 100644
> > > --- a/migration/postcopy-ram.c
> > > +++ b/migration/postcopy-ram.c
> > > @@ -113,6 +113,55 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
> > > return ctx;
> > > }
> > > +static int64List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
> > > +{
> > > + int64List *list = NULL, *entry = NULL;
> > > + int i;
> > > +
> > > + for (i = smp_cpus - 1; i >= 0; i--) {
> > > + entry = g_new0(int64List, 1);
> > > + entry->value = ctx->vcpu_blocktime[i];
> > > + entry->next = list;
> > > + list = entry;
> > > + }
> > > +
> > > + return list;
> > > +}
> > > +
> > > +/*
> > > + * This function just populates MigrationInfo from postcopy's
> > > + * blocktime context. It will not populate MigrationInfo,
> > > + * unless postcopy-blocktime capability was set.
> > > + *
> > > + * @info: pointer to MigrationInfo to populate
> > > + */
> > > +void fill_destination_postcopy_migration_info(MigrationInfo *info)
> > > +{
> > > + MigrationIncomingState *mis = migration_incoming_get_current();
> > > + PostcopyBlocktimeContext *bc = mis->blocktime_ctx;
> > > +
> > > + if (!bc) {
> > > + return;
> > > + }
> > > +
> > > + info->has_postcopy_blocktime = true;
> > > + info->postcopy_blocktime = bc->total_blocktime;
> > > + info->has_postcopy_vcpu_blocktime = true;
> > > + info->postcopy_vcpu_blocktime = get_vcpu_blocktime_list(bc);
> > > +}
> > > +
> > > +static uint64_t get_postcopy_total_blocktime(void)
> > > +{
> > > + MigrationIncomingState *mis = migration_incoming_get_current();
> > > + PostcopyBlocktimeContext *bc = mis->blocktime_ctx;
> > > +
> > > + if (!bc) {
> > > + return 0;
> > > + }
> > > +
> > > + return bc->total_blocktime;
> > > +}
> > > +
> > > /**
> > > * receive_ufd_features: check userfault fd features, to request only supported
> > > * features in the future.
> > > @@ -487,6 +536,9 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
> > > munmap(mis->postcopy_tmp_zero_page, mis->largest_page_size);
> > > mis->postcopy_tmp_zero_page = NULL;
> > > }
> > > + trace_postcopy_ram_incoming_cleanup_blocktime(
> > > + get_postcopy_total_blocktime());
> > > +
> > > trace_postcopy_ram_incoming_cleanup_exit();
> > > return 0;
> > > }
> > > @@ -958,6 +1010,11 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
> > > #else
> > > /* No target OS support, stubs just fail */
> > > +void fill_destination_postcopy_migration_info(MigrationInfo *info)
> > > +{
> > > + error_report("%s: No OS support", __func__);
> > > +}
> > > +
> > Do we want that error_report? info migrate shouldn't give an error on a
> > non-postcopy host.
> >
> > Also, don't you fancy just checking for the presence of this new info in
> > the test?
> I'll add assert, like that
> g_assert(qdict_haskey(rsp_return, "postcopy-blocktime"));
Yes, that seems reasonable.
> when host supports UFFD_FEATURE_THREAD_ID
Great; note that needs to be runtime, not the ifdef.
Dave
> --
> Best regards,
> Alexey Perevalov
> >
> > Dave
> >
> > > bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
> > > {
> > > error_report("%s: No OS support", __func__);
> > > diff --git a/migration/trace-events b/migration/trace-events
> > > index 01f30fe..741f2ae 100644
> > > --- a/migration/trace-events
> > > +++ b/migration/trace-events
> > > @@ -197,6 +197,7 @@ postcopy_ram_incoming_cleanup_closeuf(void) ""
> > > postcopy_ram_incoming_cleanup_entry(void) ""
> > > postcopy_ram_incoming_cleanup_exit(void) ""
> > > postcopy_ram_incoming_cleanup_join(void) ""
> > > +postcopy_ram_incoming_cleanup_blocktime(uint64_t total) "total blocktime %" PRIu64
> > > save_xbzrle_page_skipping(void) ""
> > > save_xbzrle_page_overflow(void) ""
> > > ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations"
> > > diff --git a/qapi/migration.json b/qapi/migration.json
> > > index 2e4a15d..55b055e 100644
> > > --- a/qapi/migration.json
> > > +++ b/qapi/migration.json
> > > @@ -150,6 +150,13 @@
> > > # @status is 'failed'. Clients should not attempt to parse the
> > > # error strings. (Since 2.7)
> > > #
> > > +# @postcopy-blocktime: total time when all vCPU were blocked during postcopy
> > > +# live migration (Since 2.11)
> > > +#
> > > +# @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU (Since 2.10)
> > > +#
> > > +
> > > +#
> > > # Since: 0.14.0
> > > ##
> > > { 'struct': 'MigrationInfo',
> > > @@ -161,7 +168,9 @@
> > > '*downtime': 'int',
> > > '*setup-time': 'int',
> > > '*cpu-throttle-percentage': 'int',
> > > - '*error-desc': 'str'} }
> > > + '*error-desc': 'str',
> > > + '*postcopy-blocktime' : 'int64',
> > > + '*postcopy-vcpu-blocktime': ['int64']} }
> > > ##
> > > # @query-migrate:
> > > --
> > > 1.9.1
> > >
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
> >
> >
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 27+ messages in thread