Messages in this thread | | | Date | Tue, 27 Dec 2005 12:51:29 +0100 | From | Ingo Molnar <> | Subject | Re: [patch 1/3] mutex subsystem: trylock |
| |
* Nicolas Pitre <nico@cam.org> wrote:
> In the spirit of uniformity, this patch provides architecture specific > trylock implementations. This allows for a pure xchg-based > implementation, as well as an optimized ARMv6 implementation.
hm, i dont really like the xchg variant:
> +static inline int > +__mutex_trylock(atomic_t *count) > +{ > + int prev = atomic_xchg(count, 0); > + > + if (unlikely(prev < 0)) { > + /* > + * The lock was marked contended so we must restore that > + * state. If while doing so we get back a prev value of 1 > + * then we just own it. > + * > + * IN all cases this has the potential to trigger the > + * slowpath for the owner's unlock path - but this is not > + * a big problem in practice. > + */ > + prev = atomic_xchg(count, -1); > + if (prev < 0) > + prev = 0; > + }
here we go to great trouble trying to avoid the 'slowpath', while we unconditionally force the next unlock into the slowpath! So we have not won anything. (on a cycle count basis it's probably even a net loss)
The same applies to atomic_dec_return() based trylock variant. Only the cmpxchg based one, and the optimized ARM variant should be used as a 'fastpath'.
another thing is that this solution still leaves that ugly #ifdef in kernel/mutex.c.
so i took a different solution that solves both problems. The API towards architectures is now:
static inline int __mutex_fastpath_trylock(atomic_t *count, int (*fn)(atomic_t *))
note the new 'fn' parameter: this enables mutex-null.h to do a simple:
#define __mutex_fastpath_trylock(count, fn_name) fn_name(count)
and the final ugly debugging related #ifdef is gone from kernel/mutex.c! Furthermore, both the mutex-xchg.h and the mutex-dec.h non-cmpxchg variant will fall back to the spinlock implementation.
Ingo - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |