Re: [PATCH v5 2/3] mm/mempolicy: add set_mempolicy_home_node syscall
From: Michal Hocko <mhocko@suse.com>
Date: 2021-11-29 12:45:32
Also in:
linux-mm
On Mon 29-11-21 16:16:05, Aneesh Kumar K.V wrote:
Michal Hocko [off-list ref] writes:quoted
On Tue 16-11-21 12:12:37, Aneesh Kumar K.V wrote:
[...]
quoted
quoted
+SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, len, + unsigned long, home_node, unsigned long, flags) +{ + struct mm_struct *mm = current->mm; + struct vm_area_struct *vma; + struct mempolicy *new; + unsigned long vmstart; + unsigned long vmend; + unsigned long end; + int err = -ENOENT; + + if (start & ~PAGE_MASK) + return -EINVAL; + /* + * flags is used for future extension if any. + */ + if (flags != 0) + return -EINVAL; + + if (!node_online(home_node)) + return -EINVAL;You really want to check the home_node before dereferencing the mask.Any reason why we want to check for home node first?
Because the given node is an index to node_states[N_ONLINE] bitmap. I do not think we do range checking there.
quoted hunk ↗ jump to hunk
quoted
quoted
+ len = (len + PAGE_SIZE - 1) & PAGE_MASK; + end = start + len; + + if (end < start) + return -EINVAL; + if (end == start) + return 0; + mmap_write_lock(mm); + vma = find_vma(mm, start); + for (; vma && vma->vm_start < end; vma = vma->vm_next) { + + vmstart = max(start, vma->vm_start); + vmend = min(end, vma->vm_end); + new = mpol_dup(vma_policy(vma)); + if (IS_ERR(new)) { + err = PTR_ERR(new); + break; + } + /* + * Only update home node if there is an existing vma policy + */ + if (!new) + continue;Your changelog only mentions MPOL_BIND and MPOL_PREFERRED_MANY as supported but you seem to be applying the home node to all existing policiesoThe restriction is done in policy_node.@@ -1801,6 +1856,11 @@ static int policy_node(gfp_t gfp, struct mempolicy *policy, int nd)WARN_ON_ONCE(policy->mode == MPOL_BIND && (gfp & __GFP_THISNODE)); } + if ((policy->mode == MPOL_BIND || + policy->mode == MPOL_PREFERRED_MANY) && + policy->home_node != NUMA_NO_NODE) + return policy->home_node; + return nd; }
But you do allow to set the home node also for other policies and that means that a default MPOL_INTERLEAVE would be different from the one with home_node set up even though they behave exactly the same. -- Michal Hocko SUSE Labs