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

From: Ingo Molnar <mingo@elte.hu>
To: Christoph Lameter <cl@linux.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Tejun Heo <tj@kernel.org>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	rusty@rustcorp.com.au, tglx@linutronix.de, x86@kernel.org,
	linux-kernel@vger.kernel.org, hpa@zytor.com,
	Paul Mundt <lethal@linux-sh.org>,
	rmk@arm.linux.org.uk, starvik@axis.com, ralf@linux-mips.org,
	davem@davemloft.net, cooloney@kernel.org, kyle@mcmartin.ca,
	matthew@wil.cx, grundler@parisc-linux.org, takata@linux-m32r.org,
	benh@kernel.crashing.org, rth@twiddle.net,
	ink@jurassic.park.msu.ru, heiko.carstens@de.ibm.com,
	Nick Piggin <npiggin@suse.de>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: [PATCH UPDATED] percpu: use dynamic percpu allocator as the default percpu allocator
Date: Wed, 8 Apr 2009 18:26:51 +0200	[thread overview]
Message-ID: <20090408162651.GA14449@elte.hu> (raw)
In-Reply-To: <alpine.DEB.1.10.0904020948570.31756@qirst.com>

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

> On Thu, 2 Apr 2009, Ingo Molnar wrote:
> 
> > 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.
> 
> What is this? Nonsense day? Consider it a common pattern although 
> its likely not common at all? April fools day?

Dude, this is a new facility freshly modernized and freshly made 
usable. What did you expect, for a thousand usecases pop up in the 
kernel overnight? _None_ of this code is "common" today per se. (the 
networking folks are working on making it more and more common 
though)

> > 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).
> 
> Speculation. A shutdown fastpath? The percpu allocation and free 
> operations are expensive and deal with teardown and setup of 
> virtual mappings. Those paths are *not* optimized for fastpath 
> use. kfree is different.

Of course a lot of this is speculation, dynamic percpu so far has 
been a rarely used facility compared to kmalloc()/kfree(). If you 
dont accept my analogy that's fine - but that is opinion against 
opinion - while you state you opinion as truism.

So my point remains: your patch had effects you clearly did not 
anticipate, and the cacheline alignment management situation is not 
nearly as clear-cut as you imagine it to be.

Unfortunately you failed to answer my detailed mail that made very 
specific points though, you only got into generalities and flames 
about my summary mail - so it's hard to judge what your opinion 
about those specific facts is - you have not stated one.

	Ingo