[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/15] postcopy: Plumb pagesize down into place
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 07/15] postcopy: Plumb pagesize down into place helpers |
Date: |
Fri, 27 Jan 2017 15:49:43 +0000 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
* Juan Quintela (address@hidden) wrote:
> "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?
It's only a temporary, so it seems easier to map it once at the start
of the migration at the size we need and clean it up at the end, that
way we don't have to allocate/deallocate on each block.
> > @@ -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:-)
Done.
Dave
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH 03/15] postcopy: Chunk discards for hugepages, (continued)
- [Qemu-devel] [PATCH 08/15] postcopy: Use temporary for placing zero huge pages, Dr. David Alan Gilbert (git), 2017/01/06
- [Qemu-devel] [PATCH 07/15] postcopy: Plumb pagesize down into place helpers, Dr. David Alan Gilbert (git), 2017/01/06
- [Qemu-devel] [PATCH 09/15] postcopy: Load huge pages in one go, Dr. David Alan Gilbert (git), 2017/01/06
- [Qemu-devel] [PATCH 10/15] postcopy: Mask fault addresses to huge page boundary, Dr. David Alan Gilbert (git), 2017/01/06
- [Qemu-devel] [PATCH 11/15] postcopy: Send whole huge pages, Dr. David Alan Gilbert (git), 2017/01/06
- [Qemu-devel] [PATCH 12/15] postcopy: Allow hugepages, Dr. David Alan Gilbert (git), 2017/01/06
- [Qemu-devel] [PATCH 13/15] postcopy: Update userfaultfd.h header, Dr. David Alan Gilbert (git), 2017/01/06