Skip to contentSkip to conversation

MIPS: use the generic atomic64 operations for perf counter support

PATCH NAK Deng-Cheng Zhu 3 participants 4 messages Apr 15, 2010 – Apr 21, 2010

Subsystems (2)
MIPS Atomic infrastructure
lore
Share & tools
lore

Deng-Cheng Zhu

author
#0 lore raw

PATCH 1/3 MIPS: use the generic atomic64 operations for perf counter support

Currently we take the generic spinlock'ed atomic64 implementation from the lib. The atomic64 types and related functions are needed for the Linux performance counter subsystem.

Signed-off-by Deng-Cheng Zhu <[email protected]>
---
 arch/mips/Kconfig              |    1 +
 arch/mips/include/asm/atomic.h |    4 ++++
 2 files changed, 5 insertions(+), 0 deletions(-)
arch/mips/Kconfig +1 -0
index 29e8692..7161751 100644
@@ -4,6 +4,7 @@ config MIPS
 	select HAVE_GENERIC_DMA_COHERENT
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select GENERIC_ATOMIC64
 	select HAVE_ARCH_KGDB
 	select HAVE_FUNCTION_TRACER
 	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
arch/mips/include/asm/atomic.h +4 -0
index 519197e..b0a932e 100644
@@ -21,6 +21,10 @@
 #include <asm/war.h>
 #include <asm/system.h>
 
+#ifdef CONFIG_GENERIC_ATOMIC64
+#include <asm-generic/atomic64.h>
+#endif
+
 #define ATOMIC_INIT(i)    { (i) }
 
 /*
Signature · 1 line
1.7.0.4
NAK
#1 lore raw
Quoted 5 lines · Deng-Cheng Zhu

Currently we take the generic spinlock'ed atomic64 implementation from the lib. The atomic64 types and related functions are needed for the Linux performance counter subsystem.

Signed-off-by Deng-Cheng Zhu<[email protected]>

NAK.

Quoted 22 lines
---
  arch/mips/Kconfig              |    1 +
  arch/mips/include/asm/atomic.h |    4 ++++
  2 files changed, 5 insertions(+), 0 deletions(-)
arch/mips/Kconfig +1 -0
index 29e8692..7161751 100644
@@ -4,6 +4,7 @@ config MIPS
  	select HAVE_GENERIC_DMA_COHERENT
  	select HAVE_IDE
  	select HAVE_OPROFILE
+	select GENERIC_ATOMIC64
  	select HAVE_ARCH_KGDB
  	select HAVE_FUNCTION_TRACER
  	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
arch/mips/include/asm/atomic.h +0 -0
index 519197e..b0a932e 100644

arch/mips/include/asm/atomic.h line 30

Quoted 10 lines
arch/mips/include/asm/atomic.h +4 -0
@@ -21,6 +21,10 @@
  #include<asm/war.h>
  #include<asm/system.h>
 
+#ifdef CONFIG_GENERIC_ATOMIC64
+#include<asm-generic/atomic64.h>
+#endif
+
  #define ATOMIC_INIT(i)    { (i) }
 
  /*

This is incorrect. For 64-bit kernels, we already have all the 64-bit atomics implemented. This will break 64-bit kernels.

David Daney

#2 lore raw
Quoted 10 lines · David Daney
Quoted 1 line
Signed-off-by Deng-Cheng Zhu<[email protected]>

NAK.

Quoted 6 lines
(quoted diff) +4 -0
+#ifdef CONFIG_GENERIC_ATOMIC64
+#include<asm-generic/atomic64.h>
+#endif
+
 #define ATOMIC_INIT(i)    { (i) }
Quoted 4 lines
Quoted 1 line

/*

This is incorrect. For 64-bit kernels, we already have all the 64-bit atomics implemented. This will break 64-bit kernels.

Here's an alternative patch. Much larger but should take make the atomic64 functions fully usable on 32-bit kernels. Some additional optimizations for 64-bit hardware running 32-bit kernels would be possible but why bother.

Ralf

MIPS: Make atomic64 functions usable on 32-bit kernels.

Signed-off-by Ralf Baechle <[email protected]>
arch/mips/include/asm/atomic.h +63 -26
index 519197e..5f00bbc 100644
@@ -15,11 +15,13 @@
 #define _ASM_ATOMIC_H
 
 #include <linux/irqflags.h>
+#include <linux/spinlock.h>
 #include <linux/types.h>
 #include <asm/barrier.h>
+#include <asm/cmpxchg.h>
 #include <asm/cpu-features.h>
+#include <asm/hashed-locks.h>
 #include <asm/war.h>
-#include <asm/system.h>
 
 #define ATOMIC_INIT(i)    { (i) }
 

@@ -401,7 +403,9 @@ static __inline__ int atomic_add_unless(atomic_t *v, int a, int u)
  */
 #define atomic_add_negative(i, v) (atomic_add_return(i, (v)) < 0)
 
-#ifdef CONFIG_64BIT
+typedef struct {
+	long long counter;
+} atomic64_t;
 
 #define ATOMIC64_INIT(i)    { (i) }
 

@@ -410,14 +414,44 @@ static __inline__ int atomic_add_unless(atomic_t *v, int a, int u)
  * @v: pointer of type atomic64_t
  *
  */
-#define atomic64_read(v)	((v)->counter)
+static long long __inline__ atomic64_read(const atomic64_t *v)
+{
+	unsigned long flags;
+	raw_spinlock_t *lock;
+	long long val;
+
+	if (cpu_has_64bit_gp_regs)	/* 64-bit regs imply 64-bit ld / sd  */
+		return v->counter;
+
+	lock = atomic_lock_addr(v);
+	raw_spin_lock_irqsave(lock, flags);
+	val = v->counter;
+	raw_spin_unlock_irqrestore(lock, flags);
+
+	return val;
+}
 
 /*
  * atomic64_set - set atomic variable
  * @v: pointer of type atomic64_t
  * @i: required value
  */
-#define atomic64_set(v, i)	((v)->counter = (i))
+static void __inline__ atomic64_set(atomic64_t *v, long long i)
+{
+	unsigned long flags;
+	raw_spinlock_t *lock;
+
+	if (cpu_has_64bit_gp_regs) {	/* 64-bit regs imply 64-bit ld / sd  */
+		v->counter = i;
+
+		return;
+	}
+
+	lock = atomic_lock_addr(v);
+	raw_spin_lock_irqsave(lock, flags);
+	v->counter = i;
+	raw_spin_unlock_irqrestore(lock, flags);
+}
 
 /*
  * atomic64_add - add integer to atomic variable

@@ -428,7 +462,7 @@ static __inline__ int atomic_add_unless(atomic_t *v, int a, int u)
  */
 static __inline__ void atomic64_add(long i, atomic64_t * v)
 {
-	if (kernel_uses_llsc && R10000_LLSC_WAR) {
+	if (kernel_uses_llsc && cpu_has_64bit_gp_regs && R10000_LLSC_WAR) {
 		long temp;
 
 		__asm__ __volatile__(

@@ -440,7 +474,7 @@ static __inline__ void atomic64_add(long i, atomic64_t * v)
 		"	.set	mips0					\n"
 		: "=&r" (temp), "=m" (v->counter)
 		: "Ir" (i), "m" (v->counter));
-	} else if (kernel_uses_llsc) {
+	} else if (kernel_uses_llsc && cpu_has_64bit_gp_regs) {
 		long temp;
 
 		__asm__ __volatile__(

@@ -456,11 +490,12 @@ static __inline__ void atomic64_add(long i, atomic64_t * v)
 		: "=&r" (temp), "=m" (v->counter)
 		: "Ir" (i), "m" (v->counter));
 	} else {
+		raw_spinlock_t *lock = atomic_lock_addr(v);
 		unsigned long flags;
 
-		raw_local_irq_save(flags);
+		raw_spin_lock_irqsave(lock, flags);
 		v->counter += i;
-		raw_local_irq_restore(flags);
+		raw_spin_unlock_irqrestore(lock, flags);
 	}
 }
 

@@ -473,7 +508,7 @@ static __inline__ void atomic64_add(long i, atomic64_t * v)
  */
 static __inline__ void atomic64_sub(long i, atomic64_t * v)
 {
-	if (kernel_uses_llsc && R10000_LLSC_WAR) {
+	if (kernel_uses_llsc && cpu_has_64bit_gp_regs && R10000_LLSC_WAR) {
 		long temp;
 
 		__asm__ __volatile__(

@@ -485,7 +520,7 @@ static __inline__ void atomic64_sub(long i, atomic64_t * v)
 		"	.set	mips0					\n"
 		: "=&r" (temp), "=m" (v->counter)
 		: "Ir" (i), "m" (v->counter));
-	} else if (kernel_uses_llsc) {
+	} else if (kernel_uses_llsc && cpu_has_64bit_gp_regs) {
 		long temp;
 
 		__asm__ __volatile__(

@@ -501,11 +536,12 @@ static __inline__ void atomic64_sub(long i, atomic64_t * v)
 		: "=&r" (temp), "=m" (v->counter)
 		: "Ir" (i), "m" (v->counter));
 	} else {
+		raw_spinlock_t *lock = atomic_lock_addr(v);
 		unsigned long flags;
 
-		raw_local_irq_save(flags);
+		raw_spin_lock_irqsave(lock, flags);
 		v->counter -= i;
-		raw_local_irq_restore(flags);
+		raw_spin_unlock_irqrestore(lock, flags);
 	}
 }
 

@@ -518,7 +554,7 @@ static __inline__ long atomic64_add_return(long i, atomic64_t * v)
 
 	smp_mb__before_llsc();
 
-	if (kernel_uses_llsc && R10000_LLSC_WAR) {
+	if (kernel_uses_llsc &&  cpu_has_64bit_gp_regs && R10000_LLSC_WAR) {
 		long temp;
 
 		__asm__ __volatile__(

@@ -532,7 +568,7 @@ static __inline__ long atomic64_add_return(long i, atomic64_t * v)
 		: "=&r" (result), "=&r" (temp), "=m" (v->counter)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
-	} else if (kernel_uses_llsc) {
+	} else if (kernel_uses_llsc && cpu_has_64bit_gp_regs) {
 		long temp;
 
 		__asm__ __volatile__(

@@ -550,13 +586,14 @@ static __inline__ long atomic64_add_return(long i, atomic64_t * v)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
 	} else {
+		raw_spinlock_t *lock = atomic_lock_addr(v);
 		unsigned long flags;
 
-		raw_local_irq_save(flags);
+		raw_spin_lock_irqsave(lock, flags);
 		result = v->counter;
 		result += i;
 		v->counter = result;
-		raw_local_irq_restore(flags);
+		raw_spin_unlock_irqrestore(lock, flags);
 	}
 
 	smp_llsc_mb();

@@ -570,7 +607,7 @@ static __inline__ long atomic64_sub_return(long i, atomic64_t * v)
 
 	smp_mb__before_llsc();
 
-	if (kernel_uses_llsc && R10000_LLSC_WAR) {
+	if (kernel_uses_llsc && cpu_has_64bit_gp_regs && R10000_LLSC_WAR) {
 		long temp;
 
 		__asm__ __volatile__(

@@ -584,7 +621,7 @@ static __inline__ long atomic64_sub_return(long i, atomic64_t * v)
 		: "=&r" (result), "=&r" (temp), "=m" (v->counter)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
-	} else if (kernel_uses_llsc) {
+	} else if (kernel_uses_llsc && cpu_has_64bit_gp_regs) {
 		long temp;
 
 		__asm__ __volatile__(

@@ -602,13 +639,14 @@ static __inline__ long atomic64_sub_return(long i, atomic64_t * v)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
 	} else {
+		raw_spinlock_t *lock = atomic_lock_addr(v);
 		unsigned long flags;
 
-		raw_local_irq_save(flags);
+		raw_spin_lock_irqsave(lock, flags);
 		result = v->counter;
 		result -= i;
 		v->counter = result;
-		raw_local_irq_restore(flags);
+		raw_spin_unlock_irqrestore(lock, flags);
 	}
 
 	smp_llsc_mb();

@@ -630,7 +668,7 @@ static __inline__ long atomic64_sub_if_positive(long i, atomic64_t * v)
 
 	smp_mb__before_llsc();
 
-	if (kernel_uses_llsc && R10000_LLSC_WAR) {
+	if (kernel_uses_llsc && cpu_has_64bit_gp_regs && R10000_LLSC_WAR) {
 		long temp;
 
 		__asm__ __volatile__(

@@ -648,7 +686,7 @@ static __inline__ long atomic64_sub_if_positive(long i, atomic64_t * v)
 		: "=&r" (result), "=&r" (temp), "=m" (v->counter)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
-	} else if (kernel_uses_llsc) {
+	} else if (kernel_uses_llsc && cpu_has_64bit_gp_regs) {
 		long temp;
 
 		__asm__ __volatile__(

@@ -670,14 +708,15 @@ static __inline__ long atomic64_sub_if_positive(long i, atomic64_t * v)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
 	} else {
+		raw_spinlock_t *lock = atomic_lock_addr(v);
 		unsigned long flags;
 
-		raw_local_irq_save(flags);
+		raw_spin_lock_irqsave(lock, flags);
 		result = v->counter;
 		result -= i;
 		if (result >= 0)
 			v->counter = result;
-		raw_local_irq_restore(flags);
+		raw_spin_unlock_irqrestore(lock, flags);
 	}
 
 	smp_llsc_mb();

@@ -782,8 +821,6 @@ static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)
  */
 #define atomic64_add_negative(i, v) (atomic64_add_return(i, (v)) < 0)
 
-#endif /* CONFIG_64BIT */
-
 /*
  * atomic*_return operations are serializing but not the non-*_return
  * versions.
arch/mips/include/asm/cmpxchg.h +30 -18

86 lines not on this page · raw message

arch/mips/include/asm/hashed-locks.h new file +38 -0
new file mode 100644
index 0000000..6d6321c
@@ -0,0 +1,38 @@
+/*
+ * Hashed locks for when we cannot or do not want to use LL/SC for the
+ * implementation of atomic operations in atomic.h, cmpxchg.h etc.
+ */
+#ifndef __ASM_HASHED_LOCKS_H
+#define __ASM_HASHED_LOCKS_H
+
+#include <linux/cache.h>
+#include <linux/spinlock_types.h>
+
+/*
+ * We use a hashed array of spinlocks to provide exclusive access
+ * to each atomic64_t variable.  Since this is expected to used on
+ * systems with small numbers of CPUs (<= 4 or so), we use a
+ * relatively small array of 16 spinlocks to avoid wasting too much
+ * memory on the spinlock array.
+ */
+#define NR_LOCKS	16
+
+/*
+ * Ensure each lock is in a separate cacheline.
+ */
+extern union atomic64_lock_union {
+	raw_spinlock_t lock;
+	char pad[L1_CACHE_BYTES];
+} __atomic_lock_array[NR_LOCKS];
+
+static inline raw_spinlock_t *atomic_lock_addr(volatile const void *v)
+{
+	unsigned long addr = (unsigned long) v;
+
+	addr >>= L1_CACHE_SHIFT;
+	addr ^= (addr >> 8) ^ (addr >> 16);
+
+	return &__atomic_lock_array[addr & (NR_LOCKS - 1)].lock;
+}
+
+#endif /* __ASM_HASHED_LOCKS_H */
arch/mips/lib/Makefile +2 -2
index 2adead5..c138aad 100644
@@ -2,8 +2,8 @@
 # Makefile for MIPS-specific library files..
 #
 
-lib-y	+= csum_partial.o delay.o memcpy.o memcpy-inatomic.o memset.o \
-	   strlen_user.o strncpy_user.o strnlen_user.o uncached.o
+lib-y	+= csum_partial.o delay.o hashed-locks.o memcpy.o memcpy-inatomic.o \
+	   memset.o strlen_user.o strncpy_user.o strnlen_user.o uncached.o
 
 obj-y			+= iomap.o
 obj-$(CONFIG_PCI)	+= iomap-pci.o
arch/mips/lib/hashed-locks.c new file +9 -0
new file mode 100644
index 0000000..2fbb68a
@@ -0,0 +1,9 @@
+#include <linux/cache.h>
+#include <linux/module.h>
+#include <asm/atomic.h>
+
+union atomic64_lock_union __atomic_lock_array[NR_LOCKS] = {
+	[0 ... NR_LOCKS - 1] .lock	= __ARCH_SPIN_LOCK_UNLOCKED,
+} __cacheline_aligned_in_smp;
+
+EXPORT_SYMBOL(__atomic_lock_array);
#3 lore raw

On 04/21/2010 10:19 AM, Ralf Baechle wrote:
[...]

arch/mips/include/asm/atomic.h static __inline__ int atomic_add_unless(atomic_t *v, int a, int u) · line 408

Quoted 6 lines
arch/mips/include/asm/atomic.h +3 -1
-#ifdef CONFIG_64BIT
+typedef struct {
+	long long counter;
+} atomic64_t;

How does this not conflict with the definition in linux/types.h for a 64-bit kernel?

Quoted 6 lines
  #define ATOMIC64_INIT(i)    { (i) }
arch/mips/include/asm/atomic.h +0 -0
@@ -410,14 +414,44 @@ static __inline__ int atomic_add_unless(atomic_t *v, int a, int u)
   * @v: pointer of type atomic64_t
   *
   */

arch/mips/include/asm/atomic.h static __inline__ int atomic_add_unless(atomic_t *v, int a, int u) · line 425

Quoted 10 lines
arch/mips/include/asm/atomic.h +9 -1
-#define atomic64_read(v)	((v)->counter)
+static long long __inline__ atomic64_read(const atomic64_t *v)
+{
+	unsigned long flags;
+	raw_spinlock_t *lock;
+	long long val;
+
+	if (cpu_has_64bit_gp_regs)	/* 64-bit regs imply 64-bit ld / sd  */
+		return v->counter;
+

How is this atomic for the o32 ABI? counter is now not volatile, in o32, u64 values are often split between two registers. There is nothing to guarantee that the compiler will use LD.

David Daney