linux-kernel - Re: [PATCH UPDATED] percpu: use dynamic percpu allocator as the default percpu allocator

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:	Thu, 2 Apr 2009 05:42:23 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Christoph Lameter <cl@...ux.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Tejun Heo <tj@...nel.org>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	rusty@...tcorp.com.au, tglx@...utronix.de, x86@...nel.org,
	linux-kernel@...r.kernel.org, hpa@...or.com,
	Paul Mundt <lethal@...ux-sh.org>, rmk@....linux.org.uk,
	starvik@...s.com, ralf@...ux-mips.org, davem@...emloft.net,
	cooloney@...nel.org, kyle@...artin.ca, matthew@....cx,
	grundler@...isc-linux.org, takata@...ux-m32r.org,
	benh@...nel.crashing.org, rth@...ddle.net,
	ink@...assic.park.msu.ru, heiko.carstens@...ibm.com,
	Nick Piggin <npiggin@...e.de>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH UPDATED] percpu: use dynamic percpu allocator as the
	default percpu allocator


* Christoph Lameter <cl@...ux.com> wrote:

> On Thu, 2 Apr 2009, Ingo Molnar wrote:
> 
> > So ... we regressed the performance of percpu_free(NULL) with a 
> > potential cross-CPU cacheline bounce. Without the patch, 
> > percpu_free(NULL) would never do such a bounce. So i dont think 
> > the patch is a good idea.
> 
> But percpu_free is not an operation typical for hot code paths. 
> Per cpu variables are allocated and freed in rarely used code 
> paths. Typically the use of the per cpu variables occurs in hot 
> code paths, not their allocations.

to quote an earlier part of my mail:

> > We encourage kfree(NULL) and it triggers commonly in the kernel 
> > today [on distro kernels we checked it can trigger once per 
> > syscall!] - so i think we should consider free_percpu(NULL) a 
> > possibly common pattern too. (even though today it's likely 
> > _not_ common at all.)

I specifically mentioned that it is not at all common now.

But there's no reason why an object shutdown fastpath with an 
optional percpu buffer (say for debug statistics, not enabled by 
default) couldnt look like this:

	percpu_free(NULL);

We actually have such patterns of kfree(ptr) use, where the _common_ 
case in a fastpath is kfree(NULL).

So, had your patch been applied as-is, we might have created this 
situation.

Yes, it can be fixed, and yes, it is probably not worth even 
bothering (percpu alloc/free goes over all online CPUs) - but i 
found it kind of interesting that this random specific example we 
ran into (i totally didnt go out and try to find some other, better 
example) showed so many variable placement ambiguities and false 
cacheline sharing problems, and showed visible inefficiencies in the 
layout (i showed a whole line of variables wastefully spaces - 
unrelated to the percpu allocator).

So these examples almost seemed to support my earlier points in the 
thread where i suggested that variable placement is often random and 
ambigious, and that we could fix inefficiencies in the layout ;-)

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