page_alloc: skip cpuset enforcement for lower zone allocations

Zone specific allocations, such as GFP_DMA32, should not be restricted to cpusets allowed node list: the zones which such allocations demand might be contained in particular nodes outside the cpuset node list.

The alternative would be to not perform such allocations from applications which are cpuset restricted, which is unrealistic.

Fixes KVM's alloc_page(gfp_mask=GFP_DMA32) with cpuset as explained.

Signed-off-by Marcelo Tosatti <[email protected]>

mm/page_alloc.c +3 -0

index 5dba293..f228039 100644
@@ -2698,6 +2698,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 	unsigned int cpuset_mems_cookie;
 	int alloc_flags = ALLOC_WMARK_LOW|ALLOC_CPUSET|ALLOC_FAIR;
 	struct mem_cgroup *memcg = NULL;
+	nodemask_t *cpuset_mems_allowed = &cpuset_current_mems_allowed;
 
 	gfp_mask &= gfp_allowed_mask;
 

@@ -2726,9 +2727,14 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 retry_cpuset:
 	cpuset_mems_cookie = read_mems_allowed_begin();
 
+#ifdef CONFIG_NUMA
+	if (gfp_zone(gfp_mask) < policy_zone)

Quoted 7 lines · Marcelo Tosatti

Zone specific allocations, such as GFP_DMA32, should not be restricted to cpusets allowed node list: the zones which such allocations demand might be contained in particular nodes outside the cpuset node list.

The alternative would be to not perform such allocations from applications which are cpuset restricted, which is unrealistic.

Or ensure applications that allocate from lowmem are allowed to do so, but I understand that might be hard to make sure always happens.

Quoted 23 lines

Fixes KVM's alloc_page(gfp_mask=GFP_DMA32) with cpuset as explained.

Signed-off-by Marcelo Tosatti <[email protected]>

mm/page_alloc.c +3 -0

index 5dba293..f228039 100644
@@ -2698,6 +2698,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 	unsigned int cpuset_mems_cookie;
 	int alloc_flags = ALLOC_WMARK_LOW|ALLOC_CPUSET|ALLOC_FAIR;
 	struct mem_cgroup *memcg = NULL;
+	nodemask_t *cpuset_mems_allowed = &cpuset_current_mems_allowed;
 
 	gfp_mask &= gfp_allowed_mask;
 

@@ -2726,9 +2727,14 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 retry_cpuset:
 	cpuset_mems_cookie = read_mems_allowed_begin();
 
+#ifdef CONFIG_NUMA
+	if (gfp_zone(gfp_mask) < policy_zone)

mm/page_alloc.c __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order, · line 2740

Quoted 10 lines

mm/page_alloc.c +4 -1

+		cpuset_mems_allowed = NULL;
+#endif
+
 	/* The preferred zone is used for statistics later */
 	first_zones_zonelist(zonelist, high_zoneidx,
-				nodemask ? : &cpuset_current_mems_allowed,
+				nodemask ? : cpuset_mems_allowed,
 				&preferred_zone);
 	if (!preferred_zone)
 		goto out;

I think this is incomplete. Correct me if I'm wrong on how this is
working: preferred_zone, today, is NULL because first_zones_zonelist() is restricted to a cpuset.mems that does not include lowmem and your patch

fixes that.
But if the fastpath allocation with mandatory ALLOC_CPUSET fails and we go to the slowpath, which may or may not have showed up in your testing, there's still issues,
particularly if __GFP_WAIT and lots of
allocators do GFP_KERNEL | __GFP_DMA32. This requires ALLOC_CPUSET on all allocations and you haven't updated __cpuset_node_allowed_softwall() with this exception nor zlc_setup().

After that's done, I think all of this is really convoluted and deserves a comment to describe the ALLOC_CPUSET and __GFP_DMA32 behavior.

Adding Li, the cpusets maintainer, to this as well.

Zone specific allocations, such as GFP_DMA32, should not be restricted to cpusets allowed node list: the zones which such allocations demand might be contained in particular nodes outside the cpuset node list.

The alternative would be to not perform such allocations from applications which are cpuset restricted, which is unrealistic.

Fixes KVM's alloc_page(gfp_mask=GFP_DMA32) with cpuset as explained.

kernel/cpuset.c +4 -0

 	if (in_interrupt() || (gfp_mask & __GFP_THISNODE))
 		return 1;
+#ifdef CONFIG_NUMA
+	if (gfp_zone(gfp_mask) < policy_zone)
+		return 1;
+#endif
 	might_sleep_if(!(gfp_mask & __GFP_HARDWALL));
 	if (node_isset(node, current->mems_allowed))
 		return 1;

mm/page_alloc.c +7 -1

index 5dba293..dfea3dc 100644
@@ -2698,6 +2698,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 	unsigned int cpuset_mems_cookie;
 	int alloc_flags = ALLOC_WMARK_LOW|ALLOC_CPUSET|ALLOC_FAIR;
 	struct mem_cgroup *memcg = NULL;
+	nodemask_t *cpuset_mems_allowed = &cpuset_current_mems_allowed;
 
 	gfp_mask &= gfp_allowed_mask;
 

@@ -2726,9 +2727,14 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 retry_cpuset:
 	cpuset_mems_cookie = read_mems_allowed_begin();
 
+#ifdef CONFIG_NUMA
+	if (gfp_zone(gfp_mask) < policy_zone)
+		cpuset_mems_allowed = NULL;
+#endif
+
 	/* The preferred zone is used for statistics later */
 	first_zones_zonelist(zonelist, high_zoneidx,
-				nodemask ? : &cpuset_current_mems_allowed,
+				nodemask ? : cpuset_mems_allowed,
 				&preferred_zone);
 	if (!preferred_zone)
 		goto out;

.

Why call __alloc_pages_nodemask at all if you want to skip the node handling? Punt to alloc_pages()

Zone specific allocations, such as GFP_DMA32, should not be restricted to cpusets allowed node list: the zones which such allocations demand might be contained in particular nodes outside the cpuset node list.

Necessary for the following usecase:
- driver which requires zone specific memory (such as KVM, which requires root pagetable at paddr < 4GB).
- user wants to limit allocations of application to nodeX, and nodeX has no memory < 4GB.

Zone specific allocations, such as GFP_DMA32, should not be restricted to cpusets allowed node list: the zones which such allocations demand might be contained in particular nodes outside the cpuset node list.

Necessary for the following usecase:
- driver which requires zone specific memory (such as KVM, which requires root pagetable at paddr < 4GB).
- user wants to limit allocations of application to nodeX, and nodeX has no memory < 4GB.

Signed-off-by Marcelo Tosatti <[email protected]>

kernel/cpuset.c +5 -0

index 3d54c41..3bbc23f 100644
@@ -2374,6 +2374,7 @@ static struct cpuset *nearest_hardwall_ancestor(struct cpuset *cs)
  * variable 'wait' is not set, and the bit ALLOC_CPUSET is not set
  * in alloc_flags.  That logic and the checks below have the combined
  * affect that:
+ *	gfp_zone(mask) < policy_zone - any node ok
  *	in_interrupt - any node ok (current task context irrelevant)
  *	GFP_ATOMIC   - any node ok
  *	TIF_MEMDIE   - any node ok

@@ -2392,6 +2393,10 @@ int __cpuset_node_allowed_softwall(int node, gfp_t gfp_mask)
 
 	if (in_interrupt() || (gfp_mask & __GFP_THISNODE))
 		return 1;
+#ifdef CONFIG_NUMA
+	if (gfp_zone(gfp_mask) < policy_zone)
+		return 1;
+#endif
 	might_sleep_if(!(gfp_mask & __GFP_HARDWALL));
 	if (node_isset(node, current->mems_allowed))
 		return 1;

mm/page_alloc.c +0 -0

index 5dba293..a0ce1ba 100644
@@ -2726,6 +2726,11 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 retry_cpuset:

There are still three issues with this, two of which are only minor and one that needs more thought:

(1) this doesn't affect only cpusets which the changelog indicates, it also bypasses mempolicies for GFP_DMA and GFP_DMA32 allocations since the nodemask != NULL in the page allocator when there is an effective mempolicy. That may be precisely what you're trying to do (do the same for mempolicies as you're doing for cpusets), but the comment now in the code specifically refers to cpusets. Can you make a case for the mempolicies exception as well? Otherwise, we'll need to do

if (!nodemask && gfp_zone(gfp_mask) < policy_zone)
nodemask = &node_states[N_ONLINE];

And the two minors:

(2) this should be &node_states[N_MEMORY], not &node_states[N_ONLINE] since memoryless nodes should not be included. Note that guarantee_online_mems() looks at N_MEMORY and cpuset_current_mems_allowed is defined for N_MEMORY without cpusets.

(3) it's unnecessary for this to be after the "retry_cpuset" label and check the gfp mask again if we need to relook at the allowed cpuset mask.

Quoted 3 lines

	if (!nodemask && gfp_zone(gfp_mask) < policy_zone)
		nodemask = &node_states[N_ONLINE];

OK, thanks, I made the patch go away for now.

Quoted 6 lines · Andrew Morton

Quoted 3 lines

	if (!nodemask && gfp_zone(gfp_mask) < policy_zone)
		nodemask = &node_states[N_ONLINE];

OK, thanks, I made the patch go away for now.

And another issue is that the policy_zone may be highmem on 32 bit platforms which will result in ZONE_NORMAL to be exempted.

policy zone can actually even be ZONE_DMA for some platforms. The check would not be useful at all on those.

Ignoring the containing cpuset only makes sense for GFP_DMA32 on 64 bit platforms and for GFP_DMA on platforms where there is an actual difference in the address spaces supported by GFP_DMA (such as x86).

Generally I think this is only useful for platforms that attempt to support legacy devices only able to DMA to a portion of the memory address space and that at the same time support NUMA for large address spaces. This is a contradiction on the one hand this is a high end system and on the other hand it attempts to support crippled DMA devices?

kernel/cpuset.c +5 -0

index 3d54c41..3bbc23f 100644
@@ -2374,6 +2374,7 @@ static struct cpuset *nearest_hardwall_ancestor(struct cpuset *cs)
  * variable 'wait' is not set, and the bit ALLOC_CPUSET is not set
  * in alloc_flags.  That logic and the checks below have the combined
  * affect that:
+ *	gfp_zone(mask) < policy_zone - any node ok
  *	in_interrupt - any node ok (current task context irrelevant)
  *	GFP_ATOMIC   - any node ok
  *	TIF_MEMDIE   - any node ok

@@ -2392,6 +2393,10 @@ int __cpuset_node_allowed_softwall(int node, gfp_t gfp_mask)
 
 	if (in_interrupt() || (gfp_mask & __GFP_THISNODE))
 		return 1;
+#ifdef CONFIG_NUMA
+	if (gfp_zone(gfp_mask) < policy_zone)
+		return 1;
+#endif
 	might_sleep_if(!(gfp_mask & __GFP_HARDWALL));
 	if (node_isset(node, current->mems_allowed))
 		return 1;

mm/page_alloc.c +0 -0

index 5dba293..0fd6923 100644
@@ -2723,6 +2723,11 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 	if (!memcg_kmem_newpage_charge(gfp_mask, &memcg, order))

Quoted 31 lines · Marcelo Tosatti

kernel/cpuset.c +5 -0

index 3d54c41..3bbc23f 100644
@@ -2374,6 +2374,7 @@ static struct cpuset *nearest_hardwall_ancestor(struct cpuset *cs)
  * variable 'wait' is not set, and the bit ALLOC_CPUSET is not set
  * in alloc_flags.  That logic and the checks below have the combined
  * affect that:
+ *	gfp_zone(mask) < policy_zone - any node ok
  *	in_interrupt - any node ok (current task context irrelevant)
  *	GFP_ATOMIC   - any node ok
  *	TIF_MEMDIE   - any node ok

@@ -2392,6 +2393,10 @@ int __cpuset_node_allowed_softwall(int node, gfp_t gfp_mask)
 
 	if (in_interrupt() || (gfp_mask & __GFP_THISNODE))
 		return 1;
+#ifdef CONFIG_NUMA
+	if (gfp_zone(gfp_mask) < policy_zone)
+		return 1;
+#endif
 	might_sleep_if(!(gfp_mask & __GFP_HARDWALL));
 	if (node_isset(node, current->mems_allowed))
 		return 1;

mm/page_alloc.c +0 -0

index 5dba293..0fd6923 100644
@@ -2723,6 +2723,11 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 	if (!memcg_kmem_newpage_charge(gfp_mask, &memcg, order))
 		return NULL;
 

mm/page_alloc.c __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order, · line 2733

Quoted 8 lines

mm/page_alloc.c +5 -0

+#ifdef CONFIG_NUMA
+	if (!nodemask && gfp_zone(gfp_mask) < policy_zone)
+		nodemask = &node_states[N_MEMORY];
+#endif
+
 retry_cpuset:
 	cpuset_mems_cookie = read_mems_allowed_begin();
 

When I said that my point about mempolicies needs more thought, I wasn't expecting that there would be no discussion -- at least _something_ that would say why we don't care about the mempolicy case.

The motivation here is identical for both cpusets and mempolicies. What is the significant difference between attaching a process to a cpuset without access to lowmem and a process doing set_mempolicy(MPOL_BIND) without access to lowmem? Is it because the process should know what it's doing if it asks for a mempolicy that doesn't include lowmem? If so, is the cpusets case different because the cpuset attacher isn't held to the same standard?

I'd argue that an application may never know if it needs to allocate GFP_DMA32 or not since its a property of the hardware that its running on and my driver may need to access lowmem while yours may not. I may even configure CONFIG_ZONE_DMA=n and CONFIG_ZONE_DMA32=n because I know the _hardware_ requirements of my platforms.

If there is no difference, then why are we allowing the exception for cpusets and not mempolicies?

I really think you want to allow both cpusets and mempolicies. I'd like to hear Christoph's thoughts on it as well, though.

Furthermore, I don't know why you're opposed to the comments that Andrew added here. In the first version of this patch, I suggested a comment and you referred to a kernel/cpuset.c comment. Nowhere in the above change to the page allocator would make anyone think of cpusets or what it is trying to do. Please comment the code accordingly so your intention is understood for everybody else who happens upon your code.

When I said that my point about mempolicies needs more thought, I wasn't expecting that there would be no discussion -- at least _something_ that would say why we don't care about the mempolicy case.

The motivation here is identical for both cpusets and mempolicies. What is the significant difference between attaching a process to a cpuset without access to lowmem and a process doing set_mempolicy(MPOL_BIND) without access to lowmem? Is it because the process should know what it's doing if it asks for a mempolicy that doesn't include lowmem? If so, is the cpusets case different because the cpuset attacher isn't held to the same standard?

I'd argue that an application may never know if it needs to allocate GFP_DMA32 or not since its a property of the hardware that its running on and my driver may need to access lowmem while yours may not. I may even configure CONFIG_ZONE_DMA=n and CONFIG_ZONE_DMA32=n because I know the _hardware_ requirements of my platforms.

If there is no difference, then why are we allowing the exception for cpusets and not mempolicies?

I really think you want to allow both cpusets and mempolicies. I'd like to hear Christoph's thoughts on it as well, though.

Quoted 3 lines · David Rientjes

When I said that my point about mempolicies needs more thought, I wasn't expecting that there would be no discussion -- at least _something_ that would say why we don't care about the mempolicy case.

Lets get Andi involved here too.

Zone specific allocations, such as GFP_DMA32, should not be restricted to cpusets allowed node list: the zones which such allocations demand might be contained in particular nodes outside the cpuset node list.

The alternative would be to not perform such allocations from applications which are cpuset restricted, which is unrealistic.

Fixes KVM's alloc_page(gfp_mask=GFP_DMA32) with cpuset as explained.

Quoted 7 lines · Marcelo Tosatti

Zone specific allocations, such as GFP_DMA32, should not be restricted to cpusets allowed node list: the zones which such allocations demand might be contained in particular nodes outside the cpuset node list.

The alternative would be to not perform such allocations from applications which are cpuset restricted, which is unrealistic.

Quoted 1 line

Fixes KVM's alloc_page(gfp_mask=GFP_DMA32) with cpuset as explained.

Memory policies are only applied to a specific zone so this is not unprecedented. However, if a user wants to limit allocation to a specific node and there is no DMA memory there then may be that is a operator error? After all the application will be using memory from a node that the operator explicitly wanted not to be used.

There is also the hardwall flag. I think its ok to allocate outside of the cpuset if that flag is not set. However, if it is set then any attempt to alloc outside of the cpuset should fail.

Quoted 9 lines · Christoph Lameter

Quoted 7 lines · Marcelo Tosatti

Zone specific allocations, such as GFP_DMA32, should not be restricted to cpusets allowed node list: the zones which such allocations demand might be contained in particular nodes outside the cpuset node list.

The alternative would be to not perform such allocations from applications which are cpuset restricted, which is unrealistic.

Quoted 7 lines

Quoted 1 line

Fixes KVM's alloc_page(gfp_mask=GFP_DMA32) with cpuset as explained.

Memory policies are only applied to a specific zone so this is not unprecedented. However, if a user wants to limit allocation to a specific node and there is no DMA memory there then may be that is a operator error? After all the application will be using memory from a node that the operator explicitly wanted not to be used.

Ok here is the use-case:

- machine contains driver which requires zone specific memory (such as KVM, which requires root pagetable at paddr < 4GB).

- user wants to limit allocation of application to nodeX, and nodeX has no memory < 4GB.

How would you solve that? Options:

1) force admin to allow allocation from node(s) which contain 0-4GB range, which unfortunately would allow every allocation, including ones which are not restricted to particular nodes, to be performed there.

or

2) allow zone specific allocations to bypass memory policies.

It seems 2) is the best option (and there is precedent for it).

Quoted 3 lines

There is also the hardwall flag. I think its ok to allocate outside of the cpuset if that flag is not set. However, if it is set then any attempt to alloc outside of the cpuset should fail.

GFP_ATOMIC bypasses hardwall:

* The second pass through get_page_from_freelist() doesn't even call
* here for GFP_ATOMIC calls. For those calls, the __alloc_pages()
* variable 'wait' is not set, and the bit ALLOC_CPUSET is not set
* in alloc_flags. That logic and the checks below have the combined
* affect that:

 *      in_interrupt - any node ok (current task context irrelevant)
 *      GFP_ATOMIC   - any node ok
 *      TIF_MEMDIE   - any node ok
 *      GFP_KERNEL   - any node in enclosing hardwalled cpuset ok
 *      GFP_USER     - only nodes in current tasks mems allowed ok.

Quoted 6 lines

Memory policies are only applied to a specific zone so this is not unprecedented. However, if a user wants to limit allocation to a specific node and there is no DMA memory there then may be that is a operator error? After all the application will be using memory from a node that the operator explicitly wanted not to be used.

Ok here is the use-case:

- machine contains driver which requires zone specific memory (such as KVM, which requires root pagetable at paddr < 4GB).

* The second pass through get_page_from_freelist() doesn't even call
* here for GFP_ATOMIC calls. For those calls, the __alloc_pages()
* variable 'wait' is not set, and the bit ALLOC_CPUSET is not set
* in alloc_flags. That logic and the checks below have the combined
* affect that:

 *      in_interrupt - any node ok (current task context irrelevant)
 *      GFP_ATOMIC   - any node ok
 *      TIF_MEMDIE   - any node ok
 *      GFP_KERNEL   - any node in enclosing hardwalled cpuset ok