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

[Andreas Färber]

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.

Andreas

> 
>> 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).
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg