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

[Perevalov Alexey]

On Fri, Jun 23, 2017 at 08:41:41PM +0100, Dr. David Alan Gilbert wrote:
> * Perevalov Alexey (address@hidden) wrote:
> > On Fri, Jun 23, 2017 at 11:29:42AM +0100, Dr. David Alan Gilbert wrote:
> > > * Alexey Perevalov (address@hidden) 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.
> > > > 
> > > > 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)
> > > >  {
> > > > +    /* received page isn't feature of blocktime calculation,
> > > > +     * it's more general entity, so keep it here,
> > > > +     * but gup betwean two following operation could be high,
> > > > +     * and in this case blocktime for such small interval will be lost 
> > > > */
> > > > +    ramblock_recv_bitmap_set(host_addr, rb);
> > > 
> > > I have a fun problem here in my world with using the same bitmap for
> > > shared memory with the vhost-user client;  for that a set bit means
> > > that the data has already arrived and we need to do a UFFDIO_WAKE on
> > > the client;
> > Do you mean vhost-user client?
> 
> Yes, I'm doing UFFDIO_WAKE calls on the userfault fd passed to me by
> the client.
> 
> > > but that means we can't set the bit in this function until
> > > the end after we've done the COPY/ZERO.
> > 
> > I have the same problem, I described it to Peter, when he asked why 
> > ramblock_recv_bitmap_set should be closer to ioctl. But even such
> > position doesn't solve that problem.
> > 
> > I could repeat here, I'm sending that bitmap to vhost-user client, and
> > it's possible situation when bitmap is set but page not yet copied.
> > Did you faced that? Or just mention it as potential problem.
> 
> A similar problem;  I've got the fault thread receiving a fault request
> from the UFD, if the bit is set then it sends a WAKE, if it's not set
> then it sends a request back to the source.
> If we set the bit before the COPY/ZERO then I could send a WAKE too
> early.
> 
> > If so, we could move ramblock_recv_bitmap_set after ioctl,
> > but we chose that way to avoid situation when new page fault happening
> > during ioctl or betwean ioctl and ramblock_recv_bitmap_set on the same vCPU.
> > Or introduce 2 bitmap, copied/received.
> 
> It's a shame to need 2 bits.   We shouldn't get another fault on the
> same page, but I guess we can get it from another CPU on the same page
> which hmm is the problem with the stats code.
Yes, 2 bitmap is bad idea. I'm going to place ramblock_recv_bitmap_set
after ioctl, and leave such trade off in stats code or return back to
tree where key was an address of the pagefault.

> 
> Dave
> 
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK


Alexey