Re: [patch 0/9] mutex subsystem, -V4

[Nicolas Pitre <nico@cam.org>]

On Thu, 22 Dec 2005, Christoph Hellwig wrote:

> On Thu, Dec 22, 2005 at 12:41:47PM +0100, Ingo Molnar wrote:
> > this is the -V4 of the mutex subsystem patch-queue. It consists of the 
> > following patches:
> > 
> >   add-atomic-xchg.patch
> >   add-atomic-call-func-i386.patch
> >   add-atomic-call-func-x86_64.patch
> >   add-atomic-call-wrappers-rest.patch
> >   mutex-core.patch
> >   mutex-switch-arm-to-xchg.patch
> >   mutex-debug.patch
> >   mutex-debug-more.patch
> >   xfs-mutex-namespace-collision-fix.patch
> > 
> > the patches are against Linus' latest GIT tree, and they should work 
> > fine on every Linux architecture.
> > 
> > Changes since -V3:
> > 
> > - imlemented an atomic_xchg() based mutex implementation. It integrated
> >   pretty nicely into the generic code, and most of the code is still
> >   shared.
> > 
> > - added __ARCH_WANT_XCHG_BASED_ATOMICS: if an architecture defines 
> >   this then the generic mutex code will switch to the atomic_xchg() 
> >   implementation.
> > 
> >   This should be conceptually equivalent to the variant Nicolas Pitre 
> >   posted - Nicolas, could you check out this one? It's much easier to 
> >   provide this in the generic implementation, and the code ends up 
> >   looking cleaner.
> > 
> > - eliminated ARCH_IMPLEMENTS_MUTEX_FASTPATH: there's no need for 
> >   architectures to override the generic code anymore, with the 
> >   introduction of __ARCH_WANT_XCHG_BASED_ATOMICS.
> > 
> > - ARM: enable __ARCH_WANT_XCHG_BASED_ATOMICS.
> 
> I must admit I really really hat __ARCH_ stuff if we can avoid it.
> An <asm/mutex.h> that usually includes two asm-generic variants is probably
> a much better choice.
> 
> Actually just havign asm/mutex.h implement the faspath per-arch and get
> rid of all the oddball atomic.h additions would be even better.  While
> this means we need per-arch code it also means the code is a lot easier
> understandable, and we don't add odd public APIs.

I'm with Christoph here.  Please preserve my 
arch_mutex_fast_lock/arch_mutex_fast_unlock helpers.  I did it that way 
because the most important thing they bring is flexibility where it is 
needed i.e. in architecture specific implementations.  And done that way 
the architecture specific part is well abstracted with the minimum 
semantics allowing flexibility in the implementation.

I insist on that because, even if ARM currently relies on the atomic 
swap behavior, on ARMv6 at least this can be improved even further, but 
a special implementation which is neither a fully qualified atomic 
decrement nor an atomic swap is needed.  That's why I insist that you 
should keep my arch_mutex_fast_lock and friends (rename them if you 
wish) and remove __ARCH_WANT_XCHG_BASED_ATOMICS entirely.


Nicolas