2024-01-26 13:23:12

by Chunsheng Luo

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

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 <[email protected]>
---
mm/mempolicy.c | 6 ++++++
1 file changed, 6 insertions(+)

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

2024-01-28 08:29:53

by Andrew Morton

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

On Fri, 26 Jan 2024 08:22:40 -0500 Chunsheng Luo <[email protected]> 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.

Thanks. Under what circumstances does userspace trigger this issue?

2024-01-29 06:15:50

by Chunsheng Luo

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

For example,  In a system with NUMA nodes 0,1,2,3, i mbind process to
node 0-2 and set home_node to node 3, it will not be allocated from node
3, then from node closer to node 3. But i think home_node should be set
directly from node 0-2, which makes more sense. So i think it needs to
return failure to prompt user.


On 2024/1/28 16:29, Andrew Morton wrote:
> On Fri, 26 Jan 2024 08:22:40 -0500 Chunsheng Luo <[email protected]> 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.
> Thanks. Under what circumstances does userspace trigger this issue?
>

2024-01-29 15:13:44

by Gregory Price

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

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 <[email protected]>
> ---
> 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)


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