linux-kernel - Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone allocations (v5)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 30 May 2014 10:12:43 -0300
From:	Marcelo Tosatti <mtosatti@...hat.com>
To:	David Rientjes <rientjes@...gle.com>
Cc:	Li Zefan <lizefan@...wei.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, Lai Jiangshan <laijs@...fujitsu.com>,
	Mel Gorman <mgorman@...e.de>, Tejun Heo <tj@...nel.org>,
	Christoph Lameter <cl@...ux.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] page_alloc: skip cpuset enforcement for lower zone
 allocations (v5)

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/