On Mon 29-11-21 20:29:05, Aneesh Kumar K.V wrote:

Michal Hocko [off-list ref] writes:

quoted

On Mon 29-11-21 19:17:06, Aneesh Kumar K.V wrote:

quoted

Michal Hocko [off-list ref] writes:

[...]

quoted

quoted

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?

quoted

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?

quoted

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