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

[Peter Xu]

On Wed, Jul 26, 2017 at 11:07:17AM +0300, Alexey Perevalov wrote:
> On 07/26/2017 04:49 AM, Peter Xu wrote:
> >On Thu, Jul 20, 2017 at 09:52:34AM +0300, Alexey Perevalov 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.
> >>
> >>Reviewed-by: Peter Xu <address@hidden>
> >>Signed-off-by: Alexey Perevalov <address@hidden>
> >>---
> >[...]
> >
> >>  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)
> >>  {
> >>+    int ret;
> >>      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);
> >>+        ret = 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);
> >>+        ret = ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
> >>+    }
> >>+    if (!ret) {
> >>+        ramblock_recv_bitmap_set(host_addr, rb);
> >Wait...
> >
> >Now we are using 4k-page/bit bitmap, do we need to take care of the
> >huge pages here?  Looks like we are only setting the first bit of it
> >if it is a huge page?
> First version was per ramblock page size, IOW bitmap was smaller in
> case of hugepages.

Yes, but this is not the first version any more. :)

This patch is using:

  bitmap_new(rb->max_length >> TARGET_PAGE_BITS);

to allocate bitmap, so it is using small pages always for bitmap,
right? (I should not really say "4k" pages, here I think the size is
host page size, which is the thing returned from getpagesize()).

> 
> 
> You mentioned that TARGET_PAGE_SIZE is reasonable for precopy case,
> in "Re: [Qemu-devel] [PATCH v1 2/2] migration: add bitmap for copied page"
> I though TARGET_PAGE_SIZE as transmition unit, is using in precopy even
> hugepage case.
> But it's not so logically, page being marked as dirty, should be sent as a
> whole page.

Sorry if I misunderstood, but I didn't see anything wrong - we are
sending pages in small pages, but when postcopy is there, we do
UFFDIO_COPY in huge page, so everything is fine?

-- 
Peter Xu