Re: [Qemu-devel] [PATCH 07/15] postcopy: Plumb pagesize down into place helpers

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <address@hidden> wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
>
> Now we deal with normal size pages and huge pages we need
> to tell the place handlers the size we're dealing with
> and make sure the temporary page is large enough.
>
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>

I understand the what you are trying here, but ....

Why do we always map/upmap with the bigger pagesize?  I would assume
that this deppends on the block we are dealing with, no?
> @@ -321,7 +321,7 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState 
> *mis)
>      migrate_send_rp_shut(mis, qemu_file_get_error(mis->from_src_file) != 0);
>  
>      if (mis->postcopy_tmp_page) {
> -        munmap(mis->postcopy_tmp_page, getpagesize());
> +        munmap(mis->postcopy_tmp_page, mis->largest_page_size);
>          mis->postcopy_tmp_page = NULL;
>      }
>      trace_postcopy_ram_incoming_cleanup_exit();

Here

>  void *postcopy_get_tmp_page(MigrationIncomingState *mis)
>  {
>      if (!mis->postcopy_tmp_page) {
> -        mis->postcopy_tmp_page = mmap(NULL, getpagesize(),
> +        mis->postcopy_tmp_page = mmap(NULL, mis->largest_page_size,
>                               PROT_READ | PROT_WRITE, MAP_PRIVATE |
>                               MAP_ANONYMOUS, -1, 0);
>          if (mis->postcopy_tmp_page == MAP_FAILED) {

And here ...

>              if (all_zero) {
>                  ret = postcopy_place_page_zero(mis,
>                                                 host + TARGET_PAGE_SIZE -
> -                                               qemu_host_page_size);
> +                                               block->page_size,
> +                                               block->page_size);
>              } else {
>                  ret = postcopy_place_page(mis, host + TARGET_PAGE_SIZE -
> -                                               qemu_host_page_size,
> -                                               place_source);
> +                                               block->page_size,
> +                                               place_source, 
> block->page_size);
>              }
>          }
>          if (!ret) {

creating a temp for

addr = host + TARGET_PAGE_SIZE - block->page_size;

would make things more readable IMHO.  I was missreading the - by a ,
and didn't understand so many parameters O:-)