parisc: Align locks for LWS syscalls to L1 cache size

PATCH In review Helge Deller 3 participants 7 messages Sep 2, 2015 – Sep 7, 2015 PA-RISC architecture

Share & tools

Helge Deller

author maintainer

2015-09-02 19:38 UTC #0 lore raw

PATCH 1/1 parisc: Align locks for LWS syscalls to L1 cache size

Align the locks for the Light weight syscall (LWS) which is used for atomic userspace operations (e.g. gcc atomic builtins) on L1 cache boundaries. This should speed up LWS calls on PA20 systems.

Reported-by John David Anglin <[email protected]>

Signed-off-by Helge Deller <[email protected]>

arch/parisc/kernel/syscall.S +6 -8

index 7ef22e3..80c2306 100644
@@ -561,9 +561,9 @@ lws_compare_and_swap:
 	extru  %r26, 27, 4, %r20
 
 	/* Find lock to use, the hash is either one of 0 to
-	   15, multiplied by 16 (keep it 16-byte aligned)
+	   15, multiplied by L1_CACHE_BYTES (keep it L1 cache aligned)
 	   and add to the lock table offset. */
-	shlw	%r20, 4, %r20
+	shlw	%r20, L1_CACHE_SHIFT, %r20
 	add	%r20, %r28, %r20
 
 # if ENABLE_LWS_DEBUG

@@ -751,9 +751,9 @@ cas2_lock_start:
 	extru  %r26, 27, 4, %r20
 
 	/* Find lock to use, the hash is either one of 0 to
-	   15, multiplied by 16 (keep it 16-byte aligned)
+	   15, multiplied by L1_CACHE_BYTES (keep it L1 cache aligned)
 	   and add to the lock table offset. */
-	shlw	%r20, 4, %r20
+	shlw	%r20, L1_CACHE_SHIFT, %r20
 	add	%r20, %r28, %r20
 
 	rsm	PSW_SM_I, %r0			/* Disable interrupts */

@@ -931,11 +931,9 @@ END(sys_call_table64)
 ENTRY(lws_lock_start)
 	/* lws locks */
 	.rept 16
-	/* Keep locks aligned at 16-bytes */
+	/* Keep locks aligned to L1_CACHE_BYTES */
 	.word 1
-	.word 0 
-	.word 0
-	.word 0
+	.align	L1_CACHE_BYTES
 	.endr
 END(lws_lock_start)
 	.previous

John David Anglin

2015-09-02 19:46 UTC #1 lore raw

The LWS locks are also used for futex operations. The shifts in arch/parisc/include/asm/futex.h need a corresponding update.

Dave

Quoted 51 lines · Helge Deller

Align the locks for the Light weight syscall (LWS) which is used for atomic userspace operations (e.g. gcc atomic builtins) on L1 cache boundaries. This should speed up LWS calls on PA20 systems.

Reported-by John David Anglin <[email protected]>

Signed-off-by Helge Deller <[email protected]>

arch/parisc/kernel/syscall.S +6 -8

index 7ef22e3..80c2306 100644
@@ -561,9 +561,9 @@ lws_compare_and_swap:
  	extru  %r26, 27, 4, %r20
 
  	/* Find lock to use, the hash is either one of 0 to
-	   15, multiplied by 16 (keep it 16-byte aligned)
+	   15, multiplied by L1_CACHE_BYTES (keep it L1 cache aligned)
  	   and add to the lock table offset. */
-	shlw	%r20, 4, %r20
+	shlw	%r20, L1_CACHE_SHIFT, %r20
  	add	%r20, %r28, %r20
 
  # if ENABLE_LWS_DEBUG

@@ -751,9 +751,9 @@ cas2_lock_start:
  	extru  %r26, 27, 4, %r20
 
  	/* Find lock to use, the hash is either one of 0 to
-	   15, multiplied by 16 (keep it 16-byte aligned)
+	   15, multiplied by L1_CACHE_BYTES (keep it L1 cache aligned)
  	   and add to the lock table offset. */
-	shlw	%r20, 4, %r20
+	shlw	%r20, L1_CACHE_SHIFT, %r20
  	add	%r20, %r28, %r20
 
  	rsm	PSW_SM_I, %r0			/* Disable interrupts */

@@ -931,11 +931,9 @@ END(sys_call_table64)
  ENTRY(lws_lock_start)
  	/* lws locks */
  	.rept 16
-	/* Keep locks aligned at 16-bytes */
+	/* Keep locks aligned to L1_CACHE_BYTES */
  	.word 1
-	.word 0
-	.word 0
-	.word 0
+	.align	L1_CACHE_BYTES
  	.endr
  END(lws_lock_start)
  	.previous

Signature · 1 line

John David Anglin  [email protected]

Helge Deller

author maintainer

2015-09-02 20:29 UTC #2 lore raw

parisc: Align locks for LWS syscalls to L1 cache size (v2)

Align the locks for the Light-weight-syscall (LWS) which are used for atomic userspace operations (e.g. gcc atomic builtins) on L1 cache boundaries. This should speed up LWS calls on PA20 systems.

Reported-by John David Anglin <[email protected]>

Signed-off-by Helge Deller <[email protected]>

arch/parisc/kernel/syscall.S +6 -8

index 7ef22e3..80c2306 100644
@@ -561,9 +561,9 @@ lws_compare_and_swap:
 	extru  %r26, 27, 4, %r20
 
 	/* Find lock to use, the hash is either one of 0 to
-	   15, multiplied by 16 (keep it 16-byte aligned)
+	   15, multiplied by L1_CACHE_BYTES (keep it L1 cache aligned)
 	   and add to the lock table offset. */
-	shlw	%r20, 4, %r20
+	shlw	%r20, L1_CACHE_SHIFT, %r20
 	add	%r20, %r28, %r20
 
 # if ENABLE_LWS_DEBUG

@@ -751,9 +751,9 @@ cas2_lock_start:
 	extru  %r26, 27, 4, %r20
 
 	/* Find lock to use, the hash is either one of 0 to
-	   15, multiplied by 16 (keep it 16-byte aligned)
+	   15, multiplied by L1_CACHE_BYTES (keep it L1 cache aligned)
 	   and add to the lock table offset. */
-	shlw	%r20, 4, %r20
+	shlw	%r20, L1_CACHE_SHIFT, %r20
 	add	%r20, %r28, %r20
 
 	rsm	PSW_SM_I, %r0			/* Disable interrupts */

@@ -931,11 +931,9 @@ END(sys_call_table64)
 ENTRY(lws_lock_start)
 	/* lws locks */
 	.rept 16
-	/* Keep locks aligned at 16-bytes */
+	/* Keep locks aligned to L1_CACHE_BYTES */
 	.word 1
-	.word 0 
-	.word 0
-	.word 0
+	.align	L1_CACHE_BYTES
 	.endr
 END(lws_lock_start)
 	.previous

arch/parisc/include/asm/futex.h +2 -2

index 49df148..47b075c 100644
@@ -15,7 +15,7 @@ static inline void
 _futex_spin_lock_irqsave(u32 __user *uaddr, unsigned long int *flags)
 {
 	extern u32 lws_lock_start[];
-	long index = ((long)uaddr & 0xf0) >> 2;
+	long index = (((long)uaddr & 0xf0) >> 4) << (L1_CACHE_SHIFT-2);
 	arch_spinlock_t *s = (arch_spinlock_t *)&lws_lock_start[index];
 	local_irq_save(*flags);
 	arch_spin_lock(s);

@@ -25,7 +25,7 @@ static inline void
 _futex_spin_unlock_irqrestore(u32 __user *uaddr, unsigned long int *flags)
 {
 	extern u32 lws_lock_start[];
-	long index = ((long)uaddr & 0xf0) >> 2;
+	long index = (((long)uaddr & 0xf0) >> 4) << (L1_CACHE_SHIFT-2);
 	arch_spinlock_t *s = (arch_spinlock_t *)&lws_lock_start[index];
 	arch_spin_unlock(s);
 	local_irq_restore(*flags);

Helge Deller

author maintainer

2015-09-05 21:48 UTC #3 lore raw

Quoted 8 lines · Helge Deller

parisc: Align locks for LWS syscalls to L1 cache size (v2)
Align the locks for the Light-weight-syscall (LWS) which are used for atomic userspace operations (e.g. gcc atomic builtins) on L1 cache boundaries. This should speed up LWS calls on PA20 systems.
Reported-by John David Anglin <[email protected]>
Signed-off-by Helge Deller <[email protected]>

Any objections to this patch ?
One idea below...

Quoted 31 lines

arch/parisc/kernel/syscall.S +4 -4

index 7ef22e3..80c2306 100644
@@ -561,9 +561,9 @@ lws_compare_and_swap:
 	extru  %r26, 27, 4, %r20
 
 	/* Find lock to use, the hash is either one of 0 to
-	   15, multiplied by 16 (keep it 16-byte aligned)
+	   15, multiplied by L1_CACHE_BYTES (keep it L1 cache aligned)
 	   and add to the lock table offset. */
-	shlw	%r20, 4, %r20
+	shlw	%r20, L1_CACHE_SHIFT, %r20
 	add	%r20, %r28, %r20
 
 # if ENABLE_LWS_DEBUG

@@ -751,9 +751,9 @@ cas2_lock_start:
 	extru  %r26, 27, 4, %r20
 
 	/* Find lock to use, the hash is either one of 0 to
-	   15, multiplied by 16 (keep it 16-byte aligned)
+	   15, multiplied by L1_CACHE_BYTES (keep it L1 cache aligned)
 	   and add to the lock table offset. */
-	shlw	%r20, 4, %r20
+	shlw	%r20, L1_CACHE_SHIFT, %r20
 	add	%r20, %r28, %r20
 
 	rsm	PSW_SM_I, %r0			/* Disable interrupts */

@@ -931,11 +931,9 @@ END(sys_call_table64)
 ENTRY(lws_lock_start)

arch/parisc/kernel/syscall.S END(sys_call_table64) · line 937

Quoted 10 lines

arch/parisc/kernel/syscall.S +2 -4

 	/* lws locks */
 	.rept 16
-	/* Keep locks aligned at 16-bytes */
+	/* Keep locks aligned to L1_CACHE_BYTES */
 	.word 1
-	.word 0 
-	.word 0
-	.word 0
+	.align	L1_CACHE_BYTES
 	.endr

I think this alignment/increase of each array entry to size of L1_CACHE_BYTES should be limited to the SMP case only... For UP 16 bytes would be ok.

Helge

Quoted 26 lines

arch/parisc/kernel/syscall.S +0 -0

 END(lws_lock_start)
 	.previous

arch/parisc/include/asm/futex.h +2 -2

index 49df148..47b075c 100644
@@ -15,7 +15,7 @@ static inline void
 _futex_spin_lock_irqsave(u32 __user *uaddr, unsigned long int *flags)
 {
 	extern u32 lws_lock_start[];
-	long index = ((long)uaddr & 0xf0) >> 2;
+	long index = (((long)uaddr & 0xf0) >> 4) << (L1_CACHE_SHIFT-2);
 	arch_spinlock_t *s = (arch_spinlock_t *)&lws_lock_start[index];
 	local_irq_save(*flags);
 	arch_spin_lock(s);

@@ -25,7 +25,7 @@ static inline void
 _futex_spin_unlock_irqrestore(u32 __user *uaddr, unsigned long int *flags)
 {
 	extern u32 lws_lock_start[];
-	long index = ((long)uaddr & 0xf0) >> 2;
+	long index = (((long)uaddr & 0xf0) >> 4) << (L1_CACHE_SHIFT-2);
 	arch_spinlock_t *s = (arch_spinlock_t *)&lws_lock_start[index];
 	arch_spin_unlock(s);
 	local_irq_restore(*flags);

Helge Deller

author maintainer

2015-09-07 20:51 UTC #4 lore raw

* Helge Deller <[email protected]>:

Quoted 9 lines · Helge Deller

Quoted 8 lines
parisc: Align locks for LWS syscalls to L1 cache size (v2)
Align the locks for the Light-weight-syscall (LWS) which are used for atomic userspace operations (e.g. gcc atomic builtins) on L1 cache boundaries. This should speed up LWS calls on PA20 systems.
Reported-by John David Anglin <[email protected]>
Signed-off-by Helge Deller <[email protected]>

Updated patch (v2):
- using 64 LWS locks (instead of 16)
- LWS lock index is calculated by offset of u32 type, because futexes operate on u32 types (before based on 16 bytes)
- LWS locks aligned to 16byte on UP and to L1 cache size on SMP (to avoid different threads/processes locking each other on futexes at different addresses

Signed-off-by Helge Deller <[email protected]>

arch/parisc/include/asm/cache.h +15 -0

index 47f11c7..bb3d952 100644
@@ -22,6 +22,21 @@
 #define L1_CACHE_SHIFT 5
 #endif
 
+
+/* Number of Light-weight-syscall (LWS) spinlocks */
+#define LWS_NUM_LOCK_BITS	6
+#define LWS_NUM_LOCKS		(1 << LWS_NUM_LOCK_BITS)
+
+/* Number of bits for alignment of LWS locks.
+ * Needs to be at least 4 (=16 bytes) for safe operation of LDCW.  For SMP
+ * align locks on L1 cache size. */
+#ifdef CONFIG_SMP
+# define LWS_LOCK_ALIGN_BITS	L1_CACHE_SHIFT
+#else
+# define LWS_LOCK_ALIGN_BITS	4
+#endif
+
+
 #ifndef __ASSEMBLY__
 
 #define SMP_CACHE_BYTES L1_CACHE_BYTES

arch/parisc/include/asm/futex.h +13 -8

index 49df148..b79e469 100644
@@ -7,16 +7,23 @@
 #include <linux/uaccess.h>
 #include <asm/atomic.h>
 #include <asm/errno.h>
+#include <asm/cache.h>
 
-/* The following has to match the LWS code in syscall.S.  We have
-   sixteen four-word locks. */
+/* The following has to match the LWS code in syscall.S. */
+static inline arch_spinlock_t *
+_lws_spinlockptr(u32 __user *uaddr)
+{
+	extern u8 lws_lock_start[]; /* in arch/parisc/kernel/syscall.S */
+	/* futexes operates on int values */
+	long index = (((unsigned long)uaddr >> 2) & (LWS_NUM_LOCKS-1));
+	index <<= LWS_LOCK_ALIGN_BITS;	/* multiply by alignment of the locks */
+	return (arch_spinlock_t *) &lws_lock_start[index];
+}
 
 static inline void
 _futex_spin_lock_irqsave(u32 __user *uaddr, unsigned long int *flags)
 {
-	extern u32 lws_lock_start[];
-	long index = ((long)uaddr & 0xf0) >> 2;
-	arch_spinlock_t *s = (arch_spinlock_t *)&lws_lock_start[index];
+	arch_spinlock_t *s = _lws_spinlockptr(uaddr);
 	local_irq_save(*flags);
 	arch_spin_lock(s);
 }

@@ -24,9 +31,7 @@ _futex_spin_lock_irqsave(u32 __user *uaddr, unsigned long int *flags)
 static inline void
 _futex_spin_unlock_irqrestore(u32 __user *uaddr, unsigned long int *flags)
 {
-	extern u32 lws_lock_start[];
-	long index = ((long)uaddr & 0xf0) >> 2;
-	arch_spinlock_t *s = (arch_spinlock_t *)&lws_lock_start[index];
+	arch_spinlock_t *s = _lws_spinlockptr(uaddr);
 	arch_spin_unlock(s);
 	local_irq_restore(*flags);
 }

arch/parisc/kernel/syscall.S +11 ~~-17~~

index 7ef22e3..fb0dd94 100644
@@ -557,13 +557,11 @@ lws_compare_and_swap:
 	ldil	L%lws_lock_start, %r20
 	ldo	R%lws_lock_start(%r20), %r28
 
-	/* Extract four bits from r26 and hash lock (Bits 4-7) */
-	extru  %r26, 27, 4, %r20
+	/* Extract lws lock entry from r26 */
+	extru  %r26, (31-2), LWS_NUM_LOCK_BITS, %r20
 
-	/* Find lock to use, the hash is either one of 0 to
-	   15, multiplied by 16 (keep it 16-byte aligned)
-	   and add to the lock table offset. */
-	shlw	%r20, 4, %r20
+	/* Find hash lock to use */
+	shlw	%r20, LWS_LOCK_ALIGN_BITS, %r20
 	add	%r20, %r28, %r20
 
 # if ENABLE_LWS_DEBUG

@@ -747,13 +745,11 @@ cas2_lock_start:
 	ldil	L%lws_lock_start, %r20
 	ldo	R%lws_lock_start(%r20), %r28
 
-	/* Extract four bits from r26 and hash lock (Bits 4-7) */
-	extru  %r26, 27, 4, %r20
+	/* Extract lws lock entry from r26 */
+	extru  %r26, (31-2), LWS_NUM_LOCK_BITS, %r20
 
-	/* Find lock to use, the hash is either one of 0 to
-	   15, multiplied by 16 (keep it 16-byte aligned)
-	   and add to the lock table offset. */
-	shlw	%r20, 4, %r20
+	/* Find hash lock to use */
+	shlw	%r20, LWS_LOCK_ALIGN_BITS, %r20
 	add	%r20, %r28, %r20
 
 	rsm	PSW_SM_I, %r0			/* Disable interrupts */

@@ -930,12 +926,10 @@ END(sys_call_table64)
 	.align	L1_CACHE_BYTES
 ENTRY(lws_lock_start)
 	/* lws locks */
-	.rept 16
-	/* Keep locks aligned at 16-bytes */
+	.rept LWS_NUM_LOCKS
+	/* Keep locks at least 16-byte aligned */
 	.word 1
-	.word 0 
-	.word 0
-	.word 0
+	.align (1 << LWS_LOCK_ALIGN_BITS)
 	.endr
 END(lws_lock_start)
 	.previous

James Bottomley

maintainer

2015-09-02 21:32 UTC #5 lore raw

In reply to Helge Deller

Quoted 3 lines · Helge Deller

Align the locks for the Light weight syscall (LWS) which is used for atomic userspace operations (e.g. gcc atomic builtins) on L1 cache boundaries. This should speed up LWS calls on PA20 systems.

Is there any evidence for this? The architectural requirement for ldcw on which all this is based is pegged at 16 bytes. This implies that the burst width on PA88/89 may indeed be 128 bytes, but the coherence width for operations may still be 16 bytes. If that speculation is true, there's no speed at all gained by aligning ldcw to 128 bytes and all you do is waste space.

James

Helge Deller

author maintainer

2015-09-02 22:18 UTC #6 lore raw

Quoted 11 lines · James Bottomley

Quoted 3 lines · Helge Deller
Align the locks for the Light weight syscall (LWS) which is used for atomic userspace operations (e.g. gcc atomic builtins) on L1 cache boundaries. This should speed up LWS calls on PA20 systems.
Is there any evidence for this? The architectural requirement for ldcw on which all this is based is pegged at 16 bytes. This implies that the burst width on PA88/89 may indeed be 128 bytes, but the coherence width for operations may still be 16 bytes. If that speculation is true, there's no speed at all gained by aligning ldcw to 128 bytes and all you do is waste space.

Sure, we'll have to measure timings here...

Helge