David Hildenbrand <da...@redhat.com> writes:
> On 23.04.21 13:14, Markus Armbruster wrote:
>> David Hildenbrand <da...@redhat.com> writes:
>>
>>> Let's provide a way to control the use of RAM_NORESERVE via memory
>>> backends using the "reserve" property which defaults to true (old
>>> behavior).
>>>
>>> Only Linux currently supports clearing the flag (and support is checked at
>>> runtime, depending on the setting of "/proc/sys/vm/overcommit_memory").
>>> Windows and other POSIX systems will bail out with "reserve=false".
>>>
>>> The target use case is virtio-mem, which dynamically exposes memory
>>> inside a large, sparse memory area to the VM. This essentially allows
>>> avoiding to set "/proc/sys/vm/overcommit_memory == 0") when using
>>> virtio-mem and also supporting hugetlbfs in the future.
>>>
>>> Reviewed-by: Peter Xu <pet...@redhat.com>
>>> Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
>>> Cc: Markus Armbruster <arm...@redhat.com>
>>> Cc: Eric Blake <ebl...@redhat.com>
>>> Cc: Igor Mammedov <imamm...@redhat.com>
>>> Signed-off-by: David Hildenbrand <da...@redhat.com>
>>> ---
>>> backends/hostmem-file.c | 11 ++++++-----
>>> backends/hostmem-memfd.c | 1 +
>>> backends/hostmem-ram.c | 1 +
>>> backends/hostmem.c | 32 ++++++++++++++++++++++++++++++++
>>> include/sysemu/hostmem.h | 2 +-
>>> qapi/qom.json | 4 ++++
>>> 6 files changed, 45 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
>>> index b683da9daf..9d550e53d4 100644
>>> --- a/backends/hostmem-file.c
>>> +++ b/backends/hostmem-file.c
>>> @@ -40,6 +40,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend,
>>> Error **errp)
>>> object_get_typename(OBJECT(backend)));
>>> #else
>>> HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
>>> + uint32_t ram_flags;
>>> gchar *name;
>>> if (!backend->size) {
>>> @@ -52,11 +53,11 @@ file_backend_memory_alloc(HostMemoryBackend *backend,
>>> Error **errp)
>>> }
>>> name = host_memory_backend_get_name(backend);
>>> - memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
>>> - name,
>>> - backend->size, fb->align,
>>> - (backend->share ? RAM_SHARED : 0) |
>>> - (fb->is_pmem ? RAM_PMEM : 0),
>>> + ram_flags = backend->share ? RAM_SHARED : 0;
>>> + ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
>>> + ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
>>> + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
>>> + backend->size, fb->align, ram_flags,
>>> fb->mem_path, fb->readonly, errp);
>>> g_free(name);
>>> #endif
>>> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
>>> index 93b5d1a4cf..f3436b623d 100644
>>> --- a/backends/hostmem-memfd.c
>>> +++ b/backends/hostmem-memfd.c
>>> @@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend,
>>> Error **errp)
>>> name = host_memory_backend_get_name(backend);
>>> ram_flags = backend->share ? RAM_SHARED : 0;
>>> + ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
>>> memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name,
>>> backend->size, ram_flags, fd, 0, errp);
>>> g_free(name);
>>> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
>>> index 741e701062..b8e55cdbd0 100644
>>> --- a/backends/hostmem-ram.c
>>> +++ b/backends/hostmem-ram.c
>>> @@ -29,6 +29,7 @@ ram_backend_memory_alloc(HostMemoryBackend *backend,
>>> Error **errp)
>>> name = host_memory_backend_get_name(backend);
>>> ram_flags = backend->share ? RAM_SHARED : 0;
>>> + ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
>>> memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend),
>>> name,
>>> backend->size, ram_flags,
>>> errp);
>>> g_free(name);
>>
>> As the commit message says, @reserve translates to RAM_NORESERVE.
>> Good.
>> I figure passing RAM_NORESERVE can't make these functions fail.
>> Correct?
>> @reserve defaults to true. The commit message assures us this gives
>> us
>> the old behavior. Good. But the patch *adds* flag RAM_NORESERVE when
>> it is true. Now I'm confused.
>
> ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
>
> translates to
>
> if (!backend->reserve)
> ram_flags |= RAM_NORESERVE;
You're right! /me uncrosses eyes...
> I thought for a while if calling the property "noreserve" would be
> cleaner, but decided against it.
I dislike "negative" flag names, too.
