Use MPOL_PREFERRED for system default policy

Now, system default policy, except during boot, is "local allocation". By using the MPOL_PREFERRED mode with a negative value of preferred node for system default policy, MPOL_DEFAULT will never occur in the 'policy' member of a struct mempolicy. Thus, we can remove all checks for MPOL_DEFAULT when converting policy to a node id/zonelist in the allocation paths.

Quoted 6 lines · Lee Schermerhorn

Now, system default policy, except during boot, is "local allocation". By using the MPOL_PREFERRED mode with a negative value of preferred node for system default policy, MPOL_DEFAULT will never occur in the 'policy' member of a struct mempolicy. Thus, we can remove all checks for MPOL_DEFAULT when converting policy to a node id/zonelist in the allocation paths.

Isnt it possible to set a task policy or VMA policy to MPOL_DEFAULT through the API? For the VMA policy this would mean fall back to task policy. Is that still possible?

Quoted 9 lines · Christoph Lameter

Quoted 6 lines · Lee Schermerhorn

Now, system default policy, except during boot, is "local allocation". By using the MPOL_PREFERRED mode with a negative value of preferred node for system default policy, MPOL_DEFAULT will never occur in the 'policy' member of a struct mempolicy. Thus, we can remove all checks for MPOL_DEFAULT when converting policy to a node id/zonelist in the allocation paths.

Quoted 3 lines

Isnt it possible to set a task policy or VMA policy to MPOL_DEFAULT through the API? For the VMA policy this would mean fall back to task policy. Is that still possible?

No. mpol_new() returns NULL if policy==MPOL_DEFAULT, so you end up just deleting any existing task policy and replacing it with a NULL pointer. This is pretty cool, I think. I have checked back, but Andi may have

done this from day 1.

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>

Quoted 10 lines · Lee Schermerhorn

Quoted 9 lines · Christoph Lameter

Quoted 6 lines · Lee Schermerhorn

Now, system default policy, except during boot, is "local allocation". By using the MPOL_PREFERRED mode with a negative value of preferred node for system default policy, MPOL_DEFAULT will never occur in the 'policy' member of a struct mempolicy. Thus, we can remove all checks for MPOL_DEFAULT when converting policy to a node id/zonelist in the allocation paths.

Quoted 7 lines

Quoted 3 lines

Isnt it possible to set a task policy or VMA policy to MPOL_DEFAULT through the API? For the VMA policy this would mean fall back to task policy. Is that still possible?

No. mpol_new() returns NULL if policy==MPOL_DEFAULT, so you end up just deleting any existing task policy and replacing it with a NULL pointer. This is pretty cool, I think. I have checked back, but Andi may have

                                   ^^^^ haven't!

Quoted 2 lines

done this from day 1.

Given that the mem policy does the right thing with this patch, can we merge it? I think it cleans up the mem policy concepts to have MPOL_DEFAULT mean "use default policy for this context/scope" rather than have an additional allocation behavior of its own.

Quoted 4 lines · Lee Schermerhorn

Given that the mem policy does the right thing with this patch, can we merge it? I think it cleans up the mem policy concepts to have MPOL_DEFAULT mean "use default policy for this context/scope" rather than have an additional allocation behavior of its own.

I still have not gotten my head around this one. Lets wait awhile.

There is another, "preferred" way to specify local allocation via the APIs. That is using the MPOL_PREFERRED policy mode with an empty nodemask. Internally, the empty nodemask gets converted to a preferred_node id of '-1'. All internal usage of MPOL_PREFERRED will convert the '-1' to the local node id.

Now, system default policy, except during boot, is "local allocation". By using the MPOL_PREFERRED mode with a negative value of preferred node for system default policy, MPOL_DEFAULT will never occur in the 'policy' member of a struct mempolicy. Thus, we can remove all checks for MPOL_DEFAULT when converting policy to a node id/zonelist in the allocation paths.

Note: in slab_node() I kept the use of MPOL_DEFAULT when the policy pointer is NULL to force the switch to take the default:
case. This seemed more efficient than pointing policy at the system default, and having to deref that. Any value not covered by one of the existing case's would have served, but MPOL_DEFAULT is guaranteed to be a different value from any of the other MPOL_* handled explicitly by the switch.

static void mpol_rebind_policy(struct mempolicy *pol,

mm/mempolicy.c +0 -2

@@ -492,8 +496,6 @@ static void get_zonemask(struct mempolic
 			node_set(zone_to_nid(p->v.zonelist->zones[i]),
 				*nodes);
 		break;
-	case MPOL_DEFAULT:
-		break;
 	case MPOL_INTERLEAVE:
 		*nodes = p->v.nodes;
 		break;

Quoted 5 lines · Lee Schermerhorn

There is another, "preferred" way to specify local allocation via the APIs. That is using the MPOL_PREFERRED policy mode with an empty nodemask. Internally, the empty nodemask gets converted to a preferred_node id of '-1'. All internal usage of MPOL_PREFERRED will convert the '-1' to the local node id.

But the comparison with an MPOL_PREFERRED policy is different from comparing with a MPOL_DEFAULT policy. MPOL_DEFAULT matches any other policy. MPOL_PREFERRED only matches other MPOL_DEFERRED policies.

Quoted 7 lines

Now, system default policy, except during boot, is "local allocation". By using the MPOL_PREFERRED mode with a negative value of preferred node for system default policy, MPOL_DEFAULT will never occur in the 'policy' member of a struct mempolicy. Thus, we can remove all checks for MPOL_DEFAULT when converting policy to a node id/zonelist in the allocation paths.

So we can also avoid having to check for NULL pointers?

Quoted 23 lines

Note: in slab_node() I kept the use of MPOL_DEFAULT when the policy pointer is NULL to force the switch to take the default:
case. This seemed more efficient than pointing policy at the system default, and having to deref that. Any value not covered by one of the existing case's would have served, but MPOL_DEFAULT is guaranteed to be a different value from any of the other MPOL_* handled explicitly by the switch.

static void mpol_rebind_policy(struct mempolicy *pol,

mm/mempolicy.c +0 -2

@@ -492,8 +496,6 @@ static void get_zonemask(struct mempolic
 			node_set(zone_to_nid(p->v.zonelist->zones[i]),
 				*nodes);
 		break;
-	case MPOL_DEFAULT:
-		break;
 	case MPOL_INTERLEAVE:
 		*nodes = p->v.nodes;
 		break;

@@ -505,7 +507,11 @@ static void get_zonemask(struct mempolic
 			node_set(p->v.preferred_node, *nodes);
 		break;
 	default:

mm/mempolicy.c static void get_zonemask(struct mempolic · line 514

Quoted 6 lines

mm/mempolicy.c +5 -1

-		BUG();
+		/*
+		 * shouldn't happen
+		 */
+		WARN_ON_ONCE(1);
+		node_set(numa_node_id(), *nodes);

Safety features? Are these triggered? Could we leave the BUG() in?

mm/mempolicy.c static struct mempolicy * get_vma_policy · line 1099

Quoted 11 lines

mm/mempolicy.c +1 -2

@@ -1087,8 +1093,7 @@ static struct mempolicy * get_vma_policy
 	if (vma) {
 		if (vma->vm_ops && vma->vm_ops->get_policy)
 			pol = vma->vm_ops->get_policy(vma, addr);
-		else if (vma->vm_policy &&
-				vma->vm_policy->policy != MPOL_DEFAULT)
+		else if (vma->vm_policy)
 			pol = vma->vm_policy;
 	}
 	if (!pol)

Good

mm/mempolicy.c static struct zonelist *zonelist_policy( · line 1122

Quoted 5 lines

mm/mempolicy.c +0 -0

@@ -1115,12 +1120,11 @@ static struct zonelist *zonelist_policy(
 				return policy->v.zonelist;
 		/*FALL THROUGH*/
 	case MPOL_INTERLEAVE: /* should not happen */

Hmmmm does the MPOL_INTERLEAVE happen at all? Does it also need a WARN_ON?

mm/mempolicy.c void __mpol_free(struct mempolicy *p) · line 1382

Quoted 7 lines

mm/mempolicy.c +2 -1

@@ -1376,7 +1378,8 @@ void __mpol_free(struct mempolicy *p)
 		return;
 	if (p->policy == MPOL_BIND)
 		kfree(p->v.zonelist);
-	p->policy = MPOL_DEFAULT;
+	p->policy = MPOL_PREFERRED;
+	p->v.preferred_node = -1;

Why are we initializing values here in an object that is then freed?

Otherwise looks okay.

Quoted 3 lines

But the comparison with an MPOL_PREFERRED policy is different from comparing with a MPOL_DEFAULT policy. MPOL_DEFAULT matches any other policy. MPOL_PREFERRED only matches other MPOL_DEFERRED policies.

MPOL_DEFAULT doesn't match anything but itself. Everywhere but in the system default policy, specifying MPOL_DEFAULT means "delete the current policy [task or vma] and replace it with a null pointer. Again, the only place that MPOL_DEFAULT actually occurs in a struct mempolicy is in the system default policy. By changing that, I can eliminate the MPOL_DEFAULT checks in the run time [allocation path] use of mempolicy.

Quoted 1 line

Safety features? Are these triggered? Could we leave the BUG() in?

I haven't seen them triggered. I'm hoping that testing in -mm will not hit them either. I suppose we could leave the BUG. Seems a bit drastic for this case, where we have a reasonable fallback. But, the BUG will more likely get someone's attention, I suppose :-).

Quoted 9 lines

mm/mempolicy.c void __mpol_free(struct mempolicy *p) · line 1382

Quoted 7 lines

mm/mempolicy.c +2 -1

@@ -1376,7 +1378,8 @@ void __mpol_free(struct mempolicy *p)
 		return;
 	if (p->policy == MPOL_BIND)
 		kfree(p->v.zonelist);
-	p->policy = MPOL_DEFAULT;
+	p->policy = MPOL_PREFERRED;
+	p->v.preferred_node = -1;

Why are we initializing values here in an object that is then freed?

I wondered that myself. I think Andi was stuffing MPOL_DEFAULT "just in case" or to NULL out the policy member. I just replaced it so that I was sure that MPOL_DEFAULT never occurs in a struct mempolicy. We always initialize the policy member when we alloc one, so I guess we can drop the reinit here.

Quoted 10 lines · Lee Schermerhorn

Quoted 3 lines

But the comparison with an MPOL_PREFERRED policy is different from comparing with a MPOL_DEFAULT policy. MPOL_DEFAULT matches any other policy. MPOL_PREFERRED only matches other MPOL_DEFERRED policies.

MPOL_DEFAULT doesn't match anything but itself. Everywhere but in the system default policy, specifying MPOL_DEFAULT means "delete the current policy [task or vma] and replace it with a null pointer. Again, the only place that MPOL_DEFAULT actually occurs in a struct mempolicy is in the system default policy. By changing that, I can eliminate the MPOL_DEFAULT checks in the run time [allocation path] use of mempolicy.

Look at mpol_equal(). If the policy to compare it MPOL_DEFAULT then it returns true. If its MPOL_PREFERRED then it requires a matching on the node. Wont your change break this?

Quoted 7 lines

Quoted 2 lines

Safety features? Are these triggered? Could we leave the BUG() in?

I haven't seen them triggered. I'm hoping that testing in -mm will not hit them either. I suppose we could leave the BUG. Seems a bit drastic for this case, where we have a reasonable fallback. But, the BUG will more likely get someone's attention, I suppose :-).

So you have never seen warnings in testing and there should not be any? Then leave the BUG() in. WARN is useful if there is something that another developer could fix.

Quoted 11 lines

Quoted 10 lines

mm/mempolicy.c void __mpol_free(struct mempolicy *p) · line 1382

Quoted 8 lines

mm/mempolicy.c +2 -1

@@ -1376,7 +1378,8 @@ void __mpol_free(struct mempolicy *p)
 		return;
 	if (p->policy == MPOL_BIND)
 		kfree(p->v.zonelist);
-	p->policy = MPOL_DEFAULT;
+	p->policy = MPOL_PREFERRED;
+	p->v.preferred_node = -1;

Why are we initializing values here in an object that is then freed?

Quoted 5 lines

I wondered that myself. I think Andi was stuffing MPOL_DEFAULT "just in case" or to NULL out the policy member. I just replaced it so that I was sure that MPOL_DEFAULT never occurs in a struct mempolicy. We always initialize the policy member when we alloc one, so I guess we can drop the reinit here.

I'd say remove the useless assignements. They are confusing.