[patch] spinlock consolidation, v2

[patch] spinlock consolidation, v2

[Ingo Molnar]

the latest version of the spinlock consolidation patch can be found at:

  http://redhat.com/~mingo/spinlock-patches/consolidate-spinlocks.patch

the patch is now complete in the sense that it does everything i wanted 
it to do. If you have any other suggestions (or i have missed to 
incorporate an earlier suggestion of yours), please yell.

Changes:

 - all architectures have been converted to the new spinlock code.
   arm, i386, ia64, ppc, ppc64, s390/s390x, x64 was build-tested via
   crosscompilers. Alpha, m32r, mips, parisc, sh, sparc, sparc64 has
   not been tested yet, but should be mostly fine. x86 and x64 was
   boot-tested in all relevant .config variations. It all brought a nice 
   reduction in source code size:

     62 files changed, 1455 insertions(+), 3025 deletions(-)

   Al, would you be interested in checking this patch on your build 
   farm? It should build on all architectures, UP and SMP alike.

   (NOTE: i've switched sparc32, sparc64, alpha, ppc, parisc to use the 
    generic spinlock debugging code. I believe the generic debugging 
    code is now capable enough to be a replacement - but especially the 
    Sparc ones are pretty advanced; so if i've missed some important
    feature please let me know and i'll implement it in the generic 
    code.)

 - linux/spinlock_types.h: new, pure header file that can be used
   by other headers to define spinlock fields - without having to
   pull in all the other include files that are needed on the
   implementational side. (Roman Zippel)

 - lib/spinlock_debug.c: got rid of the __FILE__/__LINE__ debug output 
   (suggested by Ingo Oeser), and streamlined the debug output. 
   Implemented 'lockup detection' which is a must for architectures that 
   dont have the equivalent of an NMI watchdog. (but is useful on other 
   architectures as well.) Both spinlocks and rwlocks are now fully 
   debugged.

 - linux/spinlock.h: got rid of the ATOMIC_DEC_AND_LOCK cruft. This is 
   achieved by not doing the UP-nonpreempt-nondebug specific 
   optimization but letting it pick the generic _atomic_dec_and_lock 
   function. The assembly looks sane on x86 (no locked ops, etc.) so 
   there's no performance problem. Other architectures should work fine 
   too, those which implement _atomic_dec_and_lock unconditionally might 
   want to review whether they want to use the CONFIG_HAVE_DEC_LOCK 
   mechanism to get the optimized (generic) version of the function on 
   UP.

 - asm-generic/spinlock_types_up.h: further simplifications (suggested
   by Arjan van de Ven), typo fixed

 - linux/spinlock_up.h: since this an UP-nondebug branch now, the macros 
   were simplified and streamlined significantly.

 - asm-generic/spinlock_up.h: further cleanups, reordering of op
   definitions into 'natural' op order.

 - asm-i386/spinlock.h and asm-x86_64/spinlock.h: reordering of ops, 
   cleanups

 - include/linux/spinlock.h: more cleanups, reordering

 - linux/spinlock_smp.h: cleanups, reordering of prototypes

 - kernel/spinlock.c: fixed bug in generic_raw_read_trylock and renamed 
   it to generic__raw_read_trylock to ease conversion.

 - lib/kernel_lock.c: simplification

 - (lots of small details i forgot)

	Ingo

Re: [patch] spinlock consolidation, v2

[Ingo Molnar]

* Christoph Hellwig <hch@infradead.org> wrote:

> > the latest version of the spinlock consolidation patch can be found at:
> > 
> >   http://redhat.com/~mingo/spinlock-patches/consolidate-spinlocks.patch
> > 
> > the patch is now complete in the sense that it does everything i wanted 
> > it to do. If you have any other suggestions (or i have missed to 
> > incorporate an earlier suggestion of yours), please yell.
> 
> Looks pretty nice, but your usage of asm-generic is totally wrong. 
> files in asm-generic must only ever be used for default 
> implementations of asm/ headers, and _never_ be included from common 
> code.  But your asm-generic files are only ever used from 
> linux/spinlock.h, so there's no point at all in splitting them out in 
> the first time. [...]

I see your point. My intention was to make the include file structure 
completely symmetric and thus easy to review. The following table 
illustrates how we build up the spinlock type and primitives in the SMP 
and UP cases:

   SMP                         |  UP
   ----------------------------|-----------------------------------
   asm/spinlock_types.h        |  asm-generic/spinlock_types_up.h
   linux/spinlock_types.h      |  linux/spinlock_types.h
   asm/spinlock.h              |  asm-generic/spinlock_up.h
   linux/spinlock_smp.h        |  linux/spinlock_up.h
   linux/spinlock.h            |  linux/spinlock.h

as you can see in the list above, whenever something comes from the asm 
directory, in the UP case we pick it from asm-generic. But i accept your 
point that the use of asm-generic/ for this is improper, and i've moved 
those files into include/linux/. (I also have renamed spinlock_smp.h and 
spinlock_up.h to spinlock_api_smp.h and spinlock_api_up.h, to avoid the 
filename clash.) The naming is still symmetric:

   SMP                         |  UP
   ----------------------------|-----------------------------------
   asm/spinlock_types_smp.h    |  linux/spinlock_types_up.h
   linux/spinlock_types.h      |  linux/spinlock_types.h
   asm/spinlock_smp.h          |  linux/spinlock_up.h
   linux/spinlock_api_smp.h    |  linux/spinlock_api_up.h
   linux/spinlock.h            |  linux/spinlock.h

all this splitup structure was created based on a pretty simple rule:
whenever some aspect is sufficiently dissimilar to be #ifdef
CONFIG_SMP-ed, it gets into separate _smp and _up files. If it's generic
and shared it gets into the main spinlock.h file. The splitups were only
done to enable this clean structure.

> Similarly there's no point in a separate linux/spinlock_smp.h and 
> linux/spinlock_up.h - it'll only cause some driver writers to include 
> either of them directly and break the build for either UP or SMP. If 
> you absolutely want to split them add an #error if not included from 
> spinlock.h

ok, i've added the #error protectors.

> Little nitpick no 2: please include linux/*.h always before asm/*.h 
> (in linux/jbd.h)

done.

you can find the latest patch with all these fixes included, at:

  http://redhat.com/~mingo/spinlock-patches/consolidate-spinlocks.patch

	Ingo