Re: [PATCH v5 2/3] mm/mempolicy: add set_mempolicy_home_node syscall

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

Michal Hocko <mhocko@suse.com> writes:

> On Mon 29-11-21 19:17:06, Aneesh Kumar K.V wrote:
>> Michal Hocko <mhocko@suse.com> 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;
		}


....