Re: [PATCH v6 14/15] qmp: Include "reserve" property of memory backends

[David Hildenbrand <david@redhat.com>]

On 23.04.21 14:13, Markus Armbruster wrote:
> David Hildenbrand <david@redhat.com> writes:
> 
>> On 23.04.21 13:21, Markus Armbruster wrote:
>>> David Hildenbrand <david@redhat.com> writes:
>>>
>>>> On 23.04.21 13:00, Markus Armbruster wrote:
>>>>> David Hildenbrand <david@redhat.com> writes:
>>>>>
>>>>>> Let's include the new property.
>>>>>>
>>>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>> Cc: Eric Blake <eblake@redhat.com>
>>>>>> Cc: Markus Armbruster <armbru@redhat.com>
>>>>>> Cc: Igor Mammedov <imammedo@redhat.com>
>>>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>>>> ---
>>>>>>     hw/core/machine-qmp-cmds.c | 1 +
>>>>>>     qapi/machine.json          | 4 ++++
>>>>>>     2 files changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
>>>>>> index d41db5b93b..2d135ecdd0 100644
>>>>>> --- a/hw/core/machine-qmp-cmds.c
>>>>>> +++ b/hw/core/machine-qmp-cmds.c
>>>>>> @@ -175,6 +175,7 @@ static int query_memdev(Object *obj, void *opaque)
>>>>>>             m->dump = object_property_get_bool(obj, "dump", &error_abort);
>>>>>>             m->prealloc = object_property_get_bool(obj, "prealloc", &error_abort);
>>>>>>             m->share = object_property_get_bool(obj, "share", &error_abort);
>>>>>> +        m->reserve = object_property_get_bool(obj, "reserve", &error_abort);
>>>>>>             m->policy = object_property_get_enum(obj, "policy", "HostMemPolicy",
>>>>>>                                                  &error_abort);
>>>>>>             host_nodes = object_property_get_qobject(obj,
>>>>>> diff --git a/qapi/machine.json b/qapi/machine.json
>>>>>> index 32650bfe9e..5932139d20 100644
>>>>>> --- a/qapi/machine.json
>>>>>> +++ b/qapi/machine.json
>>>>>> @@ -798,6 +798,9 @@
>>>>>>     #
>>>>>>     # @share: whether memory is private to QEMU or shared (since 6.1)
>>>>>>     #
>>>>>> +# @reserve: whether swap space (or huge pages) was reserved if applicable
>>>>>> +#           (since 6.1)
>>>>>> +#
>>>>>>     # @host-nodes: host nodes for its memory policy
>>>>>>     #
>>>>>>     # @policy: memory policy of memory backend
>>>>>> @@ -812,6 +815,7 @@
>>>>>>         'dump':       'bool',
>>>>>>         'prealloc':   'bool',
>>>>>>         'share':      'bool',
>>>>>> +    'reserve':    'bool',
>>>>>>         'host-nodes': ['uint16'],
>>>>>>         'policy':     'HostMemPolicy' }}
>>>>>
>>>>> Double-checking: true means definitely reserved, and false means
>>>>> definitely not reserved.  Correct?
>>>>
>>>> True means "reserved if applicable" which means "not reserved if not
>>>> applicable". False means "definitely not reserved".
>>>>
>>>> (any recommendation how to rephrase are appreciated; I tried my best --
>>>> this interface here makes it especially hard -- it's easier for the
>>>> property itself)
>>>
>>> When is it "applicable"?
>>
>> When the OS supports it for the memory type and it hasn't been disabled.
>>
>> Linux handling as described in
>>    [PATCH v6 09/15] util/mmap-alloc: Support RAM_NORESERVE via
>>    MAP_NORESERVE under Linux
>> and
>>
>>    https://www.kernel.org/doc/Documentation/vm/overcommit-accounting
>>
>> Summary *without* MAP_NORESERVE:
>>
>> a) !Hugetlbfs with Memory overcommit disabled via
>>      ("/proc/sys/vm/overcommit_memory == 2"): never
>>
>> b) !Hugetlbfs with Memory overcommit enabled
>>    1) Shared mappings on files: never
>>
>>    2) Private mappings on files: only when writable (for us always)
>>
>>    3) Shared anonymous memory: always
>>
>>    4) Private anonymous memory: only when writable (for us always)
>>
>> c) Hugetlbfs: Always
>>
>>
>> Under Windows: always. On other POSIX besides Linux -- don't know.
> 
> Would working some of this into the doc comment help users of the
> interface?  Up to you.
> 

I'll give it a thought. Most people will only care about explicitly 
disabling it, where we'll bail out if that doesn't work. Otherwise, they 
just use the OS default (== reserve if supported/applicable/not 
explicitly disabled).

Thanks!

-- 
Thanks,

David / dhildenb