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]
Message-ID: <20060913233741.GB4359@localhost.localdomain>
Date:	Wed, 13 Sep 2006 16:37:41 -0700
From:	Ravikiran G Thirumalai <kiran@...lex86.org>
To:	Christoph Lameter <clameter@....com>
Cc:	Andrew Morton <akpm@...l.org>, linux-kernel@...r.kernel.org,
	Alok Kataria <alok.kataria@...softinc.com>,
	"Shai Fultheim (Shai@...lex86.org)" <shai@...lex86.org>,
	Christoph Lameter <clameter@...r.sgi.com>,
	"Benzi Galili (Benzi@...leMP.com)" <benzi@...lemp.com>
Subject: Re: [patch] slab: Do not use mempolicy for kmalloc_node

On Wed, Sep 13, 2006 at 03:28:01PM -0700, Christoph Lameter wrote:
> I wish first of all that the description would accurately describe the 
> problem.

Well, I thought the subject itself described the problem in one line :), I
guess I should be more verbose?  OK.

> 
> On Wed, 13 Sep 2006, Ravikiran G Thirumalai wrote:
> 
> 
> > +/* Allocate object from the array cache of the executing cpu */
> 
> I am not sure what this adds.

Comments :). Well the idea was to describe the difference between 
__cache_alloc and ____cache_alloc, I mean, these are two similar sounding
functions with extra underscores.  OK, maybe I was being too verbose here?

> 
> > +/* 
> > + * Allocate object from the appropriate node as per mempolicy/cpuset
> > + * constraints
> > + */
> 
> We only do this under certain conditions. The main purpose of this 
> function is to allocate an object without having specified a node.

Yes, the conditions being cpuset constraints or a mempolicy being in place.
Again, since objects can be allocated off other nodes under certain
conditions, I thought it was good to document it...

> 
> >  	unsigned long save_flags;
> >  	void *objp;
> > -
> >  	cache_alloc_debugcheck_before(cachep, flags);
> > -
> >  	local_irq_save(save_flags);
> 
> Extra material.

OK

> 
> > +
> > +#ifdef CONFIG_NUMA
> > +	if (unlikely(current->flags & (PF_SPREAD_SLAB | PF_MEMPOLICY))) {
> > +		objp = alternate_node_alloc(cachep, flags);
> > +		if (objp != NULL) {
> > +			local_irq_restore(save_flags);
> > +			prefetchw(objp);
> > +			return objp;
> > +		}
> > +	}
> > +#endif
> > +
> 
> Ok.
> 
> > @@ -3303,9 +3309,10 @@ void *kmem_cache_alloc_node(struct kmem_
> >  	cache_alloc_debugcheck_before(cachep, flags);
> >  	local_irq_save(save_flags);
> >  
> > -	if (nodeid == -1 || nodeid == numa_node_id() ||
> > -			!cachep->nodelists[nodeid])
> > +	if (nodeid == numa_node_id())
> >  		ptr = ____cache_alloc(cachep, flags);
> > +	else if (nodeid == -1 || !cachep->nodelists[nodeid])
> > +		ptr = __cache_alloc(cachep, flags, __builtin_return_address(0));
> >  	else
> 
> We are still allocating according to policy if nodeid == -1 or if we have 
> an invalid node? I thought we agreed that kmalloc_node should never 
> obey memory policies?
> 
> Simply move the #ifdef CONFIG_NUMA block as we agreed last night. And fix 
> the description to specify under what conditions kmalloc_node will obey 
> memory policies.

This is the case when we are requesting an object from a non existent
node/invalid node.  So we have 2 choices, either to spread the allocations 
as per the cpuset constraints (the same treatment as kmalloc), or to 
allocate from the requesting node, either ways we are not strictly 
confirming to the user's choice of node (which we cannot).  I cannot see 
major advantage or disadvantage either ways, so I chose to keep the policy
in current mainline code -- spread according to the policy set.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ