Re: [patch 1/3] mutex subsystem: trylock

[Arjan van de Ven <arjan@infradead.org>]

On Mon, 2005-12-26 at 14:25 -0500, Nicolas Pitre wrote:
> Index: linux-2.6/include/asm-arm/mutex.h
> ===================================================================
> --- linux-2.6.orig/include/asm-arm/mutex.h
> +++ linux-2.6/include/asm-arm/mutex.h
> @@ -98,5 +98,31 @@ do {									\
>   */
>  #define __mutex_slowpath_needs_to_unlock()	1
>  
> +/*
> + * For __mutex_trylock we use another construct which could be described
> + * as an "incomplete atomic decrement" or a "single value cmpxchg" since
> + * it has two modes of failure:
> + *
> + * 1) if the exclusive store fails we fail, and
> + *
> + * 2) if the decremented value is not zero we don't even attempt the store.


btw I really think that 1) is wrong. trylock should do everything it can
to get the semaphore short of sleeping. Just because some cacheline got
written to (which might even be shared!) in the middle of the atomic op
is not a good enough reason to fail the trylock imho. Going into the
slowpath.. fine. But here it's a quality of implementation issue; you
COULD get the semaphore without sleeping (at least probably, you'd have
to retry to know for sure) but because something wrote to the same
cacheline as the lock... no. that's just not good enough.. sorry.