On Mon 29-11-21 16:16:05, Aneesh Kumar K.V wrote:
> Michal Hocko <mhocko@xxxxxxxx> writes:
>
> > On Tue 16-11-21 12:12:37, Aneesh Kumar K.V wrote:
[...]
> >> +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.
> >> + 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
> > policieso
>
>
> The 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
