All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v9 0/8] calculate blocktime for postcopy live migration
       [not found] <CGME20170616191218eucas1p2bcc5ece2b03593f16ab599c4f12eaa5e@eucas1p2.samsung.com>
@ 2017-06-16 19:11 ` Alexey Perevalov
       [not found]   ` <CGME20170616191219eucas1p197a274ce1b480821ac4813cde42aeb74@eucas1p1.samsung.com>
                     ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Alexey Perevalov @ 2017-06-16 19:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Perevalov, peterx, i.maximets, quintela, dgilbert

This is 9th version.

The rationale for that idea is following:
vCPU could suspend during postcopy live migration until faulted
page is not copied into kernel. Downtime on source side it's a value -
time interval since source turn vCPU off, till destination start runnig
vCPU. But that value was proper value for precopy migration it really shows
amount of time when vCPU is down. But not for postcopy migration, because
several vCPU threads could susppend after vCPU was started. That is important
to estimate packet drop for SDN software.

(V8 -> V9)
    - rebase
    - traces

(V7 -> V8)
    - just one comma in
"migration: fix hardcoded function name in error report"
It was really missed, but fixed in futher patch.

(V6 -> V7)
    - copied bitmap was placed into RAMBlock as another migration
related bitmaps.
    - Ordering of mark_postcopy_blocktime_end call and ordering
of checking copied bitmap were changed.
    - linewrap style defects
    - new patch "postcopy_place_page factoring out"
    - postcopy_ram_supported_by_host accepts
MigrationIncomingState in qmp_migrate_set_capabilities
    - minor fixes of documentation. 
    and huge description of get_postcopy_total_blocktime was
moved. Davids comment.

(V5 -> V6)
    - blocktime was added into hmp command. Comment from David.
    - bitmap for copied pages was added as well as check in *_begin/_end
functions. Patch uses just introduced RAMBLOCK_FOREACH. Comment from David.
    - description of receive_ufd_features/request_ufd_features. Comment from David.
    - commit message headers/@since references were modified. Comment from Eric.
    - also typos in documentation. Comment from Eric.
    - style and description of field in MigrationInfo. Comment from Eric.
    - ufd_check_and_apply (former ufd_version_check) is calling twice,
so my previous patch contained double allocation of blocktime context and
as a result memory leak. In this patch series it was fixed.

(V4 -> V5)
    - fill_destination_postcopy_migration_info empty stub was missed for none linux
build

(V3 -> V4)
    - get rid of Downtime as a name for vCPU waiting time during postcopy migration
    - PostcopyBlocktimeContext renamed (it was just BlocktimeContext)
    - atomic operations are used for dealing with fields of PostcopyBlocktimeContext
affected in both threads.
    - hardcoded function names in error_report were replaced to %s and __line__
    - this patch set includes postcopy-downtime capability, but it used on
destination, coupled with not possibility to return calculated downtime back
to source to show it in query-migrate, it looks like a big trade off
    - UFFD_API have to be sent notwithstanding need or not to ask kernel
for a feature, due to kernel expects it in any case (see patch comment)
    - postcopy_downtime included into query-migrate output
    - also this patch set includes trivial fix
migration: fix hardcoded function name in error report
maybe that is a candidate for qemu-trivial mailing list, but I already
sent "migration: Fixed code style" and it was unclaimed.

(V2 -> V3)
    - Downtime calculation approach was changed, thanks to Peter Xu
    - Due to previous point no more need to keep GTree as well as bitmap of cpus.
So glib changes aren't included in this patch set, it could be resent in
another patch set, if it will be a good reason for it.
    - No procfs traces in this patchset, if somebody wants it, you could get it
from patchwork site to track down page fault initiators.
    - UFFD_FEATURE_THREAD_ID is requesting only when kernel supports it
    - It doesn't send back the downtime, just trace it

This patch set is based on commit
[PATCH v3 0/3] Add bitmap for received pages in postcopy migration


Alexey Perevalov (8):
  userfault: add pid into uffd_msg & update UFFD_FEATURE_*
  migration: pass MigrationIncomingState* into migration check functions
  migration: fix hardcoded function name in error report
  migration: split ufd_version_check onto receive/request features part
  migration: introduce postcopy-blocktime capability
  migration: add postcopy blocktime ctx into MigrationIncomingState
  migration: calculate vCPU blocktime on dst side
  migration: postcopy_blocktime documentation

 docs/devel/migration.txt          |  10 ++
 linux-headers/linux/userfaultfd.h |   4 +
 migration/migration.c             |  12 +-
 migration/migration.h             |   9 ++
 migration/postcopy-ram.c          | 300 ++++++++++++++++++++++++++++++++++++--
 migration/postcopy-ram.h          |   2 +-
 migration/savevm.c                |   2 +-
 migration/trace-events            |   5 +-
 qapi-schema.json                  |   5 +-
 9 files changed, 334 insertions(+), 15 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v9 1/8] userfault: add pid into uffd_msg & update UFFD_FEATURE_*
       [not found]   ` <CGME20170616191219eucas1p197a274ce1b480821ac4813cde42aeb74@eucas1p1.samsung.com>
@ 2017-06-16 19:11     ` Alexey Perevalov
  0 siblings, 0 replies; 12+ messages in thread
From: Alexey Perevalov @ 2017-06-16 19:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Perevalov, peterx, i.maximets, quintela, dgilbert

This commit duplicates header of "userfaultfd: provide pid in userfault msg"
into linux kernel.

Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 linux-headers/linux/userfaultfd.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/linux-headers/linux/userfaultfd.h b/linux-headers/linux/userfaultfd.h
index 9701772..eda028c 100644
--- a/linux-headers/linux/userfaultfd.h
+++ b/linux-headers/linux/userfaultfd.h
@@ -78,6 +78,9 @@ struct uffd_msg {
 		struct {
 			__u64	flags;
 			__u64	address;
+			union {
+				__u32   ptid;
+			} feat;
 		} pagefault;
 
 		struct {
@@ -161,6 +164,7 @@ 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_THREAD_ID			(1<<7)
 	__u64 features;
 
 	__u64 ioctls;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v9 2/8] migration: pass MigrationIncomingState* into migration check functions
       [not found]   ` <CGME20170616191220eucas1p2ac6347288c68b20113882eaa38f34d5f@eucas1p2.samsung.com>
@ 2017-06-16 19:11     ` Alexey Perevalov
  0 siblings, 0 replies; 12+ messages in thread
From: Alexey Perevalov @ 2017-06-16 19:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Perevalov, peterx, i.maximets, quintela, dgilbert

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 53fbd41..56929dc 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -591,6 +591,7 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
                                   Error **errp)
 {
     MigrationState *s = migrate_get_current();
+    MigrationIncomingState *mis = migration_incoming_get_current();
     MigrationCapabilityStatusList *cap;
     bool old_postcopy_cap = migrate_postcopy_ram();
 
@@ -637,7 +638,7 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
          * 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 5d2b92d..35f3ffd 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -63,7 +63,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;
@@ -126,7 +126,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;
@@ -149,7 +149,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;
     }
 
@@ -525,7 +525,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;
     }
 
@@ -678,7 +678,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 31158da..4c3b75e 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1367,7 +1367,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.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v9 3/8] migration: fix hardcoded function name in error report
       [not found]   ` <CGME20170616191221eucas1p18f1b21abee06f0a34c2a8eb5fb0c6004@eucas1p1.samsung.com>
@ 2017-06-16 19:12     ` Alexey Perevalov
  0 siblings, 0 replies; 12+ messages in thread
From: Alexey Perevalov @ 2017-06-16 19:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Perevalov, peterx, i.maximets, quintela, dgilbert

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 35f3ffd..b35baf3 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -71,7 +71,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.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v9 4/8] migration: split ufd_version_check onto receive/request features part
       [not found]   ` <CGME20170616191222eucas1p2fc88d38fb9f84f7da04d853811dd07db@eucas1p2.samsung.com>
@ 2017-06-16 19:12     ` Alexey Perevalov
  0 siblings, 0 replies; 12+ messages in thread
From: Alexey Perevalov @ 2017-06-16 19:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Perevalov, peterx, i.maximets, quintela, dgilbert

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 b35baf3..d152f1a 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -63,16 +63,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;
     }
 
@@ -84,11 +135,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");
@@ -149,7 +231,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;
     }
 
@@ -525,7 +607,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.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v9 5/8] migration: introduce postcopy-blocktime capability
       [not found]   ` <CGME20170616191222eucas1p186aab01cb769c9c2f7a1b61be48cd9a2@eucas1p1.samsung.com>
@ 2017-06-16 19:12     ` Alexey Perevalov
  0 siblings, 0 replies; 12+ messages in thread
From: Alexey Perevalov @ 2017-06-16 19:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Perevalov, peterx, i.maximets, quintela, dgilbert

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-schema.json      | 5 ++++-
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 56929dc..f920ddf 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1233,6 +1233,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 601e4ab..cf4d04e 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -152,6 +152,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-schema.json b/qapi-schema.json
index 4b50b65..e906953 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -900,12 +900,15 @@
 #          offers more flexibility.
 #          (Since 2.10)
 #
+# @postcopy-blocktime: Calculate downtime for postcopy live migration
+#                     (since 2.10)
+#
 # Since: 1.2
 ##
 { 'enum': 'MigrationCapability',
   'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
            'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
-           'block' ] }
+           'block', 'postcopy-blocktime'] }
 
 ##
 # @MigrationCapabilityStatus:
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v9 6/8] migration: add postcopy blocktime ctx into MigrationIncomingState
       [not found]   ` <CGME20170616191223eucas1p26f48ecbb0f533561e9c43bbac1da3e04@eucas1p2.samsung.com>
@ 2017-06-16 19:12     ` Alexey Perevalov
  0 siblings, 0 replies; 12+ messages in thread
From: Alexey Perevalov @ 2017-06-16 19:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Perevalov, peterx, i.maximets, quintela, dgilbert

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 cf4d04e..2697f6c 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -22,6 +22,8 @@
 #include "exec/cpu-common.h"
 #include "qemu/coroutine_int.h"
 
+struct PostcopyBlocktimeContext;
+
 /* State for the incoming migration */
 struct MigrationIncomingState {
     QEMUFile *from_src_file;
@@ -59,6 +61,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 d152f1a..990d143 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -63,6 +63,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
@@ -155,6 +207,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.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v9 7/8] migration: calculate vCPU blocktime on dst side
       [not found]   ` <CGME20170616191223eucas1p14025ca2f7c70d0edcad51f05458e0d9c@eucas1p1.samsung.com>
@ 2017-06-16 19:12     ` Alexey Perevalov
  0 siblings, 0 replies; 12+ messages in thread
From: Alexey Perevalov @ 2017-06-16 19:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Perevalov, peterx, i.maximets, quintela, dgilbert

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 | 137 ++++++++++++++++++++++++++++++++++++++++++++++-
 migration/trace-events   |   5 +-
 2 files changed, 139 insertions(+), 3 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 990d143..3f36e4f 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -112,7 +112,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;
 }
 
@@ -561,6 +560,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((void *)addr, rb);
+    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
  */
@@ -638,8 +767,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)
@@ -716,6 +848,7 @@ static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
      * but gup betwean two following operation could be high,
      * and in this case blocktime for such small interval will be lost */
     ramblock_recv_bitmap_set(host_addr, rb);
+    mark_postcopy_blocktime_end((uint64_t)(uintptr_t)host_addr);
     if (from_addr) {
         struct uffdio_copy copy_struct;
         copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
diff --git a/migration/trace-events b/migration/trace-events
index cb2c4b5..13e6ecd 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=%" PRIx64 " rb=%s offset=%zx"
+postcopy_ram_fault_thread_request(uint64_t hostaddr, const char *ramblock, size_t offset, uint32_t pid) "Request for HVA=%" PRIx64 " rb=%s offset=%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.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v9 8/8] migration: postcopy_blocktime documentation
       [not found]   ` <CGME20170616191224eucas1p2e8c1ff33f630b6beaff096b73ab3462b@eucas1p2.samsung.com>
@ 2017-06-16 19:12     ` Alexey Perevalov
  0 siblings, 0 replies; 12+ messages in thread
From: Alexey Perevalov @ 2017-06-16 19:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Perevalov, peterx, i.maximets, quintela, dgilbert

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v9 0/8] calculate blocktime for postcopy live migration
  2017-06-16 19:11 ` [Qemu-devel] [PATCH v9 0/8] calculate blocktime for postcopy live migration Alexey Perevalov
                     ` (7 preceding siblings ...)
       [not found]   ` <CGME20170616191224eucas1p2e8c1ff33f630b6beaff096b73ab3462b@eucas1p2.samsung.com>
@ 2017-09-18 11:15   ` Dr. David Alan Gilbert
  2017-09-18 15:23     ` Alexey Perevalov
  8 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-18 11:15 UTC (permalink / raw)
  To: Alexey Perevalov; +Cc: qemu-devel, peterx, i.maximets, quintela

* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> This is 9th version.
> 
> The rationale for that idea is following:
> vCPU could suspend during postcopy live migration until faulted
> page is not copied into kernel. Downtime on source side it's a value -
> time interval since source turn vCPU off, till destination start runnig
> vCPU. But that value was proper value for precopy migration it really shows
> amount of time when vCPU is down. But not for postcopy migration, because
> several vCPU threads could susppend after vCPU was started. That is important
> to estimate packet drop for SDN software.

Hi Alexey,
  I see that the UFFD_FEATURE_THREAD_ID has landed in kernel v4.14-rc1
over the weekend, so it's probably time to reheat this patchset.

  I think you should be able to generate a first patch by running
  scripts/update-linux-headers.sh

Dave

> (V8 -> V9)
>     - rebase
>     - traces
> 
> (V7 -> V8)
>     - just one comma in
> "migration: fix hardcoded function name in error report"
> It was really missed, but fixed in futher patch.
> 
> (V6 -> V7)
>     - copied bitmap was placed into RAMBlock as another migration
> related bitmaps.
>     - Ordering of mark_postcopy_blocktime_end call and ordering
> of checking copied bitmap were changed.
>     - linewrap style defects
>     - new patch "postcopy_place_page factoring out"
>     - postcopy_ram_supported_by_host accepts
> MigrationIncomingState in qmp_migrate_set_capabilities
>     - minor fixes of documentation. 
>     and huge description of get_postcopy_total_blocktime was
> moved. Davids comment.
> 
> (V5 -> V6)
>     - blocktime was added into hmp command. Comment from David.
>     - bitmap for copied pages was added as well as check in *_begin/_end
> functions. Patch uses just introduced RAMBLOCK_FOREACH. Comment from David.
>     - description of receive_ufd_features/request_ufd_features. Comment from David.
>     - commit message headers/@since references were modified. Comment from Eric.
>     - also typos in documentation. Comment from Eric.
>     - style and description of field in MigrationInfo. Comment from Eric.
>     - ufd_check_and_apply (former ufd_version_check) is calling twice,
> so my previous patch contained double allocation of blocktime context and
> as a result memory leak. In this patch series it was fixed.
> 
> (V4 -> V5)
>     - fill_destination_postcopy_migration_info empty stub was missed for none linux
> build
> 
> (V3 -> V4)
>     - get rid of Downtime as a name for vCPU waiting time during postcopy migration
>     - PostcopyBlocktimeContext renamed (it was just BlocktimeContext)
>     - atomic operations are used for dealing with fields of PostcopyBlocktimeContext
> affected in both threads.
>     - hardcoded function names in error_report were replaced to %s and __line__
>     - this patch set includes postcopy-downtime capability, but it used on
> destination, coupled with not possibility to return calculated downtime back
> to source to show it in query-migrate, it looks like a big trade off
>     - UFFD_API have to be sent notwithstanding need or not to ask kernel
> for a feature, due to kernel expects it in any case (see patch comment)
>     - postcopy_downtime included into query-migrate output
>     - also this patch set includes trivial fix
> migration: fix hardcoded function name in error report
> maybe that is a candidate for qemu-trivial mailing list, but I already
> sent "migration: Fixed code style" and it was unclaimed.
> 
> (V2 -> V3)
>     - Downtime calculation approach was changed, thanks to Peter Xu
>     - Due to previous point no more need to keep GTree as well as bitmap of cpus.
> So glib changes aren't included in this patch set, it could be resent in
> another patch set, if it will be a good reason for it.
>     - No procfs traces in this patchset, if somebody wants it, you could get it
> from patchwork site to track down page fault initiators.
>     - UFFD_FEATURE_THREAD_ID is requesting only when kernel supports it
>     - It doesn't send back the downtime, just trace it
> 
> This patch set is based on commit
> [PATCH v3 0/3] Add bitmap for received pages in postcopy migration
> 
> 
> Alexey Perevalov (8):
>   userfault: add pid into uffd_msg & update UFFD_FEATURE_*
>   migration: pass MigrationIncomingState* into migration check functions
>   migration: fix hardcoded function name in error report
>   migration: split ufd_version_check onto receive/request features part
>   migration: introduce postcopy-blocktime capability
>   migration: add postcopy blocktime ctx into MigrationIncomingState
>   migration: calculate vCPU blocktime on dst side
>   migration: postcopy_blocktime documentation
> 
>  docs/devel/migration.txt          |  10 ++
>  linux-headers/linux/userfaultfd.h |   4 +
>  migration/migration.c             |  12 +-
>  migration/migration.h             |   9 ++
>  migration/postcopy-ram.c          | 300 ++++++++++++++++++++++++++++++++++++--
>  migration/postcopy-ram.h          |   2 +-
>  migration/savevm.c                |   2 +-
>  migration/trace-events            |   5 +-
>  qapi-schema.json                  |   5 +-
>  9 files changed, 334 insertions(+), 15 deletions(-)
> 
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v9 0/8] calculate blocktime for postcopy live migration
  2017-09-18 11:15   ` [Qemu-devel] [PATCH v9 0/8] calculate blocktime for postcopy live migration Dr. David Alan Gilbert
@ 2017-09-18 15:23     ` Alexey Perevalov
  2017-09-18 15:52       ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 12+ messages in thread
From: Alexey Perevalov @ 2017-09-18 15:23 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: qemu-devel, peterx, i.maximets, quintela

On 09/18/2017 02:15 PM, Dr. David Alan Gilbert wrote:
> * Alexey Perevalov (a.perevalov@samsung.com) wrote:
>> This is 9th version.
>>
>> The rationale for that idea is following:
>> vCPU could suspend during postcopy live migration until faulted
>> page is not copied into kernel. Downtime on source side it's a value -
>> time interval since source turn vCPU off, till destination start runnig
>> vCPU. But that value was proper value for precopy migration it really shows
>> amount of time when vCPU is down. But not for postcopy migration, because
>> several vCPU threads could susppend after vCPU was started. That is important
>> to estimate packet drop for SDN software.
> Hi Alexey,
>    I see that the UFFD_FEATURE_THREAD_ID has landed in kernel v4.14-rc1
> over the weekend, so it's probably time to reheat this patchset.
>
>    I think you should be able to generate a first patch by running
>    scripts/update-linux-headers.sh
Hi David,
ok, I'll resend it tomorrow,
I also added set capability postcopy-blocktime into tests/postcopy-test.c,
but I don't check the result of the qmp there,
I added it just to enable and test code path, is it ok for you?
>
> Dave
>
>> (V8 -> V9)
>>      - rebase
>>      - traces
>>
>> (V7 -> V8)
>>      - just one comma in
>> "migration: fix hardcoded function name in error report"
>> It was really missed, but fixed in futher patch.
>>
>> (V6 -> V7)
>>      - copied bitmap was placed into RAMBlock as another migration
>> related bitmaps.
>>      - Ordering of mark_postcopy_blocktime_end call and ordering
>> of checking copied bitmap were changed.
>>      - linewrap style defects
>>      - new patch "postcopy_place_page factoring out"
>>      - postcopy_ram_supported_by_host accepts
>> MigrationIncomingState in qmp_migrate_set_capabilities
>>      - minor fixes of documentation.
>>      and huge description of get_postcopy_total_blocktime was
>> moved. Davids comment.
>>
>> (V5 -> V6)
>>      - blocktime was added into hmp command. Comment from David.
>>      - bitmap for copied pages was added as well as check in *_begin/_end
>> functions. Patch uses just introduced RAMBLOCK_FOREACH. Comment from David.
>>      - description of receive_ufd_features/request_ufd_features. Comment from David.
>>      - commit message headers/@since references were modified. Comment from Eric.
>>      - also typos in documentation. Comment from Eric.
>>      - style and description of field in MigrationInfo. Comment from Eric.
>>      - ufd_check_and_apply (former ufd_version_check) is calling twice,
>> so my previous patch contained double allocation of blocktime context and
>> as a result memory leak. In this patch series it was fixed.
>>
>> (V4 -> V5)
>>      - fill_destination_postcopy_migration_info empty stub was missed for none linux
>> build
>>
>> (V3 -> V4)
>>      - get rid of Downtime as a name for vCPU waiting time during postcopy migration
>>      - PostcopyBlocktimeContext renamed (it was just BlocktimeContext)
>>      - atomic operations are used for dealing with fields of PostcopyBlocktimeContext
>> affected in both threads.
>>      - hardcoded function names in error_report were replaced to %s and __line__
>>      - this patch set includes postcopy-downtime capability, but it used on
>> destination, coupled with not possibility to return calculated downtime back
>> to source to show it in query-migrate, it looks like a big trade off
>>      - UFFD_API have to be sent notwithstanding need or not to ask kernel
>> for a feature, due to kernel expects it in any case (see patch comment)
>>      - postcopy_downtime included into query-migrate output
>>      - also this patch set includes trivial fix
>> migration: fix hardcoded function name in error report
>> maybe that is a candidate for qemu-trivial mailing list, but I already
>> sent "migration: Fixed code style" and it was unclaimed.
>>
>> (V2 -> V3)
>>      - Downtime calculation approach was changed, thanks to Peter Xu
>>      - Due to previous point no more need to keep GTree as well as bitmap of cpus.
>> So glib changes aren't included in this patch set, it could be resent in
>> another patch set, if it will be a good reason for it.
>>      - No procfs traces in this patchset, if somebody wants it, you could get it
>> from patchwork site to track down page fault initiators.
>>      - UFFD_FEATURE_THREAD_ID is requesting only when kernel supports it
>>      - It doesn't send back the downtime, just trace it
>>
>> This patch set is based on commit
>> [PATCH v3 0/3] Add bitmap for received pages in postcopy migration
>>
>>
>> Alexey Perevalov (8):
>>    userfault: add pid into uffd_msg & update UFFD_FEATURE_*
>>    migration: pass MigrationIncomingState* into migration check functions
>>    migration: fix hardcoded function name in error report
>>    migration: split ufd_version_check onto receive/request features part
>>    migration: introduce postcopy-blocktime capability
>>    migration: add postcopy blocktime ctx into MigrationIncomingState
>>    migration: calculate vCPU blocktime on dst side
>>    migration: postcopy_blocktime documentation
>>
>>   docs/devel/migration.txt          |  10 ++
>>   linux-headers/linux/userfaultfd.h |   4 +
>>   migration/migration.c             |  12 +-
>>   migration/migration.h             |   9 ++
>>   migration/postcopy-ram.c          | 300 ++++++++++++++++++++++++++++++++++++--
>>   migration/postcopy-ram.h          |   2 +-
>>   migration/savevm.c                |   2 +-
>>   migration/trace-events            |   5 +-
>>   qapi-schema.json                  |   5 +-
>>   9 files changed, 334 insertions(+), 15 deletions(-)
>>
>> -- 
>> 1.8.3.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
>
>

-- 
Best regards,
Alexey Perevalov

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v9 0/8] calculate blocktime for postcopy live migration
  2017-09-18 15:23     ` Alexey Perevalov
@ 2017-09-18 15:52       ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-18 15:52 UTC (permalink / raw)
  To: Alexey Perevalov; +Cc: qemu-devel, peterx, i.maximets, quintela

* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> On 09/18/2017 02:15 PM, Dr. David Alan Gilbert wrote:
> > * Alexey Perevalov (a.perevalov@samsung.com) wrote:
> > > This is 9th version.
> > > 
> > > The rationale for that idea is following:
> > > vCPU could suspend during postcopy live migration until faulted
> > > page is not copied into kernel. Downtime on source side it's a value -
> > > time interval since source turn vCPU off, till destination start runnig
> > > vCPU. But that value was proper value for precopy migration it really shows
> > > amount of time when vCPU is down. But not for postcopy migration, because
> > > several vCPU threads could susppend after vCPU was started. That is important
> > > to estimate packet drop for SDN software.
> > Hi Alexey,
> >    I see that the UFFD_FEATURE_THREAD_ID has landed in kernel v4.14-rc1
> > over the weekend, so it's probably time to reheat this patchset.
> > 
> >    I think you should be able to generate a first patch by running
> >    scripts/update-linux-headers.sh
> Hi David,
> ok, I'll resend it tomorrow,
> I also added set capability postcopy-blocktime into tests/postcopy-test.c,
> but I don't check the result of the qmp there,
> I added it just to enable and test code path, is it ok for you?

It'd be better if you just ready the value in the test via qmp; that
would mean it'd be a basic check it was OK, and should be pretty
easy to glue into postcopy-test.c

Dave

> > 
> > Dave
> > 
> > > (V8 -> V9)
> > >      - rebase
> > >      - traces
> > > 
> > > (V7 -> V8)
> > >      - just one comma in
> > > "migration: fix hardcoded function name in error report"
> > > It was really missed, but fixed in futher patch.
> > > 
> > > (V6 -> V7)
> > >      - copied bitmap was placed into RAMBlock as another migration
> > > related bitmaps.
> > >      - Ordering of mark_postcopy_blocktime_end call and ordering
> > > of checking copied bitmap were changed.
> > >      - linewrap style defects
> > >      - new patch "postcopy_place_page factoring out"
> > >      - postcopy_ram_supported_by_host accepts
> > > MigrationIncomingState in qmp_migrate_set_capabilities
> > >      - minor fixes of documentation.
> > >      and huge description of get_postcopy_total_blocktime was
> > > moved. Davids comment.
> > > 
> > > (V5 -> V6)
> > >      - blocktime was added into hmp command. Comment from David.
> > >      - bitmap for copied pages was added as well as check in *_begin/_end
> > > functions. Patch uses just introduced RAMBLOCK_FOREACH. Comment from David.
> > >      - description of receive_ufd_features/request_ufd_features. Comment from David.
> > >      - commit message headers/@since references were modified. Comment from Eric.
> > >      - also typos in documentation. Comment from Eric.
> > >      - style and description of field in MigrationInfo. Comment from Eric.
> > >      - ufd_check_and_apply (former ufd_version_check) is calling twice,
> > > so my previous patch contained double allocation of blocktime context and
> > > as a result memory leak. In this patch series it was fixed.
> > > 
> > > (V4 -> V5)
> > >      - fill_destination_postcopy_migration_info empty stub was missed for none linux
> > > build
> > > 
> > > (V3 -> V4)
> > >      - get rid of Downtime as a name for vCPU waiting time during postcopy migration
> > >      - PostcopyBlocktimeContext renamed (it was just BlocktimeContext)
> > >      - atomic operations are used for dealing with fields of PostcopyBlocktimeContext
> > > affected in both threads.
> > >      - hardcoded function names in error_report were replaced to %s and __line__
> > >      - this patch set includes postcopy-downtime capability, but it used on
> > > destination, coupled with not possibility to return calculated downtime back
> > > to source to show it in query-migrate, it looks like a big trade off
> > >      - UFFD_API have to be sent notwithstanding need or not to ask kernel
> > > for a feature, due to kernel expects it in any case (see patch comment)
> > >      - postcopy_downtime included into query-migrate output
> > >      - also this patch set includes trivial fix
> > > migration: fix hardcoded function name in error report
> > > maybe that is a candidate for qemu-trivial mailing list, but I already
> > > sent "migration: Fixed code style" and it was unclaimed.
> > > 
> > > (V2 -> V3)
> > >      - Downtime calculation approach was changed, thanks to Peter Xu
> > >      - Due to previous point no more need to keep GTree as well as bitmap of cpus.
> > > So glib changes aren't included in this patch set, it could be resent in
> > > another patch set, if it will be a good reason for it.
> > >      - No procfs traces in this patchset, if somebody wants it, you could get it
> > > from patchwork site to track down page fault initiators.
> > >      - UFFD_FEATURE_THREAD_ID is requesting only when kernel supports it
> > >      - It doesn't send back the downtime, just trace it
> > > 
> > > This patch set is based on commit
> > > [PATCH v3 0/3] Add bitmap for received pages in postcopy migration
> > > 
> > > 
> > > Alexey Perevalov (8):
> > >    userfault: add pid into uffd_msg & update UFFD_FEATURE_*
> > >    migration: pass MigrationIncomingState* into migration check functions
> > >    migration: fix hardcoded function name in error report
> > >    migration: split ufd_version_check onto receive/request features part
> > >    migration: introduce postcopy-blocktime capability
> > >    migration: add postcopy blocktime ctx into MigrationIncomingState
> > >    migration: calculate vCPU blocktime on dst side
> > >    migration: postcopy_blocktime documentation
> > > 
> > >   docs/devel/migration.txt          |  10 ++
> > >   linux-headers/linux/userfaultfd.h |   4 +
> > >   migration/migration.c             |  12 +-
> > >   migration/migration.h             |   9 ++
> > >   migration/postcopy-ram.c          | 300 ++++++++++++++++++++++++++++++++++++--
> > >   migration/postcopy-ram.h          |   2 +-
> > >   migration/savevm.c                |   2 +-
> > >   migration/trace-events            |   5 +-
> > >   qapi-schema.json                  |   5 +-
> > >   9 files changed, 334 insertions(+), 15 deletions(-)
> > > 
> > > -- 
> > > 1.8.3.1
> > > 
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> > 
> > 
> 
> -- 
> Best regards,
> Alexey Perevalov
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-09-18 15:52 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20170616191218eucas1p2bcc5ece2b03593f16ab599c4f12eaa5e@eucas1p2.samsung.com>
2017-06-16 19:11 ` [Qemu-devel] [PATCH v9 0/8] calculate blocktime for postcopy live migration Alexey Perevalov
     [not found]   ` <CGME20170616191219eucas1p197a274ce1b480821ac4813cde42aeb74@eucas1p1.samsung.com>
2017-06-16 19:11     ` [Qemu-devel] [PATCH v9 1/8] userfault: add pid into uffd_msg & update UFFD_FEATURE_* Alexey Perevalov
     [not found]   ` <CGME20170616191220eucas1p2ac6347288c68b20113882eaa38f34d5f@eucas1p2.samsung.com>
2017-06-16 19:11     ` [Qemu-devel] [PATCH v9 2/8] migration: pass MigrationIncomingState* into migration check functions Alexey Perevalov
     [not found]   ` <CGME20170616191221eucas1p18f1b21abee06f0a34c2a8eb5fb0c6004@eucas1p1.samsung.com>
2017-06-16 19:12     ` [Qemu-devel] [PATCH v9 3/8] migration: fix hardcoded function name in error report Alexey Perevalov
     [not found]   ` <CGME20170616191222eucas1p2fc88d38fb9f84f7da04d853811dd07db@eucas1p2.samsung.com>
2017-06-16 19:12     ` [Qemu-devel] [PATCH v9 4/8] migration: split ufd_version_check onto receive/request features part Alexey Perevalov
     [not found]   ` <CGME20170616191222eucas1p186aab01cb769c9c2f7a1b61be48cd9a2@eucas1p1.samsung.com>
2017-06-16 19:12     ` [Qemu-devel] [PATCH v9 5/8] migration: introduce postcopy-blocktime capability Alexey Perevalov
     [not found]   ` <CGME20170616191223eucas1p26f48ecbb0f533561e9c43bbac1da3e04@eucas1p2.samsung.com>
2017-06-16 19:12     ` [Qemu-devel] [PATCH v9 6/8] migration: add postcopy blocktime ctx into MigrationIncomingState Alexey Perevalov
     [not found]   ` <CGME20170616191223eucas1p14025ca2f7c70d0edcad51f05458e0d9c@eucas1p1.samsung.com>
2017-06-16 19:12     ` [Qemu-devel] [PATCH v9 7/8] migration: calculate vCPU blocktime on dst side Alexey Perevalov
     [not found]   ` <CGME20170616191224eucas1p2e8c1ff33f630b6beaff096b73ab3462b@eucas1p2.samsung.com>
2017-06-16 19:12     ` [Qemu-devel] [PATCH v9 8/8] migration: postcopy_blocktime documentation Alexey Perevalov
2017-09-18 11:15   ` [Qemu-devel] [PATCH v9 0/8] calculate blocktime for postcopy live migration Dr. David Alan Gilbert
2017-09-18 15:23     ` Alexey Perevalov
2017-09-18 15:52       ` Dr. David Alan Gilbert

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.