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

Commit Message

David Hildenbrand April 21, 2021, 12:26 p.m. UTC

Let's include the new property.

Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
Cc: Eric Blake <[e-mail]>
Cc: Markus Armbruster <[e-mail]>
Cc: Igor Mammedov <[e-mail]>
Signed-off-by: David Hildenbrand <[e-mail]>
---
 hw/core/machine-qmp-cmds.c | 1 +
 qapi/machine.json          | 4 ++++
 2 files changed, 5 insertions(+)

Comments

Eduardo Habkost April 21, 2021, 9:05 p.m. UTC | #1

On Wed, Apr 21, 2021 at 02:26:23PM +0200, David Hildenbrand wrote:
> Let's include the new property.
> 
> Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
> Cc: Eric Blake <[e-mail]>
> Cc: Markus Armbruster <[e-mail]>
> Cc: Igor Mammedov <[e-mail]>
> Signed-off-by: David Hildenbrand <[e-mail]>

Reviewed-by: Eduardo Habkost <[e-mail]>

Markus Armbruster April 23, 2021, 11 a.m. UTC | #2

David Hildenbrand <[e-mail]> writes:

> Let's include the new property.
>
> Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
> Cc: Eric Blake <[e-mail]>
> Cc: Markus Armbruster <[e-mail]>
> Cc: Igor Mammedov <[e-mail]>
> Signed-off-by: David Hildenbrand <[e-mail]>
> ---
>  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?

I'm asking because the discussion of v4 + passage of time = me
uncertain.

David Hildenbrand April 23, 2021, 11:03 a.m. UTC | #3

On 23.04.21 13:00, Markus Armbruster wrote:
> David Hildenbrand <[e-mail]> writes:
> 
>> Let's include the new property.
>>
>> Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
>> Cc: Eric Blake <[e-mail]>
>> Cc: Markus Armbruster <[e-mail]>
>> Cc: Igor Mammedov <[e-mail]>
>> Signed-off-by: David Hildenbrand <[e-mail]>
>> ---
>>   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)

Markus Armbruster April 23, 2021, 11:21 a.m. UTC | #4

David Hildenbrand <[e-mail]> writes:

> On 23.04.21 13:00, Markus Armbruster wrote:
>> David Hildenbrand <[e-mail]> writes:
>> 
>>> Let's include the new property.
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
>>> Cc: Eric Blake <[e-mail]>
>>> Cc: Markus Armbruster <[e-mail]>
>>> Cc: Igor Mammedov <[e-mail]>
>>> Signed-off-by: David Hildenbrand <[e-mail]>
>>> ---
>>>   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"?

David Hildenbrand April 23, 2021, 11:33 a.m. UTC | #5

On 23.04.21 13:21, Markus Armbruster wrote:
> David Hildenbrand <[e-mail]> writes:
> 
>> On 23.04.21 13:00, Markus Armbruster wrote:
>>> David Hildenbrand <[e-mail]> writes:
>>>
>>>> Let's include the new property.
>>>>
>>>> Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
>>>> Cc: Eric Blake <[e-mail]>
>>>> Cc: Markus Armbruster <[e-mail]>
>>>> Cc: Igor Mammedov <[e-mail]>
>>>> Signed-off-by: David Hildenbrand <[e-mail]>
>>>> ---
>>>>    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.

Markus Armbruster April 23, 2021, 12:13 p.m. UTC | #6

David Hildenbrand <[e-mail]> writes:

> On 23.04.21 13:21, Markus Armbruster wrote:
>> David Hildenbrand <[e-mail]> writes:
>> 
>>> On 23.04.21 13:00, Markus Armbruster wrote:
>>>> David Hildenbrand <[e-mail]> writes:
>>>>
>>>>> Let's include the new property.
>>>>>
>>>>> Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
>>>>> Cc: Eric Blake <[e-mail]>
>>>>> Cc: Markus Armbruster <[e-mail]>
>>>>> Cc: Igor Mammedov <[e-mail]>
>>>>> Signed-off-by: David Hildenbrand <[e-mail]>
>>>>> ---
>>>>>    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.

David Hildenbrand April 23, 2021, 12:29 p.m. UTC | #7

On 23.04.21 14:13, Markus Armbruster wrote:
> David Hildenbrand <[e-mail]> writes:
> 
>> On 23.04.21 13:21, Markus Armbruster wrote:
>>> David Hildenbrand <[e-mail]> writes:
>>>
>>>> On 23.04.21 13:00, Markus Armbruster wrote:
>>>>> David Hildenbrand <[e-mail]> writes:
>>>>>
>>>>>> Let's include the new property.
>>>>>>
>>>>>> Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
>>>>>> Cc: Eric Blake <[e-mail]>
>>>>>> Cc: Markus Armbruster <[e-mail]>
>>>>>> Cc: Igor Mammedov <[e-mail]>
>>>>>> Signed-off-by: David Hildenbrand <[e-mail]>
>>>>>> ---
>>>>>>     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!

David Hildenbrand April 23, 2021, 2:13 p.m. UTC | #8

On 23.04.21 14:29, David Hildenbrand wrote:
> On 23.04.21 14:13, Markus Armbruster wrote:
>> David Hildenbrand <[e-mail]> writes:
>>
>>> On 23.04.21 13:21, Markus Armbruster wrote:
>>>> David Hildenbrand <[e-mail]> writes:
>>>>
>>>>> On 23.04.21 13:00, Markus Armbruster wrote:
>>>>>> David Hildenbrand <[e-mail]> writes:
>>>>>>
>>>>>>> Let's include the new property.
>>>>>>>
>>>>>>> Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
>>>>>>> Cc: Eric Blake <[e-mail]>
>>>>>>> Cc: Markus Armbruster <[e-mail]>
>>>>>>> Cc: Igor Mammedov <[e-mail]>
>>>>>>> Signed-off-by: David Hildenbrand <[e-mail]>
>>>>>>> ---
>>>>>>>      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!

Markus Armbruster April 23, 2021, 2:55 p.m. UTC | #9

David Hildenbrand <[e-mail]> writes:

> On 23.04.21 14:29, David Hildenbrand wrote:
>> On 23.04.21 14:13, Markus Armbruster wrote:
>>> David Hildenbrand <[e-mail]> writes:
>>>
>>>> On 23.04.21 13:21, Markus Armbruster wrote:
>>>>> David Hildenbrand <[e-mail]> writes:
>>>>>
>>>>>> On 23.04.21 13:00, Markus Armbruster wrote:
>>>>>>> David Hildenbrand <[e-mail]> writes:
>>>>>>>
>>>>>>>> Let's include the new property.
>>>>>>>>
>>>>>>>> Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
>>>>>>>> Cc: Eric Blake <[e-mail]>
>>>>>>>> Cc: Markus Armbruster <[e-mail]>
>>>>>>>> Cc: Igor Mammedov <[e-mail]>
>>>>>>>> Signed-off-by: David Hildenbrand <[e-mail]>
>>>>>>>> ---
>>>>>>>>      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!

Good balance between too little and too much detail.

Reviewed-by: Markus Armbruster <[e-mail]>

David Hildenbrand April 23, 2021, 3:04 p.m. UTC | #10

On 23.04.21 16:55, Markus Armbruster wrote:
> Reviewed-by: Markus Armbruster<[e-mail]>

Thanks Markus -- have a nice weekend!

Patch

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' }}