On Mon 29-11-21 20:29:05, Aneesh Kumar K.V wrote:
> Michal Hocko <mhocko@xxxxxxxx> writes:
>
> > On Mon 29-11-21 19:17:06, Aneesh Kumar K.V wrote:
> >> Michal Hocko <mhocko@xxxxxxxx> writes:
> > [...]
> >> > 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.
> >>
> >> I agree that there is no error returned if we try to set the home_node
> >> for other memory policies. But there should not be any behaviour
> >> differences. We ignore home_node for policies other than BIND and
> >> PREFERRED_MANY.
> >>
> >> The reason I avoided erroring out for other policies was to simplify the
> >> error handling.
> >
> > But this leads to a future extensions problems. How do you tell whether
> > a specific policy has a support for home node?
> >
> >> For example, for a range of addr with a mix of memory
> >> policy MPOLD_BIND and MPOL_INTERLEAVE what should be the state after the
> >> above system call?
> >
> > Do we even allow to combinate different memory policies?
> >
> >> We could say, we ignore setting home_node for vma
> >> with policy MPOL_INTERLEAVE and leave the home node set for vma with
> >> policy MPOL_BIND. Or should we make the system call return error also
> >> undoing the changes done for vmas for which we have set the home_node?
> >
> > The error behavior is really nasty with the existing behavior. The
> > caller has no way to tell which vmas have been updated. The only way is
> > to query the state. So if we return an error because of an incompatible
> > mempolicy in place we are not much worse than now. If the "error" is
> > silent then we establish a dependency on the specific implementation.
>
> How about
> 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;
>
> /*
> * If any vma in the range got policy other than MPOL_BIND
> * or MPOL_PREFERRED_MANY we return error. We don't reset
> * the home node for vmas we already updated before.
> */
> if (new->mode != MPOL_BIND && new->mode != MPOL_PREFERRED_MANY) {
> err = -EINVAL;
> break;
> }
Maybe ENOSUPP to make the error handling slightly easier.
--
Michal Hocko
SUSE Labs
