linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations
@ 2014-05-23 19:37 Marcelo Tosatti
  2014-05-23 20:51 ` David Rientjes
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2014-05-23 19:37 UTC (permalink / raw)
  To: linux-mm, linux-kernel
  Cc: Lai Jiangshan, Mel Gorman, Tejun Heo, Christoph Lameter,
	David Rientjes, Andrew Morton


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 <mtosatti@redhat.com>

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5dba293..f228039 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -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;

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations
  2014-05-23 19:37 [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations Marcelo Tosatti
@ 2014-05-23 20:51 ` David Rientjes
  2014-05-23 23:33   ` Marcelo Tosatti
  2014-05-26 18:53 ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v2) Marcelo Tosatti
  2014-05-27 14:21 ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations Christoph Lameter
  2 siblings, 1 reply; 23+ messages in thread
From: David Rientjes @ 2014-05-23 20:51 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman, Tejun Heo,
	Christoph Lameter, Li Zefan, Andrew Morton, cgroups

On Fri, 23 May 2014, Marcelo Tosatti wrote:

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

> Fixes KVM's alloc_page(gfp_mask=GFP_DMA32) with cpuset as explained.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5dba293..f228039 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -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;
> 

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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations
  2014-05-23 20:51 ` David Rientjes
@ 2014-05-23 23:33   ` Marcelo Tosatti
  0 siblings, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2014-05-23 23:33 UTC (permalink / raw)
  To: David Rientjes
  Cc: linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman, Tejun Heo,
	Christoph Lameter, Li Zefan, Andrew Morton, cgroups

On Fri, May 23, 2014 at 01:51:12PM -0700, David Rientjes wrote:
> On Fri, 23 May 2014, Marcelo Tosatti wrote:
> 
> > 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.
> 
> > Fixes KVM's alloc_page(gfp_mask=GFP_DMA32) with cpuset as explained.
> > 
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > 
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 5dba293..f228039 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -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;
> > 
> 
> 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().

Yes, thanks. Can you please review updated patch below.

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

The comment at mm/mempolicy.c seems sufficient:

/* Highest zone. An specific allocation for a zone below that is not
   policied. */
enum zone_type policy_zone = 0;

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


diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 3d54c41..b70a336 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -2392,6 +2392,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;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5dba293..dfea3dc 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -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;

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v2)
  2014-05-23 19:37 [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations Marcelo Tosatti
  2014-05-23 20:51 ` David Rientjes
@ 2014-05-26 18:53 ` Marcelo Tosatti
  2014-05-28  7:02   ` Li Zefan
  2014-05-27 14:21 ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations Christoph Lameter
  2 siblings, 1 reply; 23+ messages in thread
From: Marcelo Tosatti @ 2014-05-26 18:53 UTC (permalink / raw)
  To: linux-mm, linux-kernel
  Cc: Lai Jiangshan, Mel Gorman, Tejun Heo, Christoph Lameter,
	David Rientjes, Andrew Morton, Li Zefan


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 <mtosatti@redhat.com>

v2: fix slowpath as well (David Rientjes)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 3d54c41..b70a336 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -2392,6 +2392,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;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5dba293..dfea3dc 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -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;

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations
  2014-05-23 19:37 [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations Marcelo Tosatti
  2014-05-23 20:51 ` David Rientjes
  2014-05-26 18:53 ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v2) Marcelo Tosatti
@ 2014-05-27 14:21 ` Christoph Lameter
  2014-05-27 14:53   ` Marcelo Tosatti
  2 siblings, 1 reply; 23+ messages in thread
From: Christoph Lameter @ 2014-05-27 14:21 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman, Tejun Heo,
	David Rientjes, Andrew Morton

On Fri, 23 May 2014, Marcelo Tosatti wrote:

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

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.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations
  2014-05-27 14:21 ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations Christoph Lameter
@ 2014-05-27 14:53   ` Marcelo Tosatti
  2014-05-27 14:57     ` Marcelo Tosatti
  2014-05-27 15:31     ` Christoph Lameter
  0 siblings, 2 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2014-05-27 14:53 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman, Tejun Heo,
	David Rientjes, Andrew Morton

On Tue, May 27, 2014 at 09:21:32AM -0500, Christoph Lameter wrote:
> On Fri, 23 May 2014, Marcelo Tosatti wrote:
> 
> > 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.
> 
> 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).

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





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations
  2014-05-27 14:53   ` Marcelo Tosatti
@ 2014-05-27 14:57     ` Marcelo Tosatti
  2014-05-27 15:31     ` Christoph Lameter
  1 sibling, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2014-05-27 14:57 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman, Tejun Heo,
	David Rientjes, Andrew Morton

On Tue, May 27, 2014 at 11:53:52AM -0300, Marcelo Tosatti wrote:
> On Tue, May 27, 2014 at 09:21:32AM -0500, Christoph Lameter wrote:
> > On Fri, 23 May 2014, Marcelo Tosatti wrote:
> > 
> > > 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.
> > 
> > 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).
> 
> > 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.

Thats softwall nevermind. 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations
  2014-05-27 14:53   ` Marcelo Tosatti
  2014-05-27 14:57     ` Marcelo Tosatti
@ 2014-05-27 15:31     ` Christoph Lameter
  1 sibling, 0 replies; 23+ messages in thread
From: Christoph Lameter @ 2014-05-27 15:31 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman, Tejun Heo,
	David Rientjes, Andrew Morton

On Tue, 27 May 2014, Marcelo Tosatti wrote:

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

GFP_KERNEL is used for page tables.

>
>  * 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

Page table allocations are GFP_KERNEL allocations. So the above use case
is ok if you switch off the hardwall flag in the cpuset.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v2)
  2014-05-26 18:53 ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v2) Marcelo Tosatti
@ 2014-05-28  7:02   ` Li Zefan
  2014-05-28 22:43     ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v3) Marcelo Tosatti
  0 siblings, 1 reply; 23+ messages in thread
From: Li Zefan @ 2014-05-28  7:02 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman, Tejun Heo,
	Christoph Lameter, David Rientjes, Andrew Morton

On 2014/5/27 2:53, Marcelo Tosatti wrote:
> 
> 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.
> 

Could you add the use case that you described in a previous email to
the changelog?

> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> v2: fix slowpath as well (David Rientjes)
> 
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index 3d54c41..b70a336 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -2392,6 +2392,10 @@ int __cpuset_node_allowed_softwall(int node, gfp_t gfp_mask)
>  

Add a comment accordingly?

	 *      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.

>  	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;
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5dba293..dfea3dc 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -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;
> .
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v3)
  2014-05-28  7:02   ` Li Zefan
@ 2014-05-28 22:43     ` Marcelo Tosatti
  2014-05-28 23:45       ` Christoph Lameter
  2014-05-29 18:43       ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v4) Marcelo Tosatti
  0 siblings, 2 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2014-05-28 22:43 UTC (permalink / raw)
  To: Li Zefan
  Cc: linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman, Tejun Heo,
	Christoph Lameter, David Rientjes, Andrew Morton


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 <mtosatti@redhat.com>

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 3d54c41..3bbc23f 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -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;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5dba293..dfea3dc 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -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;

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v3)
  2014-05-28 22:43     ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v3) Marcelo Tosatti
@ 2014-05-28 23:45       ` Christoph Lameter
  2014-05-29 18:46         ` Marcelo Tosatti
  2014-05-29 18:43       ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v4) Marcelo Tosatti
  1 sibling, 1 reply; 23+ messages in thread
From: Christoph Lameter @ 2014-05-28 23:45 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Li Zefan, linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman,
	Tejun Heo, David Rientjes, Andrew Morton

On Wed, 28 May 2014, Marcelo Tosatti wrote:

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5dba293..dfea3dc 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -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;

Why do you need this one?

>  	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;

nodemask = &node_states[N_ONLINE];

> +#endif


> +
>  	/* The preferred zone is used for statistics later */
>  	first_zones_zonelist(zonelist, high_zoneidx,
> -				nodemask ? : &cpuset_current_mems_allowed,
> +				nodemask ? : cpuset_mems_allowed,

Skip this?

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v4)
  2014-05-28 22:43     ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v3) Marcelo Tosatti
  2014-05-28 23:45       ` Christoph Lameter
@ 2014-05-29 18:43       ` Marcelo Tosatti
  2014-05-29 22:40         ` Andrew Morton
  2014-05-29 23:01         ` David Rientjes
  1 sibling, 2 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2014-05-29 18:43 UTC (permalink / raw)
  To: Li Zefan
  Cc: linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman, Tejun Heo,
	Christoph Lameter, David Rientjes, Andrew Morton


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 <mtosatti@redhat.com>

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 3d54c41..3bbc23f 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -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;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5dba293..a0ce1ba 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2726,6 +2726,11 @@ __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)
+		nodemask = &node_states[N_ONLINE];
+#endif
+
 	/* The preferred zone is used for statistics later */
 	first_zones_zonelist(zonelist, high_zoneidx,
 				nodemask ? : &cpuset_current_mems_allowed,

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v3)
  2014-05-28 23:45       ` Christoph Lameter
@ 2014-05-29 18:46         ` Marcelo Tosatti
  0 siblings, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2014-05-29 18:46 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Li Zefan, linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman,
	Tejun Heo, David Rientjes, Andrew Morton


On Wed, May 28, 2014 at 06:45:04PM -0500, Christoph Lameter wrote:

<snip>

Much cleaner, sent v4 with your suggestions.

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

- __alloc_pages_nodemask ignored GFP_DMA32 on older kernels, so the
interface should remain functional.
- There are others callers of alloc_pages(GFP_DMA) that can suffer
from the same problem.
- Mirrors mempolicy behaviour.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v4)
  2014-05-29 18:43       ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v4) Marcelo Tosatti
@ 2014-05-29 22:40         ` Andrew Morton
  2014-05-29 23:01         ` David Rientjes
  1 sibling, 0 replies; 23+ messages in thread
From: Andrew Morton @ 2014-05-29 22:40 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Li Zefan, linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman,
	Tejun Heo, Christoph Lameter, David Rientjes

On Thu, 29 May 2014 15:43:03 -0300 Marcelo Tosatti <mtosatti@redhat.com> wrote:

> 
> 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.
> 
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -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

It's not very obvious why this code is doing what it does, so I'm
thinking a comment is needed.  And that changelog text looks good, so

--- a/kernel/cpuset.c~page_alloc-skip-cpuset-enforcement-for-lower-zone-allocations-v4-fix
+++ a/kernel/cpuset.c
@@ -2388,6 +2388,11 @@ int __cpuset_node_allowed_softwall(int n
 	if (in_interrupt() || (gfp_mask & __GFP_THISNODE))
 		return 1;
 #ifdef CONFIG_NUMA
+	/*
+	 * Zone specific allocations such as GFP_DMA32 should not be restricted
+	 * to cpusets allowed node list: the zones which such allocations
+	 * demand be contained in particular nodes outside the cpuset node list
+	 */
 	if (gfp_zone(gfp_mask) < policy_zone)
 		return 1;
 #endif
--- a/mm/page_alloc.c~page_alloc-skip-cpuset-enforcement-for-lower-zone-allocations-v4-fix
+++ a/mm/page_alloc.c
@@ -2742,6 +2742,11 @@ retry_cpuset:
 	cpuset_mems_cookie = read_mems_allowed_begin();
 
 #ifdef CONFIG_NUMA
+	/*
+	 * Zone specific allocations such as GFP_DMA32 should not be restricted
+	 * to cpusets allowed node list: the zones which such allocations
+	 * demand be contained in particular nodes outside the cpuset node list
+	 */
 	if (gfp_zone(gfp_mask) < policy_zone)
 		nodemask = &node_states[N_ONLINE];
 #endif



However perhaps it would be nicer to do



#ifdef CONFIG_NUMA
/*
 * Zone specific allocations such as GFP_DMA32 should not be restricted to
 * cpusets allowed node list: the zones which such allocations demand be
 * contained in particular nodes outside the cpuset node list
 */
static inline bool i_cant_think_of_a_name(gfp_t mask)
{
	return gfp_zone(gfp_mask) < policy_zone;
}
#else
static inline bool i_cant_think_of_a_name(gfp_t mask)
{
	return false;
}
#endif

This encapsulates it all in a single place and zaps those ifdefs?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v4)
  2014-05-29 18:43       ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v4) Marcelo Tosatti
  2014-05-29 22:40         ` Andrew Morton
@ 2014-05-29 23:01         ` David Rientjes
  2014-05-29 23:12           ` Andrew Morton
  2014-05-29 23:28           ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v5) Marcelo Tosatti
  1 sibling, 2 replies; 23+ messages in thread
From: David Rientjes @ 2014-05-29 23:01 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Li Zefan, linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman,
	Tejun Heo, Christoph Lameter, Andrew Morton

On Thu, 29 May 2014, Marcelo Tosatti wrote:

> 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 <mtosatti@redhat.com>
> 
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index 3d54c41..3bbc23f 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -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;
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5dba293..a0ce1ba 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2726,6 +2726,11 @@ __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)
> +		nodemask = &node_states[N_ONLINE];
> +#endif
> +
>  	/* The preferred zone is used for statistics later */
>  	first_zones_zonelist(zonelist, high_zoneidx,
>  				nodemask ? : &cpuset_current_mems_allowed,

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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v4)
  2014-05-29 23:01         ` David Rientjes
@ 2014-05-29 23:12           ` Andrew Morton
  2014-05-30 13:48             ` Christoph Lameter
  2014-05-29 23:28           ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v5) Marcelo Tosatti
  1 sibling, 1 reply; 23+ messages in thread
From: Andrew Morton @ 2014-05-29 23:12 UTC (permalink / raw)
  To: David Rientjes
  Cc: Marcelo Tosatti, Li Zefan, linux-mm, linux-kernel, Lai Jiangshan,
	Mel Gorman, Tejun Heo, Christoph Lameter

On Thu, 29 May 2014 16:01:55 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:

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

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v5)
  2014-05-29 23:01         ` David Rientjes
  2014-05-29 23:12           ` Andrew Morton
@ 2014-05-29 23:28           ` Marcelo Tosatti
  2014-05-29 23:54             ` David Rientjes
  1 sibling, 1 reply; 23+ messages in thread
From: Marcelo Tosatti @ 2014-05-29 23:28 UTC (permalink / raw)
  To: David Rientjes
  Cc: Li Zefan, linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman,
	Tejun Heo, Christoph Lameter, Andrew Morton


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 <mtosatti@redhat.com>


diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 3d54c41..3bbc23f 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -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;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5dba293..0fd6923 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -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;
 
+#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();
 


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v5)
  2014-05-29 23:28           ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v5) Marcelo Tosatti
@ 2014-05-29 23:54             ` David Rientjes
  2014-05-30 13:12               ` Marcelo Tosatti
  2014-05-30 13:50               ` Christoph Lameter
  0 siblings, 2 replies; 23+ messages in thread
From: David Rientjes @ 2014-05-29 23:54 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Li Zefan, linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman,
	Tejun Heo, Christoph Lameter, Andrew Morton

On Thu, 29 May 2014, Marcelo Tosatti wrote:

> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index 3d54c41..3bbc23f 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -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;
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5dba293..0fd6923 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -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;
>  
> +#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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v5)
  2014-05-29 23:54             ` David Rientjes
@ 2014-05-30 13:12               ` Marcelo Tosatti
  2014-05-30 13:50               ` Christoph Lameter
  1 sibling, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2014-05-30 13:12 UTC (permalink / raw)
  To: David Rientjes
  Cc: Li Zefan, linux-mm, linux-kernel, Lai Jiangshan, Mel Gorman,
	Tejun Heo, Christoph Lameter, Andrew Morton

On Thu, May 29, 2014 at 04:54:00PM -0700, David Rientjes wrote:
> On Thu, 29 May 2014, Marcelo Tosatti wrote:
> 
> > diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> > index 3d54c41..3bbc23f 100644
> > --- a/kernel/cpuset.c
> > +++ b/kernel/cpuset.c
> > @@ -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;
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 5dba293..0fd6923 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -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;
> >  
> > +#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.

We care about the mempolicy case, and that is taken care of by
apply_policy_zone.

Or does that code fail to handle a particular 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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v4)
  2014-05-29 23:12           ` Andrew Morton
@ 2014-05-30 13:48             ` Christoph Lameter
  2014-05-30 21:43               ` Marcelo Tosatti
  0 siblings, 1 reply; 23+ messages in thread
From: Christoph Lameter @ 2014-05-30 13:48 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Rientjes, Marcelo Tosatti, Li Zefan, linux-mm,
	linux-kernel, Lai Jiangshan, Mel Gorman, Tejun Heo, Andi Kleen

On Thu, 29 May 2014, Andrew Morton wrote:

> >
> > 	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?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v5)
  2014-05-29 23:54             ` David Rientjes
  2014-05-30 13:12               ` Marcelo Tosatti
@ 2014-05-30 13:50               ` Christoph Lameter
  2014-05-30 21:18                 ` Andi Kleen
  1 sibling, 1 reply; 23+ messages in thread
From: Christoph Lameter @ 2014-05-30 13:50 UTC (permalink / raw)
  To: David Rientjes
  Cc: Marcelo Tosatti, Li Zefan, linux-mm, linux-kernel, Lai Jiangshan,
	Mel Gorman, Tejun Heo, Andrew Morton, Andi Kleen

On Thu, 29 May 2014, David Rientjes wrote:

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

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

Right. This is a hardware issue and the hardware is pretty messed up. And
now one wants to use NUMA features?

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

I said something elsewhere in the thread.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v5)
  2014-05-30 13:50               ` Christoph Lameter
@ 2014-05-30 21:18                 ` Andi Kleen
  0 siblings, 0 replies; 23+ messages in thread
From: Andi Kleen @ 2014-05-30 21:18 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: David Rientjes, Marcelo Tosatti, Li Zefan, linux-mm,
	linux-kernel, Lai Jiangshan, Mel Gorman, Tejun Heo,
	Andrew Morton, Andi Kleen

On Fri, May 30, 2014 at 08:50:56AM -0500, Christoph Lameter wrote:
> On Thu, 29 May 2014, David Rientjes wrote:
> 
> > 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.

I'm not fully sure about the use case for this. On the NUMA systems
I'm aware of usually only node 0 has <4GB, so mem policy
is pointless.

But anyways it seems ok to me to ignore mempolicies. Mempolicies
are primarily for user space, which doesn't use GFP_DMA32.

-ANdi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v4)
  2014-05-30 13:48             ` Christoph Lameter
@ 2014-05-30 21:43               ` Marcelo Tosatti
  0 siblings, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2014-05-30 21:43 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Andrew Morton, David Rientjes, Li Zefan, linux-mm, linux-kernel,
	Lai Jiangshan, Mel Gorman, Tejun Heo, Andi Kleen

On Fri, May 30, 2014 at 08:48:41AM -0500, Christoph Lameter wrote:
> On Thu, 29 May 2014, Andrew Morton wrote:
> 
> > >
> > > 	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?

OK we will handle this in userspace.


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2014-05-30 21:44 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-23 19:37 [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations Marcelo Tosatti
2014-05-23 20:51 ` David Rientjes
2014-05-23 23:33   ` Marcelo Tosatti
2014-05-26 18:53 ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v2) Marcelo Tosatti
2014-05-28  7:02   ` Li Zefan
2014-05-28 22:43     ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v3) Marcelo Tosatti
2014-05-28 23:45       ` Christoph Lameter
2014-05-29 18:46         ` Marcelo Tosatti
2014-05-29 18:43       ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v4) Marcelo Tosatti
2014-05-29 22:40         ` Andrew Morton
2014-05-29 23:01         ` David Rientjes
2014-05-29 23:12           ` Andrew Morton
2014-05-30 13:48             ` Christoph Lameter
2014-05-30 21:43               ` Marcelo Tosatti
2014-05-29 23:28           ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v5) Marcelo Tosatti
2014-05-29 23:54             ` David Rientjes
2014-05-30 13:12               ` Marcelo Tosatti
2014-05-30 13:50               ` Christoph Lameter
2014-05-30 21:18                 ` Andi Kleen
2014-05-27 14:21 ` [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations Christoph Lameter
2014-05-27 14:53   ` Marcelo Tosatti
2014-05-27 14:57     ` Marcelo Tosatti
2014-05-27 15:31     ` Christoph Lameter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).