[v2,5/6] migration/postcopy: enable random order target page arrival
diff mbox series

Message ID 20191107123907.29791-6-richardw.yang@linux.intel.com
State New
Headers show
Series
  • migration/postcopy: enable compress during postcopy
Related show

Commit Message

Wei Yang Nov. 7, 2019, 12:39 p.m. UTC
After using number of target page received to track one host page, we
could have the capability to handle random order target page arrival in
one host page.

This is a preparation for enabling compress during postcopy.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

---
v2:
   * use uintptr_t to calculate place_dest
   * check target pages belongs to the same host page
---
 migration/ram.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

Comments

Dr. David Alan Gilbert Nov. 7, 2019, 2:30 p.m. UTC | #1
* Wei Yang (richardw.yang@linux.intel.com) wrote:
> After using number of target page received to track one host page, we
> could have the capability to handle random order target page arrival in
> one host page.
> 
> This is a preparation for enabling compress during postcopy.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

Yep, that's better

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


> ---
> v2:
>    * use uintptr_t to calculate place_dest
>    * check target pages belongs to the same host page
> ---
>  migration/ram.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index b5759793a9..666ad69284 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -4015,7 +4015,7 @@ static int ram_load_postcopy(QEMUFile *f)
>      MigrationIncomingState *mis = migration_incoming_get_current();
>      /* Temporary page that is later 'placed' */
>      void *postcopy_host_page = mis->postcopy_tmp_page;
> -    void *last_host = NULL;
> +    void *this_host = NULL;
>      bool all_zero = false;
>      int target_pages = 0;
>  
> @@ -4062,24 +4062,26 @@ static int ram_load_postcopy(QEMUFile *f)
>               * that's moved into place later.
>               * The migration protocol uses,  possibly smaller, target-pages
>               * however the source ensures it always sends all the components
> -             * of a host page in order.
> +             * of a host page in one chunk.
>               */
>              page_buffer = postcopy_host_page +
>                            ((uintptr_t)host & (block->page_size - 1));
>              /* If all TP are zero then we can optimise the place */
>              if (target_pages == 1) {
>                  all_zero = true;
> +                this_host = (void *)QEMU_ALIGN_DOWN((uintptr_t)host,
> +                                                    block->page_size);
>              } else {
>                  /* not the 1st TP within the HP */
> -                if (host != (last_host + TARGET_PAGE_SIZE)) {
> -                    error_report("Non-sequential target page %p/%p",
> -                                  host, last_host);
> +                if (QEMU_ALIGN_DOWN((uintptr_t)host, block->page_size) !=
> +                    (uintptr_t)this_host) {
> +                    error_report("Non-same host page %p/%p",
> +                                  host, this_host);
>                      ret = -EINVAL;
>                      break;
>                  }
>              }
>  
> -
>              /*
>               * If it's the last part of a host page then we place the host
>               * page
> @@ -4090,7 +4092,6 @@ static int ram_load_postcopy(QEMUFile *f)
>              }
>              place_source = postcopy_host_page;
>          }
> -        last_host = host;
>  
>          switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
>          case RAM_SAVE_FLAG_ZERO:
> @@ -4143,7 +4144,8 @@ static int ram_load_postcopy(QEMUFile *f)
>  
>          if (!ret && place_needed) {
>              /* This gets called at the last target page in the host page */
> -            void *place_dest = host + TARGET_PAGE_SIZE - block->page_size;
> +            void *place_dest = (void *)QEMU_ALIGN_DOWN((uintptr_t)host,
> +                                                       block->page_size);
>  
>              if (all_zero) {
>                  ret = postcopy_place_page_zero(mis, place_dest,
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Wei Yang Nov. 8, 2019, 12:40 a.m. UTC | #2
On Thu, Nov 07, 2019 at 02:30:25PM +0000, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.yang@linux.intel.com) wrote:
>> After using number of target page received to track one host page, we
>> could have the capability to handle random order target page arrival in
>> one host page.
>> 
>> This is a preparation for enabling compress during postcopy.
>> 
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>
>Yep, that's better
>
>Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>

Thanks for your suggestion :-)
Juan Quintela Jan. 9, 2020, 9:42 a.m. UTC | #3
Wei Yang <richardw.yang@linux.intel.com> wrote:
> After using number of target page received to track one host page, we
> could have the capability to handle random order target page arrival in
> one host page.
>
> This is a preparation for enabling compress during postcopy.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>
> ---
> v2:
>    * use uintptr_t to calculate place_dest
>    * check target pages belongs to the same host page

Reviewed-by: Juan Quintela <quintela@redhat.com>

Patch
diff mbox series

diff --git a/migration/ram.c b/migration/ram.c
index b5759793a9..666ad69284 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4015,7 +4015,7 @@  static int ram_load_postcopy(QEMUFile *f)
     MigrationIncomingState *mis = migration_incoming_get_current();
     /* Temporary page that is later 'placed' */
     void *postcopy_host_page = mis->postcopy_tmp_page;
-    void *last_host = NULL;
+    void *this_host = NULL;
     bool all_zero = false;
     int target_pages = 0;
 
@@ -4062,24 +4062,26 @@  static int ram_load_postcopy(QEMUFile *f)
              * that's moved into place later.
              * The migration protocol uses,  possibly smaller, target-pages
              * however the source ensures it always sends all the components
-             * of a host page in order.
+             * of a host page in one chunk.
              */
             page_buffer = postcopy_host_page +
                           ((uintptr_t)host & (block->page_size - 1));
             /* If all TP are zero then we can optimise the place */
             if (target_pages == 1) {
                 all_zero = true;
+                this_host = (void *)QEMU_ALIGN_DOWN((uintptr_t)host,
+                                                    block->page_size);
             } else {
                 /* not the 1st TP within the HP */
-                if (host != (last_host + TARGET_PAGE_SIZE)) {
-                    error_report("Non-sequential target page %p/%p",
-                                  host, last_host);
+                if (QEMU_ALIGN_DOWN((uintptr_t)host, block->page_size) !=
+                    (uintptr_t)this_host) {
+                    error_report("Non-same host page %p/%p",
+                                  host, this_host);
                     ret = -EINVAL;
                     break;
                 }
             }
 
-
             /*
              * If it's the last part of a host page then we place the host
              * page
@@ -4090,7 +4092,6 @@  static int ram_load_postcopy(QEMUFile *f)
             }
             place_source = postcopy_host_page;
         }
-        last_host = host;
 
         switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
         case RAM_SAVE_FLAG_ZERO:
@@ -4143,7 +4144,8 @@  static int ram_load_postcopy(QEMUFile *f)
 
         if (!ret && place_needed) {
             /* This gets called at the last target page in the host page */
-            void *place_dest = host + TARGET_PAGE_SIZE - block->page_size;
+            void *place_dest = (void *)QEMU_ALIGN_DOWN((uintptr_t)host,
+                                                       block->page_size);
 
             if (all_zero) {
                 ret = postcopy_place_page_zero(mis, place_dest,