Re: [Qemu-devel] [PATCH v2 10/16] postcopy: Load huge pages in one go

[Laurent Vivier]

On 06/02/2017 18:33, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
> 
> The existing postcopy RAM load loop already ensures that it
> glues together whole host-pages from the target page size chunks sent
> over the wire.  Modify the definition of host page that it uses
> to be the RAM block page size and thus be huge pages where appropriate.
> 
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> Reviewed-by: Juan Quintela <address@hidden>
> ---
>  migration/ram.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index ff448ef..88d9444 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2342,7 +2342,7 @@ static int ram_load_postcopy(QEMUFile *f)
>  {
>      int flags = 0, ret = 0;
>      bool place_needed = false;
> -    bool matching_page_sizes = qemu_host_page_size == TARGET_PAGE_SIZE;
> +    bool matching_page_sizes = false;

The false value is not obvious.
Is gcc smart enough to detect you use "matching_page_sizes" (in the
"switch ()") only when it has been really initialized (in the "if ()")?

>      MigrationIncomingState *mis = migration_incoming_get_current();
>      /* Temporary page that is later 'placed' */
>      void *postcopy_host_page = postcopy_get_tmp_page(mis);
> @@ -2372,8 +2372,11 @@ static int ram_load_postcopy(QEMUFile *f)
>                  ret = -EINVAL;
>                  break;
>              }
> +            matching_page_sizes = block->page_size == TARGET_PAGE_SIZE;
>              /*
> -             * Postcopy requires that we place whole host pages atomically.
> +             * Postcopy requires that we place whole host pages atomically;
> +             * these may be huge pages for RAMBlocks that are backed by
> +             * hugetlbfs.
>               * To make it atomic, the data is read into a temporary page
>               * that's moved into place later.
>               * The migration protocol uses,  possibly smaller, target-pages
> @@ -2381,9 +2384,9 @@ static int ram_load_postcopy(QEMUFile *f)
>               * of a host page in order.
>               */
>              page_buffer = postcopy_host_page +
> -                          ((uintptr_t)host & ~qemu_host_page_mask);
> +                          ((uintptr_t)host & (block->page_size - 1));
>              /* If all TP are zero then we can optimise the place */
> -            if (!((uintptr_t)host & ~qemu_host_page_mask)) {
> +            if (!((uintptr_t)host & (block->page_size - 1))) {
>                  all_zero = true;
>              } else {
>                  /* not the 1st TP within the HP */
> @@ -2401,7 +2404,7 @@ static int ram_load_postcopy(QEMUFile *f)
>               * page
>               */
>              place_needed = (((uintptr_t)host + TARGET_PAGE_SIZE) &
> -                                     ~qemu_host_page_mask) == 0;
> +                                     (block->page_size - 1)) == 0;
>              place_source = postcopy_host_page;
>          }
>          last_host = host;
> 

Reviewed-by: Laurent Vivier <address@hidden>