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

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

[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 <dengcheng.zhu@gmail.com>
---
 arch/mips/Kconfig              |    1 +
 arch/mips/include/asm/atomic.h |    4 ++++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 29e8692..7161751 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -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
diff --git a/arch/mips/include/asm/atomic.h b/arch/mips/include/asm/atomic.h
index 519197e..b0a932e 100644
--- a/arch/mips/include/asm/atomic.h
+++ b/arch/mips/include/asm/atomic.h
@@ -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) }
 
 /*
-- 
1.7.0.4

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

[David Daney]

On 04/15/2010 09:38 AM, Deng-Cheng Zhu wrote:
> 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<dengcheng.zhu@gmail.com>

NAK.

> ---
>   arch/mips/Kconfig              |    1 +
>   arch/mips/include/asm/atomic.h |    4 ++++
>   2 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 29e8692..7161751 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -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
> diff --git a/arch/mips/include/asm/atomic.h b/arch/mips/include/asm/atomic.h
> index 519197e..b0a932e 100644
> --- a/arch/mips/include/asm/atomic.h
> +++ b/arch/mips/include/asm/atomic.h
> @@ -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

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

[Ralf Baechle]

On Tue, Apr 20, 2010 at 09:25:51AM -0700, David Daney wrote:

> >Signed-off-by: Deng-Cheng Zhu<dengcheng.zhu@gmail.com>
> 
> NAK.

> >+#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.

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 <ralf@linux-mips.org>

diff --git a/arch/mips/include/asm/atomic.h b/arch/mips/include/asm/atomic.h
index 519197e..5f00bbc 100644
--- a/arch/mips/include/asm/atomic.h
+++ b/arch/mips/include/asm/atomic.h
@@ -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.
diff --git a/arch/mips/include/asm/cmpxchg.h b/arch/mips/include/asm/cmpxchg.h
index 2d28017..7ed77bd 100644
--- a/arch/mips/include/asm/cmpxchg.h
+++ b/arch/mips/include/asm/cmpxchg.h
@@ -9,6 +9,8 @@
 #define __ASM_CMPXCHG_H
 
 #include <linux/irqflags.h>
+#include <linux/spinlock_types.h>
+#include <asm/hashed-locks.h>
 
 #define __HAVE_ARCH_CMPXCHG 1
 
@@ -53,16 +55,8 @@
 		: "=&r" (__ret), "=R" (*m)				\
 		: "R" (*m), "Jr" (old), "Jr" (new)			\
 		: "memory");						\
-	} else {							\
-		unsigned long __flags;					\
-									\
-		raw_local_irq_save(__flags);				\
-		__ret = *m;						\
-		if (__ret == old)					\
-			*m = new;					\
-		raw_local_irq_restore(__flags);				\
-	}								\
-									\
+	} else								\
+		BUG();							\
 	__ret;								\
 })
 
@@ -78,19 +72,42 @@ extern void __cmpxchg_called_with_bad_pointer(void);
 	__typeof__(*(ptr)) __old = (old);				\
 	__typeof__(*(ptr)) __new = (new);				\
 	__typeof__(*(ptr)) __res = 0;					\
+	unsigned long __flags;						\
+	raw_spinlock_t *lock;						\
 									\
 	pre_barrier;							\
 									\
 	switch (sizeof(*(__ptr))) {					\
 	case 4:								\
-		__res = __cmpxchg_asm("ll", "sc", __ptr, __old, __new);	\
+		if (kernel_uses_llsc) {					\
+			__res = __cmpxchg_asm("ll", "sc", __ptr, __old, \
+					       __new);			\
+			break;						\
+		}							\
+									\
+		lock = atomic_lock_addr(__ptr);				\
+		raw_spin_lock_irqsave(lock, __flags);			\
+		__res = *__ptr;						\
+		if (__res == __old)					\
+			*__ptr = __new;					\
+		raw_spin_unlock_irqrestore(lock, __flags);		\
 		break;							\
+									\
 	case 8:								\
-		if (sizeof(long) == 8) {				\
+		if (kernel_uses_llsc && cpu_has_64bit_gp_regs) {	\
 			__res = __cmpxchg_asm("lld", "scd", __ptr,	\
-					   __old, __new);		\
+					      __old, __new);		\
 			break;						\
 		}							\
+									\
+		lock = atomic_lock_addr(__ptr);				\
+		raw_spin_lock_irqsave(lock, __flags);			\
+		__res = *__ptr;						\
+		if (__res == __old)					\
+			*__ptr = __new;					\
+		raw_spin_unlock_irqrestore(lock, __flags);		\
+		break;							\
+									\
 	default:							\
 		__cmpxchg_called_with_bad_pointer();			\
 		break;							\
@@ -110,15 +127,10 @@ extern void __cmpxchg_called_with_bad_pointer(void);
 	cmpxchg((ptr), (o), (n));					\
   })
 
-#ifdef CONFIG_64BIT
 #define cmpxchg64_local(ptr, o, n)					\
   ({									\
 	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
 	cmpxchg_local((ptr), (o), (n));					\
   })
-#else
-#include <asm-generic/cmpxchg-local.h>
-#define cmpxchg64_local(ptr, o, n) __cmpxchg64_local_generic((ptr), (o), (n))
-#endif
 
 #endif /* __ASM_CMPXCHG_H */
diff --git a/arch/mips/include/asm/hashed-locks.h b/arch/mips/include/asm/hashed-locks.h
new file mode 100644
index 0000000..6d6321c
--- /dev/null
+++ b/arch/mips/include/asm/hashed-locks.h
@@ -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 */
diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
index 2adead5..c138aad 100644
--- a/arch/mips/lib/Makefile
+++ b/arch/mips/lib/Makefile
@@ -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
diff --git a/arch/mips/lib/hashed-locks.c b/arch/mips/lib/hashed-locks.c
new file mode 100644
index 0000000..2fbb68a
--- /dev/null
+++ b/arch/mips/lib/hashed-locks.c
@@ -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);

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

[David Daney]

On 04/21/2010 10:19 AM, Ralf Baechle wrote:
[...]
>
> -#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?


>   #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;
> +

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