Re: [Qemu-devel] [RFC v2 06/32] postcopy: use UFFDIO_ZEROPAGE only when available

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Marc-André Lureau (marcandre.lureau@gmail.com) wrote:
> Hi
> 
> On Thu, Aug 24, 2017 at 9:27 PM, Dr. David Alan Gilbert (git)
> <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Use a flag on the RAMBlock to state whether it has the
> > UFFDIO_ZEROPAGE capability, use it when it's available.
> >
> > This allows the use of postcopy on tmpfs as well as hugepage
> > backed files.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  exec.c                    | 15 +++++++++++++++
> >  include/exec/cpu-common.h |  3 +++
> >  migration/postcopy-ram.c  | 14 +++++++++++---
> >  3 files changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/exec.c b/exec.c
> > index 35b4cea2ed..80c3d1d121 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -103,6 +103,11 @@ static MemoryRegion io_mem_unassigned;
> >   */
> >  #define RAM_RESIZEABLE (1 << 2)
> >
> > +/* UFFDIO_ZEROPAGE is available on this RAMBlock to atomically
> > + * zero the page and wake waiting processes.
> > + * (Set during postcopy)
> > + */
> > +#define RAM_UF_ZEROPAGE (1 << 3)
> >  #endif
> >
> >  #ifdef TARGET_PAGE_BITS_VARY
> > @@ -1705,6 +1710,16 @@ bool qemu_ram_is_shared(RAMBlock *rb)
> >      return rb->flags & RAM_SHARED;
> >  }
> >
> > +bool qemu_ram_is_uf_zeroable(RAMBlock *rb)
> > +{
> > +    return rb->flags & RAM_UF_ZEROPAGE;
> > +}
> > +
> > +void qemu_ram_set_uf_zeroable(RAMBlock *rb)
> > +{
> > +    rb->flags |= RAM_UF_ZEROPAGE;
> > +}
> > +
> >  /* Called with iothread lock held.  */
> >  void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState *dev)
> >  {
> > diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> > index 0d861a6289..24d335f95d 100644
> > --- a/include/exec/cpu-common.h
> > +++ b/include/exec/cpu-common.h
> > @@ -73,6 +73,9 @@ void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev);
> >  void qemu_ram_unset_idstr(RAMBlock *block);
> >  const char *qemu_ram_get_idstr(RAMBlock *rb);
> >  bool qemu_ram_is_shared(RAMBlock *rb);
> > +bool qemu_ram_is_uf_zeroable(RAMBlock *rb);
> > +void qemu_ram_set_uf_zeroable(RAMBlock *rb);
> > +
> >  size_t qemu_ram_pagesize(RAMBlock *block);
> >  size_t qemu_ram_pagesize_largest(void);
> >
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 7a414ebad8..640b72d86d 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -408,6 +408,11 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,
> >          error_report("%s userfault: Region doesn't support COPY", __func__);
> >          return -1;
> >      }
> > +    if (reg_struct.ioctls & ((__u64)1 << _UFFDIO_ZEROPAGE)) {
> > +        RAMBlock *rb = qemu_ram_block_by_name(block_name);
> > +        qemu_ram_set_uf_zeroable(rb);
> > +    }
> > +
> >
> 
> extra empty line

Thanks, gone.

> >      return 0;
> >  }
> > @@ -617,11 +622,14 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
> >  int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
> >                               RAMBlock *rb)
> >  {
> > +    size_t pagesize = qemu_ram_pagesize(rb);
> >      trace_postcopy_place_page_zero(host);
> >
> > -    if (qemu_ram_pagesize(rb) == getpagesize()) {
> 
> Is this check drop intentionally?

Yes, it used to be the case that we knew that for hugepages we couldn't
do zeroing and that was the rule we used.   Now we're using the
capability flag returned from the uffdio registration on this RAMBlock
and it tells us if we can use the ZERO ioctl on this block.

Dave

> > -        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, getpagesize(),
> > -                                rb)) {
> > +    /* Normal RAMBlocks can zero a page using UFFDIO_ZEROPAGE
> > +     * but it's not available for everything (e.g. hugetlbpages)
> > +     */
> > +    if (qemu_ram_is_uf_zeroable(rb)) {
> > +        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, pagesize, rb)) {
> >              int e = errno;
> >              error_report("%s: %s zero host: %p",
> >                           __func__, strerror(e), host);
> > --
> > 2.13.5
> >
> >
> 
> 
> 
> -- 
> Marc-André Lureau
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK