All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Marc-André Lureau" <marcandre.lureau@gmail.com>
To: Raphael Norwitz <raphael.norwitz@nutanix.com>
Cc: Raphael Norwitz <raphael.s.norwitz@gmail.com>,
	QEMU <qemu-devel@nongnu.org>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [PATCH v4 06/10] Refactor out libvhost-user fault generation logic
Date: Thu, 4 Jun 2020 16:48:30 +0200	[thread overview]
Message-ID: <CAJ+F1CK1=UqTA2FzyvVz7UcspYQhSG1NO5mph3DmS-_mieDj_Q@mail.gmail.com> (raw)
In-Reply-To: <1588533678-23450-7-git-send-email-raphael.norwitz@nutanix.com>

[-- Attachment #1: Type: text/plain, Size: 8273 bytes --]

Hi

On Thu, May 21, 2020 at 7:00 AM Raphael Norwitz <raphael.norwitz@nutanix.com>
wrote:

> In libvhost-user, the incoming postcopy migration path for setting the
> backend's memory tables has become convolued. In particular, moving the
> logic which starts generating faults, having received the final ACK from
> qemu can be moved to a separate function. This simplifies the code
> substantially.
>
> This logic will also be needed by the postcopy path once the
> VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS feature is supported.
>
> Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---
>  contrib/libvhost-user/libvhost-user.c | 147
> ++++++++++++++++++----------------
>  1 file changed, 79 insertions(+), 68 deletions(-)
>
> diff --git a/contrib/libvhost-user/libvhost-user.c
> b/contrib/libvhost-user/libvhost-user.c
> index 3bca996..cccfa22 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -584,6 +584,84 @@ map_ring(VuDev *dev, VuVirtq *vq)
>  }
>
>  static bool
> +generate_faults(VuDev *dev) {
> +    int i;
> +    for (i = 0; i < dev->nregions; i++) {
> +        VuDevRegion *dev_region = &dev->regions[i];
> +        int ret;
> +#ifdef UFFDIO_REGISTER
> +        /*
> +         * We should already have an open ufd. Mark each memory
> +         * range as ufd.
> +         * Discard any mapping we have here; note I can't use MADV_REMOVE
> +         * or fallocate to make the hole since I don't want to lose
> +         * data that's already arrived in the shared process.
> +         * TODO: How to do hugepage
> +         */
> +        ret = madvise((void *)(uintptr_t)dev_region->mmap_addr,
> +                      dev_region->size + dev_region->mmap_offset,
> +                      MADV_DONTNEED);
> +        if (ret) {
> +            fprintf(stderr,
> +                    "%s: Failed to madvise(DONTNEED) region %d: %s\n",
> +                    __func__, i, strerror(errno));
> +        }
> +        /*
> +         * Turn off transparent hugepages so we dont get lose wakeups
> +         * in neighbouring pages.
> +         * TODO: Turn this backon later.
> +         */
> +        ret = madvise((void *)(uintptr_t)dev_region->mmap_addr,
> +                      dev_region->size + dev_region->mmap_offset,
> +                      MADV_NOHUGEPAGE);
> +        if (ret) {
> +            /*
> +             * Note: This can happen legally on kernels that are
> configured
> +             * without madvise'able hugepages
> +             */
> +            fprintf(stderr,
> +                    "%s: Failed to madvise(NOHUGEPAGE) region %d: %s\n",
> +                    __func__, i, strerror(errno));
> +        }
> +        struct uffdio_register reg_struct;
> +        reg_struct.range.start = (uintptr_t)dev_region->mmap_addr;
> +        reg_struct.range.len = dev_region->size + dev_region->mmap_offset;
> +        reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING;
> +
> +        if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, &reg_struct)) {
> +            vu_panic(dev, "%s: Failed to userfault region %d "
> +                          "@%p + size:%zx offset: %zx: (ufd=%d)%s\n",
> +                     __func__, i,
> +                     dev_region->mmap_addr,
> +                     dev_region->size, dev_region->mmap_offset,
> +                     dev->postcopy_ufd, strerror(errno));
> +            return false;
> +        }
> +        if (!(reg_struct.ioctls & ((__u64)1 << _UFFDIO_COPY))) {
> +            vu_panic(dev, "%s Region (%d) doesn't support COPY",
> +                     __func__, i);
> +            return false;
> +        }
> +        DPRINT("%s: region %d: Registered userfault for %"
> +               PRIx64 " + %" PRIx64 "\n", __func__, i,
> +               (uint64_t)reg_struct.range.start,
> +               (uint64_t)reg_struct.range.len);
> +        /* Now it's registered we can let the client at it */
> +        if (mprotect((void *)(uintptr_t)dev_region->mmap_addr,
> +                     dev_region->size + dev_region->mmap_offset,
> +                     PROT_READ | PROT_WRITE)) {
> +            vu_panic(dev, "failed to mprotect region %d for postcopy
> (%s)",
> +                     i, strerror(errno));
> +            return false;
> +        }
> +        /* TODO: Stash 'zero' support flags somewhere */
> +#endif
> +    }
> +
> +    return true;
> +}
> +
> +static bool
>  vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg)
>  {
>      int i;
> @@ -655,74 +733,7 @@ vu_set_mem_table_exec_postcopy(VuDev *dev,
> VhostUserMsg *vmsg)
>      }
>
>      /* OK, now we can go and register the memory and generate faults */
> -    for (i = 0; i < dev->nregions; i++) {
> -        VuDevRegion *dev_region = &dev->regions[i];
> -        int ret;
> -#ifdef UFFDIO_REGISTER
> -        /* We should already have an open ufd. Mark each memory
> -         * range as ufd.
> -         * Discard any mapping we have here; note I can't use MADV_REMOVE
> -         * or fallocate to make the hole since I don't want to lose
> -         * data that's already arrived in the shared process.
> -         * TODO: How to do hugepage
> -         */
> -        ret = madvise((void *)(uintptr_t)dev_region->mmap_addr,
> -                      dev_region->size + dev_region->mmap_offset,
> -                      MADV_DONTNEED);
> -        if (ret) {
> -            fprintf(stderr,
> -                    "%s: Failed to madvise(DONTNEED) region %d: %s\n",
> -                    __func__, i, strerror(errno));
> -        }
> -        /* Turn off transparent hugepages so we dont get lose wakeups
> -         * in neighbouring pages.
> -         * TODO: Turn this backon later.
> -         */
> -        ret = madvise((void *)(uintptr_t)dev_region->mmap_addr,
> -                      dev_region->size + dev_region->mmap_offset,
> -                      MADV_NOHUGEPAGE);
> -        if (ret) {
> -            /* Note: This can happen legally on kernels that are
> configured
> -             * without madvise'able hugepages
> -             */
> -            fprintf(stderr,
> -                    "%s: Failed to madvise(NOHUGEPAGE) region %d: %s\n",
> -                    __func__, i, strerror(errno));
> -        }
> -        struct uffdio_register reg_struct;
> -        reg_struct.range.start = (uintptr_t)dev_region->mmap_addr;
> -        reg_struct.range.len = dev_region->size + dev_region->mmap_offset;
> -        reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING;
> -
> -        if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, &reg_struct)) {
> -            vu_panic(dev, "%s: Failed to userfault region %d "
> -                          "@%p + size:%zx offset: %zx: (ufd=%d)%s\n",
> -                     __func__, i,
> -                     dev_region->mmap_addr,
> -                     dev_region->size, dev_region->mmap_offset,
> -                     dev->postcopy_ufd, strerror(errno));
> -            return false;
> -        }
> -        if (!(reg_struct.ioctls & ((__u64)1 << _UFFDIO_COPY))) {
> -            vu_panic(dev, "%s Region (%d) doesn't support COPY",
> -                     __func__, i);
> -            return false;
> -        }
> -        DPRINT("%s: region %d: Registered userfault for %"
> -               PRIx64 " + %" PRIx64 "\n", __func__, i,
> -               (uint64_t)reg_struct.range.start,
> -               (uint64_t)reg_struct.range.len);
> -        /* Now it's registered we can let the client at it */
> -        if (mprotect((void *)(uintptr_t)dev_region->mmap_addr,
> -                     dev_region->size + dev_region->mmap_offset,
> -                     PROT_READ | PROT_WRITE)) {
> -            vu_panic(dev, "failed to mprotect region %d for postcopy
> (%s)",
> -                     i, strerror(errno));
> -            return false;
> -        }
> -        /* TODO: Stash 'zero' support flags somewhere */
> -#endif
> -    }
> +    (void)generate_faults(dev);
>

>      return false;
>  }
> --
> 1.8.3.1
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 10694 bytes --]

  reply	other threads:[~2020-06-04 14:49 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-21  5:00 [PATCH v4 00/10] vhost-user: Lift Max Ram Slots Limitation Raphael Norwitz
2020-05-21  5:00 ` [PATCH v4 01/10] Add helper to populate vhost-user message regions Raphael Norwitz
2020-06-04 14:40   ` Marc-André Lureau
2020-05-21  5:00 ` [PATCH v4 02/10] Add vhost-user helper to get MemoryRegion data Raphael Norwitz
2020-06-04 14:41   ` Marc-André Lureau
2020-05-21  5:00 ` [PATCH v4 03/10] Add VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS Raphael Norwitz
2020-06-04 14:42   ` Marc-André Lureau
2020-05-21  5:00 ` [PATCH v4 04/10] Transmit vhost-user memory regions individually Raphael Norwitz
2020-06-04 14:44   ` Marc-André Lureau
2020-06-09 14:13     ` Raphael Norwitz
2020-05-21  5:00 ` [PATCH v4 05/10] Lift max memory slots limit imposed by vhost-user Raphael Norwitz
2020-06-04 14:45   ` Marc-André Lureau
2020-05-21  5:00 ` [PATCH v4 06/10] Refactor out libvhost-user fault generation logic Raphael Norwitz
2020-06-04 14:48   ` Marc-André Lureau [this message]
2020-05-21  5:00 ` [PATCH v4 07/10] Support ram slot configuration in libvhost-user Raphael Norwitz
2020-06-04 14:49   ` Marc-André Lureau
2020-05-21  5:00 ` [PATCH v4 08/10] Support adding individual regions " Raphael Norwitz
2020-05-21  5:00 ` [PATCH v4 09/10] Support individual region unmap " Raphael Norwitz
2020-05-21  5:00 ` [PATCH v4 10/10] Lift max ram slots limit " Raphael Norwitz
2020-06-04  4:11 ` [PATCH v4 00/10] vhost-user: Lift Max Ram Slots Limitation Raphael Norwitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJ+F1CK1=UqTA2FzyvVz7UcspYQhSG1NO5mph3DmS-_mieDj_Q@mail.gmail.com' \
    --to=marcandre.lureau@gmail.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=raphael.norwitz@nutanix.com \
    --cc=raphael.s.norwitz@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.