Generic Mutex Subsystem, add-atomic-call-func-x86_64.patch
PATCH 4/15 Generic Mutex Subsystem, add-atomic-call-func-x86_64.patch
add two new atomic ops to x86_64: atomic_dec_call_if_negative() and atomic_inc_call_if_nonpositive(), which are conditional-call-if atomic operations. Needed by the new mutex code.
----
include/asm-x86_64/atomic.h | 56 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 56 insertions(+)
include/asm-x86_64/atomic.h +56 -0
@@ -203,6 +203,62 @@ static __inline__ int atomic_sub_return(#define atomic_inc_return (v) (atomic_add_return(1,v))#define atomic_dec_return (v) (atomic_sub_return(1,v)) +/** +* atomic_dec_call_if_negative - decrement and call function if negative +* @v: pointer of type atomic_t +* @fn: function to call if the result is negative +* +* Atomically decrements @v and calls a function if the result is negative. +* NOTE: the function is type-checked and must be a fastcall. +*/ +#define atomic_dec_call_if_negative (v, fn_name) \ +do { \ + fastcall void (*__tmp)(atomic_t *) = fn_name; \ + \ + (void)__tmp; \ + typecheck(atomic_t *, v); \ + \ + __asm__ __volatile__( \ + LOCK "decl (%%rdi)\n" \ + "js 2f\n" \ + "1:\n" \ + LOCK_SECTION_START("") \ + "2: call "#fn_name"\n\t" \ + "jmp 1b\n" \ + LOCK_SECTION_END \ + : \ + :"D" (v) \ + :"memory"); \ +} while (0) + +/** +* atomic_inc_call_if_nonpositive - increment and call function if nonpositive +* @v: pointer of type atomic_t +* @fn: function to call if the result is nonpositive +* +* Atomically increments @v and calls a function if the result is nonpositive. +* NOTE: the function is type-checked and must be a fastcall. +*/ +#define atomic_inc_call_if_nonpositive (v, fn_name) \ +do { \ + fastcall void (*__tmp)(atomic_t *) = fn_name; \ + \ + (void)__tmp; \ + typecheck(atomic_t *, v); \ + \ + __asm__ __volatile__( \ + LOCK "incl (%%rdi)\n" \ + "jle 2f\n" \ + "1:\n" \ + LOCK_SECTION_START("") \ + "2: call "#fn_name"\n\t" \ + "jmp 1b\n" \ + LOCK_SECTION_END \ + : \ + :"D" (v) \ + :"memory"); \ +} while (0) +/* An 64bit atomic type */ typedef struct {volatile long counter; }atomic64_t ;
Quoted 9 lines · Ingo Molnar
include/asm-x86_64/atomic.h +9
-0+#define atomic_dec_call_if_negative (v, fn_name) \ +do { \ + fastcall void (*__tmp)(atomic_t *) = fn_name; \ + \ + (void)__tmp; \ + typecheck(atomic_t *, v); \ + \ + __asm__ __volatile__( \ + LOCK "decl (%%rdi)\n" \
include/asm-x86_64/atomic.h static __inline__ int atomic_sub_return( · line 232
Quoted 10 lines
include/asm-x86_64/atomic.h +10
-0+"js 2f\n" \ +"1:\n" \ + LOCK_SECTION_START("") \ + "2: call "#fn_name"\n\t" \ + "jmp 1b\n" \ + LOCK_SECTION_END \ + : \ + :"D" (v) \ + :"memory"); \ +} while (0)
Hi Ingo,
Doesn't this corrupt caller saved registers?
Quoted 2 lines · Zwane Mwaikambo
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?
Signature · 1 line
dwmw2
Quoted 6 lines · David Woodhouse
Quoted 2 lines · Zwane Mwaikambo
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.
Ingo
Quoted 7 lines · Ingo Molnar
Quoted 4 lines · David Woodhouse
Quoted 2 lines · Zwane Mwaikambo
Hi Ingo,
Doesn't this corrupt caller saved registers?
Quoted 6 lines
Quoted 2 lines
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
Quoted 11 lines · Nicolas Pitre
Quoted 8 lines · Ingo Molnar
Quoted 5 lines · David Woodhouse
Quoted 2 lines · Zwane Mwaikambo
Hi Ingo,
Doesn't this corrupt caller saved registers?
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
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. Althoughatomic_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:
Considering that on UP, the arm should not need to disable interrupts for this function (or has someone refuted Linus?), how about:
#ifndef CONFIG_SMP
typedef struct { volatile int counter; } mutex_counter_t;
static inline int mutex_counter_dec_return(mutex_counter *v)
{
return --v->counter;
}...
#else
#define mutex_counter_t atomic_t
...
#endif
Or does that get too bulky or have other problems?
MP ARMs should have an adequate atomic_dec_return.
Signature · 3 lines
SUSE Labs, Novell Inc. Send instant messages to your online friends http://au.messenger.yahoo.com
Quoted 20 lines · Nick Piggin
Quoted 19 lines · Nicolas Pitre
Quoted 14 lines · Ingo Molnar
Quoted 7 lines · David Woodhouse
Quoted 2 lines · Zwane Mwaikambo
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.
Quoted 8 lines
Quoted 5 lines
Please consider using (a variant of)
xchg()instead. Althoughatomic_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:Considering that on UP, the arm should not need to disable interrupts for this function (or has someone refuted Linus?), how about:
Kernel preemption.
Nicolas
Quoted 8 lines · Nicolas Pitre
Quoted 2 lines · Nick Piggin
Considering that on UP, the arm should not need to disable interrupts for this function (or has someone refuted Linus?), how about:
Kernel preemption.
preempt_disable() ?
Signature · 3 lines
SUSE Labs, Novell Inc. Send instant messages to your online friends http://au.messenger.yahoo.com
Quoted 11 lines · Nick Piggin
Quoted 8 lines · Nicolas Pitre
Quoted 2 lines · Nick Piggin
Considering that on UP, the arm should not need to disable interrupts for this function (or has someone refuted Linus?), how about:
Kernel preemption.
preempt_disable()?
Sure, and we're now more costly than the current implementation with irq disabling.
If we go with simple mutexes that's because there is a gain, even a huge one on ARM, especially for the fast uncontended case. If you guys insist on making things so generic and rigid then there is no gain anymore worth the bother.
Nicolas
Quoted 11 lines · Nicolas Pitre
Quoted 8 lines · Nick Piggin
Quoted 6 lines · Nicolas Pitre
Quoted 2 lines · Nick Piggin
Considering that on UP, the arm should not need to disable interrupts for this function (or has someone refuted Linus?), how about:
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Kernel preemption.
preempt_disable()?Sure, and we're now more costly than the current implementation with irq disabling.
Why? It is just a plain increment of a location that will almost certainly be in cache. I can't see how it would be more than half the cost of the irq implementation (based on looking at your measurements). How do you figure?
Also, preempt_disable is a very frequent operation on preempt kernels so if you have CONFIG_PREEMPT then you don't care about preempt_disable in down() (and if you do then you are calling down too often).
Quoted 2 lines
If we go with simple mutexes that's because there is a gain, even a huge one on ARM, especially for the fast uncontended case. If you guys insist on making things so generic and rigid then there is no gain anymore worth the bother.
I guess there is no bother for you, but maintaining code for 1 generic platform versus two dozen architectures is a huge win for many.
Signature · 3 lines
SUSE Labs, Novell Inc. Send instant messages to your online friends http://au.messenger.yahoo.com
Quoted 18 lines · Nick Piggin
Quoted 17 lines · Nicolas Pitre
Quoted 12 lines · Nick Piggin
Quoted 8 lines · Nicolas Pitre
Quoted 2 lines · Nick Piggin
Considering that on UP, the arm should not need to disable interrupts for this function (or has someone refuted Linus?), how about:
Kernel preemption.
preempt_disable()?
Quoted 8 lines
Quoted 3 lines
Sure, and we're now more costly than the current implementation with irq disabling.
Why? It is just a plain increment of a location that will almost certainly be in cache. I can't see how it would be more than half the cost of the irq implementation (based on looking at your measurements). How do you figure?
ARM is a load/store architecture. So the preempt_count has to be loaded, incremented, and stored back (3 instructions just there). Then the preempt_count itself isn't straight there, it is reached with the thread_info pointer which requires 2 additional instructions to generate. So we're already up to 5 instructions while the disabling of interrupts takes 3.
OK now we want to decrement the semaphore count. One load, one decrement, one store, i.e. 3 more instructions. We're up to 8.
Oh and we're still supposed to be in a fast path. And we even aren't ready to decide if we acquired the semaphore just yet.
Now we have to call preempt_enable(). Given that preempt_count() will use 2 instructions again to get to the thread_info pointer, let's say for the demonstration that we instead open coded our own preempt_enable() and preempt_disable() so to be able to cache the thread_info pointer amongst those two. So let's forget about those 2 additional instructions.
Let's also suppose that we preserved the original preempt count so that we don't have to re-load and decrement it which would have used 2 other additional instructions. Good! But register pressure is increasing, and btw we're using completely non generic kernel infrastructure at this point.
We nevertheless still have to store the original preempt count back. One instruction i.e. we're up to 9.
And with 9 instructions we're already worse than the implementation with interrupt disabling. Maybe not in terms of cycles but hey we're not done yet!
Any preempt_enable() equivalent look at the TIF_NEED_RESCHED flag (luckily we still have the thread_info pointer handy since we're not using test_thread_flag() but loading the flag directly i.e. one instruction), testing it (one instruction), conditionally branching to preempt_schedule (one instruction). Now up to 12 instructions. Amd to make things even worse: since the flag has to be tested right after it has been loaded you must account for result delays (more cycles) for the load.
OK, did we acquire that damn semaphore or not? Oh since we clobbered the processor's condition flag while testing the TIF_NEED_RESCHED flag we are now forced to test that semaphore count again to see if it went
negative: one instruction. And finally branch to the contention routine if the semaphore was already locked: one instruction.
So 14 instructions total with preemption disabling, and that's with the best implementation possible by open coding everything instead of relying on generic functions/macros.
Compare that to my 4 (or 3 when gcc can cse a constant) instructions needed for the mutex equivalent.
Nicolas
Quoted 3 lines · Nicolas Pitre
So 14 instructions total with preemption disabling, and that's with the best implementation possible by open coding everything instead of relying on generic functions/macros.
I agree with your analysis Nicolas.
However, don't forget to compare this with our existing semaphore implementation which is 9 instructions, or 8 for the SMP version.
In total, this means that mutexes will be _FAR MORE EXPENSIVE_ on ARM than our semaphores.
Forcing architectures down the "lets make everything generic" path does not always hack it. It can't do by its very nature. Generic implementations are *always* sub-optimal and it is pretty clear that any gain that mutexes _may_ give is completely wasted on ARM by the overhead of having a generic framework imposed upon us.
So, to sum up, if this path is persued, mutexes will be a bloody disaster on ARM. We'd be far better off sticking to semaphores and ignoring this mutex stuff.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core
Quoted 4 lines
So, to sum up, if this path is persued, mutexes will be a bloody disaster on ARM. We'd be far better off sticking to semaphores and ignoring this mutex stuff.
on x86 the fastpath is the same for both basically.. is there a fundamental reason it can't be for ARM?
Quoted 7 lines · Arjan van de Ven
Quoted 4 lines
So, to sum up, if this path is persued, mutexes will be a bloody disaster on ARM. We'd be far better off sticking to semaphores and ignoring this mutex stuff.
on x86 the fastpath is the same for both basically.. is there a fundamental reason it can't be for ARM?
If we're talking about implementing mutexes as a semaphore, then they will be identical. But what's the point of that? It's a semaphore which is just called a mutex.
So you might as well just save the patch noise and do nothing instead. It seems to me that you'll get _exactly_ the same result with a lot less effort.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core
Quoted 2 lines · Arjan van de Ven
on x86 the fastpath is the same for both basically.. is there a fundamental reason it can't be for ARM?
ARM prior ARM architecture level 6 (which means about 99% of all ARM processors in the field currently) cannot do an atomic decrement/increment. It must be done manually with interrupt disabled.
The only truly atomic instruction it has is a swap which lays itself pretty well for mutex support.
Nicolas
Quoted 19 lines · Russell King
Quoted 3 lines · Nicolas Pitre
So 14 instructions total with preemption disabling, and that's with the best implementation possible by open coding everything instead of relying on generic functions/macros.
I agree with your analysis Nicolas.
However, don't forget to compare this with our existing semaphore implementation which is 9 instructions, or 8 for the SMP version.
In total, this means that mutexes will be
_FARMOREEXPENSIVE_on ARM than our semaphores.Forcing architectures down the "lets make everything generic" path does not always hack it. It can't do by its very nature. Generic implementations are *always* sub-optimal and it is pretty clear that any gain that mutexes _may_ give is completely wasted on ARM by the overhead of having a generic framework imposed upon us.
Quoted 4 lines
So, to sum up, if this path is persued, mutexes will be a bloody disaster on ARM. We'd be far better off sticking to semaphores and ignoring this mutex stuff.
So what's wrong with having the generic code, and for those with a fast semapore add an arch specific?
#define mutex_lock down
#define mutex_unlock up
#define mutex_trylock(x) (!down_trylock(x))
Until the mutex code is updated to a fast arch specific implementation.
Let me restate, that the generic code should not be this, but each arch can have this if they already went through great lengths in making a fast semaphore.
Heck put the above defines in the generic code, with a define
linux/mutex.h:
#ifdef HAVE_ARCH_MUTEX
#include <asm/mutex.h>
#else
#ifdef HAVE_FAST_SEMAPHORE
#define <defines here>
#else
generic code here
#endif /* HAVE_FAST_SEMAPHORE */
#endif /* HAVE_ARCH_MUTEX */
-- Steve
Quoted 7 lines · Steven Rostedt
So what's wrong with having the generic code, and for those with a fast semapore add an arch specific?
#define
mutex_lockdown
#definemutex_unlockup
#definemutex_trylock(x)(!down_trylock(x))
Quoted 5 lines
Until the mutex code is updated to a fast arch specific implementation.
Let me restate, that the generic code should not be this, but each arch can have this if they already went through great lengths in making a fast semaphore.
I have no problem with this since we can then use Nico's swp-based implementation. Great! What seems to be happening though is that there's a move to make these operations be generic across all architectures.
What both Nico and myself have demonstrated is that if architectures are placed into the generic strait-jacket, any alleged performance benefit of mutexes is completely swamped, which in turn makes the whole mutex idea entirely pointless.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core
Steven Rostedt <[email protected]> wrote:
[...]
Quoted 12 lines
Let me restate, that the generic code should not be this, but each arch can have this if they already went through great lengths in making a fast semaphore.
Heck put the above defines in the generic code, with a define
linux/mutex.h:#ifdef
HAVE_ARCH_MUTEX
#include <asm/mutex.h>
#else
Quoted 7 lines
#ifdef
HAVE_FAST_SEMAPHORE#define <defines here>
#else
generic code here
Anything to go here could/should very well be in the above arch-specific file. Saves you a #define ;-)
Quoted 2 lines
#endif /*
HAVE_FAST_SEMAPHORE*/
#endif /*HAVE_ARCH_MUTEX*/
Signature · 4 lines
Dr. Horst H. von Brand User #22616 counter.li.org Departamento de Informatica Fono: +56 32 654431 Universidad Tecnica Federico Santa Maria +56 32 654239 Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513
Quoted 5 lines · Russell King
Quoted 3 lines · Nicolas Pitre
So 14 instructions total with preemption disabling, and that's with the best implementation possible by open coding everything instead of relying on generic functions/macros.
Quoted 7 lines
I agree with your analysis Nicolas.
However, don't forget to compare this with our existing semaphore implementation which is 9 instructions, or 8 for the SMP version.
In total, this means that mutexes will be
_FARMOREEXPENSIVE_on ARM than our semaphores.
I don't follow you at all. I'm arguing that mutexes are much less expensive than any semaphore implementation (except on SMP where semaphores and mutexes will probably look the same).
Quoted 5 lines
Forcing architectures down the "lets make everything generic" path does not always hack it. It can't do by its very nature. Generic implementations are *always* sub-optimal and it is pretty clear that any gain that mutexes _may_ give is completely wasted on ARM by the overhead of having a generic framework imposed upon us.
Agreed. And that's why I'm suggesting that the mutex locking/unlocking fast path should be architecture specific. And to that effect I'm working on a patch against Ingo's mutex code to illustrate my point.
What should still remain generic is the contention fallback. That's where the actual complexity lives and that part doesn't have to be optimized for best inline performances.
Nicolas
Quoted 3 lines · Nicolas Pitre
Sure, and we're now more costly than the current implementation with irq disabling.
Do the timing. It may be more instructions, but I think it was you yourself that timed the current thing at 23 cycles, no?
Special registers are almost always slower than nicely cached accesses (and they all basically will be).
Linus
Quoted 7 lines · Linus Torvalds
Quoted 3 lines · Nicolas Pitre
Sure, and we're now more costly than the current implementation with irq disabling.
Do the timing. It may be more instructions, but I think it was you yourself that timed the current thing at 23 cycles, no?
That's PXA, which is Intel. Most other ARM CPUs are far faster than that at IRQ disable. Typically you're looking at 6 cycles to disable + 3 cycles to re-enable.
However, Nico's analysis of 14 instructions vs 9 instructions pretty much paints the picture - those 14 instructions for the preempt_disable _will_ be more heavy weight than Nico's idea.
Also, Nico has an alternative idea for mutexes which does not involve decrementing or incrementing - it's an atomic swap. That works out at about the same cycle count on non-Intel ARM CPUs as the present semaphore path. I'm willing to bet that it will be faster than the present semaphore path on Intel ARM CPUs.
Here's the cycle counts for ARM926, assuming hot caches and the failure path not taken for the existing semaphore code:
mrs ip, cpsr 2 orr lr, ip, #128 1 msr cpsr_c, lr 3 ldr lr, [%0] 2 subs lr, lr, %1 1 str lr, [%0] 1 msr cpsr_c, ip 3 movmi ip, %0 1 blmi failure 1
total: 15 cycles
Here's nico's version (with a couple of fixes to ensure we don't schedule if preempt count is non-zero):
mov r0, sp, lsr #13 1
mov r0, r0, lsl #13 1
ldr r1, [r0, #PREEMPT_COUNT] 2
add r2, r1, #1 1
str r2, [r0, #PREEMPT_COUNT] 1
ldr r4, [r3] 2
sub r4, r4, #1 1
str r4, [r3] 1
str r1, [r0, #PREEMPT_COUNT] 1
teq r1, #0 1
bne no_preempt_check 1
ldr r1, [r0, #FLAGS] 2
tst r1, #TIF_NEED_RESCHED 1
blne schedule 1
no_preempt_check:
cmp r4, #0 1
blmi failed 1
total: 19 cyclesThat's a 26% increase in the cost of a mutex implemented this way over a plain semaphore.
Hence, mutexes implemented this way will be _more_ costly. Significantly so. Enough to make them worthless.
I think the approach needs completely rethinking.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core
Quoted 7 lines · Russell King
Also, Nico has an alternative idea for mutexes which does not involve decrementing or incrementing - it's an atomic swap. That works out at about the same cycle count on non-Intel ARM CPUs as the present semaphore path. I'm willing to bet that it will be faster than the present semaphore path on Intel ARM CPUs.
Don't be so sure, especially not in the future.
An atomic "swap" operation is, from a CPU design standpoint, fundamentally more expensive that a "load + store".
Now, most ARM architectures don't notice this, because they are all in-order, and not SMP-aware anyway. No suble memory ordering, no nothing. Which is the only case when "swap" basically becomes a cheap "load+store".
What I'm trying to say is that a plain "load + store" is almost always going to be the best option in the long run.
It's also almost certainly always the best option for UP + non-preempt, for both present and future CPU's. The reason is simply that a microarchitecture will _always_ be optimized for that case, since it's pretty much by definition the common situation.
Is preemption even the common case on ARM? I'd assume not. Why are people so interested in the preemption case? IOW, why don't you just do
ldr lr,[%0] subs lr, lr, %1 str lr,[%0] blmi failure
as the _base_ timings, since that should be the common case. That's the drop-dead fastest UP case.
There's an additional advantage of the regular load/store case: if some CPU has scheduling issues, you can actually write this out as C code (with an extra empty ASM to make sure that the compiler doesn't move anything out of the critical region). Again, that probably doesn't matter on most ARM chips, but in the general case it sure does matter.
(Btw, inlining _any_ of these except perhaps the above trivial case, is probably wrong. None of the ARM chips tend to have caches all that big, I bet).
Linus
Quoted 4 lines · Linus Torvalds
Quoted 1 line · Russell King
Quoted 8 lines
Quoted 6 lines
Also, Nico has an alternative idea for mutexes which does not involve decrementing or incrementing - it's an atomic swap. That works out at about the same cycle count on non-Intel ARM CPUs as the present semaphore path. I'm willing to bet that it will be faster than the present semaphore path on Intel ARM CPUs.
Don't be so sure, especially not in the future.
I think we agree. But we still live in the present.
Quoted 6 lines
An atomic "swap" operation is, from a CPU design standpoint, fundamentally more expensive that a "load + store".
Now, most ARM architectures don't notice this, because they are all in-order, and not SMP-aware anyway. No suble memory ordering, no nothing. Which is the only case when "swap" basically becomes a cheap "load+store".
Indeed.
Quoted 7 lines
What I'm trying to say is that a plain "load + store" is almost always going to be the best option in the long run.
It's also almost certainly always the best option for UP + non-preempt, for both present and future CPU's. The reason is simply that a microarchitecture will _always_ be optimized for that case, since it's pretty much by definition the common situation.
Which is why each architecture must always have the option of providing its own fast path implementation according to a number of factors that cannot be made into a generic layer. But this is the same issue whether we talk about semaphores or mutexes.
Quoted 2 lines
Is preemption even the common case on ARM? I'd assume not. Why are people so interested in the preemption case?
Because embedded people want it. ARM is also about the only architecture besides x86 that currently sees most of the RT work for the same reason. And yes preemption does make a difference.
Quoted 9 lines
IOW, why don't you just do
ldr lr,[%0] subs lr, lr, %1 str lr,[%0] blmi failureas the _base_ timings, since that should be the common case. That's the drop-dead fastest UP case.
The above is 5 cycles. About the same as the preemption-safe swp-based mutex implementation on non-Intel ARM. It is broken wrt interrupts when the swp is not. It doesn't work with preemption while the swp implementation is potentially smaller with cse and it works in all cases.
I mean...... what is it with mutexes that you dislike to the point of bending backward that far, and even after seeing the numbers, with such a semaphore implementation that _I_ even wouldn't trust people to use correctly? Yes we should use complete() from interrupt handlers but I can bet you that a lot of people are still using up() there, and with the current up() implementation it just _works_ at least on ARM.
Quoted 3 lines
(Btw, inlining _any_ of these except perhaps the above trivial case, is probably wrong. None of the ARM chips tend to have caches all that big, I bet).
On XScale you should add 2 cycles to branch to the out of line code and 2 other cycles to branch back.
The ARM mutex implementation can save that and have an extremely small inlined footprint already.
Again what do you dislike so much about mutexes? It must not have much to do with technical issues at this point. ;-)
Nicolas
Quoted 5 lines · Nicolas Pitre
I mean...... what is it with mutexes that you dislike to the point of bending backward that far, and even after seeing the numbers, with such a semaphore implementation that _I_ even wouldn't trust people to use correctly?
Quite frankly, what has disgusted me about this mutex discussion is the totally specious arguments for the new mutexes that just rubs me entirely the wrong way.
If it had _started_ with a mutex implementation that was faster, simpler, and didn't rename the old and working semaphores, I'd have been perfectly fine with it.
As it is, the discussion has been pretty much everything but that.
And then people who argue about single cycles, end up dismissing the single cycles when I argue that "ld+st" is faster - like you just did.
Be consistent, dammit. If single cycles matter, they matter. If they don't, then the existing code is better, since it's existing and works. You can't have it both ways.
In other words: if people didn't mix up issues that had nothing to do with this into it, I'd be happier. I've already said that a mutex that does _not_ replace semaphore (and doesn't mess with naming) is acceptable.
We've done that before. But do it RIGHT, dammit. And don't mix existing semaphores into it (for example, completions didn't change any old users).
Linus
Quoted 5 lines · Linus Torvalds
In other words: if people didn't mix up issues that had nothing to do with this into it, I'd be happier. I've already said that a mutex that does _not_ replace semaphore (and doesn't mess with naming) is acceptable.
At least I'm very happy with Linus' decision. Can we now end this thread, and move forward. You may bring up your issues in what comes next.
Thank you,
-- Steve
Quoted 4 lines · Linus Torvalds
Quite frankly, what has disgusted me about this mutex discussion is the totally specious arguments for the new mutexes that just rubs me entirely the wrong way.
[...]
Quoted 4 lines
In other words: if people didn't mix up issues that had nothing to do with this into it, I'd be happier. I've already said that a mutex that does _not_ replace semaphore (and doesn't mess with naming) is acceptable.
Oh if it is so we are in _violent_ agreement then. I don't dispute that at all and I pretty agree with a separate namespace for mutexes. Actually I think no one contested that either.
Current semaphores can be migrated to mutexes individually when that makes sense to do so, separately.
With regards to my _implementation_ concerns about the proposed mutex patches I guess I can discuss them with Ingo (and an actual patch is coming to fix them).
Nicolas
Quoted 3 lines · Linus Torvalds
If it had _started_ with a mutex implementation that was faster, simpler, and didn't rename the old and working semaphores, I'd have been perfectly fine with it.
oh, i'm totally OK with not doing the renames and leaving semaphores alone!
Just in case it wasnt clear: i very much expected that the migration helper patches would be controverial, and that they would probably not go upstream. [Christoph said last week that they were fit for an obfuscated C contest, not the kernel, and i didnt expect this sentiment to change overnight.] Look at my patch order:
xfs-mutex-namespace-collision-fix.patch
add-atomic-xchg-i386.patch
add-atomic-xchg-x86_64.patch
add-atomic-xchg-ia64.patch
add-atomic-call-func-i386.patch
add-atomic-call-func-x86_64.patch
add-atomic-call-func-ia64.patch
add-atomic-call-wrappers-all-other-arches.patch
mutex-core.patch
mutex-debug.patch
mutex-debug-more.patch
mutex-migration-helper-i386.patch # not upstream from here
mutex-migration-helper-x86_64.patch
mutex-migration-helper-ia64.patch
mutex-migration-helper-core.patch
sx8-sem2completions.patch
cpu5wdt-sem2completions.patch
ide-gendev-sem-to-completion.patch
loop-to-completions.patch
arch-semaphores.patch
The first 11 patches are finegrained and contain _zero_ of the migration and rename stuff. I specifically created the patch-series in such a way, so that we could simply chop off the last few patches.
in the future i'll only send patches up to mutex-debug-more.patch, as suggested by Christoph and you as well. So there's really no controversy. Btw., that was true for my first queue already, as noticed by Christoph [*].
Basically all of the activity in the last 2 days was in the first 11 patches. I'll send an updated queue later today.
Ingo
[*] the migration helpers were incredibly useful for pulling this off.
Without the wide exposure of mutexes to existing semaphore users i'd not have been able to profile the system, to measure the impact the effects of mutexes on performance. I'd also not have been able to say what percentage of semaphores could move over to mutexes. We could also not have carried the mutex implementation in the -rt tree for more than a year, in which year millions of lines were changed in the upstream kernel! It would have been simply impossible to even attempt this, without the type-sensitive APIs and the minimal renames to categorize semaphores.
Quoted 9 lines · Nicolas Pitre
Quoted 7 lines · Linus Torvalds
IOW, why don't you just do
ldr lr,[%0] subs lr, lr, %1 str lr,[%0] blmi failure
Quoted 4 lines
Quoted 1 line
as the _base_ timings, since that should be the common case. That's the drop-dead fastest UP case.
The above is 5 cycles. About the same as the preemption-safe swp-based mutex implementation on non-Intel ARM. It is broken wrt interrupts when the swp is not.
How it is broken WRT interrupts? (sorry, I haven't had it explained to me in words of two syllables or less)
Signature · 3 lines
SUSE Labs, Novell Inc. Send instant messages to your online friends http://au.messenger.yahoo.com
Quoted 9 lines · Linus Torvalds
Quoted 3 lines · Nicolas Pitre
Sure, and we're now more costly than the current implementation with irq disabling.
Do the timing. It may be more instructions, but I think it was you yourself that timed the current thing at 23 cycles, no?
Yes. And with my hand-optimized assembly version using
(the equivalent of) preempt_disable/preempt_enable I get 20 cycles over 14 instructions. If I let gcc do it using the standard macros it gets worse.
The irq disabling implementation spends 23 cycles over 8 instructions.
The mutex swp-based implementation spends 8 cycles over 4 instructions.
If we make the preempt_disable/enable implementation out of line that'll reduce the .text size bloat, but it'LL add more cycles due to the additional two branches plus preserve of the return address making it above 23 cycles for sure.
Quoted 2 lines
Special registers are almost always slower than nicely cached accesses (and they all basically will be).
Sure, but do the numbers above tell you anything else? It seems pretty obvious to me that simple mutexes are a real gain. Suffice for the implementation to let architecture do their best to lock the mutex and fall back to generic code when there is contention (just like we already do for semaphores).
Nicolas
Quoted 11 lines · Nick Piggin
Quoted 8 lines · Nicolas Pitre
Quoted 2 lines · Nick Piggin
Considering that on UP, the arm should not need to disable interrupts for this function (or has someone refuted Linus?), how about:
Kernel preemption.
preempt_disable()?
Yes. It should almost always be faster than physically disabling interrupts anyway. Even if it's more instructions.
Linus
Quoted 6 lines · Nick Piggin
Quoted 4 lines
Quoted 2 lines
Considering that on UP, the arm should not need to disable interrupts for this function (or has someone refuted Linus?), how about:
Kernel preemption.
preempt_disable()?
please take a look at kernel/mutex.c, there's a define at the top of the file:
// #define MUTEX_IRQ_SAFE
which, if off, makes the mutex code use preempt_disable() and preempt_enable() to make it preemption-safe. If it's on, the mutex implementation uses IRQ flags.
in my current tree i've already eliminated this define, and have switched the code to use preempt_disable()/preempt_enable() exclusively, because preempt_*() is equally fast on all platforms, while IRQ disable costs vary largely. (and they are rarely faster than preempt_disable()).
my current tree also provides a mechanism for architectures to hand-code the mutex lock and unlock fastpath, if they choose to do so. So i think we can really stop the cycle counting.
Ingo
Quoted 12 lines · Nicolas Pitre
Quoted 10 lines · Ingo Molnar
Quoted 6 lines · David Woodhouse
Quoted 2 lines · Zwane Mwaikambo
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?
Quoted 7 lines
Quoted 3 lines
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. Althoughatomic_dec()is available on all architectures, its implementation is far from being the most efficient thing to do for them all.
Actually, the best thing to do is to let the architecture do what is the most efficient. Please consider this patch that adds the flexibility for any architecture to use the optimal mechanism it has for atomic locking. It also let it decide whether inlining the fast path is a good thing or not. This patch should leave i386 unchanged. It applies on top of your last posted mutex patch serie.
include/linux/mutex.h +41 -2
@@ -14,6 +14,8 @@#include <asm/atomic.h> #include <linux/list.h> #include <linux/spinlock_types.h> +#include <linux/linkage.h> +#include <asm/mutex.h> /* * Simple, straightforward mutexes with strict semantics:
@@ -93,12 +95,49 @@ extern void FASTCALL(__mutex_init(struct#define DEFINE_MUTEX (mutexname) \ struct mutex mutexname = __MUTEX_INITIALIZER(mutexname) +/* +* We can speed up the lock-acquire, but only if the architecture +* supports it and if there's no debugging state +* to be set up (!DEBUG_MUTEXESS). +*/ +#ifdef CONFIG_DEBUG_MUTEXESS +#undef MUTEX_LOCKLESS_FASTPATH +#endif + +/* +* We can inline the lock-acquire, but only if the architecture +* wants it, and if MUTEX_LOCKLESS_FASTPATH is active. +*/ +#ifndef MUTEX_LOCKLESS_FASTPATH +#undef MUTEX_INLINE_FASTPATH +#endif + +/* out of line contention handlers */ +extern void FASTCALL (__mutex_lock_noinline (atomic_t *lock_count)); +extern void FASTCALL (__mutex_unlock_noinline (atomic_t *lock_count)); + +#ifdef MUTEX_INLINE_FASTPATH + +#define mutex_lock (lock) \ + __arch_fast_mutex_lock(&lock->count, __mutex_lock_noinline) +#define mutex_unlock (lock) \ + __arch_fast_mutex_unlock(&lock->count, __mutex_unlock_noinline) +#define mutex_trylock (lock) \ + __arch_fast_mutex_trylock(&lock->count) +#define mutex_is_locked (lock) \ + ({ mb(); atomic_read(&lock->count) != 1; }) + +#else +extern void FASTCALL (mutex_lock (struct mutex *lock)); -extern int FASTCALL (mutex_lock_interruptible (struct mutex *lock)); -extern int FASTCALL (mutex_trylock (struct mutex *lock));extern void FASTCALL (mutex_unlock (struct mutex *lock)); +extern int FASTCALL (mutex_trylock (struct mutex *lock));extern int FASTCALL (mutex_is_locked (struct mutex *lock)); +#endif + +extern int FASTCALL (mutex_lock_interruptible (struct mutex *lock)); +/* * Debugging variant of mutexes. The only difference is that they accept * the semaphore APIs too:
kernel/mutex.c +28 -37
134 lines not on this page · raw message
include/asm-generic/mutex.h new file +22 -0
@@ -0,0 +1,22 @@ +/* +* linux/include/asm-generic/mutex.h +* +* Reference implementation for lockless fast path mutex operations +*/ + +#define MUTEX_LOCKLESS_FASTPATH + +#define __arch_fast_mutex_lock (count, contention_fn) \ +do { \ + if (atomic_xchg(count, 0) != 1) \ + contention_fn(count); \ +} while (0) + +#define __arch_fast_mutex_unlock (count, contention_fn) \ +do { \ + if (atomic_xchg(count, 1) != 0) \ + contention_fn(count); \ +} while (0) + +#define __arch_fast_mutex_trylock (count) \ + (atomic_cmpxchg(count, 1, 0) == 1)
include/asm-i386/mutex.h new file +44 -0
44 lines not on this page · raw message
include/asm-arm/mutex.h new file +9 -0
@@ -0,0 +1,9 @@ +#include <asm/system.h> + +#define atomic_xchg (v, new) (xchg(&((v)->counter), new)) + +#include <asm-generic/mutex.h> + +#ifndef swp_is_buggy +#define MUTEX_INLINE_FASTPATH +#endif
Nicolas
Quoted 6 lines · Nicolas Pitre
Quoted 3 lines
Please consider using (a variant of)
xchg()instead. Althoughatomic_dec()is available on all architectures, its implementation is far from being the most efficient thing to do for them all.Actually, the best thing to do is to let the architecture do what is the most efficient. [...]
i have already added something similar. Furthermore, i have also eliminated the CMPXCHG requirement from MUTEX_FASTPATH, which should open ARM up to the generic lockless fastpath. (but you can still choose to reimplement it in the architecture) I'll send a new queue later today.
Ingo
Quoted 16 lines · Zwane Mwaikambo
Quoted 14 lines · Ingo Molnar
include/asm-x86_64/atomic.h +14
-0+#define atomic_dec_call_if_negative (v, fn_name) \ +do { \ + fastcall void (*__tmp)(atomic_t *) = fn_name; \ + \ + (void)__tmp; \ + typecheck(atomic_t *, v); \ + \ + __asm__ __volatile__( \ + LOCK "decl (%%rdi)\n" \ + "js 2f\n" \ + "1:\n" \ + LOCK_SECTION_START("") \ + "2: call "#fn_name"\n\t" \ + "jmp 1b\n" \
Quoted 8 lines
include/asm-x86_64/atomic.h static __inline__ int atomic_sub_return( · line 232
Quoted 5 lines
include/asm-x86_64/atomic.h +5
-0+ LOCK_SECTION_END \ + : \ + :"D" (v) \ + :"memory"); \ +}while (0 )Hi Ingo,
Doesn't this corrupt caller saved registers?
good catch - i correctly marked them clobbered on i386, but not on x86_64. The function using this code uses nothing else, so this bug caused no real corruption - but that was just pure luck.
Ingo