On 15.05.20 11:54, Dr. David Alan Gilbert wrote:
> * David Hildenbrand (david@xxxxxxxxxx) wrote:
>> We want to replace qemu_balloon_inhibit() by something more generic.
>> Especially, we want to make sure that technologies that really rely on
>> RAM block discards to work reliably to run mutual exclusive with
>> technologies that break it.
>>
>> E.g., vfio will usually pin all guest memory, turning the virtio-balloon
>> basically useless and make the VM consume more memory than reported via
>> the balloon. While the balloon is special already (=> no guarantees, same
>> behavior possible afer reboots and with huge pages), this will be
>> different, especially, with virtio-mem.
>>
>> Let's implement a way such that we can make both types of technology run
>> mutually exclusive. We'll convert existing balloon inhibitors in successive
>> patches and add some new ones. Add the check to
>> qemu_balloon_is_inhibited() for now. We might want to make
>> virtio-balloon an acutal inhibitor in the future - however, that
>> requires more thought to not break existing setups.
>>
>> Cc: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
>> Cc: Richard Henderson <rth@xxxxxxxxxxx>
>> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
>> ---
>> balloon.c | 3 ++-
>> exec.c | 48 +++++++++++++++++++++++++++++++++++++++++++
>> include/exec/memory.h | 41 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 91 insertions(+), 1 deletion(-)
>>
>> diff --git a/balloon.c b/balloon.c
>> index f104b42961..c49f57c27b 100644
>> --- a/balloon.c
>> +++ b/balloon.c
>> @@ -40,7 +40,8 @@ static int balloon_inhibit_count;
>>
>> bool qemu_balloon_is_inhibited(void)
>> {
>> - return atomic_read(&balloon_inhibit_count) > 0;
>> + return atomic_read(&balloon_inhibit_count) > 0 ||
>> + ram_block_discard_is_broken();
>> }
>>
>> void qemu_balloon_inhibit(bool state)
>> diff --git a/exec.c b/exec.c
>> index 2874bb5088..52a6e40e99 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -4049,4 +4049,52 @@ void mtree_print_dispatch(AddressSpaceDispatch *d, MemoryRegion *root)
>> }
>> }
>>
>> +static int ram_block_discard_broken;
>
> This could do with a comment; if I'm reading this right then
> +ve means broken
> -ve means required
>
I'll add to ram_block_discard_broken:
"If positive, discarding RAM is broken. If negative, discarding of RAM
is required to work correctly."
[...]
>>
>> +/*
>> + * Inhibit technologies that rely on discarding of parts of RAM blocks to work
>> + * reliably, e.g., to manage the actual amount of memory consumed by the VM
>> + * (then, the memory provided by RAM blocks might be bigger than the desired
>> + * memory consumption). This *must* be set if:
>
> 'technologies that rely on discarding of parts of RAM blocks to work
> reliably' is pretty long; I'm not sure of a better way of saying it
> though.
Maybe simply
"Inhibit technologies that rely on discarding of pages in RAM blocks to
work"?
?
--
Thanks,
David / dhildenb
