[Qemu-devel] [PATCH v2 0/3] Add bitmap for received pages in postcopy migration

[Qemu-devel] [PATCH v2 0/3] Add bitmap for received pages in postcopy migration

[Alexey Perevalov]

This is second version of
[PATCH v1 0/2] Add bitmap for copied pages in postcopy migration
cover message from there

This is a separate patch set, it derived from
https://www.mail-archive.com/qemu-devel@nongnu.org/msg456004.html

There are several possible use cases:
1. solve issue with postcopy live migration and shared memory.
OVS-VSWITCH requires information about copied pages, to fallocate
newly allocated pages.
2. calculation vCPU blocktime
for more details see
https://www.mail-archive.com/qemu-devel@nongnu.org/msg456004.html
3. Recovery after fail in the middle of postcopy migration 

Declaration is placed in two places include/migration/migration.h and into
migration/postcopy-ram.h, because some functions are required in virtio.

----------------------------------------------------------------


V1 -> V2
    - change in terminology s/copied/received/g
    - granularity became TARGET_PAGE_SIZE, but not actual page size of the
ramblock
    - movecopiedmap & get_copiedmap_size were removed, until patch set where
it will be necessary
    - releasing memory of receivedmap was added into ram_load_cleanup
    - new patch "migration: introduce qemu_ufd_copy_ioctl helper"

Patchset is based on Juan's patchset:
[PATCH v2 0/5] Create setup/cleanup methods for migration incoming side

Alexey Perevalov (3):
  migration: postcopy_place_page factoring out
  migration: introduce qemu_ufd_copy_ioctl helper
  migration: add bitmap for received page

 include/exec/ram_addr.h  |  2 ++
 migration/postcopy-ram.c | 53 ++++++++++++++++++++++++++++++++----------------
 migration/postcopy-ram.h |  4 ++--
 migration/ram.c          | 37 +++++++++++++++++++++++++++++++--
 migration/ram.h          |  5 +++++
 migration/savevm.c       |  1 +
 6 files changed, 80 insertions(+), 22 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/3] migration: postcopy_place_page factoring out

[Alexey Perevalov]

Need to mark copied pages as closer as possible to the place where it
tracks down. That will be necessary in futher patch.

Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 migration/postcopy-ram.c | 13 ++++++++-----
 migration/postcopy-ram.h |  4 ++--
 migration/ram.c          |  4 ++--
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index c8c4500..743ca6c 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -566,9 +566,10 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
  * returns 0 on success
  */
 int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
-                        size_t pagesize)
+                        RAMBlock *rb)
 {
     struct uffdio_copy copy_struct;
+    size_t pagesize = qemu_ram_pagesize(rb);
 
     copy_struct.dst = (uint64_t)(uintptr_t)host;
     copy_struct.src = (uint64_t)(uintptr_t)from;
@@ -597,10 +598,12 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
  * returns 0 on success
  */
 int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
-                             size_t pagesize)
+                             RAMBlock *rb)
 {
+    size_t pagesize;
     trace_postcopy_place_page_zero(host);
 
+    pagesize = qemu_ram_pagesize(rb);
     if (pagesize == getpagesize()) {
         struct uffdio_zeropage zero_struct;
         zero_struct.range.start = (uint64_t)(uintptr_t)host;
@@ -631,7 +634,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
             memset(mis->postcopy_tmp_zero_page, '\0', mis->largest_page_size);
         }
         return postcopy_place_page(mis, host, mis->postcopy_tmp_zero_page,
-                                   pagesize);
+                                   rb);
     }
 
     return 0;
@@ -694,14 +697,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
 }
 
 int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
-                        size_t pagesize)
+                        RAMBlock *rb)
 {
     assert(0);
     return -1;
 }
 
 int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
-                        size_t pagesize)
+                        RAMBlock *rb)
 {
     assert(0);
     return -1;
diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
index 52d51e8..78a3591 100644
--- a/migration/postcopy-ram.h
+++ b/migration/postcopy-ram.h
@@ -72,14 +72,14 @@ void postcopy_discard_send_finish(MigrationState *ms,
  * returns 0 on success
  */
 int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
-                        size_t pagesize);
+                        RAMBlock *rb);
 
 /*
  * Place a zero page at (host) atomically
  * returns 0 on success
  */
 int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
-                             size_t pagesize);
+                             RAMBlock *rb);
 
 /* The current postcopy state is read/set by postcopy_state_get/set
  * which update it atomically.
diff --git a/migration/ram.c b/migration/ram.c
index 8dbdfdb..f50479d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2465,10 +2465,10 @@ static int ram_load_postcopy(QEMUFile *f)
 
             if (all_zero) {
                 ret = postcopy_place_page_zero(mis, place_dest,
-                                               block->page_size);
+                                               block);
             } else {
                 ret = postcopy_place_page(mis, place_dest,
-                                          place_source, block->page_size);
+                                          place_source, block);
             }
         }
         if (!ret) {
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/3] migration: introduce qemu_ufd_copy_ioctl helper

[Alexey Perevalov]

Just for placing auxilary operations inside helper,
auxilary operations like: track received pages,
notify about copying operation in futher patches.

Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 migration/postcopy-ram.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 743ca6c..38a429a 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -561,6 +561,26 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
     return 0;
 }
 
+static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
+        void *from_addr, uint64_t pagesize)
+{
+    if (from_addr) {
+        struct uffdio_copy copy_struct;
+        copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
+        copy_struct.src = (uint64_t)(uintptr_t)from_addr;
+        copy_struct.len = pagesize;
+        copy_struct.mode = 0;
+        return ioctl(userfault_fd, UFFDIO_COPY, &copy_struct);
+    } else {
+        struct uffdio_zeropage zero_struct;
+        zero_struct.range.start = (uint64_t)(uintptr_t)host_addr;
+        zero_struct.range.len = pagesize;
+        zero_struct.mode = 0;
+        return ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
+    }
+    return 0;
+}
+
 /*
  * Place a host page (from) at (host) atomically
  * returns 0 on success
@@ -568,20 +588,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
 int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
                         RAMBlock *rb)
 {
-    struct uffdio_copy copy_struct;
     size_t pagesize = qemu_ram_pagesize(rb);
 
-    copy_struct.dst = (uint64_t)(uintptr_t)host;
-    copy_struct.src = (uint64_t)(uintptr_t)from;
-    copy_struct.len = pagesize;
-    copy_struct.mode = 0;
-
     /* copy also acks to the kernel waking the stalled thread up
      * TODO: We can inhibit that ack and only do it if it was requested
      * which would be slightly cheaper, but we'd have to be careful
      * of the order of updating our page state.
      */
-    if (ioctl(mis->userfault_fd, UFFDIO_COPY, &copy_struct)) {
+    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
         int e = errno;
         error_report("%s: %s copy host: %p from: %p (size: %zd)",
                      __func__, strerror(e), host, from, pagesize);
@@ -605,12 +619,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
 
     pagesize = qemu_ram_pagesize(rb);
     if (pagesize == getpagesize()) {
-        struct uffdio_zeropage zero_struct;
-        zero_struct.range.start = (uint64_t)(uintptr_t)host;
-        zero_struct.range.len = getpagesize();
-        zero_struct.mode = 0;
-
-        if (ioctl(mis->userfault_fd, UFFDIO_ZEROPAGE, &zero_struct)) {
+        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize())) {
             int e = errno;
             error_report("%s: %s zero host: %p",
                          __func__, strerror(e), host);
-- 
1.9.1

[Qemu-devel] [PATCH v2 3/3] migration: add bitmap for received page

[Alexey Perevalov]

This patch adds ability to track down already received
pages, it's necessary for calculation vCPU block time in
postcopy migration feature, maybe for restore after
postcopy migration failure.
Also it's necessary to solve shared memory issue in
postcopy livemigration. Information about received pages
will be transferred to the software virtual bridge
(e.g. OVS-VSWITCHD), to avoid fallocate (unmap) for
already received pages. fallocate syscall is required for
remmaped shared memory, due to remmaping itself blocks
ioctl(UFFDIO_COPY, ioctl in this case will end with EEXIT
error (struct page is exists after remmap).

Bitmap is placed into RAMBlock as another postcopy/precopy
related bitmaps.

Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 include/exec/ram_addr.h  |  2 ++
 migration/postcopy-ram.c | 11 ++++++++---
 migration/ram.c          | 33 +++++++++++++++++++++++++++++++++
 migration/ram.h          |  5 +++++
 migration/savevm.c       |  1 +
 5 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 140efa8..c2c1dfe 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -47,6 +47,8 @@ struct RAMBlock {
      * of the postcopy phase
      */
     unsigned long *unsentmap;
+    /* bitmap of already received pages in postcopy */
+    unsigned long *receivedmap;
 };
 
 static inline bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset)
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 38a429a..5ac3ed2 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -562,8 +562,13 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
 }
 
 static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
-        void *from_addr, uint64_t pagesize)
+        void *from_addr, uint64_t pagesize, RAMBlock *rb)
 {
+    /* received page isn't feature of blocktime calculation,
+     * it's more general entity, so keep it here,
+     * but gup betwean two following operation could be high,
+     * and in this case blocktime for such small interval will be lost */
+    set_receivedmap_by_addr(host_addr, rb);
     if (from_addr) {
         struct uffdio_copy copy_struct;
         copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
@@ -595,7 +600,7 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
      * which would be slightly cheaper, but we'd have to be careful
      * of the order of updating our page state.
      */
-    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
+    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize, rb)) {
         int e = errno;
         error_report("%s: %s copy host: %p from: %p (size: %zd)",
                      __func__, strerror(e), host, from, pagesize);
@@ -619,7 +624,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
 
     pagesize = qemu_ram_pagesize(rb);
     if (pagesize == getpagesize()) {
-        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize())) {
+        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize(), rb)) {
             int e = errno;
             error_report("%s: %s zero host: %p",
                          __func__, strerror(e), host);
diff --git a/migration/ram.c b/migration/ram.c
index f50479d..f5ea3c2 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -151,6 +151,34 @@ out:
     return ret;
 }
 
+void init_receivedmap(void)
+{
+    RAMBlock *rb;
+
+    RAMBLOCK_FOREACH(rb) {
+        unsigned long pages;
+        pages = rb->max_length >> TARGET_PAGE_BITS;
+        rb->receivedmap = bitmap_new(pages);
+    }
+}
+
+static unsigned long int get_received_bit_offset(void *host_addr, RAMBlock *rb)
+{
+    uint64_t host_addr_offset = (uint64_t)(uintptr_t)(host_addr
+                                                      - (void *)rb->host);
+    return host_addr_offset >> TARGET_PAGE_BITS;
+}
+
+int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
+{
+    return test_bit(get_received_bit_offset(host_addr, rb), rb->receivedmap);
+}
+
+void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
+{
+    set_bit_atomic(get_received_bit_offset(host_addr, rb), rb->receivedmap);
+}
+
 /*
  * An outstanding page request, on the source, having been received
  * and queued
@@ -2324,8 +2352,13 @@ static int ram_load_setup(QEMUFile *f, void *opaque)
 
 static int ram_load_cleanup(void *opaque)
 {
+    RAMBlock *rb;
     xbzrle_load_cleanup();
     compress_threads_load_cleanup();
+
+    RAMBLOCK_FOREACH(rb) {
+        g_free(rb->receivedmap);
+    }
     return 0;
 }
 
diff --git a/migration/ram.h b/migration/ram.h
index c081fde..7048ff9 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -52,4 +52,9 @@ int ram_discard_range(const char *block_name, uint64_t start, size_t length);
 int ram_postcopy_incoming_init(MigrationIncomingState *mis);
 
 void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
+
+void init_receivedmap(void);
+int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
+void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
+
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 31158da..668d3bb 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1372,6 +1372,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
         return -1;
     }
 
+    init_receivedmap();
     remote_pagesize_summary = qemu_get_be64(mis->from_src_file);
     local_pagesize_summary = ram_pagesize_summary();
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 1/3] migration: postcopy_place_page factoring out

[Peter Xu]

On Thu, Jun 15, 2017 at 07:36:13PM +0300, Alexey Perevalov wrote:
> Need to mark copied pages as closer as possible to the place where it
> tracks down. That will be necessary in futher patch.
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
>  migration/postcopy-ram.c | 13 ++++++++-----
>  migration/postcopy-ram.h |  4 ++--
>  migration/ram.c          |  4 ++--
>  3 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index c8c4500..743ca6c 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -566,9 +566,10 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>   * returns 0 on success
>   */
>  int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
> -                        size_t pagesize)
> +                        RAMBlock *rb)
>  {
>      struct uffdio_copy copy_struct;
> +    size_t pagesize = qemu_ram_pagesize(rb);
>  
>      copy_struct.dst = (uint64_t)(uintptr_t)host;
>      copy_struct.src = (uint64_t)(uintptr_t)from;
> @@ -597,10 +598,12 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
>   * returns 0 on success
>   */
>  int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
> -                             size_t pagesize)
> +                             RAMBlock *rb)
>  {
> +    size_t pagesize;
>      trace_postcopy_place_page_zero(host);
>  
> +    pagesize = qemu_ram_pagesize(rb);

Nit: we can remove pagesize val since it's only used in below line?

Other than that:

Reviewed-by: Peter Xu <peterx@redhat.com>

>      if (pagesize == getpagesize()) {
>          struct uffdio_zeropage zero_struct;
>          zero_struct.range.start = (uint64_t)(uintptr_t)host;
> @@ -631,7 +634,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>              memset(mis->postcopy_tmp_zero_page, '\0', mis->largest_page_size);
>          }
>          return postcopy_place_page(mis, host, mis->postcopy_tmp_zero_page,
> -                                   pagesize);
> +                                   rb);
>      }
>  
>      return 0;
> @@ -694,14 +697,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>  }
>  
>  int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
> -                        size_t pagesize)
> +                        RAMBlock *rb)
>  {
>      assert(0);
>      return -1;
>  }
>  
>  int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
> -                        size_t pagesize)
> +                        RAMBlock *rb)
>  {
>      assert(0);
>      return -1;
> diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> index 52d51e8..78a3591 100644
> --- a/migration/postcopy-ram.h
> +++ b/migration/postcopy-ram.h
> @@ -72,14 +72,14 @@ void postcopy_discard_send_finish(MigrationState *ms,
>   * returns 0 on success
>   */
>  int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
> -                        size_t pagesize);
> +                        RAMBlock *rb);
>  
>  /*
>   * Place a zero page at (host) atomically
>   * returns 0 on success
>   */
>  int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
> -                             size_t pagesize);
> +                             RAMBlock *rb);
>  
>  /* The current postcopy state is read/set by postcopy_state_get/set
>   * which update it atomically.
> diff --git a/migration/ram.c b/migration/ram.c
> index 8dbdfdb..f50479d 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2465,10 +2465,10 @@ static int ram_load_postcopy(QEMUFile *f)
>  
>              if (all_zero) {
>                  ret = postcopy_place_page_zero(mis, place_dest,
> -                                               block->page_size);
> +                                               block);
>              } else {
>                  ret = postcopy_place_page(mis, place_dest,
> -                                          place_source, block->page_size);
> +                                          place_source, block);
>              }
>          }
>          if (!ret) {
> -- 
> 1.9.1
> 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 2/3] migration: introduce qemu_ufd_copy_ioctl helper

[Peter Xu]

On Thu, Jun 15, 2017 at 07:36:14PM +0300, Alexey Perevalov wrote:
> Just for placing auxilary operations inside helper,
> auxilary operations like: track received pages,
> notify about copying operation in futher patches.
> 
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
>  migration/postcopy-ram.c | 35 ++++++++++++++++++++++-------------
>  1 file changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 743ca6c..38a429a 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -561,6 +561,26 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>      return 0;
>  }
>  
> +static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
> +        void *from_addr, uint64_t pagesize)
> +{
> +    if (from_addr) {
> +        struct uffdio_copy copy_struct;
> +        copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
> +        copy_struct.src = (uint64_t)(uintptr_t)from_addr;
> +        copy_struct.len = pagesize;
> +        copy_struct.mode = 0;
> +        return ioctl(userfault_fd, UFFDIO_COPY, &copy_struct);
> +    } else {
> +        struct uffdio_zeropage zero_struct;
> +        zero_struct.range.start = (uint64_t)(uintptr_t)host_addr;
> +        zero_struct.range.len = pagesize;
> +        zero_struct.mode = 0;
> +        return ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
> +    }
> +    return 0;

Remove this line?

> +}
> +
>  /*
>   * Place a host page (from) at (host) atomically
>   * returns 0 on success
> @@ -568,20 +588,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>  int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
>                          RAMBlock *rb)
>  {
> -    struct uffdio_copy copy_struct;
>      size_t pagesize = qemu_ram_pagesize(rb);
>  
> -    copy_struct.dst = (uint64_t)(uintptr_t)host;
> -    copy_struct.src = (uint64_t)(uintptr_t)from;
> -    copy_struct.len = pagesize;
> -    copy_struct.mode = 0;
> -
>      /* copy also acks to the kernel waking the stalled thread up
>       * TODO: We can inhibit that ack and only do it if it was requested
>       * which would be slightly cheaper, but we'd have to be careful
>       * of the order of updating our page state.
>       */
> -    if (ioctl(mis->userfault_fd, UFFDIO_COPY, &copy_struct)) {
> +    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {

(hmm I know "from" won't be zero, but naturally if without the context
 I would doubt it. Shall we add an assert here? You call.)

Besides:

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

>          int e = errno;
>          error_report("%s: %s copy host: %p from: %p (size: %zd)",
>                       __func__, strerror(e), host, from, pagesize);
> @@ -605,12 +619,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>  
>      pagesize = qemu_ram_pagesize(rb);
>      if (pagesize == getpagesize()) {
> -        struct uffdio_zeropage zero_struct;
> -        zero_struct.range.start = (uint64_t)(uintptr_t)host;
> -        zero_struct.range.len = getpagesize();
> -        zero_struct.mode = 0;
> -
> -        if (ioctl(mis->userfault_fd, UFFDIO_ZEROPAGE, &zero_struct)) {
> +        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize())) {
>              int e = errno;
>              error_report("%s: %s zero host: %p",
>                           __func__, strerror(e), host);
> -- 
> 1.9.1
> 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 3/3] migration: add bitmap for received page

[Peter Xu]

On Thu, Jun 15, 2017 at 07:36:15PM +0300, Alexey Perevalov wrote:

[...]

> +void init_receivedmap(void)
> +{
> +    RAMBlock *rb;
> +
> +    RAMBLOCK_FOREACH(rb) {
> +        unsigned long pages;
> +        pages = rb->max_length >> TARGET_PAGE_BITS;

Nit: I would assert(!rb->receivedmap) before new it in case for leak.

> +        rb->receivedmap = bitmap_new(pages);
> +    }
> +}
> +
> +static unsigned long int get_received_bit_offset(void *host_addr, RAMBlock *rb)
> +{
> +    uint64_t host_addr_offset = (uint64_t)(uintptr_t)(host_addr
> +                                                      - (void *)rb->host);
> +    return host_addr_offset >> TARGET_PAGE_BITS;
> +}

Can we rename this function to ramblock_get_pfn() and export it in
ramblock API (exec/ram_addr.h)?

static inline uint64_t ramblock_get_pfn(RAMBlock *rb, void *host)
{
    return (host - rb->host) >> TARGET_PAGE_BITS;
}

> +
> +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
> +{
> +    return test_bit(get_received_bit_offset(host_addr, rb), rb->receivedmap);
> +}
> +
> +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
> +{
> +    set_bit_atomic(get_received_bit_offset(host_addr, rb), rb->receivedmap);
> +}
> +
>  /*
>   * An outstanding page request, on the source, having been received
>   * and queued
> @@ -2324,8 +2352,13 @@ static int ram_load_setup(QEMUFile *f, void *opaque)
>  
>  static int ram_load_cleanup(void *opaque)
>  {
> +    RAMBlock *rb;
>      xbzrle_load_cleanup();
>      compress_threads_load_cleanup();
> +
> +    RAMBLOCK_FOREACH(rb) {

Nit: I'd prefer: if (rb->receivedmap) {...}

> +        g_free(rb->receivedmap);

and: rb->receivedmap = NULL later.

> +    }
>      return 0;
>  }
>  
> diff --git a/migration/ram.h b/migration/ram.h
> index c081fde..7048ff9 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -52,4 +52,9 @@ int ram_discard_range(const char *block_name, uint64_t start, size_t length);
>  int ram_postcopy_incoming_init(MigrationIncomingState *mis);
>  
>  void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
> +
> +void init_receivedmap(void);
> +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
> +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb);

Nit: I would name these names as: ramblock_recv_map_*().

> +
>  #endif
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 31158da..668d3bb 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1372,6 +1372,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
>          return -1;
>      }
>  
> +    init_receivedmap();
>      remote_pagesize_summary = qemu_get_be64(mis->from_src_file);
>      local_pagesize_summary = ram_pagesize_summary();
>  
> -- 
> 1.9.1
> 

Again, did you trap precopy?

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 3/3] migration: add bitmap for received page

[Alexey Perevalov]

On 06/16/2017 12:06 PM, Peter Xu wrote:
> On Thu, Jun 15, 2017 at 07:36:15PM +0300, Alexey Perevalov wrote:
>
> [...]
>
>> +void init_receivedmap(void)
>> +{
>> +    RAMBlock *rb;
>> +
>> +    RAMBLOCK_FOREACH(rb) {
>> +        unsigned long pages;
>> +        pages = rb->max_length >> TARGET_PAGE_BITS;
> Nit: I would assert(!rb->receivedmap) before new it in case for leak.
ok,
>
>> +        rb->receivedmap = bitmap_new(pages);
>> +    }
>> +}
>> +
>> +static unsigned long int get_received_bit_offset(void *host_addr, RAMBlock *rb)
>> +{
>> +    uint64_t host_addr_offset = (uint64_t)(uintptr_t)(host_addr
>> +                                                      - (void *)rb->host);
>> +    return host_addr_offset >> TARGET_PAGE_BITS;
>> +}
> Can we rename this function to ramblock_get_pfn() and export it in
> ramblock API (exec/ram_addr.h)?
if you need it, I'll place export into ram_addr.h
but name, PFN could confuse, because in case of hugepage
address this function will not return real PFN, due to

TARGET_PAGE_BITS usage.

>
> static inline uint64_t ramblock_get_pfn(RAMBlock *rb, void *host)
> {
>      return (host - rb->host) >> TARGET_PAGE_BITS;
> }
>
>> +
>> +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
>> +{
>> +    return test_bit(get_received_bit_offset(host_addr, rb), rb->receivedmap);
>> +}
>> +
>> +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
>> +{
>> +    set_bit_atomic(get_received_bit_offset(host_addr, rb), rb->receivedmap);
>> +}
>> +
>>   /*
>>    * An outstanding page request, on the source, having been received
>>    * and queued
>> @@ -2324,8 +2352,13 @@ static int ram_load_setup(QEMUFile *f, void *opaque)
>>   
>>   static int ram_load_cleanup(void *opaque)
>>   {
>> +    RAMBlock *rb;
>>       xbzrle_load_cleanup();
>>       compress_threads_load_cleanup();
>> +
>> +    RAMBLOCK_FOREACH(rb) {
> Nit: I'd prefer: if (rb->receivedmap) {...}
g_free already contains it
>
>> +        g_free(rb->receivedmap);
> and: rb->receivedmap = NULL later.
why not )
>
>> +    }
>>       return 0;
>>   }
>>   
>> diff --git a/migration/ram.h b/migration/ram.h
>> index c081fde..7048ff9 100644
>> --- a/migration/ram.h
>> +++ b/migration/ram.h
>> @@ -52,4 +52,9 @@ int ram_discard_range(const char *block_name, uint64_t start, size_t length);
>>   int ram_postcopy_incoming_init(MigrationIncomingState *mis);
>>   
>>   void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
>> +
>> +void init_receivedmap(void);
>> +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
>> +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
> Nit: I would name these names as: ramblock_recv_map_*().
>
>> +
>>   #endif
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 31158da..668d3bb 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1372,6 +1372,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
>>           return -1;
>>       }
>>   
>> +    init_receivedmap();
>>       remote_pagesize_summary = qemu_get_be64(mis->from_src_file);
>>       local_pagesize_summary = ram_pagesize_summary();
>>   
>> -- 
>> 1.9.1
>>
> Again, did you trap precopy?
sorry, I really forgot, now it's not copied bitmap, it's
received bitmap. So, I know about
case RAM_SAVE_FLAG_PAGE, but it seems another cases exist 
(compressed/xbzrle)
and there is no single point where coping is doing.


>
> Thanks,
>

-- 
Best regards,
Alexey Perevalov

Re: [Qemu-devel] [PATCH v2 1/3] migration: postcopy_place_page factoring out

[Dr. David Alan Gilbert]

* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> Need to mark copied pages as closer as possible to the place where it
> tracks down. That will be necessary in futher patch.
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/postcopy-ram.c | 13 ++++++++-----
>  migration/postcopy-ram.h |  4 ++--
>  migration/ram.c          |  4 ++--
>  3 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index c8c4500..743ca6c 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -566,9 +566,10 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>   * returns 0 on success
>   */
>  int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
> -                        size_t pagesize)
> +                        RAMBlock *rb)
>  {
>      struct uffdio_copy copy_struct;
> +    size_t pagesize = qemu_ram_pagesize(rb);
>  
>      copy_struct.dst = (uint64_t)(uintptr_t)host;
>      copy_struct.src = (uint64_t)(uintptr_t)from;
> @@ -597,10 +598,12 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
>   * returns 0 on success
>   */
>  int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
> -                             size_t pagesize)
> +                             RAMBlock *rb)
>  {
> +    size_t pagesize;
>      trace_postcopy_place_page_zero(host);
>  
> +    pagesize = qemu_ram_pagesize(rb);
>      if (pagesize == getpagesize()) {
>          struct uffdio_zeropage zero_struct;
>          zero_struct.range.start = (uint64_t)(uintptr_t)host;
> @@ -631,7 +634,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>              memset(mis->postcopy_tmp_zero_page, '\0', mis->largest_page_size);
>          }
>          return postcopy_place_page(mis, host, mis->postcopy_tmp_zero_page,
> -                                   pagesize);
> +                                   rb);
>      }
>  
>      return 0;
> @@ -694,14 +697,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>  }
>  
>  int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
> -                        size_t pagesize)
> +                        RAMBlock *rb)
>  {
>      assert(0);
>      return -1;
>  }
>  
>  int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
> -                        size_t pagesize)
> +                        RAMBlock *rb)
>  {
>      assert(0);
>      return -1;
> diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> index 52d51e8..78a3591 100644
> --- a/migration/postcopy-ram.h
> +++ b/migration/postcopy-ram.h
> @@ -72,14 +72,14 @@ void postcopy_discard_send_finish(MigrationState *ms,
>   * returns 0 on success
>   */
>  int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
> -                        size_t pagesize);
> +                        RAMBlock *rb);
>  
>  /*
>   * Place a zero page at (host) atomically
>   * returns 0 on success
>   */
>  int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
> -                             size_t pagesize);
> +                             RAMBlock *rb);
>  
>  /* The current postcopy state is read/set by postcopy_state_get/set
>   * which update it atomically.
> diff --git a/migration/ram.c b/migration/ram.c
> index 8dbdfdb..f50479d 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2465,10 +2465,10 @@ static int ram_load_postcopy(QEMUFile *f)
>  
>              if (all_zero) {
>                  ret = postcopy_place_page_zero(mis, place_dest,
> -                                               block->page_size);
> +                                               block);
>              } else {
>                  ret = postcopy_place_page(mis, place_dest,
> -                                          place_source, block->page_size);
> +                                          place_source, block);
>              }
>          }
>          if (!ret) {
> -- 
> 1.9.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 2/3] migration: introduce qemu_ufd_copy_ioctl helper

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> On Thu, Jun 15, 2017 at 07:36:14PM +0300, Alexey Perevalov wrote:
> > Just for placing auxilary operations inside helper,
> > auxilary operations like: track received pages,
> > notify about copying operation in futher patches.
> > 
> > Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> > ---
> >  migration/postcopy-ram.c | 35 ++++++++++++++++++++++-------------
> >  1 file changed, 22 insertions(+), 13 deletions(-)
> > 
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 743ca6c..38a429a 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -561,6 +561,26 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
> >      return 0;
> >  }
> >  
> > +static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
> > +        void *from_addr, uint64_t pagesize)
> > +{
> > +    if (from_addr) {
> > +        struct uffdio_copy copy_struct;
> > +        copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
> > +        copy_struct.src = (uint64_t)(uintptr_t)from_addr;
> > +        copy_struct.len = pagesize;
> > +        copy_struct.mode = 0;
> > +        return ioctl(userfault_fd, UFFDIO_COPY, &copy_struct);
> > +    } else {
> > +        struct uffdio_zeropage zero_struct;
> > +        zero_struct.range.start = (uint64_t)(uintptr_t)host_addr;
> > +        zero_struct.range.len = pagesize;
> > +        zero_struct.mode = 0;
> > +        return ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
> > +    }
> > +    return 0;
> 
> Remove this line?

Yes, probably should; only minor.

> > +}
> > +
> >  /*
> >   * Place a host page (from) at (host) atomically
> >   * returns 0 on success
> > @@ -568,20 +588,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
> >  int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
> >                          RAMBlock *rb)
> >  {
> > -    struct uffdio_copy copy_struct;
> >      size_t pagesize = qemu_ram_pagesize(rb);
> >  
> > -    copy_struct.dst = (uint64_t)(uintptr_t)host;
> > -    copy_struct.src = (uint64_t)(uintptr_t)from;
> > -    copy_struct.len = pagesize;
> > -    copy_struct.mode = 0;
> > -
> >      /* copy also acks to the kernel waking the stalled thread up
> >       * TODO: We can inhibit that ack and only do it if it was requested
> >       * which would be slightly cheaper, but we'd have to be careful
> >       * of the order of updating our page state.
> >       */
> > -    if (ioctl(mis->userfault_fd, UFFDIO_COPY, &copy_struct)) {
> > +    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
> 
> (hmm I know "from" won't be zero, but naturally if without the context
>  I would doubt it. Shall we add an assert here? You call.)

Doesn't seem worth it.

> Besides:
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> 
> Thanks,
> 
> >          int e = errno;
> >          error_report("%s: %s copy host: %p from: %p (size: %zd)",
> >                       __func__, strerror(e), host, from, pagesize);
> > @@ -605,12 +619,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
> >  
> >      pagesize = qemu_ram_pagesize(rb);
> >      if (pagesize == getpagesize()) {
> > -        struct uffdio_zeropage zero_struct;
> > -        zero_struct.range.start = (uint64_t)(uintptr_t)host;
> > -        zero_struct.range.len = getpagesize();
> > -        zero_struct.mode = 0;
> > -
> > -        if (ioctl(mis->userfault_fd, UFFDIO_ZEROPAGE, &zero_struct)) {
> > +        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize())) {
> >              int e = errno;
> >              error_report("%s: %s zero host: %p",
> >                           __func__, strerror(e), host);
> > -- 
> > 1.9.1
> > 
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 3/3] migration: add bitmap for received page

[Dr. David Alan Gilbert]

* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> This patch adds ability to track down already received
> pages, it's necessary for calculation vCPU block time in
> postcopy migration feature, maybe for restore after
> postcopy migration failure.
> Also it's necessary to solve shared memory issue in
> postcopy livemigration. Information about received pages
> will be transferred to the software virtual bridge
> (e.g. OVS-VSWITCHD), to avoid fallocate (unmap) for
> already received pages. fallocate syscall is required for
> remmaped shared memory, due to remmaping itself blocks
> ioctl(UFFDIO_COPY, ioctl in this case will end with EEXIT
> error (struct page is exists after remmap).
> 
> Bitmap is placed into RAMBlock as another postcopy/precopy
> related bitmaps.
> 
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
>  include/exec/ram_addr.h  |  2 ++
>  migration/postcopy-ram.c | 11 ++++++++---
>  migration/ram.c          | 33 +++++++++++++++++++++++++++++++++
>  migration/ram.h          |  5 +++++
>  migration/savevm.c       |  1 +
>  5 files changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 140efa8..c2c1dfe 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -47,6 +47,8 @@ struct RAMBlock {
>       * of the postcopy phase
>       */
>      unsigned long *unsentmap;
> +    /* bitmap of already received pages in postcopy */
> +    unsigned long *receivedmap;
>  };
>  
>  static inline bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset)
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 38a429a..5ac3ed2 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -562,8 +562,13 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>  }
>  
>  static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
> -        void *from_addr, uint64_t pagesize)
> +        void *from_addr, uint64_t pagesize, RAMBlock *rb)
>  {
> +    /* received page isn't feature of blocktime calculation,
> +     * it's more general entity, so keep it here,
> +     * but gup betwean two following operation could be high,
> +     * and in this case blocktime for such small interval will be lost */
> +    set_receivedmap_by_addr(host_addr, rb);
>      if (from_addr) {
>          struct uffdio_copy copy_struct;
>          copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
> @@ -595,7 +600,7 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
>       * which would be slightly cheaper, but we'd have to be careful
>       * of the order of updating our page state.
>       */
> -    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
> +    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize, rb)) {

In your first patch of the series you changed postcopy_place_page to
take a RAMBlock* *rather* than a pagesize; perhaps it would be better
to do the same here?

Other than that looks pretty good.

Dave

>          int e = errno;
>          error_report("%s: %s copy host: %p from: %p (size: %zd)",
>                       __func__, strerror(e), host, from, pagesize);
> @@ -619,7 +624,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>  
>      pagesize = qemu_ram_pagesize(rb);
>      if (pagesize == getpagesize()) {
> -        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize())) {
> +        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize(), rb)) {
>              int e = errno;
>              error_report("%s: %s zero host: %p",
>                           __func__, strerror(e), host);
> diff --git a/migration/ram.c b/migration/ram.c
> index f50479d..f5ea3c2 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -151,6 +151,34 @@ out:
>      return ret;
>  }
>  
> +void init_receivedmap(void)
> +{
> +    RAMBlock *rb;
> +
> +    RAMBLOCK_FOREACH(rb) {
> +        unsigned long pages;
> +        pages = rb->max_length >> TARGET_PAGE_BITS;
> +        rb->receivedmap = bitmap_new(pages);
> +    }
> +}
> +
> +static unsigned long int get_received_bit_offset(void *host_addr, RAMBlock *rb)
> +{
> +    uint64_t host_addr_offset = (uint64_t)(uintptr_t)(host_addr
> +                                                      - (void *)rb->host);
> +    return host_addr_offset >> TARGET_PAGE_BITS;
> +}
> +
> +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
> +{
> +    return test_bit(get_received_bit_offset(host_addr, rb), rb->receivedmap);
> +}
> +
> +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
> +{
> +    set_bit_atomic(get_received_bit_offset(host_addr, rb), rb->receivedmap);
> +}
> +
>  /*
>   * An outstanding page request, on the source, having been received
>   * and queued
> @@ -2324,8 +2352,13 @@ static int ram_load_setup(QEMUFile *f, void *opaque)
>  
>  static int ram_load_cleanup(void *opaque)
>  {
> +    RAMBlock *rb;
>      xbzrle_load_cleanup();
>      compress_threads_load_cleanup();
> +
> +    RAMBLOCK_FOREACH(rb) {
> +        g_free(rb->receivedmap);
> +    }
>      return 0;
>  }
>  
> diff --git a/migration/ram.h b/migration/ram.h
> index c081fde..7048ff9 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -52,4 +52,9 @@ int ram_discard_range(const char *block_name, uint64_t start, size_t length);
>  int ram_postcopy_incoming_init(MigrationIncomingState *mis);
>  
>  void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
> +
> +void init_receivedmap(void);
> +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
> +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
> +
>  #endif
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 31158da..668d3bb 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1372,6 +1372,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
>          return -1;
>      }
>  
> +    init_receivedmap();
>      remote_pagesize_summary = qemu_get_be64(mis->from_src_file);
>      local_pagesize_summary = ram_pagesize_summary();
>  
> -- 
> 1.9.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 3/3] migration: add bitmap for received page

[Dr. David Alan Gilbert]

* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> On 06/16/2017 12:06 PM, Peter Xu wrote:
> > On Thu, Jun 15, 2017 at 07:36:15PM +0300, Alexey Perevalov wrote:
> > 
> > [...]
> > 
> > > +void init_receivedmap(void)
> > > +{
> > > +    RAMBlock *rb;
> > > +
> > > +    RAMBLOCK_FOREACH(rb) {
> > > +        unsigned long pages;
> > > +        pages = rb->max_length >> TARGET_PAGE_BITS;
> > Nit: I would assert(!rb->receivedmap) before new it in case for leak.
> ok,
> > 
> > > +        rb->receivedmap = bitmap_new(pages);
> > > +    }
> > > +}
> > > +
> > > +static unsigned long int get_received_bit_offset(void *host_addr, RAMBlock *rb)
> > > +{
> > > +    uint64_t host_addr_offset = (uint64_t)(uintptr_t)(host_addr
> > > +                                                      - (void *)rb->host);
> > > +    return host_addr_offset >> TARGET_PAGE_BITS;
> > > +}
> > Can we rename this function to ramblock_get_pfn() and export it in
> > ramblock API (exec/ram_addr.h)?
> if you need it, I'll place export into ram_addr.h
> but name, PFN could confuse, because in case of hugepage
> address this function will not return real PFN, due to
> 
> TARGET_PAGE_BITS usage.
> 
> > 
> > static inline uint64_t ramblock_get_pfn(RAMBlock *rb, void *host)
> > {
> >      return (host - rb->host) >> TARGET_PAGE_BITS;
> > }
> > 
> > > +
> > > +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
> > > +{
> > > +    return test_bit(get_received_bit_offset(host_addr, rb), rb->receivedmap);
> > > +}
> > > +
> > > +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
> > > +{
> > > +    set_bit_atomic(get_received_bit_offset(host_addr, rb), rb->receivedmap);
> > > +}
> > > +
> > >   /*
> > >    * An outstanding page request, on the source, having been received
> > >    * and queued
> > > @@ -2324,8 +2352,13 @@ static int ram_load_setup(QEMUFile *f, void *opaque)
> > >   static int ram_load_cleanup(void *opaque)
> > >   {
> > > +    RAMBlock *rb;
> > >       xbzrle_load_cleanup();
> > >       compress_threads_load_cleanup();
> > > +
> > > +    RAMBLOCK_FOREACH(rb) {
> > Nit: I'd prefer: if (rb->receivedmap) {...}
> g_free already contains it
> > 
> > > +        g_free(rb->receivedmap);
> > and: rb->receivedmap = NULL later.
> why not )
> > 
> > > +    }
> > >       return 0;
> > >   }
> > > diff --git a/migration/ram.h b/migration/ram.h
> > > index c081fde..7048ff9 100644
> > > --- a/migration/ram.h
> > > +++ b/migration/ram.h
> > > @@ -52,4 +52,9 @@ int ram_discard_range(const char *block_name, uint64_t start, size_t length);
> > >   int ram_postcopy_incoming_init(MigrationIncomingState *mis);
> > >   void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
> > > +
> > > +void init_receivedmap(void);
> > > +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
> > > +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
> > Nit: I would name these names as: ramblock_recv_map_*().
> > 
> > > +
> > >   #endif
> > > diff --git a/migration/savevm.c b/migration/savevm.c
> > > index 31158da..668d3bb 100644
> > > --- a/migration/savevm.c
> > > +++ b/migration/savevm.c
> > > @@ -1372,6 +1372,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
> > >           return -1;
> > >       }
> > > +    init_receivedmap();
> > >       remote_pagesize_summary = qemu_get_be64(mis->from_src_file);
> > >       local_pagesize_summary = ram_pagesize_summary();
> > > -- 
> > > 1.9.1
> > > 
> > Again, did you trap precopy?
> sorry, I really forgot, now it's not copied bitmap, it's
> received bitmap. So, I know about
> case RAM_SAVE_FLAG_PAGE, but it seems another cases exist
> (compressed/xbzrle)
> and there is no single point where coping is doing.

Yes we probably should trap precopy as well; I'm not sure yet
whether it'll trigger during the network cases; but certainly
in the recovery-from-postcopy-failure case you want to know
which pages have been received.
You'd also want to clear flags in this bitmap during the discard
phase.

Dave

> 
> 
> > 
> > Thanks,
> > 
> 
> -- 
> Best regards,
> Alexey Perevalov
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 3/3] migration: add bitmap for received page

[Alexey Perevalov]

On 06/16/2017 08:14 PM, Dr. David Alan Gilbert wrote:
> * Alexey Perevalov (a.perevalov@samsung.com) wrote:
>> This patch adds ability to track down already received
>> pages, it's necessary for calculation vCPU block time in
>> postcopy migration feature, maybe for restore after
>> postcopy migration failure.
>> Also it's necessary to solve shared memory issue in
>> postcopy livemigration. Information about received pages
>> will be transferred to the software virtual bridge
>> (e.g. OVS-VSWITCHD), to avoid fallocate (unmap) for
>> already received pages. fallocate syscall is required for
>> remmaped shared memory, due to remmaping itself blocks
>> ioctl(UFFDIO_COPY, ioctl in this case will end with EEXIT
>> error (struct page is exists after remmap).
>>
>> Bitmap is placed into RAMBlock as another postcopy/precopy
>> related bitmaps.
>>
>> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
>> ---
>>   include/exec/ram_addr.h  |  2 ++
>>   migration/postcopy-ram.c | 11 ++++++++---
>>   migration/ram.c          | 33 +++++++++++++++++++++++++++++++++
>>   migration/ram.h          |  5 +++++
>>   migration/savevm.c       |  1 +
>>   5 files changed, 49 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
>> index 140efa8..c2c1dfe 100644
>> --- a/include/exec/ram_addr.h
>> +++ b/include/exec/ram_addr.h
>> @@ -47,6 +47,8 @@ struct RAMBlock {
>>        * of the postcopy phase
>>        */
>>       unsigned long *unsentmap;
>> +    /* bitmap of already received pages in postcopy */
>> +    unsigned long *receivedmap;
>>   };
>>   
>>   static inline bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset)
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index 38a429a..5ac3ed2 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -562,8 +562,13 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>>   }
>>   
>>   static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
>> -        void *from_addr, uint64_t pagesize)
>> +        void *from_addr, uint64_t pagesize, RAMBlock *rb)
>>   {
>> +    /* received page isn't feature of blocktime calculation,
>> +     * it's more general entity, so keep it here,
>> +     * but gup betwean two following operation could be high,
>> +     * and in this case blocktime for such small interval will be lost */
>> +    set_receivedmap_by_addr(host_addr, rb);
>>       if (from_addr) {
>>           struct uffdio_copy copy_struct;
>>           copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
>> @@ -595,7 +600,7 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
>>        * which would be slightly cheaper, but we'd have to be careful
>>        * of the order of updating our page state.
>>        */
>> -    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
>> +    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize, rb)) {
> In your first patch of the series you changed postcopy_place_page to
> take a RAMBlock* *rather* than a pagesize; perhaps it would be better
> to do the same here?
that's true only for postcopy_place_page where pagesize is getting from 
ramblock,
but not true for postcopy_place_page_zero, where initially was len = 
getpagesize()
>
> Other than that looks pretty good.
>
> Dave
>
>>           int e = errno;
>>           error_report("%s: %s copy host: %p from: %p (size: %zd)",
>>                        __func__, strerror(e), host, from, pagesize);
>> @@ -619,7 +624,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>>   
>>       pagesize = qemu_ram_pagesize(rb);
>>       if (pagesize == getpagesize()) {
>> -        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize())) {
>> +        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize(), rb)) {
>>               int e = errno;
>>               error_report("%s: %s zero host: %p",
>>                            __func__, strerror(e), host);
>> diff --git a/migration/ram.c b/migration/ram.c
>> index f50479d..f5ea3c2 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -151,6 +151,34 @@ out:
>>       return ret;
>>   }
>>   
>> +void init_receivedmap(void)
>> +{
>> +    RAMBlock *rb;
>> +
>> +    RAMBLOCK_FOREACH(rb) {
>> +        unsigned long pages;
>> +        pages = rb->max_length >> TARGET_PAGE_BITS;
>> +        rb->receivedmap = bitmap_new(pages);
>> +    }
>> +}
>> +
>> +static unsigned long int get_received_bit_offset(void *host_addr, RAMBlock *rb)
>> +{
>> +    uint64_t host_addr_offset = (uint64_t)(uintptr_t)(host_addr
>> +                                                      - (void *)rb->host);
>> +    return host_addr_offset >> TARGET_PAGE_BITS;
>> +}
>> +
>> +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
>> +{
>> +    return test_bit(get_received_bit_offset(host_addr, rb), rb->receivedmap);
>> +}
>> +
>> +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
>> +{
>> +    set_bit_atomic(get_received_bit_offset(host_addr, rb), rb->receivedmap);
>> +}
>> +
>>   /*
>>    * An outstanding page request, on the source, having been received
>>    * and queued
>> @@ -2324,8 +2352,13 @@ static int ram_load_setup(QEMUFile *f, void *opaque)
>>   
>>   static int ram_load_cleanup(void *opaque)
>>   {
>> +    RAMBlock *rb;
>>       xbzrle_load_cleanup();
>>       compress_threads_load_cleanup();
>> +
>> +    RAMBLOCK_FOREACH(rb) {
>> +        g_free(rb->receivedmap);
>> +    }
>>       return 0;
>>   }
>>   
>> diff --git a/migration/ram.h b/migration/ram.h
>> index c081fde..7048ff9 100644
>> --- a/migration/ram.h
>> +++ b/migration/ram.h
>> @@ -52,4 +52,9 @@ int ram_discard_range(const char *block_name, uint64_t start, size_t length);
>>   int ram_postcopy_incoming_init(MigrationIncomingState *mis);
>>   
>>   void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
>> +
>> +void init_receivedmap(void);
>> +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
>> +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
>> +
>>   #endif
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 31158da..668d3bb 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1372,6 +1372,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
>>           return -1;
>>       }
>>   
>> +    init_receivedmap();
>>       remote_pagesize_summary = qemu_get_be64(mis->from_src_file);
>>       local_pagesize_summary = ram_pagesize_summary();
>>   
>> -- 
>> 1.9.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
>
>

-- 
Best regards,
Alexey Perevalov

Re: [Qemu-devel] [PATCH v2 3/3] migration: add bitmap for received page

[Dr. David Alan Gilbert]

* Alexey Perevalov (a.perevalov@samsung.com) wrote:
> On 06/16/2017 08:14 PM, Dr. David Alan Gilbert wrote:
> > * Alexey Perevalov (a.perevalov@samsung.com) wrote:
> > > This patch adds ability to track down already received
> > > pages, it's necessary for calculation vCPU block time in
> > > postcopy migration feature, maybe for restore after
> > > postcopy migration failure.
> > > Also it's necessary to solve shared memory issue in
> > > postcopy livemigration. Information about received pages
> > > will be transferred to the software virtual bridge
> > > (e.g. OVS-VSWITCHD), to avoid fallocate (unmap) for
> > > already received pages. fallocate syscall is required for
> > > remmaped shared memory, due to remmaping itself blocks
> > > ioctl(UFFDIO_COPY, ioctl in this case will end with EEXIT
> > > error (struct page is exists after remmap).
> > > 
> > > Bitmap is placed into RAMBlock as another postcopy/precopy
> > > related bitmaps.
> > > 
> > > Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> > > ---
> > >   include/exec/ram_addr.h  |  2 ++
> > >   migration/postcopy-ram.c | 11 ++++++++---
> > >   migration/ram.c          | 33 +++++++++++++++++++++++++++++++++
> > >   migration/ram.h          |  5 +++++
> > >   migration/savevm.c       |  1 +
> > >   5 files changed, 49 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> > > index 140efa8..c2c1dfe 100644
> > > --- a/include/exec/ram_addr.h
> > > +++ b/include/exec/ram_addr.h
> > > @@ -47,6 +47,8 @@ struct RAMBlock {
> > >        * of the postcopy phase
> > >        */
> > >       unsigned long *unsentmap;
> > > +    /* bitmap of already received pages in postcopy */
> > > +    unsigned long *receivedmap;
> > >   };
> > >   static inline bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset)
> > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > > index 38a429a..5ac3ed2 100644
> > > --- a/migration/postcopy-ram.c
> > > +++ b/migration/postcopy-ram.c
> > > @@ -562,8 +562,13 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
> > >   }
> > >   static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
> > > -        void *from_addr, uint64_t pagesize)
> > > +        void *from_addr, uint64_t pagesize, RAMBlock *rb)
> > >   {
> > > +    /* received page isn't feature of blocktime calculation,
> > > +     * it's more general entity, so keep it here,
> > > +     * but gup betwean two following operation could be high,
> > > +     * and in this case blocktime for such small interval will be lost */
> > > +    set_receivedmap_by_addr(host_addr, rb);
> > >       if (from_addr) {
> > >           struct uffdio_copy copy_struct;
> > >           copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
> > > @@ -595,7 +600,7 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
> > >        * which would be slightly cheaper, but we'd have to be careful
> > >        * of the order of updating our page state.
> > >        */
> > > -    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
> > > +    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize, rb)) {
> > In your first patch of the series you changed postcopy_place_page to
> > take a RAMBlock* *rather* than a pagesize; perhaps it would be better
> > to do the same here?
> that's true only for postcopy_place_page where pagesize is getting from
> ramblock,
> but not true for postcopy_place_page_zero, where initially was len =
> getpagesize()

Ah, yes:

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> > 
> > Other than that looks pretty good.
> > 
> > Dave
> > 
> > >           int e = errno;
> > >           error_report("%s: %s copy host: %p from: %p (size: %zd)",
> > >                        __func__, strerror(e), host, from, pagesize);
> > > @@ -619,7 +624,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
> > >       pagesize = qemu_ram_pagesize(rb);
> > >       if (pagesize == getpagesize()) {
> > > -        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize())) {
> > > +        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize(), rb)) {
> > >               int e = errno;
> > >               error_report("%s: %s zero host: %p",
> > >                            __func__, strerror(e), host);
> > > diff --git a/migration/ram.c b/migration/ram.c
> > > index f50479d..f5ea3c2 100644
> > > --- a/migration/ram.c
> > > +++ b/migration/ram.c
> > > @@ -151,6 +151,34 @@ out:
> > >       return ret;
> > >   }
> > > +void init_receivedmap(void)
> > > +{
> > > +    RAMBlock *rb;
> > > +
> > > +    RAMBLOCK_FOREACH(rb) {
> > > +        unsigned long pages;
> > > +        pages = rb->max_length >> TARGET_PAGE_BITS;
> > > +        rb->receivedmap = bitmap_new(pages);
> > > +    }
> > > +}
> > > +
> > > +static unsigned long int get_received_bit_offset(void *host_addr, RAMBlock *rb)
> > > +{
> > > +    uint64_t host_addr_offset = (uint64_t)(uintptr_t)(host_addr
> > > +                                                      - (void *)rb->host);
> > > +    return host_addr_offset >> TARGET_PAGE_BITS;
> > > +}
> > > +
> > > +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
> > > +{
> > > +    return test_bit(get_received_bit_offset(host_addr, rb), rb->receivedmap);
> > > +}
> > > +
> > > +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
> > > +{
> > > +    set_bit_atomic(get_received_bit_offset(host_addr, rb), rb->receivedmap);
> > > +}
> > > +
> > >   /*
> > >    * An outstanding page request, on the source, having been received
> > >    * and queued
> > > @@ -2324,8 +2352,13 @@ static int ram_load_setup(QEMUFile *f, void *opaque)
> > >   static int ram_load_cleanup(void *opaque)
> > >   {
> > > +    RAMBlock *rb;
> > >       xbzrle_load_cleanup();
> > >       compress_threads_load_cleanup();
> > > +
> > > +    RAMBLOCK_FOREACH(rb) {
> > > +        g_free(rb->receivedmap);
> > > +    }
> > >       return 0;
> > >   }
> > > diff --git a/migration/ram.h b/migration/ram.h
> > > index c081fde..7048ff9 100644
> > > --- a/migration/ram.h
> > > +++ b/migration/ram.h
> > > @@ -52,4 +52,9 @@ int ram_discard_range(const char *block_name, uint64_t start, size_t length);
> > >   int ram_postcopy_incoming_init(MigrationIncomingState *mis);
> > >   void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
> > > +
> > > +void init_receivedmap(void);
> > > +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
> > > +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
> > > +
> > >   #endif
> > > diff --git a/migration/savevm.c b/migration/savevm.c
> > > index 31158da..668d3bb 100644
> > > --- a/migration/savevm.c
> > > +++ b/migration/savevm.c
> > > @@ -1372,6 +1372,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
> > >           return -1;
> > >       }
> > > +    init_receivedmap();
> > >       remote_pagesize_summary = qemu_get_be64(mis->from_src_file);
> > >       local_pagesize_summary = ram_pagesize_summary();
> > > -- 
> > > 1.9.1
> > > 
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> > 
> > 
> 
> -- 
> Best regards,
> Alexey Perevalov
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 3/3] migration: add bitmap for received page

[Alexey Perevalov]

On 06/16/2017 09:46 PM, Dr. David Alan Gilbert wrote:
> * Alexey Perevalov (a.perevalov@samsung.com) wrote:
>> On 06/16/2017 08:14 PM, Dr. David Alan Gilbert wrote:
>>> * Alexey Perevalov (a.perevalov@samsung.com) wrote:
>>>> This patch adds ability to track down already received
>>>> pages, it's necessary for calculation vCPU block time in
>>>> postcopy migration feature, maybe for restore after
>>>> postcopy migration failure.
>>>> Also it's necessary to solve shared memory issue in
>>>> postcopy livemigration. Information about received pages
>>>> will be transferred to the software virtual bridge
>>>> (e.g. OVS-VSWITCHD), to avoid fallocate (unmap) for
>>>> already received pages. fallocate syscall is required for
>>>> remmaped shared memory, due to remmaping itself blocks
>>>> ioctl(UFFDIO_COPY, ioctl in this case will end with EEXIT
>>>> error (struct page is exists after remmap).
>>>>
>>>> Bitmap is placed into RAMBlock as another postcopy/precopy
>>>> related bitmaps.
>>>>
>>>> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
>>>> ---
>>>>    include/exec/ram_addr.h  |  2 ++
>>>>    migration/postcopy-ram.c | 11 ++++++++---
>>>>    migration/ram.c          | 33 +++++++++++++++++++++++++++++++++
>>>>    migration/ram.h          |  5 +++++
>>>>    migration/savevm.c       |  1 +
>>>>    5 files changed, 49 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
>>>> index 140efa8..c2c1dfe 100644
>>>> --- a/include/exec/ram_addr.h
>>>> +++ b/include/exec/ram_addr.h
>>>> @@ -47,6 +47,8 @@ struct RAMBlock {
>>>>         * of the postcopy phase
>>>>         */
>>>>        unsigned long *unsentmap;
>>>> +    /* bitmap of already received pages in postcopy */
>>>> +    unsigned long *receivedmap;
>>>>    };
>>>>    static inline bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset)
>>>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>>>> index 38a429a..5ac3ed2 100644
>>>> --- a/migration/postcopy-ram.c
>>>> +++ b/migration/postcopy-ram.c
>>>> @@ -562,8 +562,13 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>>>>    }
>>>>    static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
>>>> -        void *from_addr, uint64_t pagesize)
>>>> +        void *from_addr, uint64_t pagesize, RAMBlock *rb)
>>>>    {
>>>> +    /* received page isn't feature of blocktime calculation,
>>>> +     * it's more general entity, so keep it here,
>>>> +     * but gup betwean two following operation could be high,
>>>> +     * and in this case blocktime for such small interval will be lost */
>>>> +    set_receivedmap_by_addr(host_addr, rb);
>>>>        if (from_addr) {
>>>>            struct uffdio_copy copy_struct;
>>>>            copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
>>>> @@ -595,7 +600,7 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
>>>>         * which would be slightly cheaper, but we'd have to be careful
>>>>         * of the order of updating our page state.
>>>>         */
>>>> -    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
>>>> +    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize, rb)) {
>>> In your first patch of the series you changed postcopy_place_page to
>>> take a RAMBlock* *rather* than a pagesize; perhaps it would be better
>>> to do the same here?
>> that's true only for postcopy_place_page where pagesize is getting from
>> ramblock,
>> but not true for postcopy_place_page_zero, where initially was len =
>> getpagesize()
> Ah, yes:
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
don't rush ;) I resent patch and added precopy/discard cases there.
I'll be in vacation during next week, but I'll try to check mailing list.
>
>>> Other than that looks pretty good.
>>>
>>> Dave
>>>
>>>>            int e = errno;
>>>>            error_report("%s: %s copy host: %p from: %p (size: %zd)",
>>>>                         __func__, strerror(e), host, from, pagesize);
>>>> @@ -619,7 +624,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>>>>        pagesize = qemu_ram_pagesize(rb);
>>>>        if (pagesize == getpagesize()) {
>>>> -        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize())) {
>>>> +        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize(), rb)) {
>>>>                int e = errno;
>>>>                error_report("%s: %s zero host: %p",
>>>>                             __func__, strerror(e), host);
>>>> diff --git a/migration/ram.c b/migration/ram.c
>>>> index f50479d..f5ea3c2 100644
>>>> --- a/migration/ram.c
>>>> +++ b/migration/ram.c
>>>> @@ -151,6 +151,34 @@ out:
>>>>        return ret;
>>>>    }
>>>> +void init_receivedmap(void)
>>>> +{
>>>> +    RAMBlock *rb;
>>>> +
>>>> +    RAMBLOCK_FOREACH(rb) {
>>>> +        unsigned long pages;
>>>> +        pages = rb->max_length >> TARGET_PAGE_BITS;
>>>> +        rb->receivedmap = bitmap_new(pages);
>>>> +    }
>>>> +}
>>>> +
>>>> +static unsigned long int get_received_bit_offset(void *host_addr, RAMBlock *rb)
>>>> +{
>>>> +    uint64_t host_addr_offset = (uint64_t)(uintptr_t)(host_addr
>>>> +                                                      - (void *)rb->host);
>>>> +    return host_addr_offset >> TARGET_PAGE_BITS;
>>>> +}
>>>> +
>>>> +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
>>>> +{
>>>> +    return test_bit(get_received_bit_offset(host_addr, rb), rb->receivedmap);
>>>> +}
>>>> +
>>>> +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb)
>>>> +{
>>>> +    set_bit_atomic(get_received_bit_offset(host_addr, rb), rb->receivedmap);
>>>> +}
>>>> +
>>>>    /*
>>>>     * An outstanding page request, on the source, having been received
>>>>     * and queued
>>>> @@ -2324,8 +2352,13 @@ static int ram_load_setup(QEMUFile *f, void *opaque)
>>>>    static int ram_load_cleanup(void *opaque)
>>>>    {
>>>> +    RAMBlock *rb;
>>>>        xbzrle_load_cleanup();
>>>>        compress_threads_load_cleanup();
>>>> +
>>>> +    RAMBLOCK_FOREACH(rb) {
>>>> +        g_free(rb->receivedmap);
>>>> +    }
>>>>        return 0;
>>>>    }
>>>> diff --git a/migration/ram.h b/migration/ram.h
>>>> index c081fde..7048ff9 100644
>>>> --- a/migration/ram.h
>>>> +++ b/migration/ram.h
>>>> @@ -52,4 +52,9 @@ int ram_discard_range(const char *block_name, uint64_t start, size_t length);
>>>>    int ram_postcopy_incoming_init(MigrationIncomingState *mis);
>>>>    void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
>>>> +
>>>> +void init_receivedmap(void);
>>>> +int test_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
>>>> +void set_receivedmap_by_addr(void *host_addr, RAMBlock *rb);
>>>> +
>>>>    #endif
>>>> diff --git a/migration/savevm.c b/migration/savevm.c
>>>> index 31158da..668d3bb 100644
>>>> --- a/migration/savevm.c
>>>> +++ b/migration/savevm.c
>>>> @@ -1372,6 +1372,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
>>>>            return -1;
>>>>        }
>>>> +    init_receivedmap();
>>>>        remote_pagesize_summary = qemu_get_be64(mis->from_src_file);
>>>>        local_pagesize_summary = ram_pagesize_summary();
>>>> -- 
>>>> 1.9.1
>>>>
>>> --
>>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>>>
>>>
>>>
>> -- 
>> Best regards,
>> Alexey Perevalov
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
>
>

-- 
Best regards,
Alexey Perevalov