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

[Paolo Bonzini]

Il 06/08/2013 18:55, Andreas Färber ha scritto:
> Am 25.07.2013 12:32, schrieb 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.
> 
> So if we add the DONTFORK unconditionally here, why not drop it in said
> kvm_setup_guest_memory()? That would make the patch more
> self-documenting while at it.

This is mentioned in the commit message:

"In the common case we can ignore the error if MADV_DONTFORK is not
available. Leave a second invocation that errors out in the KVM path if
MMU notifiers are missing and KVM is enabled, to abort in such case".

Paolo