Before this patch set, numatune only has three memory modes:
static, interleave and prefered. These memory policies are
ultimately set by mbind() system call.

Memory policy could be 'hard coded' into the kernel, but none of
above policies fit our requirment under this case. mbind() support
default memory policy, but it requires a NULL nodemask. So obviously
setting allowed memory nodes is cgroups' mission under this case.
So we introduce a new option for mode in numatune named 'restrictive'.

<numatune>
   <memory mode="restrictive" nodeset="1-4,^3"/>
   <memnode cellid="0" mode="restrictive" nodeset="1"/>
   <memnode cellid="2" mode="restrictive" nodeset="2"/>
</numatune>

The config above means we only use cgroups to restrict the allowed
memory nodes and not setting any specific memory policies explicitly.

RFC discussion:
https://www.redhat.com/archives/libvir-list/2020-November/msg01256.html

Regards,
Luyao

Luyao Zhong (3):
  docs: add docs for 'restrictive' option for mode in numatune
  schema: add 'restrictive' config option for mode in numatune
  qemu: add parser and formatter for 'restrictive' mode in numatune

 docs/formatdomain.rst                         |  7 +++-
 docs/schemas/domaincommon.rng                 |  2 +
 include/libvirt/libvirt-domain.h              |  1 +
 src/conf/numa_conf.c                          |  9 ++++
 src/qemu/qemu_command.c                       |  6 ++-
 src/qemu/qemu_process.c                       | 27 ++++++++++++
 src/util/virnuma.c                            |  3 ++
 .../numatune-memnode-invalid-mode.err         |  1 +
 .../numatune-memnode-invalid-mode.xml         | 33 +++++++++++++++
 ...emnode-restrictive-mode.x86_64-latest.args | 40 ++++++++++++++++++
 .../numatune-memnode-restrictive-mode.xml     | 33 +++++++++++++++
 tests/qemuxml2argvtest.c                      |  2 +
 ...memnode-restrictive-mode.x86_64-latest.xml | 41 +++++++++++++++++++
 tests/qemuxml2xmltest.c                       |  1 +
 14 files changed, 203 insertions(+), 3 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.err
 create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.xml
 create mode 100644 tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml
 create mode 100644 tests/qemuxml2xmloutdata/numatune-memnode-restrictive-mode.x86_64-latest.xml

-- 
2.25.4

On Tue, Mar 23, 2021 at 10:59:02AM +0800, Luyao Zhong wrote:
> Before this patch set, numatune only has three memory modes:
> static, interleave and prefered. These memory policies are
> ultimately set by mbind() system call.
> 
> Memory policy could be 'hard coded' into the kernel, but none of
> above policies fit our requirment under this case. mbind() support
> default memory policy, but it requires a NULL nodemask. So obviously
> setting allowed memory nodes is cgroups' mission under this case.
> So we introduce a new option for mode in numatune named 'restrictive'.
> 
> <numatune>
>    <memory mode="restrictive" nodeset="1-4,^3"/>
>    <memnode cellid="0" mode="restrictive" nodeset="1"/>
>    <memnode cellid="2" mode="restrictive" nodeset="2"/>
> </numatune>

'restrictive' is rather a wierd name and doesn't really tell me what
the memory policy is going to be. As far as I can tell from the
patches, it seems this causes us to not set any memory alllocation
policy at all. IOW, we're using some undefined host default policy.

Given this I think we should be calling it either "none" or "default"


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Tue, Mar 23, 2021 at 09:48:02AM +0000, Daniel P. Berrangé wrote:
>On Tue, Mar 23, 2021 at 10:59:02AM +0800, Luyao Zhong wrote:
>> Before this patch set, numatune only has three memory modes:
>> static, interleave and prefered. These memory policies are
>> ultimately set by mbind() system call.
>>
>> Memory policy could be 'hard coded' into the kernel, but none of
>> above policies fit our requirment under this case. mbind() support
>> default memory policy, but it requires a NULL nodemask. So obviously
>> setting allowed memory nodes is cgroups' mission under this case.
>> So we introduce a new option for mode in numatune named 'restrictive'.
>>
>> <numatune>
>>    <memory mode="restrictive" nodeset="1-4,^3"/>
>>    <memnode cellid="0" mode="restrictive" nodeset="1"/>
>>    <memnode cellid="2" mode="restrictive" nodeset="2"/>
>> </numatune>
>
>'restrictive' is rather a wierd name and doesn't really tell me what
>the memory policy is going to be. As far as I can tell from the
>patches, it seems this causes us to not set any memory alllocation
>policy at all. IOW, we're using some undefined host default policy.
>
>Given this I think we should be calling it either "none" or "default"
>

I was against "default" because having such option possible, but the actual
default being different sounds stupid.  Similarly "none" sounds like no
restrictions are applied or that it is the same as if nothing was specified.  It
is funny to imagine the situation when I am explaining to someone how to achieve
this solution:

   "The default is 'strict', you need to explicitly set it to 'default'."

or

   "What setting did you use?"
   "None"
   "As in no mode or in mode='none'?"

As I said before, please come up with any name, but not these that are IMHO
actually more confusing.

>
>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>

On Wed, Mar 24, 2021 at 09:46:23PM +0100, Martin Kletzander wrote:
> On Tue, Mar 23, 2021 at 09:48:02AM +0000, Daniel P. Berrangé wrote:
> > On Tue, Mar 23, 2021 at 10:59:02AM +0800, Luyao Zhong wrote:
> > > Before this patch set, numatune only has three memory modes:
> > > static, interleave and prefered. These memory policies are
> > > ultimately set by mbind() system call.
> > > 
> > > Memory policy could be 'hard coded' into the kernel, but none of
> > > above policies fit our requirment under this case. mbind() support
> > > default memory policy, but it requires a NULL nodemask. So obviously
> > > setting allowed memory nodes is cgroups' mission under this case.
> > > So we introduce a new option for mode in numatune named 'restrictive'.
> > > 
> > > <numatune>
> > >    <memory mode="restrictive" nodeset="1-4,^3"/>
> > >    <memnode cellid="0" mode="restrictive" nodeset="1"/>
> > >    <memnode cellid="2" mode="restrictive" nodeset="2"/>
> > > </numatune>
> > 
> > 'restrictive' is rather a wierd name and doesn't really tell me what
> > the memory policy is going to be. As far as I can tell from the
> > patches, it seems this causes us to not set any memory alllocation
> > policy at all. IOW, we're using some undefined host default policy.
> > 
> > Given this I think we should be calling it either "none" or "default"
> > 
> 
> I was against "default" because having such option possible, but the actual
> default being different sounds stupid.  Similarly "none" sounds like no
> restrictions are applied or that it is the same as if nothing was specified.  It
> is funny to imagine the situation when I am explaining to someone how to achieve
> this solution:
> 
>   "The default is 'strict', you need to explicitly set it to 'default'."

These patches aren't claiming the default is strict though - they're saying
the default is whatever the kernel has been configured to be.  The kernel
could apply interleave, or preferred or strict.  So using "default" as the
term is fine, because we explicitly aren't guaranteing which behaviour is
used.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Thu, Mar 25, 2021 at 08:36:17AM +0000, Daniel P. Berrangé wrote:
>On Wed, Mar 24, 2021 at 09:46:23PM +0100, Martin Kletzander wrote:
>> On Tue, Mar 23, 2021 at 09:48:02AM +0000, Daniel P. Berrangé wrote:
>> > On Tue, Mar 23, 2021 at 10:59:02AM +0800, Luyao Zhong wrote:
>> > > Before this patch set, numatune only has three memory modes:
>> > > static, interleave and prefered. These memory policies are
>> > > ultimately set by mbind() system call.
>> > >
>> > > Memory policy could be 'hard coded' into the kernel, but none of
>> > > above policies fit our requirment under this case. mbind() support
>> > > default memory policy, but it requires a NULL nodemask. So obviously
>> > > setting allowed memory nodes is cgroups' mission under this case.
>> > > So we introduce a new option for mode in numatune named 'restrictive'.
>> > >
>> > > <numatune>
>> > >    <memory mode="restrictive" nodeset="1-4,^3"/>
>> > >    <memnode cellid="0" mode="restrictive" nodeset="1"/>
>> > >    <memnode cellid="2" mode="restrictive" nodeset="2"/>
>> > > </numatune>
>> >
>> > 'restrictive' is rather a wierd name and doesn't really tell me what
>> > the memory policy is going to be. As far as I can tell from the
>> > patches, it seems this causes us to not set any memory alllocation
>> > policy at all. IOW, we're using some undefined host default policy.
>> >
>> > Given this I think we should be calling it either "none" or "default"
>> >
>>
>> I was against "default" because having such option possible, but the actual
>> default being different sounds stupid.  Similarly "none" sounds like no
>> restrictions are applied or that it is the same as if nothing was specified.  It
>> is funny to imagine the situation when I am explaining to someone how to achieve
>> this solution:
>>
>>   "The default is 'strict', you need to explicitly set it to 'default'."
>
>These patches aren't claiming the default is strict though - they're saying
>the default is whatever the kernel has been configured to be.  The kernel
>could apply interleave, or preferred or strict.  So using "default" as the
>term is fine, because we explicitly aren't guaranteing which behaviour is
>used.
>

Sorry, I was not clear.  Our (libvirt) current default is "strict".
That's why it seems weird to have that new value be called "default".

>
>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

> -----Original Message-----
> From: Martin Kletzander <mkletzan@redhat.com>
> Sent: Thursday, March 25, 2021 4:46 AM
> To: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Zhong, Luyao <luyao.zhong@intel.com>; libvir-list@redhat.com
> Subject: Re: [libvirt][PATCH v4 0/3] introduce 'restrictive' mode in numatune
> 
> On Tue, Mar 23, 2021 at 09:48:02AM +0000, Daniel P. Berrangé wrote:
> >On Tue, Mar 23, 2021 at 10:59:02AM +0800, Luyao Zhong wrote:
> >> Before this patch set, numatune only has three memory modes:
> >> static, interleave and prefered. These memory policies are ultimately
> >> set by mbind() system call.
> >>
> >> Memory policy could be 'hard coded' into the kernel, but none of
> >> above policies fit our requirment under this case. mbind() support
> >> default memory policy, but it requires a NULL nodemask. So obviously
> >> setting allowed memory nodes is cgroups' mission under this case.
> >> So we introduce a new option for mode in numatune named 'restrictive'.
> >>
> >> <numatune>
> >>    <memory mode="restrictive" nodeset="1-4,^3"/>
> >>    <memnode cellid="0" mode="restrictive" nodeset="1"/>
> >>    <memnode cellid="2" mode="restrictive" nodeset="2"/> </numatune>
> >
> >'restrictive' is rather a wierd name and doesn't really tell me what
> >the memory policy is going to be. As far as I can tell from the
> >patches, it seems this causes us to not set any memory alllocation
> >policy at all. IOW, we're using some undefined host default policy.
> >
> >Given this I think we should be calling it either "none" or "default"
> >
> 
> I was against "default" because having such option possible, but the actual
> default being different sounds stupid.  Similarly "none" sounds like no
> restrictions are applied or that it is the same as if nothing was specified.  It is
> funny to imagine the situation when I am explaining to someone how to achieve
> this solution:
> 
>    "The default is 'strict', you need to explicitly set it to 'default'."
> 
> or
> 
>    "What setting did you use?"
>    "None"
>    "As in no mode or in mode='none'?"
> 
> As I said before, please come up with any name, but not these that are IMHO
> actually more confusing.
> 

Hi Daniel and Martin, thanks for your reply, just as Martin said current default mode is "strict", so "default" was deprecated at the beginning when I proposed this change.  And actually we have cgroups restricting the memory resource so could we call this a "none" mode? I still don't have a better name. ☹

> >
> >Regards,
> >Daniel
> >--
> >|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> >|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> >|: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
> >

On Thu, Mar 25, 2021 at 09:11:02AM +0000, Zhong, Luyao wrote:
>
>
>> -----Original Message-----
>> From: Martin Kletzander <mkletzan@redhat.com>
>> Sent: Thursday, March 25, 2021 4:46 AM
>> To: Daniel P. Berrangé <berrange@redhat.com>
>> Cc: Zhong, Luyao <luyao.zhong@intel.com>; libvir-list@redhat.com
>> Subject: Re: [libvirt][PATCH v4 0/3] introduce 'restrictive' mode in numatune
>>
>> On Tue, Mar 23, 2021 at 09:48:02AM +0000, Daniel P. Berrangé wrote:
>> >On Tue, Mar 23, 2021 at 10:59:02AM +0800, Luyao Zhong wrote:
>> >> Before this patch set, numatune only has three memory modes:
>> >> static, interleave and prefered. These memory policies are ultimately
>> >> set by mbind() system call.
>> >>
>> >> Memory policy could be 'hard coded' into the kernel, but none of
>> >> above policies fit our requirment under this case. mbind() support
>> >> default memory policy, but it requires a NULL nodemask. So obviously
>> >> setting allowed memory nodes is cgroups' mission under this case.
>> >> So we introduce a new option for mode in numatune named 'restrictive'.
>> >>
>> >> <numatune>
>> >>    <memory mode="restrictive" nodeset="1-4,^3"/>
>> >>    <memnode cellid="0" mode="restrictive" nodeset="1"/>
>> >>    <memnode cellid="2" mode="restrictive" nodeset="2"/> </numatune>
>> >
>> >'restrictive' is rather a wierd name and doesn't really tell me what
>> >the memory policy is going to be. As far as I can tell from the
>> >patches, it seems this causes us to not set any memory alllocation
>> >policy at all. IOW, we're using some undefined host default policy.
>> >
>> >Given this I think we should be calling it either "none" or "default"
>> >
>>
>> I was against "default" because having such option possible, but the actual
>> default being different sounds stupid.  Similarly "none" sounds like no
>> restrictions are applied or that it is the same as if nothing was specified.  It is
>> funny to imagine the situation when I am explaining to someone how to achieve
>> this solution:
>>
>>    "The default is 'strict', you need to explicitly set it to 'default'."
>>
>> or
>>
>>    "What setting did you use?"
>>    "None"
>>    "As in no mode or in mode='none'?"
>>
>> As I said before, please come up with any name, but not these that are IMHO
>> actually more confusing.
>>
>
>Hi Daniel and Martin, thanks for your reply, just as Martin said
>current default mode is "strict", so "default" was deprecated at the
>beginning when I proposed this change.  And actually we have cgroups
>restricting the memory resource so could we call this a "none" mode? I
>still don't have a better name. ☹
>

Me neither as figuring out the names when our names do not precisely map
to anything else (since we are using multiple solutions to get as close
to the desired result as possible) is difficult because there is no
similar pre-existing setting.  And using anything like "cgroups-only"
would limit us in the future, probably.

>> >
>> >Regards,
>> >Daniel
>> >--
>> >|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>> >|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>> >|: https://entangle-photo.org    -o-
>> https://www.instagram.com/dberrange :|
>> >