Messages in this thread | | | Subject | Re: [2.6.24 regression][BUGFIX] numactl --interleave=all doesn't works on memoryless node. | From | Lee Schermerhorn <> | Date | Wed, 06 Feb 2008 11:00:17 -0500 |
| |
On Tue, 2008-02-05 at 14:12 -0800, Christoph Lameter wrote: > On Tue, 5 Feb 2008, Lee Schermerhorn wrote: > > > mbind(2), on the other hand, just masks off any nodes in the > > nodemask that are not included in the caller's mems_allowed. > > Ok so we temporarily adopt these semantics for set_mempolicy. > > > 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. > > Right make ssense. we already contextualize for cpusets.
Only for mbind(). set_mempolicy(), via contextualize_policy() was just returning EINVAL for invalid nodes in the mask. I don't know if it always worked like this, or if we did this in the memoryless nodes series...
> > > 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; > > } > > Hmmm... That is a pretty drastic change.
the nodes_subset() would always return true, once we mask it with cpuset_current_mems_allowed(), right? mems_allowed can now only contain nodes with memory and if cpusets are not configured, cpuset_current_mems_allowed() just returns node_states[N_HIGH_MEMORY].
So, I think this is a no-op.
> > > @@ -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); > > Do we really need to remove these lines if we change set_mempolicy?
Again, with the change to contextualize_policy(), the nodemask is guaranteed to only contain nodes with memory, so this was redundant.
> > > @@ -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); > > } > > > > Ditto?
This is the main change in the patch: masking off the invalid nodes [like sys_mbind() did inline] rather than complaining about them.
However, after I finish testing, I will post an update to this patch which restores some of the error checks that this change lost.
> > > @@ -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 > > Would just removing #ifdef CONFIG_CPUSETS work? mems_allowed falls back to > node_possible_map.... Shouldnt that be node_online_map?
I removed this because I changed do_mbind() to call the revised contextualize_policy() that does exactly this masking. I didn't see any reason to leave the duplicate code there.
I think that mems_allowed now falls back to nodes with memory. Or it should in the current code. When Paul adds his new magic, that might change.
Lee >
| |