Re: [PATCH] mempolicy: check home_node is in the nodes of policy

[Chunsheng Luo <luochunsheng@xxxxxxxx>]

On 2024/1/30 10:29, Chunsheng Luo wrote:

On 2024/1/29 23:12, Gregory Price wrote:

On Fri, Jan 26, 2024 at 08:22:40AM -0500, Chunsheng Luo wrote:

set_mempolicy_home_node should be used after setting the memory
policy. If the home_node isn't in the nodes of policy, we should
return failure to avoid misunderstanding.

Signed-off-by: Chunsheng Luo <luochunsheng@xxxxxxxx>
---
 mm/mempolicy.c | 6 ++++++
 1 file changed, 6 insertions(+)

Since it's not possible to add/remove a node to a mask without also
erasing the home node, this seems reasonable.

e.g. this is what happens presently
mbind(0-2)    :   mask(0,1,2),   home_node(NUMA_NO_NODE)
home_node(3)  :   mask(0,1,2),   home_node(3)
mbind(0-3)    :   mask(0,1,2,3), home_node(NUMA_NO_NODE)

However, it is possible for a cgroup migration or a change to
cpusets.mems_allowed to change a nodemask without somping the home_node.

e.g.:
mbind(2-3)    :   mask(2-3),  home_node(NUMA_NO_NODE)
home_node(3)  :   mask(2-3),  home_node(3)
cpusets(0-1)  :   mask(0-1),  home_node(3)

Should the rebind code also shift the home-node or un-set it accordingly
to keep the mask/home_node behavior consistent with the syscalls?

(see mpol_rebind_nodemask)

Thank you for your reply.

First, home_node can only be set for VMA policy.

second, the result of cgroup migration is consistent with the result of the first e.g.

when process mem_allowed updates, vma policy will alse be updated.

Function call:

update_tasks_nodemask

     ->mpol_rebind_mm

         -> for_each_vma: mpol_rebind_policy

             -> mpol_rebind_nodemask

    Sorry, I make a mistake. The
      mpol_rebind_nodemask() only modifies the nodemask of the vma
      policy, 
      but doesn't modify the home_node, so the behavior is different
      from do_mbind(), so you are right.
> > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > > index 10a590ee1c89..9282be2ae18e 100644
> > > --- a/mm/mempolicy.c
> > > +++ b/mm/mempolicy.c
> > > @@ -1536,6 +1536,12 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le
> > >  			err = -EOPNOTSUPP;
> > >  			break;
> > >  		}
> > > +
> > > +		if (!node_isset(home_node, old->nodes)) {
> > > +			err = -EINVAL;
> > > +			break;
> > > +		}
> > > +
> > >  		new = mpol_dup(old);
> > >  		if (IS_ERR(new)) {
> > >  			err = PTR_ERR(new);
> > > -- 
> > > 2.43.0
> > >
> > >
> > >         
> >       
>