On Tue, 20 Dec 2005, Ingo Molnar wrote:
>
> * David Woodhouse <[email protected]> wrote:
>
> > On Mon, 2005-12-19 at 09:49 -0800, Zwane Mwaikambo wrote:
> > > Hi Ingo,
> > > Doesn't this corrupt caller saved registers?
> >
> > Looks like it. I _really_ don't like calling functions from inline
> > asm. It's not nice. Can't we use atomic_dec_return() for this?
>
> we can use atomic_dec_return(), but that will add one more instruction
> to the fastpath. OTOH, atomic_dec_return() is available on every
> architecture, so it's a really tempting thing. I'll experiment with it.
Please consider using (a variant of) xchg() instead. Although
atomic_dec() is available on all architectures, its implementation is
far from being the most efficient thing to do for them all. For
example, see my discussion about swp on ARM:
http://www.ussg.iu.edu/hypermail/linux/kernel/0512.2/0727.html
What would be the most efficient implementation on ARM might look like:
static inline void mutex_lock(struct mutex *m)
{
if (unlikely(atomic_xchg(&m->count, 0) != 1))
__mutex_lock_nonatomic(m);
}
static inline void mutex_unlock(struct mutex *m)
{
if (unlikely(atomic_xchg(&m->count, 1) == -1))
__mutex_unlock_nonatomic(m);
}
Yet we might want to use special wrappers with non-standard calling
convention for getting to the contention handlers
(__mutex_lock_nonatomic() and __mutex_unlock_nonatomic() in this case).
Furthermore trying to make atomic_inc_call_if_nonpositive() looks like a
generic thing is rather counter productive. It is likely to be useful
to the mutex code only anyway, so why not make it implicit what the
contention handlers are? This will allow for each architectures to
implement the mutex interface with the best fast path they can come
with.
I'd propose this:
First, rename some functions to make it clearer what they are used for:
s/__mutex_lock_nonatomic/__mutex_lock_contended/
s/__mutex_unlock_nonatomic/__mutex_unlock_contended/
Next, please make it possible for architecture specific implementation
of mutex_lock(), mutex_unlock(), mutex_trylock(), and so on to exist.
A default implementation in include/asm-generic/mutex.h should probably
exist and the two examples above is certainly a good start.
mutex_lock should have the following definition: it should try to lock
the mutex (set the count to 0, or any value < 1). If the count was 1
prior setting it to 0 then the lock is successful and we're done.
Otherwise it should call __mutex_lock_contended. Knowing the previous
value and setting the new value must be done atomically. Whether it uses
atomic_xchg() or atomic_dec_return(), or even open code some clever
assembly trick to achieve that like your i386
atomic_dec_call_if_negative() implementation is the architecture's own
business. I can imagine the ARM implementation which inlined fast path
would be between 3 and 4 instructions only. But that's possible only if
the implementation allows any flexibility in achieving the above.
Similarly, mutex_unlock should set the mutex count to 1. It also should
call __mutex_unlock_contended if the count wasn't 0 before. And again
the atomic nature of the count (which might be renamed to "state" since
"count" is misleading for a mutex) must be preserved of course. Again
the architecture is free to implement it in whatever way as long as it
has this behavior.
The main header file for mutex users would be include/linux/mutex.h.
If mutex debugging is enabled, then architecture fast path is ignored so
mutex(lock() would simply become an alias for __mutex_lock_contended()
or whatever wrapper you have for that purpose. If debugging is disabled
only then is asm/mutex.h included from linux/mutex.h to get the
architecture fast path code (which is possibly including
asm-generic/mutex.h with reasonnable reference/default implementations).
What do you think?
Nicolas
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
