Re: [Qemu-stable] [Qemu-devel] [PATCH] KVM: always use MADV_DONTFORK

[Andrea Arcangeli]

On Thu, Jul 25, 2013 at 11:16:44AM +0100, Peter Maydell wrote:
> On 25 July 2013 11:11, Andrea Arcangeli <address@hidden> wrote:
> > diff --git a/exec.c b/exec.c
> > index c99a883..d3bb58d 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1162,6 +1162,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, 
> > void *host,
> >
> >      qemu_ram_setup_dump(new_block->host, size);
> >      qemu_madvise(new_block->host, size, QEMU_MADV_HUGEPAGE);
> > +    qemu_madvise(new_block->host, size, QEMU_MADV_DONTFORK);
> >
> >      if (kvm_enabled())
> >          kvm_setup_guest_memory(new_block->host, size);
> >
> 
> kvm_setup_guest_memory() already calls
>   qemu_madvise(start, size, QEMU_MADV_DONTFORK)
> so why do we need to do it here as well?

That only runs if kvm is enabled and mmu is not sync. But we need it
in the common case too, to prevent -ENOMEM (if MADV_DONTFORK is
available in the host OS, otherwise well we'll just do best effort and
skip). See commit message for more details.

> If we should be doing it in all cases presumably the right
> fix is to move the if (!kvm_has_sync_mmu()) check in
> kvm_setup_guest_memory() from "do we call madvise" to
> "do we fail with an error if it failed".

We could pass an error to kvm_setup_guest_memory but it's not worth it
considering more likely we should abort if kvm is enabled and mmu is
not sync (without bothering to call MADV_DONTFORK there).