Couple of questions about mempolicy rebinding

DISCUSSION Lee Schermerhorn 3 participants 3 messages Mar 17, 2008 – Mar 21, 2008 4 messages not in the archive Memory management

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): 1205437802.5300.69.camel@localhost

A message that is not in the archive (sent off-list or not yet received): [email protected]

Lee Schermerhorn

author

2008-03-17 20:36 UTC #4 lore raw

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Paul Jackson

2008-03-20 07:11 UTC #5 lore raw

Quoted 7 lines · Lee

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.

Signature · 9 lines

                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.940.382.4214

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

David Rientjes

2008-03-21 20:32 UTC #6 lore raw

In reply to Lee Schermerhorn

Quoted 6 lines · Lee Schermerhorn

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.

Quoted 8 lines

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().

Quoted 11 lines

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