Couple of questions about mempolicy rebinding

Couple of questions about mempolicy rebinding

[Lee Schermerhorn]

Paul, David:

Like the subject says, I have a couple of questions that arose while
looking through the mempolicy code for corner cases that might be
affected by some pending cleanup patches.  Nothing major, I think, I
just want to understand.  I'm looking at 25-rc5-mm1 plus the recent
patch:  "disallow static or relative flags for local preferred".

1) In __mpol_copy():  when the "current_cpuset_is_being_rebound", why do
we rebind the old policy policy and then copy it to the new?  Seems like
the old policy will get rebound in due time if, indeed, it needs to be
rebound.  I don't see any usage now, where it won't, but this seems less
general than just rebinding the new copy.  E.g., the old mempolicy being
copied may be a context-free policy that shouln't be rebound.   I think
we should at least add a comment to warn future callers.  Comments?

2) In mpol_rebind_nodemask():  this function is shared by bind and
interleave policy, and is used to rebind both task and vma policies.
However, we unconditionally update current->il_next.  Probably not an
issue for interleave vs bind because, in the case of bind policy,
il_next is meaningless.  However, il_next is used only to interleave
based on task policy [for page cache and slab allocations].  Any
allocation based on a vma policy will use the address offset into the
vma to interleave.  So, I think it's technically incorrect to update
il_next when we're rebinding a vma policy.  It may contain nodes that
are not in the task policy and vice versa.  

If we were to address this, a couple of methods come to mind:

a) add an internal mode flag--e.g., MPOL_F_TASK_POLICY--to indicate task
vs vma policy to the rebind ops, or

b) pass a task vs vma flag parameter to mpol_rebind_policy() and down to
the rebind ops.  However, how would we tell in __mpol_copy() which we're
dealing with?

Comments?

Lee

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Couple of questions about mempolicy rebinding

[Paul Jackson]

Lee wrote:
> 1) In __mpol_copy():  when the "current_cpuset_is_being_rebound", why do
> we rebind the old policy policy and then copy it to the new?  Seems like
> the old policy will get rebound in due time if, indeed, it needs to be
> rebound.  I don't see any usage now, where it won't, but this seems less
> general than just rebinding the new copy.  E.g., the old mempolicy being
> copied may be a context-free policy that shouln't be rebound.   I think
> we should at least add a comment to warn future callers.  Comments?

Sorry for the delay responding.

You're probably right; I'm not sure.  I have no record nor recollection
of why I rebound the old policy before copying, instead of rebinding the
new policy after copying.  I suspect this might be a case of "shoot
everything in sight, and hope I get them all."  Most code that I write
in that state of mind eventually gets fixed, to be more precise, once
someone has a better understanding.  Looks like this is that time.

I'd be more than comfortable you changing this to copy first and then
only rebind the new policy.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Couple of questions about mempolicy rebinding

[David Rientjes]

On Mon, 17 Mar 2008, Lee Schermerhorn wrote:

> Like the subject says, I have a couple of questions that arose while
> looking through the mempolicy code for corner cases that might be
> affected by some pending cleanup patches.  Nothing major, I think, I
> just want to understand.  I'm looking at 25-rc5-mm1 plus the recent
> patch:  "disallow static or relative flags for local preferred".
> 

Sorry for the delay in getting to this email.

> 1) In __mpol_copy():  when the "current_cpuset_is_being_rebound", why do
> we rebind the old policy policy and then copy it to the new?  Seems like
> the old policy will get rebound in due time if, indeed, it needs to be
> rebound.  I don't see any usage now, where it won't, but this seems less
> general than just rebinding the new copy.  E.g., the old mempolicy being
> copied may be a context-free policy that shouln't be rebound.   I think
> we should at least add a comment to warn future callers.  Comments?
> 

I don't quite understand.  The mempolicy being returned by __mpol_copy() 
is brand new for a forked task and has no relation to the old policy 
except it shares all of the same characteristics.  So if we have yet to 
call mpol_rebind_mm() after updating current->cpuset->mems_allowed in 
update_nodemask(), we'll get the old nodemask in the policy returned by 
__mpol_copy().

> 2) In mpol_rebind_nodemask():  this function is shared by bind and
> interleave policy, and is used to rebind both task and vma policies.
> However, we unconditionally update current->il_next.  Probably not an
> issue for interleave vs bind because, in the case of bind policy,
> il_next is meaningless.  However, il_next is used only to interleave
> based on task policy [for page cache and slab allocations].  Any
> allocation based on a vma policy will use the address offset into the
> vma to interleave.  So, I think it's technically incorrect to update
> il_next when we're rebinding a vma policy.  It may contain nodes that
> are not in the task policy and vice versa.  
> 

I don't see how this behavior has recently changed from what is currently 
in HEAD.  Are you concerned about an unnecessary update to 
current->il_next that affects the next node for a task's interleave if a 
VMA with a VMA policy gets rebound but there is an underlying task policy 
also of interleave?

To avoid updating current->il_next for MPOL_INTERLEAVE vs. MPOL_BIND, we 
can simply test pol->policy in mpol_rebind_nodemask().

		David

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>