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

[Ingo Molnar <mingo@elte.hu>]

* Christoph Lameter <cl@linux.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