Thread (31 messages) 31 messages, 6 authors, 2012-01-05

Re: [GIT PULL] slab fixes for 3.2-rc4

From: Pekka Enberg <penberg@kernel.org>
Date: 2011-12-20 09:47:35
Also in: lkml

Hi,

(I'm CC'ing Tejun and Thomas.)

On Tue, Nov 29, 2011 at 10:02 AM, Pekka Enberg [off-list ref] wrote:
quoted
The most important ones are from Christoph Lameter and Eric Dumazet that fix
stability issues on non-x86 architectures.

Christoph Lameter (1):
     slub: use irqsafe_cpu_cmpxchg for put_cpu_partial
On Tue, Nov 29, 2011 at 9:29 PM, Linus Torvalds
[off-list ref] wrote:
Quite frankly, I think "this_cpu_cmpxchg()" is pure crap, and this fix
was wrong, wrong, WRONG.

Dammit, the emulation is just bad.

The *only* architecture that implements "this_cpu_cmpxchg()" specially
seems to be x86.

So every other architecture uses the emulated one, AND THE EMULATED
ONE DOES NOT MATCH THE X86 SEMANTICS!

Sorry for shouting, but that's just *incredibly* stupid.

What's extra stupid is that s390 actually separately implements the
irq-safe version, and does it with code that seems to be generic, and
actually uses "cmpxchg()" like the regular "this_cpu_cmpxchg()" name
implies!

In other words, I would suggest that:

 - we get rid of that totally crazy idiotic "irqsafe" crap. The normal
one should be irq-safe. It's named "cmpxchg", for chrissake!

 - we make the non-arch-specific "this_cpu_cmpxchg()" use the correct
s390 routines instead of the pure and utter *shit* it uses now.

Yes, I'm upset. This "random percpu crap for SLUB" has been going on
too f*cking long, and it has been *continually* plagued by just pure
sh*t like this.

Seriously. Stop taking patches from Christoph in this area until they
have been vetted for sanity and completeness. This kind of ugly "let's
mis-name our functions in really subtle ways so that they work on x86
but nowhere else" needs to stop.
So, I actually looked into doing something like this and wasn't
actually able to understand the purpose of the various percpu
variants. It seems rather obvious that we can just drop the
non-irqsafe cmpxchg() variant but what about the rest of the percpu
ops? Why do we have preempt safe, irqsafe, and unsafe variants? How
are they supposed to be used?

To illustrate the issue, for "per cpu add" we have:

__this_cpu_add()
this_cpu_add()
irqsafe_cpu_add()
percpu_add()

Why do we need all of them?

                        Pekka
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help