[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 24/32] vub+postcopy: madvises
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [RFC v2 24/32] vub+postcopy: madvises |
Date: |
Thu, 7 Sep 2017 13:30:08 +0100 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
* Marc-André Lureau (address@hidden) wrote:
> Hi
>
> "libvhost-user: madvises for postcopy" for ex, would be nicer imho
Done.
> On Thu, Aug 24, 2017 at 12:27 PM, Dr. David Alan Gilbert (git)
> <address@hidden> wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > Clear the area and turn off THP.
> >
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> > contrib/libvhost-user/libvhost-user.c | 32 ++++++++++++++++++++++++++++++--
> > 1 file changed, 30 insertions(+), 2 deletions(-)
> >
> > diff --git a/contrib/libvhost-user/libvhost-user.c
> > b/contrib/libvhost-user/libvhost-user.c
> > index 5ec54f7d60..d816851c6d 100644
> > --- a/contrib/libvhost-user/libvhost-user.c
> > +++ b/contrib/libvhost-user/libvhost-user.c
> > @@ -450,11 +450,39 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
> > }
> >
> > if (dev->postcopy_listening) {
> > + int ret;
> > /* We should already have an open ufd need to mark each memory
> > * range as ufd.
> > - * Note: Do we need any madvises? Well it's not been accessed
> > - * yet, still probably need no THP to be safe, discard to be
> > safe?
> > */
> > +
> > + /* 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 *)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 *)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;
> > --
> > 2.13.5
> >
>
> Errors are non-fatal? patch looks ok to me, despite the TODOs :).
The DONTNEED is actually not critical in this case; since we've
only just mmap'd it there should be nothing mapped in there that
needs clearing with DONTNEED; however it's a good safe guard.
There's no equivalent syscall we can make for postcopy.
The madvise(NOHUGEPAGE) can fail on a kernel that's configured
without CONFIG_TRANSPARENT_HUGEPAGE_MADVISE if TRANSPARENT_HUGEPAGE=n
then that's OK since it just means we don't have transparent hugepages
and thus we can't turn them off (I think we saw this on s390 or aarch64)
if CONFIG_TRANSAPRENT_HUGEPAGE_ALWAYS is set then things get
messy. IMHO there's way too many kernel config flags for transparent
hugepages!
> Reviewed-by: Marc-André Lureau <address@hidden>
Thanks,
Dave
>
>
>
>
> --
> Marc-André Lureau
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [RFC v2 24/32] vub+postcopy: madvises,
Dr. David Alan Gilbert <=