Re: [patch 0/3] Allow inlined spinlocks again V3

[Linus Torvalds <torvalds@linux-foundation.org>]

On Wed, 12 Aug 2009, Heiko Carstens wrote:
> 
> This patch set allows to have inlined spinlocks again.

I'm not convinced this is the right approach.

The thing is, CONFIG_SPINLOCK_INLINE doesn't look like the right thing to 
do. On x86, for example, functions calls are usually quite cheap, and it 
may well make sense to keep 'spin_lock()' out-of-line, especially once you 
start doing things like playing with the softirq counts or interrupts.

At the same time, the special case of spin *unlock* is generally just a 
single (small) instruction on x86, so that one almost certainly is worth 
inlining - at least as long as it's not one of the softirq ones.

In other words, I think your hammer is too large. If you want to inline 
the lock functions, you should _not_ make this a single "everything or 
nothing" thing.

> If one considers that server kernels are usually compiled with
> !CONFIG_PREEMPT a simple spin_lock is just a compare and swap loop.
> The extra overhead for a function call is significant.
> With inlined spinlocks overall cpu usage gets reduced by 1%-5% on s390.
> These numbers were taken with some network benchmarks. However I expect
> any workload that calls frequently into the kernel and which grabs a few
> locks to perform better.

Have you tried to determine which kinds of spinlocks you care most about? 
With networking, I guess the softirq-safe ones (and perhaps the irq ones) 
are pretty common, but maybe even there it's really just a couple of 
cases. 

Now, there is one advantage to inlining that I really like - and that I 
personally have missed from the days when we used to always do it: 
profiling. Inlining the spinlocks tends to do _wonders_ for showing 
exactly which spinlock ends up being the contention cases for certain 
loads.

So there are advantages outside of the "size vs speed" kind of things, but 
at the same time I do suspect that the disadvantages mean that we really 
should make this be something where an architecture can specify things at 
a finer granularity. Perhaps by using per-symbol defines (ie have the 
<asm/spinlock.h> file do things like

	#define __spin_lock_is_small
	#define __spin_unlock_is_small

kind of things for the particular sequences that are worth inlining.

		Linus