ARM: locks: remove opencoded #16 for ticket shift
Subsystems (2)
PATCH 1/1 ARM: locks: remove opencoded #16 for ticket shift
The ticket width of our spinlocks is defined by TICKET_SHIFT, so remove the opencoded #16 from the trylock implementation.
--- arch/arm/include/asm/spinlock.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
arch/arm/include/asm/spinlock.h +3 -3
@@ -104,11 +104,11 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock) __asm__ __volatile__(" ldrex %0, [%3]\n" " mov %2, #0\n" -" subs %1, %0, %0, ror #16\n" -" addeq %0, %0, %4\n" +" subs %1, %0, %0, ror %4\n" +" addeq %0, %0, %5\n" " strexeq %2, %0, [%3]" :"=&r" (slock),"=&r" (contended),"=r" (res) - :"r" (&lock->slock),"I" (1 <<TICKET_SHIFT ) + :"r" (&lock->slock),"I" (TICKET_SHIFT ),"I" (1 <<TICKET_SHIFT ) :"cc" ); }while (res);
Signature · 1 line
1.8.2.2
PATCH 1/1 ARM: locks: remove opencoded #16 for ticket shift
Quoted 13 lines · Will Deacon
The ticket width of our spinlocks is defined by
TICKET_SHIFT, so remove the opencoded #16 from the trylock implementation.Signed-off-by Will Deacon <[email protected]>--- arch/arm/include/asm/spinlock.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)arch/arm/include/asm/spinlock.h +0
-0
arch/arm/include/asm/spinlock.h static inline int arch_spin_trylock(arch_spinlock_t *lock) · line 108
Quoted 7 lines
arch/arm/include/asm/spinlock.h +2
-2@@ -104,11 +104,11 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock) __asm__ __volatile__(" ldrex %0, [%3]\n" " mov %2, #0\n" -" subs %1, %0, %0, ror #16\n" - " addeq %0, %0, %4\n" +" subs %1, %0, %0, ror %4\n" + " addeq %0, %0, %5\n"
%5! Argh.
arch/arm/include/asm/spinlock.h static inline int arch_spin_trylock(arch_spinlock_t *lock) · line 111
Quoted 4 lines
arch/arm/include/asm/spinlock.h +1
-1" strexeq %2, %0, [%3]" : "=&r" (slock), "=&r" (contended), "=r" (res) - : "r" (&lock->slock), "I" (1 << TICKET_SHIFT) + : "r" (&lock->slock), "I" (TICKET_SHIFT), "I" (1 << TICKET_SHIFT)
I guess that should work.
Strictly speaking, "M" is the right constraint to use for explicit shift counts, though it won't make much difference in practice.
(Unless it ICEs the compiler for you ... seems to work on the 4.6.3 GCC I have here.)
Cheers
---Dave
Quoted 11 lines
arch/arm/include/asm/spinlock.h +0
-0: "cc"); }while (res);Signature · 7 lines
1.8.2.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 ARM: locks: remove opencoded #16 for ticket shift
In reply to Will DeaconQuoted 4 lines · Will Deacon
The ticket width of our spinlocks is defined by
TICKET_SHIFT, so remove the opencoded #16 from the trylock implementation.Signed-off-by Will Deacon <[email protected]>
This is pointless.
arch/arm/include/asm/spinlock.h static inline int arch_spin_trylock(arch_spinlock_t *lock) · line 107
Quoted 3 lines
arch/arm/include/asm/spinlock.h +0
-1" ldrex %0, [%3]\n" " mov %2, #0\n" -" subs %1, %0, %0, ror #16\n"
Any value other than 16 would break that code. You'd rather ensure it never gets defined to anything else.
Nicolas
PATCH 1/1 ARM: locks: remove opencoded #16 for ticket shift
Quoted 8 lines · Nicolas Pitre
Quoted 4 lines · Will Deacon
The ticket width of our spinlocks is defined by
TICKET_SHIFT, so remove the opencoded #16 from the trylock implementation.Signed-off-by Will Deacon <[email protected]>This is pointless.
I disagree (see below).
Quoted 6 lines
arch/arm/include/asm/spinlock.h static inline int arch_spin_trylock(arch_spinlock_t *lock) · line 107
Quoted 3 lines
arch/arm/include/asm/spinlock.h +0
-1" ldrex %0, [%3]\n" " mov %2, #0\n" -" subs %1, %0, %0, ror #16\n" Any value other than 16 would break that code. You'd rather ensure it never gets defined to anything else.
There are two aspects to the ticket lock:
1. The size of each ticket
2. The location of the next ticket within the 32-bit lock word
TICKET_SHIFT actually defines (2) and could be 0 or 16, hence why I was making this small cosmetic change. I admit that the naming isn't very good and I should change the commit message to reflect what's actually going on.
Cheers,
Will
PATCH 1/1 ARM: locks: remove opencoded #16 for ticket shift
Quoted 19 lines · Will Deacon
Quoted 8 lines · Nicolas Pitre
Quoted 4 lines · Will Deacon
The ticket width of our spinlocks is defined by
TICKET_SHIFT, so remove the opencoded #16 from the trylock implementation.Signed-off-by Will Deacon <[email protected]>This is pointless.
I disagree (see below).
Quoted 6 lines
arch/arm/include/asm/spinlock.h static inline int arch_spin_trylock(arch_spinlock_t *lock) · line 107
Quoted 3 lines
arch/arm/include/asm/spinlock.h +0
-1" ldrex %0, [%3]\n" " mov %2, #0\n" -" subs %1, %0, %0, ror #16\n" Any value other than 16 would break that code. You'd rather ensure it never gets defined to anything else.
Quoted 7 lines
There are two aspects to the ticket lock:
1. The size of each ticket
2. The location of the next ticket within the 32-bit lock word
TICKET_SHIFTactually defines (2) and could be 0 or 16, hence why I was making this small cosmetic change.
Doesn't matter where the location of the next ticket is. Whether it is 0 or 16, for the above code to work, it *must* always perform a ror #16. Hence tying the ror constant with TICKET_SHIFT is wrong.
Nicolas
PATCH 1/1 ARM: locks: remove opencoded #16 for ticket shift
Quoted 6 lines · Nicolas Pitre
Quoted 5 lines · Will Deacon
There are two aspects to the ticket lock:
1. The size of each ticket
2. The location of the next ticket within the 32-bit lock word
Quoted 6 lines
Quoted 2 lines
TICKET_SHIFTactually defines (2) and could be 0 or 16, hence why I was making this small cosmetic change.Doesn't matter where the location of the next ticket is. Whether it is 0 or 16, for the above code to work, it *must* always perform a ror #16. Hence tying the ror constant with
TICKET_SHIFTis wrong.
You're right; we always need to refer to the higher ticket, rather than owner' or next'. Patch dropped.
Cheers,
Will