Re: [patch 3/3] mutex subsystem: move the core to the new atomic helpers

[Matt Mackall <mpm@selenic.com>]

On Thu, Dec 22, 2005 at 12:12:18AM +0100, Ingo Molnar wrote:
> 
> * Nicolas Pitre <nico@cam.org> wrote:
> 
> > This patch moves the core mutex code over to the atomic helpers from 
> > previous patch.  There is no change for i386 and x86_64, except for 
> > the forced unlock state that is now done outside the spinlock (doing 
> > so doesn't matter since another CPU could have locked the mutex right 
> > away even if it was unlocked inside the spinlock).  This however 
> > brings great improvements on ARM for example.
> 
> i'm wondering how much difference it makes on ARM - could you show us 
> the before and after disassembly of the fastpath, to see the 
> improvement?
> 
> your patches look OK to me, only one small detail sticks out: i'd 
> suggest to rename the atomic_*_contended macros to be arch_mutex_*_..., 
> i dont think any other code can make use of it. Also, it would be nice 
> to see the actual ARM patches as well, which make use of the new 
> infrastructure.

I'm personally a little worried about the recent proliferation of
atomic_*.

My take on atomic_* functions has always been: a "sensible" arch [1]
implements the functionality in a single atomic instruction and this
simply exposes that instruction at the C level which otherwise lacks
appropriate semantics.

So functions like atomic_dec_call_if_negative seem a) excessively
special purpose b) not fundamental in the
ought-to-be-a-single-instruction sense c) a bit out of place in the in
the atomic_* set. These might even encourage people to roll their own
special-purpose locking primitives and we have way too many of those
already.

[1] In Linus' famous sense of what an ideal architecture should look like

-- 
Mathematics is the supreme nostalgia of our time.