Re: [2.6.24 regression][BUGFIX] numactl --interleave=all doesn'tworks on memoryless node.

[Lee Schermerhorn]

Here's a patch that addresses the problem w/o requiring change to
numactl or libnuma.  It DOES have side affects, discussed in the
description.

Tested with memoryless nodes and restricted cpusets using the numactl
installed with RHEL5.1.

Altho' nominally against 24-mm1, applies cleanly to 2.6.24.  Should be
suitable for 'stable' if everyone agrees.

Lee
----------------------------------

[PATCH] 2.6.24-mm1 - mempolicy:  silently restrict to allowed nodes

Kosaki-san noted that "numactl --interleave=all ..." failed in the
presence of memoryless nodes.  This patch attempts to fix that 
problem.

Some background:  

numactl --interleave=all calls set_mempolicy(2) with a fully
populated [out to MAXNUMNODES] nodemask.  set_mempolicy()
[in do_set_mempolicy()] calls contextualize_policy() which
requires that the nodemask be a subset of the current task's
mems_allowed; else EINVAL will be returned.  A task's
mems_allowed will always be a subset of node_states[N_HIGH_MEMORY]--
i.e., nodes with memory.  So, a fully populated nodemask will
be declared invalid if it includes memoryless nodes.

  NOTE:  the same thing will occur when running in a cpuset
         with restricted mem_allowed--for the same reason:
         node mask contains dis-allowed nodes.

mbind(2), on the other hand, just masks off any nodes in the 
nodemask that are not included in the caller's mems_allowed.

In each case [mbind() and set_mempolicy()], mpol_check_policy()
will complain [again, resulting in EINVAL] if the nodemask contains 
any memoryless nodes.  This is somewhat redundant as mpol_new() 
will remove memoryless nodes for interleave policy, as will 
bind_zonelist()--called by mpol_new() for BIND policy.

Proposed fix:

1) modify contextualize_policy to just remove the non-allowed
   nodes, as is currently done in-line for mbind().  This
   guarantees that the resulting mask includes only nodes with
   memory.

   NOTE:  this is a [benign, IMO] change in behavior for
          set_mempolicy().  Dis-allowed nodes will be silently
          ignored, rather than returning an error.

          Another, perhaps less benign, change in behavior:
          MPOL_PREFERRED policy that specifies only memoryless nodes
          or nodes that are disallowed in the cpuset will be interpreted
          as "local allocation" as the nodemask will be empty after
          the masking in contextualize_policy().  With a bit of
          additional hackery I can make this return EINVAL.

          Comments?

2) modify mbind() to use contextualize_policy(), like set_mempolicy(),
   instead of masking nodes in-line.

3) remove the now redundant check for memoryless nodes from
   mpol_check_policy().

4) remove the masking of policy nodes for interleave policy from
   mpol_new().

Signed-off-by:  Lee Schermerhorn <lee.schermerhorn@xxxxxx>

 mm/mempolicy.c |   18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

Index: Linux/mm/mempolicy.c
===================================================================
--- Linux.orig/mm/mempolicy.c	2008-02-05 11:25:17.000000000 -0500
+++ Linux/mm/mempolicy.c	2008-02-05 16:03:11.000000000 -0500
@@ -131,7 +131,7 @@ static int mpol_check_policy(int mode, n
 			return -EINVAL;
 		break;
 	}
- 	return nodes_subset(*nodes, node_states[N_HIGH_MEMORY]) ? 0 : -EINVAL;
+ 	return 0;
 }
 
 /* Generate a custom zonelist for the BIND policy. */
@@ -188,8 +188,6 @@ static struct mempolicy *mpol_new(int mo
 	switch (mode) {
 	case MPOL_INTERLEAVE:
 		policy->v.nodes = *nodes;
-		nodes_and(policy->v.nodes, policy->v.nodes,
-					node_states[N_HIGH_MEMORY]);
 		if (nodes_weight(policy->v.nodes) == 0) {
 			kmem_cache_free(policy_cache, policy);
 			return ERR_PTR(-EINVAL);
@@ -426,9 +424,13 @@ static int contextualize_policy(int mode
 	if (!nodes)
 		return 0;
 
+	/*
+	 * Restrict the nodes to the allowed nodes in the cpuset.
+	 * This is guaranteed to be a subset of nodes with memory.
+	 */
 	cpuset_update_task_memory_state();
-	if (!cpuset_nodes_subset_current_mems_allowed(*nodes))
-		return -EINVAL;
+	nodes_and(*nodes, *nodes, cpuset_current_mems_allowed);
+
 	return mpol_check_policy(mode, nodes);
 }
 
@@ -797,7 +799,7 @@ static long do_mbind(unsigned long start
 	if (end == start)
 		return 0;
 
-	if (mpol_check_policy(mode, nmask))
+	if (contextualize_policy(mode, nmask))
 		return -EINVAL;
 
 	new = mpol_new(mode, nmask);
@@ -915,10 +917,6 @@ asmlinkage long sys_mbind(unsigned long 
 	err = get_nodes(&nodes, nmask, maxnode);
 	if (err)
 		return err;
-#ifdef CONFIG_CPUSETS
-	/* Restrict the nodes to the allowed nodes in the cpuset */
-	nodes_and(nodes, nodes, current->mems_allowed);
-#endif
 	return do_mbind(start, len, mode, &nodes, flags);
 }
 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/