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

[David Hildenbrand <david@redhat.com>]

On 23.04.21 14:29, David Hildenbrand wrote:
> 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

Actually, it's a bit simpler in our case (writable mappings). The more complex part is the MAP_NORESERVE handling.

a) !Hugetlbfs
	1) Shared mappings on files: never reserve swap space

	2) Other mappings: always reserve swap space

b) Hugetlbfs: Always reserve huge pages.

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

I think something like the following might make sense:

# @reserve: whether swap space (or huge pages) was reserved if applicable.
#           This corresponds to the user configuration and not the actual
#           behavior implemented in the OS to perform a reservation;
#           For example, Linux will never reserve swap space for shared
#           file mappings. (since 6.1)

Thanks!

-- 
Thanks,

David / dhildenb