frv cmpxchg_local compile error

frv cmpxchg_local compile error

[Adrian Bunk]

Commit 14e0cb3c60b89c4a2512852ffc18601c72314a0f broke frv compilation:

<--  snip  -->

...
  CC      arch/frv/kernel/asm-offsets.s
In file included from include/asm/system.h:271,
                 from include/asm/bitops.h:19,
                 from include/linux/bitops.h:17,
                 from include/linux/kernel.h:15,
                 from include/linux/sched.h:52,
                 from arch/frv/kernel/asm-offsets.c:7:
include/asm-generic/cmpxchg-local.h: In function '__cmpxchg_local_generic':
include/asm-generic/cmpxchg-local.h:23: error: implicit declaration of function 'typecheck'
include/asm-generic/cmpxchg-local.h:23: error: expected expression before 'unsigned'
include/asm-generic/cmpxchg-local.h:23: error: expected expression before 'unsigned'
include/asm-generic/cmpxchg-local.h:44: error: expected expression before 'unsigned'
include/asm-generic/cmpxchg-local.h: In function '__cmpxchg64_local_generic':
include/asm-generic/cmpxchg-local.h:57: error: expected expression before 'unsigned'
include/asm-generic/cmpxchg-local.h:57: error: expected expression before 'unsigned'
include/asm-generic/cmpxchg-local.h:61: error: expected expression before 'unsigned'
In file included from include/asm/bitops.h:19,
                 from include/linux/bitops.h:17,
                 from include/linux/kernel.h:15,
                 from include/linux/sched.h:52,
                 from arch/frv/kernel/asm-offsets.c:7:
include/asm/system.h: In function '__cmpxchg_local':
include/asm/system.h:279: error: variable or field '__xg_orig' declared void
include/asm/system.h:279: error: variable or field '__xg_test' declared void
include/asm/system.h:279: error: variable or field '__xg_new' declared void
include/asm/system.h:279: error: void value not ignored as it ought to be
make[1]: *** [arch/frv/kernel/asm-offsets.s] Error 1

<--  snip  -->


An architecture specific patch that breaks the one architecture it 
touches at the first file being compiled is even for kernel standards 
unusually bad...


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: frv cmpxchg_local compile error

[Mathieu Desnoyers]

* Adrian Bunk (bunk@kernel.org) wrote:
> Commit 14e0cb3c60b89c4a2512852ffc18601c72314a0f broke frv compilation:
> 
> <--  snip  -->
> 
> ...
>   CC      arch/frv/kernel/asm-offsets.s
> In file included from include/asm/system.h:271,
>                  from include/asm/bitops.h:19,
>                  from include/linux/bitops.h:17,
>                  from include/linux/kernel.h:15,
>                  from include/linux/sched.h:52,
>                  from arch/frv/kernel/asm-offsets.c:7:
> include/asm-generic/cmpxchg-local.h: In function '__cmpxchg_local_generic':
> include/asm-generic/cmpxchg-local.h:23: error: implicit declaration of function 'typecheck'


Isn't typecheck() supposed to be defined in kernel.h which is included
_everywhere_ ?

Hrm, I see, dependency from kernel.h on bitops.h on asm/bitops.h on
asm/system.h.



> include/asm-generic/cmpxchg-local.h:23: error: expected expression before 'unsigned'
> include/asm-generic/cmpxchg-local.h:23: error: expected expression before 'unsigned'
> include/asm-generic/cmpxchg-local.h:44: error: expected expression before 'unsigned'
> include/asm-generic/cmpxchg-local.h: In function '__cmpxchg64_local_generic':
> include/asm-generic/cmpxchg-local.h:57: error: expected expression before 'unsigned'
> include/asm-generic/cmpxchg-local.h:57: error: expected expression before 'unsigned'
> include/asm-generic/cmpxchg-local.h:61: error: expected expression before 'unsigned'
> In file included from include/asm/bitops.h:19,
>                  from include/linux/bitops.h:17,
>                  from include/linux/kernel.h:15,
>                  from include/linux/sched.h:52,
>                  from arch/frv/kernel/asm-offsets.c:7:
> include/asm/system.h: In function '__cmpxchg_local':
> include/asm/system.h:279: error: variable or field '__xg_orig' declared void
> include/asm/system.h:279: error: variable or field '__xg_test' declared void
> include/asm/system.h:279: error: variable or field '__xg_new' declared void
> include/asm/system.h:279: error: void value not ignored as it ought to be
> make[1]: *** [arch/frv/kernel/asm-offsets.s] Error 1
> 
> <--  snip  -->
> 
> 
> An architecture specific patch that breaks the one architecture it 
> touches at the first file being compiled is even for kernel standards 
> unusually bad...
> 

I did this cmpxchg_local patchset to support Christoph Lameter's slub
performance improvement work. It implies touching every kernel
architecture. Each architecture is touched by a different patch. And no,
I did not happen to have an FRV toolchain handy. Sorry. I'll dig into
this.

Mathieu


> 
> cu
> Adrian
> 
> -- 
> 
>        "Is there not promise of rain?" Ling Tan asked suddenly out
>         of the darkness. There had been need of rain for many days.
>        "Only a promise," Lao Er said.
>                                        Pearl S. Buck - Dragon Seed
> 

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[PATCH] Fix FRV cmpxchg_local

[Mathieu Desnoyers]

Fix the FRV cmpxchg_local by breaking the following header dependency loop :

linux/kernel.h -> linux/bitops.h -> asm-frv/bitops.h -> asm-frv/atomic.h
  -> asm-frv/system.h -> 
  asm-generic/cmpxchg_local.h -> typecheck() defined in linux/kernel.h

and

linux/kernel.h -> linux/bitops.h -> asm-frv/bitops.h -> asm-frv/atomic.h -> 
  asm-generic/cmpxchg_local.h -> typecheck() defined in linux/kernel.h

In order to fix this :
- Move the atomic_test_and_ *_mask inlines from asm-frv/atomic.h (why are they
  there at all anyway ? They are not touching atomic_t variables!) to
  asm-frv/bitops.h.

Also fix a build issue with cmpxchg : it does not cast to (unsigned long *)
like other architectures, to deal with it in the cmpxchg_local macro.

FRV builds fine with this patch.

Thanks to Adrian Bunk <bunk@kernel.org> for spotting this bug.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Adrian Bunk <bunk@kernel.org>
---
 include/asm-frv/atomic.h |   81 ---------------------------------------------
 include/asm-frv/bitops.h |   83 +++++++++++++++++++++++++++++++++++++++++++++--
 include/asm-frv/system.h |    3 +
 3 files changed, 83 insertions(+), 84 deletions(-)

Index: linux-2.6-lttng/include/asm-frv/atomic.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-frv/atomic.h	2008-02-07 17:29:09.000000000 -0500
+++ linux-2.6-lttng/include/asm-frv/atomic.h	2008-02-07 17:32:05.000000000 -0500
@@ -125,87 +125,6 @@
 #define atomic_dec_and_test(v)		(atomic_sub_return(1, (v)) == 0)
 #define atomic_inc_and_test(v)		(atomic_add_return(1, (v)) == 0)
 
-#ifndef CONFIG_FRV_OUTOFLINE_ATOMIC_OPS
-static inline
-unsigned long atomic_test_and_ANDNOT_mask(unsigned long mask, volatile unsigned long *v)
-{
-	unsigned long old, tmp;
-
-	asm volatile(
-		"0:						\n"
-		"	orcc		gr0,gr0,gr0,icc3	\n"	/* set ICC3.Z */
-		"	ckeq		icc3,cc7		\n"
-		"	ld.p		%M0,%1			\n"	/* LD.P/ORCR are atomic */
-		"	orcr		cc7,cc7,cc3		\n"	/* set CC3 to true */
-		"	and%I3		%1,%3,%2		\n"
-		"	cst.p		%2,%M0		,cc3,#1	\n"	/* if store happens... */
-		"	corcc		gr29,gr29,gr0	,cc3,#1	\n"	/* ... clear ICC3.Z */
-		"	beq		icc3,#0,0b		\n"
-		: "+U"(*v), "=&r"(old), "=r"(tmp)
-		: "NPr"(~mask)
-		: "memory", "cc7", "cc3", "icc3"
-		);
-
-	return old;
-}
-
-static inline
-unsigned long atomic_test_and_OR_mask(unsigned long mask, volatile unsigned long *v)
-{
-	unsigned long old, tmp;
-
-	asm volatile(
-		"0:						\n"
-		"	orcc		gr0,gr0,gr0,icc3	\n"	/* set ICC3.Z */
-		"	ckeq		icc3,cc7		\n"
-		"	ld.p		%M0,%1			\n"	/* LD.P/ORCR are atomic */
-		"	orcr		cc7,cc7,cc3		\n"	/* set CC3 to true */
-		"	or%I3		%1,%3,%2		\n"
-		"	cst.p		%2,%M0		,cc3,#1	\n"	/* if store happens... */
-		"	corcc		gr29,gr29,gr0	,cc3,#1	\n"	/* ... clear ICC3.Z */
-		"	beq		icc3,#0,0b		\n"
-		: "+U"(*v), "=&r"(old), "=r"(tmp)
-		: "NPr"(mask)
-		: "memory", "cc7", "cc3", "icc3"
-		);
-
-	return old;
-}
-
-static inline
-unsigned long atomic_test_and_XOR_mask(unsigned long mask, volatile unsigned long *v)
-{
-	unsigned long old, tmp;
-
-	asm volatile(
-		"0:						\n"
-		"	orcc		gr0,gr0,gr0,icc3	\n"	/* set ICC3.Z */
-		"	ckeq		icc3,cc7		\n"
-		"	ld.p		%M0,%1			\n"	/* LD.P/ORCR are atomic */
-		"	orcr		cc7,cc7,cc3		\n"	/* set CC3 to true */
-		"	xor%I3		%1,%3,%2		\n"
-		"	cst.p		%2,%M0		,cc3,#1	\n"	/* if store happens... */
-		"	corcc		gr29,gr29,gr0	,cc3,#1	\n"	/* ... clear ICC3.Z */
-		"	beq		icc3,#0,0b		\n"
-		: "+U"(*v), "=&r"(old), "=r"(tmp)
-		: "NPr"(mask)
-		: "memory", "cc7", "cc3", "icc3"
-		);
-
-	return old;
-}
-
-#else
-
-extern unsigned long atomic_test_and_ANDNOT_mask(unsigned long mask, volatile unsigned long *v);
-extern unsigned long atomic_test_and_OR_mask(unsigned long mask, volatile unsigned long *v);
-extern unsigned long atomic_test_and_XOR_mask(unsigned long mask, volatile unsigned long *v);
-
-#endif
-
-#define atomic_clear_mask(mask, v)	atomic_test_and_ANDNOT_mask((mask), (v))
-#define atomic_set_mask(mask, v)	atomic_test_and_OR_mask((mask), (v))
-
 /*****************************************************************************/
 /*
  * exchange value with memory
Index: linux-2.6-lttng/include/asm-frv/bitops.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-frv/bitops.h	2008-02-07 17:27:48.000000000 -0500
+++ linux-2.6-lttng/include/asm-frv/bitops.h	2008-02-07 17:32:20.000000000 -0500
@@ -16,8 +16,6 @@
 
 #include <linux/compiler.h>
 #include <asm/byteorder.h>
-#include <asm/system.h>
-#include <asm/atomic.h>
 
 #ifdef __KERNEL__
 
@@ -33,6 +31,87 @@
 #define smp_mb__before_clear_bit()	barrier()
 #define smp_mb__after_clear_bit()	barrier()
 
+#ifndef CONFIG_FRV_OUTOFLINE_ATOMIC_OPS
+static inline
+unsigned long atomic_test_and_ANDNOT_mask(unsigned long mask, volatile unsigned long *v)
+{
+	unsigned long old, tmp;
+
+	asm volatile(
+		"0:						\n"
+		"	orcc		gr0,gr0,gr0,icc3	\n"	/* set ICC3.Z */
+		"	ckeq		icc3,cc7		\n"
+		"	ld.p		%M0,%1			\n"	/* LD.P/ORCR are atomic */
+		"	orcr		cc7,cc7,cc3		\n"	/* set CC3 to true */
+		"	and%I3		%1,%3,%2		\n"
+		"	cst.p		%2,%M0		,cc3,#1	\n"	/* if store happens... */
+		"	corcc		gr29,gr29,gr0	,cc3,#1	\n"	/* ... clear ICC3.Z */
+		"	beq		icc3,#0,0b		\n"
+		: "+U"(*v), "=&r"(old), "=r"(tmp)
+		: "NPr"(~mask)
+		: "memory", "cc7", "cc3", "icc3"
+		);
+
+	return old;
+}
+
+static inline
+unsigned long atomic_test_and_OR_mask(unsigned long mask, volatile unsigned long *v)
+{
+	unsigned long old, tmp;
+
+	asm volatile(
+		"0:						\n"
+		"	orcc		gr0,gr0,gr0,icc3	\n"	/* set ICC3.Z */
+		"	ckeq		icc3,cc7		\n"
+		"	ld.p		%M0,%1			\n"	/* LD.P/ORCR are atomic */
+		"	orcr		cc7,cc7,cc3		\n"	/* set CC3 to true */
+		"	or%I3		%1,%3,%2		\n"
+		"	cst.p		%2,%M0		,cc3,#1	\n"	/* if store happens... */
+		"	corcc		gr29,gr29,gr0	,cc3,#1	\n"	/* ... clear ICC3.Z */
+		"	beq		icc3,#0,0b		\n"
+		: "+U"(*v), "=&r"(old), "=r"(tmp)
+		: "NPr"(mask)
+		: "memory", "cc7", "cc3", "icc3"
+		);
+
+	return old;
+}
+
+static inline
+unsigned long atomic_test_and_XOR_mask(unsigned long mask, volatile unsigned long *v)
+{
+	unsigned long old, tmp;
+
+	asm volatile(
+		"0:						\n"
+		"	orcc		gr0,gr0,gr0,icc3	\n"	/* set ICC3.Z */
+		"	ckeq		icc3,cc7		\n"
+		"	ld.p		%M0,%1			\n"	/* LD.P/ORCR are atomic */
+		"	orcr		cc7,cc7,cc3		\n"	/* set CC3 to true */
+		"	xor%I3		%1,%3,%2		\n"
+		"	cst.p		%2,%M0		,cc3,#1	\n"	/* if store happens... */
+		"	corcc		gr29,gr29,gr0	,cc3,#1	\n"	/* ... clear ICC3.Z */
+		"	beq		icc3,#0,0b		\n"
+		: "+U"(*v), "=&r"(old), "=r"(tmp)
+		: "NPr"(mask)
+		: "memory", "cc7", "cc3", "icc3"
+		);
+
+	return old;
+}
+
+#else
+
+extern unsigned long atomic_test_and_ANDNOT_mask(unsigned long mask, volatile unsigned long *v);
+extern unsigned long atomic_test_and_OR_mask(unsigned long mask, volatile unsigned long *v);
+extern unsigned long atomic_test_and_XOR_mask(unsigned long mask, volatile unsigned long *v);
+
+#endif
+
+#define atomic_clear_mask(mask, v)	atomic_test_and_ANDNOT_mask((mask), (v))
+#define atomic_set_mask(mask, v)	atomic_test_and_OR_mask((mask), (v))
+
 static inline int test_and_clear_bit(int nr, volatile void *addr)
 {
 	volatile unsigned long *ptr = addr;
Index: linux-2.6-lttng/include/asm-frv/system.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-frv/system.h	2008-02-07 17:34:08.000000000 -0500
+++ linux-2.6-lttng/include/asm-frv/system.h	2008-02-07 17:35:12.000000000 -0500
@@ -14,6 +14,7 @@
 
 #include <linux/types.h>
 #include <linux/linkage.h>
+#include <linux/kernel.h>
 
 struct thread_struct;
 
@@ -276,7 +277,7 @@
 {
 	switch (size) {
 	case 4:
-		return cmpxchg(ptr, old, new);
+		return cmpxchg((unsigned long *)ptr, old, new);
 	default:
 		return __cmpxchg_local_generic(ptr, old, new, size);
 	}

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68