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

From: Ingo Molnar
Date: Tue Dec 27 2005 - 06:52:59 EST



* Nicolas Pitre <nico@xxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/