[parisc-linux] [RFC] rewrite kernel spinlock code to work better with gcc

[parisc-linux] [RFC] rewrite kernel spinlock code to work better with gcc

[Randolph Chung]

This has been discussed in bits and pieces several times on the list,
let me summarize:

- On at least some PA processors, addresses passed to ldcw() must be
16-byte aligned
- gcc doesn't guarantee alignment of automatic variables even if the
structure is marked with aligned(16). In gcc-3.0.x, this worked most of
the time because stack alignment was set to 128 bits, but this caused
various problems so the change was reverted in later revisions of gcc.

In glibc, Carlos and Dave implemented "auto-aligning" locks by using an
array of 4 ints and doing ldcw on the 16-byte aligned word inside that
array. This makes the code work all the time irregardless of how it is
placed in memory. Here's a patch that implements similar locking
mechanisms for the kernel. It compiles, but as SMP still doesn't boot
on 2.6, i haven't really tried to run it. 

there is some concern this will make structures bigger, but at least 
in some situations this actually makes them smaller. e.g.
if you have:

struct { int x; spinlock_t lock; };

with the current scheme (using the aligned(16) attribute) the structure
is 32 bytes. with the new scheme it is only 20 bytes. actually i don't
think there are any cases where it will make any structures bigger 
than they are now..... 

Any comments?

Index: include/asm-parisc/spinlock.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-parisc/spinlock.h,v
retrieving revision 1.1
diff -u -p -r1.1 spinlock.h
--- include/asm-parisc/spinlock.h	29 Jul 2003 17:02:04 -0000	1.1
+++ include/asm-parisc/spinlock.h	26 Nov 2003 07:00:12 -0000
@@ -4,35 +4,42 @@
 #include <asm/system.h>
 
 /* Note that PA-RISC has to use `1' to mean unlocked and `0' to mean locked
- * since it only has load-and-zero.
+ * since it only has load-and-zero. Moreover, at least on some PA processors,
+ * the semaphore address has to be 16-byte aligned.
  */
 
 #undef SPIN_LOCK_UNLOCKED
-#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 }
+#define SPIN_LOCK_UNLOCKED (spinlock_t) { { 1, 1, 1, 1 } }
 
-#define spin_lock_init(x)	do { (x)->lock = 1; } while(0)
+#define spin_lock_init(x)	do { *(x) = SPIN_LOCK_UNLOCKED; } while(0)
 
-#define spin_is_locked(x) ((x)->lock == 0)
-
-#define spin_unlock_wait(x)	do { barrier(); } while(((volatile spinlock_t *)(x))->lock == 0)
-
-#if 1
-#define _raw_spin_lock(x) do { \
-	while (__ldcw (&(x)->lock) == 0) \
-		while (((x)->lock) == 0) ; } while (0)
-
-#else
-#define _raw_spin_lock(x) \
-	do { while(__ldcw(&(x)->lock) == 0); } while(0)
-#endif
+static inline int spin_is_locked(spinlock_t *x)
+{
+	volatile unsigned int *a = __ldcw_align(x);
+	return *a == 0;
+}
+
+#define spin_unlock_wait(x)	do { barrier(); } while(spin_is_locked(x))
+
+static inline void _raw_spin_lock(spinlock_t *x)
+{
+	volatile unsigned int *a = __ldcw_align(x);
+	while (__ldcw(a) == 0)
+		while (*a == 0);
+}
+
+static inline void _raw_spin_unlock(spinlock_t *x)
+{
+	volatile unsigned int *a = __ldcw_align(x);
+	*a = 1;
+}
+
+static inline int _raw_spin_trylock(spinlock_t *x)
+{
+	volatile unsigned int *a = __ldcw_align(x);
+	return __ldcw(a) != 0;
+}
 	
-#define _raw_spin_unlock(x) \
-	do { (x)->lock = 1; } while(0)
-
-#define _raw_spin_trylock(x) (__ldcw(&(x)->lock) != 0)
-
-
-
 /*
  * Read-write spinlocks, allowing multiple readers
  * but only one writer.
@@ -42,7 +49,7 @@ typedef struct {
 	volatile int counter;
 } rwlock_t;
 
-#define RW_LOCK_UNLOCKED (rwlock_t) { {1}, 0 }
+#define RW_LOCK_UNLOCKED (rwlock_t) { { { 1, 1, 1, 1 } }, 0 }
 
 #define rwlock_init(lp)	do { *(lp) = RW_LOCK_UNLOCKED; } while (0)
 
Index: include/asm-parisc/system.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-parisc/system.h,v
retrieving revision 1.1
diff -u -p -r1.1 system.h
--- include/asm-parisc/system.h	29 Jul 2003 17:02:04 -0000	1.1
+++ include/asm-parisc/system.h	26 Nov 2003 07:00:12 -0000
@@ -145,6 +145,19 @@ static inline void set_eiem(unsigned lon
 	__ret; \
 })
 
+/* Because kmalloc only guarantees 8-byte alignment for kmalloc'd data,
+   and GCC only guarantees 8-byte alignment for stack locals, we can't
+   be assured of 16-byte alignment for atomic lock data even if we
+   specify "__attribute ((aligned(16)))" in the type declaration.  So,
+   we use a struct containing an array of four ints for the atomic lock
+   type and dynamically select the 16-byte aligned int from the array
+   for the semaphore.  */
+#define __PA_LDCW_ALIGNMENT 16
+#define __ldcw_align(a) ({ \
+  unsigned long __ret = (unsigned long) a;                     		\
+  __ret = (__ret + __PA_LDCW_ALIGNMENT - 1) & ~(__PA_LDCW_ALIGNMENT - 1); \
+  (unsigned int *) __ret;                                               \
+})
 
 #ifdef CONFIG_SMP
 /*
@@ -152,7 +165,7 @@ static inline void set_eiem(unsigned lon
  */
 
 typedef struct {
-	volatile unsigned int __attribute__((aligned(16))) lock;
+	volatile unsigned int lock[4];
 } spinlock_t;
 #endif
Index: include/asm-parisc/atomic.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-parisc/atomic.h,v
retrieving revision 1.5
diff -u -p -r1.5 atomic.h
--- include/asm-parisc/atomic.h	22 Sep 2003 14:28:12 -0000	1.5
+++ include/asm-parisc/atomic.h	26 Nov 2003 07:00:12 -0000
@@ -24,11 +24,18 @@
 
 extern spinlock_t __atomic_hash[ATOMIC_HASH_SIZE];
 /* copied from <asm/spinlock.h> and modified */
-#  define SPIN_LOCK(x) \
-	do { while(__ldcw(&(x)->lock) == 0); } while(0)
-	
-#  define SPIN_UNLOCK(x) \
-	do { (x)->lock = 1; } while(0)
+static inline void SPIN_LOCK(spinlock_t *x)
+{
+	volatile unsigned int *a = __ldcw_align(x);
+	while (__ldcw(a) == 0)
+		while (*a == 0);
+}
+
+static inline void SPIN_UNLOCK(spinlock_t *x)
+{
+	volatile unsigned int *a = __ldcw_align(x);
+	*a = 1;
+}
 #else
 #  define ATOMIC_HASH_SIZE 1
 #  define ATOMIC_HASH(a)	(0)


randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [parisc-linux] [RFC] rewrite kernel spinlock code to work better with gcc

[John David Anglin]

> +/* Because kmalloc only guarantees 8-byte alignment for kmalloc'd data,
> +   and GCC only guarantees 8-byte alignment for stack locals, we can't
> +   be assured of 16-byte alignment for atomic lock data even if we
> +   specify "__attribute ((aligned(16)))" in the type declaration.  So,
> +   we use a struct containing an array of four ints for the atomic lock
> +   type and dynamically select the 16-byte aligned int from the array
> +   for the semaphore.  */
> +#define __PA_LDCW_ALIGNMENT 16
> +#define __ldcw_align(a) ({ \
> +  unsigned long __ret = (unsigned long) a;                     		\
> +  __ret = (__ret + __PA_LDCW_ALIGNMENT - 1) & ~(__PA_LDCW_ALIGNMENT - 1); \
> +  (unsigned int *) __ret;                                               \
> +})

Change cast to "volatile unsigned int *".

>  typedef struct {
> -	volatile unsigned int __attribute__((aligned(16))) lock;
> +	volatile unsigned int lock[4];
>  } spinlock_t;
>  #endif

Is the struct necessary?  For example,

  typedef volatile unsigned int spinlock_t[4];

I believe that there are situations where the rest of the cache line
holding the active lock word should not be used.  The PA 1.x document
says

  When using semaphores to synchonize with I/O, care must be taken
  in placing other information in the same cache line as the semaphore.
  Data which is writeable can only be placed in the same cache line
  as a semaphore if access to write the data is controlled by the
  semaphore.

I think this restriction only applies to semaphores used both by
I/O and CPU processors, and it's not a concern for semaphores used
solely by CPU processors.  If we have locks used by I/O processors
and they are stack allocated, then some extra padding would appear
to be needed for these locks.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [parisc-linux] [RFC] rewrite kernel spinlock code to work better with gcc

[Joel Soete]

[-- Attachment #1: Type: text/plain, Size: 7922 bytes --]

Hi Randolph,

may be this alignement with 2.4 is still relevant:
---------><---------
--- system.h-rc 2003-11-30 00:21:55.000000000 +0100
+++ system.h    2003-11-30 00:26:17.000000000 +0100
@@ -138,12 +138,36 @@
  #define set_wmb(var, value)            do { var = value; wmb(); } 
while (0)


-/* LDCW, the only atomic read-write operation PA-RISC has. *sigh*.  */
+/* LDCW, the only atomic read-write operation PA-RISC has. *sigh*.
+ *
+ * Note that PA-RISC has to use `1' to mean unlocked and `0' to mean locked
+ * since it only has load-and-zero. Moreover, at least on some PA 
processors,+ * the semaphore address has to be 16-byte aligned.
+ */
+#ifdef CONFIG_PA20
+/*
+> From: "Jim Hull" <jim.hull of hp.com>
+> Delivery-date: Wed, 29 Jan 2003 13:57:05 -0500
+> I've attached a summary of the change, but basically, for PA 2.0, as
+> long as the ",CO" (coherent operation) completer is specified, then the
+> 16-byte alignment requirement for ldcw and ldcd is relaxed, and instead
+> they only require "natural" alignment (4-byte for ldcw, 8-byte for
+> ldcd).
+*/
+
+#define __ldcw(a) ({ \
+       unsigned __ret; \
+       __asm__ __volatile__("ldcw,co 0(%1),%0" : "=r" (__ret) : "r" (a)); \
+       __ret; \
+})
+#else
  #define __ldcw(a) ({ \
         unsigned __ret; \
         __asm__ __volatile__("ldcw 0(%1),%0" : "=r" (__ret) : "r" (a)); \
         __ret; \
  })
+#endif
+

  /* Because kmalloc only guarantees 8-byte alignment for kmalloc'd data,
     and GCC only guarantees 8-byte alignment for stack locals, we can't
---------><---------

hth,
	Joel

ps: also join as attachment (in case of bad wraping)

Randolph Chung wrote:
> This has been discussed in bits and pieces several times on the list,
> let me summarize:
> 
> - On at least some PA processors, addresses passed to ldcw() must be
> 16-byte aligned
> - gcc doesn't guarantee alignment of automatic variables even if the
> structure is marked with aligned(16). In gcc-3.0.x, this worked most of
> the time because stack alignment was set to 128 bits, but this caused
> various problems so the change was reverted in later revisions of gcc.
> 
> In glibc, Carlos and Dave implemented "auto-aligning" locks by using an
> array of 4 ints and doing ldcw on the 16-byte aligned word inside that
> array. This makes the code work all the time irregardless of how it is
> placed in memory. Here's a patch that implements similar locking
> mechanisms for the kernel. It compiles, but as SMP still doesn't boot
> on 2.6, i haven't really tried to run it. 
> 
> there is some concern this will make structures bigger, but at least 
> in some situations this actually makes them smaller. e.g.
> if you have:
> 
> struct { int x; spinlock_t lock; };
> 
> with the current scheme (using the aligned(16) attribute) the structure
> is 32 bytes. with the new scheme it is only 20 bytes. actually i don't
> think there are any cases where it will make any structures bigger 
> than they are now..... 
> 
> Any comments?
> 
> Index: include/asm-parisc/spinlock.h
> ===================================================================
> RCS file: /var/cvs/linux-2.6/include/asm-parisc/spinlock.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 spinlock.h
> --- include/asm-parisc/spinlock.h	29 Jul 2003 17:02:04 -0000	1.1
> +++ include/asm-parisc/spinlock.h	26 Nov 2003 07:00:12 -0000
> @@ -4,35 +4,42 @@
>  #include <asm/system.h>
>  
>  /* Note that PA-RISC has to use `1' to mean unlocked and `0' to mean locked
> - * since it only has load-and-zero.
> + * since it only has load-and-zero. Moreover, at least on some PA processors,
> + * the semaphore address has to be 16-byte aligned.
>   */
>  
>  #undef SPIN_LOCK_UNLOCKED
> -#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 }
> +#define SPIN_LOCK_UNLOCKED (spinlock_t) { { 1, 1, 1, 1 } }
>  
> -#define spin_lock_init(x)	do { (x)->lock = 1; } while(0)
> +#define spin_lock_init(x)	do { *(x) = SPIN_LOCK_UNLOCKED; } while(0)
>  
> -#define spin_is_locked(x) ((x)->lock == 0)
> -
> -#define spin_unlock_wait(x)	do { barrier(); } while(((volatile spinlock_t *)(x))->lock == 0)
> -
> -#if 1
> -#define _raw_spin_lock(x) do { \
> -	while (__ldcw (&(x)->lock) == 0) \
> -		while (((x)->lock) == 0) ; } while (0)
> -
> -#else
> -#define _raw_spin_lock(x) \
> -	do { while(__ldcw(&(x)->lock) == 0); } while(0)
> -#endif
> +static inline int spin_is_locked(spinlock_t *x)
> +{
> +	volatile unsigned int *a = __ldcw_align(x);
> +	return *a == 0;
> +}
> +
> +#define spin_unlock_wait(x)	do { barrier(); } while(spin_is_locked(x))
> +
> +static inline void _raw_spin_lock(spinlock_t *x)
> +{
> +	volatile unsigned int *a = __ldcw_align(x);
> +	while (__ldcw(a) == 0)
> +		while (*a == 0);
> +}
> +
> +static inline void _raw_spin_unlock(spinlock_t *x)
> +{
> +	volatile unsigned int *a = __ldcw_align(x);
> +	*a = 1;
> +}
> +
> +static inline int _raw_spin_trylock(spinlock_t *x)
> +{
> +	volatile unsigned int *a = __ldcw_align(x);
> +	return __ldcw(a) != 0;
> +}
>  	
> -#define _raw_spin_unlock(x) \
> -	do { (x)->lock = 1; } while(0)
> -
> -#define _raw_spin_trylock(x) (__ldcw(&(x)->lock) != 0)
> -
> -
> -
>  /*
>   * Read-write spinlocks, allowing multiple readers
>   * but only one writer.
> @@ -42,7 +49,7 @@ typedef struct {
>  	volatile int counter;
>  } rwlock_t;
>  
> -#define RW_LOCK_UNLOCKED (rwlock_t) { {1}, 0 }
> +#define RW_LOCK_UNLOCKED (rwlock_t) { { { 1, 1, 1, 1 } }, 0 }
>  
>  #define rwlock_init(lp)	do { *(lp) = RW_LOCK_UNLOCKED; } while (0)
>  
> Index: include/asm-parisc/system.h
> ===================================================================
> RCS file: /var/cvs/linux-2.6/include/asm-parisc/system.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 system.h
> --- include/asm-parisc/system.h	29 Jul 2003 17:02:04 -0000	1.1
> +++ include/asm-parisc/system.h	26 Nov 2003 07:00:12 -0000
> @@ -145,6 +145,19 @@ static inline void set_eiem(unsigned lon
>  	__ret; \
>  })
>  
> +/* Because kmalloc only guarantees 8-byte alignment for kmalloc'd data,
> +   and GCC only guarantees 8-byte alignment for stack locals, we can't
> +   be assured of 16-byte alignment for atomic lock data even if we
> +   specify "__attribute ((aligned(16)))" in the type declaration.  So,
> +   we use a struct containing an array of four ints for the atomic lock
> +   type and dynamically select the 16-byte aligned int from the array
> +   for the semaphore.  */
> +#define __PA_LDCW_ALIGNMENT 16
> +#define __ldcw_align(a) ({ \
> +  unsigned long __ret = (unsigned long) a;                     		\
> +  __ret = (__ret + __PA_LDCW_ALIGNMENT - 1) & ~(__PA_LDCW_ALIGNMENT - 1); \
> +  (unsigned int *) __ret;                                               \
> +})
>  
>  #ifdef CONFIG_SMP
>  /*
> @@ -152,7 +165,7 @@ static inline void set_eiem(unsigned lon
>   */
>  
>  typedef struct {
> -	volatile unsigned int __attribute__((aligned(16))) lock;
> +	volatile unsigned int lock[4];
>  } spinlock_t;
>  #endif
> Index: include/asm-parisc/atomic.h
> ===================================================================
> RCS file: /var/cvs/linux-2.6/include/asm-parisc/atomic.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 atomic.h
> --- include/asm-parisc/atomic.h	22 Sep 2003 14:28:12 -0000	1.5
> +++ include/asm-parisc/atomic.h	26 Nov 2003 07:00:12 -0000
> @@ -24,11 +24,18 @@
>  
>  extern spinlock_t __atomic_hash[ATOMIC_HASH_SIZE];
>  /* copied from <asm/spinlock.h> and modified */
> -#  define SPIN_LOCK(x) \
> -	do { while(__ldcw(&(x)->lock) == 0); } while(0)
> -	
> -#  define SPIN_UNLOCK(x) \
> -	do { (x)->lock = 1; } while(0)
> +static inline void SPIN_LOCK(spinlock_t *x)
> +{
> +	volatile unsigned int *a = __ldcw_align(x);
> +	while (__ldcw(a) == 0)
> +		while (*a == 0);
> +}
> +
> +static inline void SPIN_UNLOCK(spinlock_t *x)
> +{
> +	volatile unsigned int *a = __ldcw_align(x);
> +	*a = 1;
> +}
>  #else
>  #  define ATOMIC_HASH_SIZE 1
>  #  define ATOMIC_HASH(a)	(0)
> 
> 
> randolph

[-- Attachment #2: system.h-align.diff --]
[-- Type: text/plain, Size: 1397 bytes --]

--- system.h-rc	2003-11-30 00:21:55.000000000 +0100
+++ system.h	2003-11-30 00:26:17.000000000 +0100
@@ -138,12 +138,36 @@
 #define set_wmb(var, value)		do { var = value; wmb(); } while (0)
 
 
-/* LDCW, the only atomic read-write operation PA-RISC has. *sigh*.  */
+/* LDCW, the only atomic read-write operation PA-RISC has. *sigh*.
+ *
+ * Note that PA-RISC has to use `1' to mean unlocked and `0' to mean locked
+ * since it only has load-and-zero. Moreover, at least on some PA processors,
+ * the semaphore address has to be 16-byte aligned.
+ */
+#ifdef CONFIG_PA20
+/* 
+> From: "Jim Hull" <jim.hull of hp.com>
+> Delivery-date: Wed, 29 Jan 2003 13:57:05 -0500
+> I've attached a summary of the change, but basically, for PA 2.0, as
+> long as the ",CO" (coherent operation) completer is specified, then the
+> 16-byte alignment requirement for ldcw and ldcd is relaxed, and instead
+> they only require "natural" alignment (4-byte for ldcw, 8-byte for
+> ldcd).
+*/
+
+#define __ldcw(a) ({ \
+	unsigned __ret; \
+	__asm__ __volatile__("ldcw,co 0(%1),%0" : "=r" (__ret) : "r" (a)); \
+	__ret; \
+})
+#else
 #define __ldcw(a) ({ \
 	unsigned __ret; \
 	__asm__ __volatile__("ldcw 0(%1),%0" : "=r" (__ret) : "r" (a)); \
 	__ret; \
 })
+#endif
+
 
 /* Because kmalloc only guarantees 8-byte alignment for kmalloc'd data,
    and GCC only guarantees 8-byte alignment for stack locals, we can't

Re: [parisc-linux] [RFC] rewrite kernel spinlock code to work better with gcc

[Joel Soete]

John David Anglin wrote:
>>+/* Because kmalloc only guarantees 8-byte alignment for kmalloc'd data,
>>+   and GCC only guarantees 8-byte alignment for stack locals, we can't
>>+   be assured of 16-byte alignment for atomic lock data even if we
>>+   specify "__attribute ((aligned(16)))" in the type declaration.  So,
>>+   we use a struct containing an array of four ints for the atomic lock
>>+   type and dynamically select the 16-byte aligned int from the array
>>+   for the semaphore.  */
>>+#define __PA_LDCW_ALIGNMENT 16
>>+#define __ldcw_align(a) ({ \
>>+  unsigned long __ret = (unsigned long) a;                     		\
>>+  __ret = (__ret + __PA_LDCW_ALIGNMENT - 1) & ~(__PA_LDCW_ALIGNMENT - 1); \
>>+  (unsigned int *) __ret;                                               \
>>+})
> 
> 
> Change cast to "volatile unsigned int *".
> 
> 
>> typedef struct {
>>-	volatile unsigned int __attribute__((aligned(16))) lock;
>>+	volatile unsigned int lock[4];
>> } spinlock_t;
>> #endif
> 
> 
> Is the struct necessary?  For example,
> 
hmm i supposed that came from 2.4 struct:
typedef struct {
#ifdef CONFIG_PA20
	volatile unsigned int lock;
#else
	volatile unsigned int __attribute__((aligned(16))) lock;
#endif
#ifdef CONFIG_DEBUG_SPINLOCK
	volatile unsigned long owner_pc;
	volatile unsigned long owner_cpu;
#endif
} spinlock_t;

But I don't know yet if CONFIG_DEBUG_SPINLOCK is still foreseen for 2.6

Thanks,
	Joel

[parisc-linux] spinlock 2.4 re-organise a la 2.6 [was: [RFC] rewrite kernel spinlock code to work better with gcc]

[Joel Soete]

[-- Attachment #1: Type: text/plain, Size: 213 bytes --]

Hello,


btw here is attached a patch to re-organize spinlock stuff in 2.4 as in 2.6.

Can somebody help me to apply and test it before ci (not yet time to 
test and no access to cvs ci)

thanks in advance,
	Joel

[-- Attachment #2: spinlock-parisc.bp.diff --]
[-- Type: text/plain, Size: 8850 bytes --]

diff -Naur linux-2.4.23-rc5-pa17/include/asm-parisc/atomic.h linux-2.4.23-rc5-pa17-bp/include/asm-parisc/atomic.h
--- linux-2.4.23-rc5-pa17/include/asm-parisc/atomic.h	2003-11-29 14:11:51.000000000 +0100
+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/atomic.h	2003-11-30 01:23:57.000000000 +0100
@@ -1,7 +1,6 @@
 #ifndef _ASM_PARISC_ATOMIC_H_
 #define _ASM_PARISC_ATOMIC_H_
 
-#include <linux/config.h>
 #include <asm/system.h>
 
 /* Copyright (C) 2000 Philipp Rumpf <prumpf@tux.org>.  */
@@ -193,4 +192,4 @@
 #define smp_mb__before_atomic_inc()	smp_mb()
 #define smp_mb__after_atomic_inc()	smp_mb()
 
-#endif
+#endif /* _ASM_PARISC_ATOMIC_H_ */
diff -Naur linux-2.4.23-rc5-pa17/include/asm-parisc/spinlock.h linux-2.4.23-rc5-pa17-bp/include/asm-parisc/spinlock.h
--- linux-2.4.23-rc5-pa17/include/asm-parisc/spinlock.h	2003-11-29 14:10:38.000000000 +0100
+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/spinlock.h	2003-11-30 01:21:40.000000000 +0100
@@ -1,9 +1,55 @@
 #ifndef __ASM_SPINLOCK_H
 #define __ASM_SPINLOCK_H
 
-#include <asm/spinlock_t.h>		/* get spinlock primitives */
-#include <asm/psw.h>			/* local_* primitives need PSW_I */
-#include <asm/system_irqsave.h>		/* get local_* primitives */
+#include <asm/system.h>
+
+#ifndef CONFIG_DEBUG_SPINLOCK
+#define SPIN_LOCK_UNLOCKED_INIT { 1 }
+#define SPIN_LOCK_UNLOCKED (spinlock_t) SPIN_LOCK_UNLOCKED_INIT
+
+/* Define 6 spinlock primitives that don't depend on anything else. */
+
+#define spin_lock_init(x)       do { (x)->lock = 1; } while(0)
+#define spin_is_locked(x)       ((x)->lock == 0)
+#define spin_trylock(x)		(__ldcw(&(x)->lock) != 0)
+ 
+/* 
+ * PA2.0 is not strongly ordered.  PA1.X is strongly ordered.
+ * ldcw enforces ordering and we need to make sure ordering is
+ * enforced on the unlock too.
+ * "stw,ma" with Zero index is an alias for "stw,o".
+ * But PA 1.x can assemble the "stw,ma" while it doesn't know about "stw,o".
+ * And PA 2.0 will generate the right insn using either form.
+ * Thanks to John David Anglin for this cute trick.
+ *
+ * Writing this with asm also ensures that the unlock doesn't
+ * get reordered
+ */
+#define spin_unlock(x) \
+	__asm__ __volatile__ ("stw,ma  %%sp,0(%0)" : : "r" (&(x)->lock) : "memory" )
+
+#define spin_unlock_wait(x)     do { barrier(); } while(((volatile spinlock_t *)(x))->lock == 0)
+
+#define spin_lock(x) do { \
+	while (__ldcw (&(x)->lock) == 0) \
+		while ((x)->lock == 0) ; \
+} while (0)
+
+#else /* ! CONFIG_DEBUG_SPINLOCK */
+
+#define SPIN_LOCK_UNLOCKED_INIT { 1, 0L, 0L }
+#define SPIN_LOCK_UNLOCKED (spinlock_t) SPIN_LOCK_UNLOCKED_INIT
+
+/* Define 6 spinlock primitives that don't depend on anything else. */
+
+#define spin_lock_init(x)       do { (x)->lock = 1; (x)->owner_cpu = 0; (x)->owner_pc = 0; } while(0)
+#define spin_is_locked(x)       ((x)->lock == 0)
+void spin_lock(spinlock_t *lock);
+int spin_trylock(spinlock_t *lock);
+void spin_unlock(spinlock_t *lock);
+#define spin_unlock_wait(x)     do { barrier(); } while(((volatile spinlock_t *)(x))->lock == 0)
+
+#endif /* ! CONFIG_DEBUG_SPINLOCK */
 
 /*
  * Read-write spinlocks, allowing multiple readers
diff -Naur linux-2.4.23-rc5-pa17/include/asm-parisc/spinlock_t.h linux-2.4.23-rc5-pa17-bp/include/asm-parisc/spinlock_t.h
--- linux-2.4.23-rc5-pa17/include/asm-parisc/spinlock_t.h	2003-11-29 16:13:04.000000000 +0100
+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/spinlock_t.h	1970-01-01 01:00:00.000000000 +0100
@@ -1,97 +0,0 @@
-#ifndef __PARISC_SPINLOCK_T_H
-#define __PARISC_SPINLOCK_T_H
-
-/* LDCW, the only atomic read-write operation PA-RISC has. *sigh*.
- *
- * Note that PA-RISC has to use `1' to mean unlocked and `0' to mean locked
- * since it only has load-and-zero.
- */
-#ifdef CONFIG_PA20
-/* 
-> From: "Jim Hull" <jim.hull of hp.com>
-> Delivery-date: Wed, 29 Jan 2003 13:57:05 -0500
-> I've attached a summary of the change, but basically, for PA 2.0, as
-> long as the ",CO" (coherent operation) completer is specified, then the
-> 16-byte alignment requirement for ldcw and ldcd is relaxed, and instead
-> they only require "natural" alignment (4-byte for ldcw, 8-byte for
-> ldcd).
-*/
-
-#define __ldcw(a) ({ \
-	unsigned __ret; \
-	__asm__ __volatile__("ldcw,co 0(%1),%0" : "=r" (__ret) : "r" (a)); \
-	__ret; \
-})
-#else
-#define __ldcw(a) ({ \
-	unsigned __ret; \
-	__asm__ __volatile__("ldcw 0(%1),%0" : "=r" (__ret) : "r" (a)); \
-	__ret; \
-})
-#endif
-
-/*
- * Your basic SMP spinlocks, allowing only a single CPU anywhere
- */
-
-typedef struct {
-#ifdef CONFIG_PA20
-	volatile unsigned int lock;
-#else
-	volatile unsigned int __attribute__((aligned(16))) lock;
-#endif
-#ifdef CONFIG_DEBUG_SPINLOCK
-	volatile unsigned long owner_pc;
-	volatile unsigned long owner_cpu;
-#endif
-} spinlock_t;
-
-#ifndef CONFIG_DEBUG_SPINLOCK
-#define SPIN_LOCK_UNLOCKED_INIT { 1 }
-#define SPIN_LOCK_UNLOCKED (spinlock_t) SPIN_LOCK_UNLOCKED_INIT
-
-/* Define 6 spinlock primitives that don't depend on anything else. */
-
-#define spin_lock_init(x)       do { (x)->lock = 1; } while(0)
-#define spin_is_locked(x)       ((x)->lock == 0)
-#define spin_trylock(x)		(__ldcw(&(x)->lock) != 0)
- 
-/* 
- * PA2.0 is not strongly ordered.  PA1.X is strongly ordered.
- * ldcw enforces ordering and we need to make sure ordering is
- * enforced on the unlock too.
- * "stw,ma" with Zero index is an alias for "stw,o".
- * But PA 1.x can assemble the "stw,ma" while it doesn't know about "stw,o".
- * And PA 2.0 will generate the right insn using either form.
- * Thanks to John David Anglin for this cute trick.
- *
- * Writing this with asm also ensures that the unlock doesn't
- * get reordered
- */
-#define spin_unlock(x) \
-	__asm__ __volatile__ ("stw,ma  %%sp,0(%0)" : : "r" (&(x)->lock) : "memory" )
-
-#define spin_unlock_wait(x)     do { barrier(); } while(((volatile spinlock_t *)(x))->lock == 0)
-
-#define spin_lock(x) do { \
-	while (__ldcw (&(x)->lock) == 0) \
-		while ((x)->lock == 0) ; \
-} while (0)
-
-#else
-
-#define SPIN_LOCK_UNLOCKED_INIT { 1, 0L, 0L }
-#define SPIN_LOCK_UNLOCKED (spinlock_t) SPIN_LOCK_UNLOCKED_INIT
-
-/* Define 6 spinlock primitives that don't depend on anything else. */
-
-#define spin_lock_init(x)       do { (x)->lock = 1; (x)->owner_cpu = 0; (x)->owner_pc = 0; } while(0)
-#define spin_is_locked(x)       ((x)->lock == 0)
-void spin_lock(spinlock_t *lock);
-int spin_trylock(spinlock_t *lock);
-void spin_unlock(spinlock_t *lock);
-#define spin_unlock_wait(x)     do { barrier(); } while(((volatile spinlock_t *)(x))->lock == 0)
-
-#endif
-
-#endif /* __PARISC_SPINLOCK_T_H */
diff -Naur linux-2.4.23-rc5-pa17/include/asm-parisc/system.h linux-2.4.23-rc5-pa17-bp/include/asm-parisc/system.h
--- linux-2.4.23-rc5-pa17/include/asm-parisc/system.h	2003-11-29 14:11:16.000000000 +0100
+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/system.h	2003-11-30 01:34:35.000000000 +0100
@@ -5,10 +5,6 @@
 #include <asm/psw.h>
 #include <asm/system_irqsave.h>
 
-#ifdef CONFIG_SMP
-#include <asm/spinlock_t.h>
-#endif
-
 /* The program status word as bitfields.  */
 struct pa_psw {
 	unsigned int y:1;
@@ -69,7 +65,7 @@
 #define save_and_cli(x) do { save_flags(x); cli(); } while(0);
 #define save_and_sti(x) do { save_flags(x); sti(); } while(0);
 
-#else
+#else /* CONFIG_SMP */
 
 #define cli() __cli()
 #define sti() __sti()
@@ -78,7 +74,7 @@
 #define save_and_cli(x) __save_and_cli(x)
 #define save_and_sti(x) __save_and_sti(x)
 
-#endif
+#endif /* CONFIG_SMP */
 
 
 #define mfctl(reg)	({		\
@@ -147,4 +143,49 @@
 
 #define set_mb(var, value) do { var = value; mb(); } while (0)
 
+/* LDCW, the only atomic read-write operation PA-RISC has. *sigh*.
+ *
+ * Note that PA-RISC has to use `1' to mean unlocked and `0' to mean locked
+ * since it only has load-and-zero.
+ */
+#ifdef CONFIG_PA20
+/* 
+> From: "Jim Hull" <jim.hull of hp.com>
+> Delivery-date: Wed, 29 Jan 2003 13:57:05 -0500
+> I've attached a summary of the change, but basically, for PA 2.0, as
+> long as the ",CO" (coherent operation) completer is specified, then the
+> 16-byte alignment requirement for ldcw and ldcd is relaxed, and instead
+> they only require "natural" alignment (4-byte for ldcw, 8-byte for
+> ldcd).
+*/
+
+#define __ldcw(a) ({ \
+	unsigned __ret; \
+	__asm__ __volatile__("ldcw,co 0(%1),%0" : "=r" (__ret) : "r" (a)); \
+	__ret; \
+})
+#else /* CONFIG_PA20 */
+#define __ldcw(a) ({ \
+	unsigned __ret; \
+	__asm__ __volatile__("ldcw 0(%1),%0" : "=r" (__ret) : "r" (a)); \
+	__ret; \
+})
+#endif /* CONFIG_PA20 */
+
+/*
+ * Your basic SMP spinlocks, allowing only a single CPU anywhere
+ */
+
+typedef struct {
+#ifdef CONFIG_PA20
+	volatile unsigned int lock;
+#else
+	volatile unsigned int __attribute__((aligned(16))) lock;
+#endif
+#ifdef CONFIG_DEBUG_SPINLOCK
+	volatile unsigned long owner_pc;
+	volatile unsigned long owner_cpu;
 #endif
+} spinlock_t;
+
+#endif /* __PARISC_SYSTEM_H */

Re: [parisc-linux] [RFC] rewrite kernel spinlock code to work better with gcc

[Joel Soete]

:)

a trivial atomic.h patch:
---------><---------
--- atomic.h.orig       2003-11-30 01:47:49.000000000 +0100
+++ atomic.h    2003-11-30 01:48:01.000000000 +0100
@@ -1,7 +1,6 @@
  #ifndef _ASM_PARISC_ATOMIC_H_
  #define _ASM_PARISC_ATOMIC_H_

-#include <linux/config.h>
  #include <asm/system.h>

  /* Copyright (C) 2000 Philipp Rumpf <prumpf@tux.org>.  */
---------><---------

hth,
	J.

Re: [parisc-linux] spinlock 2.4 re-organise a la 2.6 [was: [RFC] rewrite kernel spinlock code to work better with gcc]

[Grant Grundler]

On Sun, Nov 30, 2003 at 12:43:51AM +0000, Joel Soete wrote:
...
> +++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/atomic.h	2003-11-30 01:23:57.000000000 +0100
> @@ -1,7 +1,6 @@
>  #ifndef _ASM_PARISC_ATOMIC_H_
>  #define _ASM_PARISC_ATOMIC_H_
>  
> -#include <linux/config.h>
>  #include <asm/system.h>

Joel,
This is wrong - atomic.h uses CONFIG_SMP and thus is
required to include config.h.

grant

Re: [parisc-linux] spinlock 2.4 re-organise a la 2.6 [was: [RFC] rewrite kernel spinlock code to work better with gcc]

[Joel Soete]

Grant Grundler wrote:
> On Sun, Nov 30, 2003 at 12:43:51AM +0000, Joel Soete wrote:
> ...
> 
>>+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/atomic.h	2003-11-30 01:23:57.000000000 +0100
>>@@ -1,7 +1,6 @@
>> #ifndef _ASM_PARISC_ATOMIC_H_
>> #define _ASM_PARISC_ATOMIC_H_
>> 
>>-#include <linux/config.h>
>> #include <asm/system.h>
> 
> 
> Joel,
> This is wrong - atomic.h uses CONFIG_SMP and thus is
> required to include config.h.
> 
I trust you.
Just a small explanation: i was confused by the begining of asm/system.h 
which is:
---------><---------
#ifndef __PARISC_SYSTEM_H
#define __PARISC_SYSTEM_H

#include <linux/config.h>
#include <asm/psw.h>

[...]
---------><---------

so I trusted that asm/system.h firstly include  <linux/config.h>.
So there should be something wrong elsewhere but don't yet find.

Sorry for my bad understand,

	Joel

Re: [parisc-linux] spinlock 2.4 re-organise a la 2.6 [was: [RFC] rewrite kernel spinlock code to work better with gcc]

[Joel Soete]

Yes forgive that try, it breaks something in 'make dep' I presume 
because make vmlinux failled oribly:

gcc -D__KERNEL__ -I/Debian-apt/SRC/Test/linux-2.4.23-rc5-pa17/include 
-Wall -Wstrict-prototypes -Wno-trigraphs -O2 -fno-strict-aliasing 
-fno-common -D__linux__ -pipe -fno-strength-reduce -mno-space-regs 
-mfast-indirect-calls -mdisable-fpregs -ffunction-sections -march=1.1 
-mschedule=7100   -DKBUILD_BASENAME=main -c -o init/main.o init/main.c
In file included from 
/Debian-apt/SRC/Test/linux-2.4.23-rc5-pa17/include/linux/wait.h:16,
                  from 
/Debian-apt/SRC/Test/linux-2.4.23-rc5-pa17/include/linux/fs.h:12,
                  from 
/Debian-apt/SRC/Test/linux-2.4.23-rc5-pa17/include/linux/capability.h:17,
                  from 
/Debian-apt/SRC/Test/linux-2.4.23-rc5-pa17/include/linux/binfmts.h:5,
                  from 
/Debian-apt/SRC/Test/linux-2.4.23-rc5-pa17/include/linux/sched.h:9,
                  from 
/Debian-apt/SRC/Test/linux-2.4.23-rc5-pa17/include/linux/mm.h:4,
                  from 
/Debian-apt/SRC/Test/linux-2.4.23-rc5-pa17/include/linux/slab.h:14,
                  from 
/Debian-apt/SRC/Test/linux-2.4.23-rc5-pa17/include/linux/proc_fs.h:5,
                  from init/main.c:15:
/Debian-apt/SRC/Test/linux-2.4.23-rc5-pa17/include/linux/spinlock.h:75: 
error: conflicting types for `spinlock_t'
/Debian-apt/SRC/Test/linux-2.4.23-rc5-pa17/include/asm/system.h:189: 
error: previous declaration of `spinlock_t'
make: *** [init/main.o] Error 1

My bad, sorry,
	Joel

Grant Grundler wrote:
> On Sun, Nov 30, 2003 at 12:43:51AM +0000, Joel Soete wrote:
> ...
> 
>>+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/atomic.h	2003-11-30 01:23:57.000000000 +0100
>>@@ -1,7 +1,6 @@
>> #ifndef _ASM_PARISC_ATOMIC_H_
>> #define _ASM_PARISC_ATOMIC_H_
>> 
>>-#include <linux/config.h>
>> #include <asm/system.h>
> 
> 
> Joel,
> This is wrong - atomic.h uses CONFIG_SMP and thus is
> required to include config.h.
> 
> grant
> _______________________________________________
> parisc-linux mailing list
> parisc-linux@lists.parisc-linux.org
> http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
> 

Re: [parisc-linux] spinlock 2.4 re-organise a la 2.6 [was: [RFC] rewrite kernel spinlock code to work better with gcc]

[Joel Soete]

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]



Grant Grundler wrote:
> On Sun, Nov 30, 2003 at 12:43:51AM +0000, Joel Soete wrote:
> ...
> 
>>+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/atomic.h	2003-11-30 01:23:57.000000000 +0100
>>@@ -1,7 +1,6 @@
>> #ifndef _ASM_PARISC_ATOMIC_H_
>> #define _ASM_PARISC_ATOMIC_H_
>> 
>>-#include <linux/config.h>
>> #include <asm/system.h>
> 
> 
> Joel,
> This is wrong - atomic.h uses CONFIG_SMP and thus is
> required to include config.h.
> 
No, i think it is right but I forgot to embrace spinlock_t declaration 
with "#ifdef CONFIG_SMP ...#endif" in system.h. Done in the new attched 
patch (tested and run fine on c100 (32bit up) with kernel up and smp)
My bad in previous test: forgot make distclean ; make mrproper (to be sure).

Still have to test in 64bit up (no means to test in smp neither 32 or 64 
bit :( ).

If you find some interest can you ci (I would like to test Randolph 
patch on n4k just to be sure).

Thanks for help,
	Joel

[-- Attachment #2: spinlock-parisc.bp2.diff --]
[-- Type: text/plain, Size: 9609 bytes --]

diff -Naur linux-2.4.23-rc5-pa17/include/asm-parisc/atomic.h linux-2.4.23-rc5-pa17-bp/include/asm-parisc/atomic.h
--- linux-2.4.23-rc5-pa17/include/asm-parisc/atomic.h	2003-11-29 14:11:51.000000000 +0100
+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/atomic.h	2003-11-30 17:13:58.000000000 +0100
@@ -1,7 +1,6 @@
 #ifndef _ASM_PARISC_ATOMIC_H_
 #define _ASM_PARISC_ATOMIC_H_
 
-#include <linux/config.h>
 #include <asm/system.h>
 
 /* Copyright (C) 2000 Philipp Rumpf <prumpf@tux.org>.  */
@@ -14,7 +13,6 @@
  * have to write any serious assembly.   prumpf
  */
 #ifdef CONFIG_SMP
-#include <asm/spinlock_t.h>
 
 /* Use an array of spinlocks for our atomic_ts.
 ** Hash function to index into a different SPINLOCK.
@@ -193,4 +191,4 @@
 #define smp_mb__before_atomic_inc()	smp_mb()
 #define smp_mb__after_atomic_inc()	smp_mb()
 
-#endif
+#endif /* _ASM_PARISC_ATOMIC_H_ */
diff -Naur linux-2.4.23-rc5-pa17/include/asm-parisc/processor.h linux-2.4.23-rc5-pa17-bp/include/asm-parisc/processor.h
--- linux-2.4.23-rc5-pa17/include/asm-parisc/processor.h	2003-11-30 02:11:17.000000000 +0100
+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/processor.h	2003-11-30 02:12:04.000000000 +0100
@@ -18,9 +18,6 @@
 #include <asm/ptrace.h>
 #include <asm/types.h>
 #include <asm/system.h>
-#ifdef CONFIG_SMP
-#include <asm/spinlock_t.h>
-#endif
 #endif /* __ASSEMBLY__ */
 
 /*
diff -Naur linux-2.4.23-rc5-pa17/include/asm-parisc/spinlock.h linux-2.4.23-rc5-pa17-bp/include/asm-parisc/spinlock.h
--- linux-2.4.23-rc5-pa17/include/asm-parisc/spinlock.h	2003-11-29 14:10:38.000000000 +0100
+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/spinlock.h	2003-11-30 01:21:40.000000000 +0100
@@ -1,9 +1,55 @@
 #ifndef __ASM_SPINLOCK_H
 #define __ASM_SPINLOCK_H
 
-#include <asm/spinlock_t.h>		/* get spinlock primitives */
-#include <asm/psw.h>			/* local_* primitives need PSW_I */
-#include <asm/system_irqsave.h>		/* get local_* primitives */
+#include <asm/system.h>
+
+#ifndef CONFIG_DEBUG_SPINLOCK
+#define SPIN_LOCK_UNLOCKED_INIT { 1 }
+#define SPIN_LOCK_UNLOCKED (spinlock_t) SPIN_LOCK_UNLOCKED_INIT
+
+/* Define 6 spinlock primitives that don't depend on anything else. */
+
+#define spin_lock_init(x)       do { (x)->lock = 1; } while(0)
+#define spin_is_locked(x)       ((x)->lock == 0)
+#define spin_trylock(x)		(__ldcw(&(x)->lock) != 0)
+ 
+/* 
+ * PA2.0 is not strongly ordered.  PA1.X is strongly ordered.
+ * ldcw enforces ordering and we need to make sure ordering is
+ * enforced on the unlock too.
+ * "stw,ma" with Zero index is an alias for "stw,o".
+ * But PA 1.x can assemble the "stw,ma" while it doesn't know about "stw,o".
+ * And PA 2.0 will generate the right insn using either form.
+ * Thanks to John David Anglin for this cute trick.
+ *
+ * Writing this with asm also ensures that the unlock doesn't
+ * get reordered
+ */
+#define spin_unlock(x) \
+	__asm__ __volatile__ ("stw,ma  %%sp,0(%0)" : : "r" (&(x)->lock) : "memory" )
+
+#define spin_unlock_wait(x)     do { barrier(); } while(((volatile spinlock_t *)(x))->lock == 0)
+
+#define spin_lock(x) do { \
+	while (__ldcw (&(x)->lock) == 0) \
+		while ((x)->lock == 0) ; \
+} while (0)
+
+#else /* ! CONFIG_DEBUG_SPINLOCK */
+
+#define SPIN_LOCK_UNLOCKED_INIT { 1, 0L, 0L }
+#define SPIN_LOCK_UNLOCKED (spinlock_t) SPIN_LOCK_UNLOCKED_INIT
+
+/* Define 6 spinlock primitives that don't depend on anything else. */
+
+#define spin_lock_init(x)       do { (x)->lock = 1; (x)->owner_cpu = 0; (x)->owner_pc = 0; } while(0)
+#define spin_is_locked(x)       ((x)->lock == 0)
+void spin_lock(spinlock_t *lock);
+int spin_trylock(spinlock_t *lock);
+void spin_unlock(spinlock_t *lock);
+#define spin_unlock_wait(x)     do { barrier(); } while(((volatile spinlock_t *)(x))->lock == 0)
+
+#endif /* ! CONFIG_DEBUG_SPINLOCK */
 
 /*
  * Read-write spinlocks, allowing multiple readers
diff -Naur linux-2.4.23-rc5-pa17/include/asm-parisc/spinlock_t.h linux-2.4.23-rc5-pa17-bp/include/asm-parisc/spinlock_t.h
--- linux-2.4.23-rc5-pa17/include/asm-parisc/spinlock_t.h	2003-11-29 16:13:04.000000000 +0100
+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/spinlock_t.h	1970-01-01 01:00:00.000000000 +0100
@@ -1,97 +0,0 @@
-#ifndef __PARISC_SPINLOCK_T_H
-#define __PARISC_SPINLOCK_T_H
-
-/* LDCW, the only atomic read-write operation PA-RISC has. *sigh*.
- *
- * Note that PA-RISC has to use `1' to mean unlocked and `0' to mean locked
- * since it only has load-and-zero.
- */
-#ifdef CONFIG_PA20
-/* 
-> From: "Jim Hull" <jim.hull of hp.com>
-> Delivery-date: Wed, 29 Jan 2003 13:57:05 -0500
-> I've attached a summary of the change, but basically, for PA 2.0, as
-> long as the ",CO" (coherent operation) completer is specified, then the
-> 16-byte alignment requirement for ldcw and ldcd is relaxed, and instead
-> they only require "natural" alignment (4-byte for ldcw, 8-byte for
-> ldcd).
-*/
-
-#define __ldcw(a) ({ \
-	unsigned __ret; \
-	__asm__ __volatile__("ldcw,co 0(%1),%0" : "=r" (__ret) : "r" (a)); \
-	__ret; \
-})
-#else
-#define __ldcw(a) ({ \
-	unsigned __ret; \
-	__asm__ __volatile__("ldcw 0(%1),%0" : "=r" (__ret) : "r" (a)); \
-	__ret; \
-})
-#endif
-
-/*
- * Your basic SMP spinlocks, allowing only a single CPU anywhere
- */
-
-typedef struct {
-#ifdef CONFIG_PA20
-	volatile unsigned int lock;
-#else
-	volatile unsigned int __attribute__((aligned(16))) lock;
-#endif
-#ifdef CONFIG_DEBUG_SPINLOCK
-	volatile unsigned long owner_pc;
-	volatile unsigned long owner_cpu;
-#endif
-} spinlock_t;
-
-#ifndef CONFIG_DEBUG_SPINLOCK
-#define SPIN_LOCK_UNLOCKED_INIT { 1 }
-#define SPIN_LOCK_UNLOCKED (spinlock_t) SPIN_LOCK_UNLOCKED_INIT
-
-/* Define 6 spinlock primitives that don't depend on anything else. */
-
-#define spin_lock_init(x)       do { (x)->lock = 1; } while(0)
-#define spin_is_locked(x)       ((x)->lock == 0)
-#define spin_trylock(x)		(__ldcw(&(x)->lock) != 0)
- 
-/* 
- * PA2.0 is not strongly ordered.  PA1.X is strongly ordered.
- * ldcw enforces ordering and we need to make sure ordering is
- * enforced on the unlock too.
- * "stw,ma" with Zero index is an alias for "stw,o".
- * But PA 1.x can assemble the "stw,ma" while it doesn't know about "stw,o".
- * And PA 2.0 will generate the right insn using either form.
- * Thanks to John David Anglin for this cute trick.
- *
- * Writing this with asm also ensures that the unlock doesn't
- * get reordered
- */
-#define spin_unlock(x) \
-	__asm__ __volatile__ ("stw,ma  %%sp,0(%0)" : : "r" (&(x)->lock) : "memory" )
-
-#define spin_unlock_wait(x)     do { barrier(); } while(((volatile spinlock_t *)(x))->lock == 0)
-
-#define spin_lock(x) do { \
-	while (__ldcw (&(x)->lock) == 0) \
-		while ((x)->lock == 0) ; \
-} while (0)
-
-#else
-
-#define SPIN_LOCK_UNLOCKED_INIT { 1, 0L, 0L }
-#define SPIN_LOCK_UNLOCKED (spinlock_t) SPIN_LOCK_UNLOCKED_INIT
-
-/* Define 6 spinlock primitives that don't depend on anything else. */
-
-#define spin_lock_init(x)       do { (x)->lock = 1; (x)->owner_cpu = 0; (x)->owner_pc = 0; } while(0)
-#define spin_is_locked(x)       ((x)->lock == 0)
-void spin_lock(spinlock_t *lock);
-int spin_trylock(spinlock_t *lock);
-void spin_unlock(spinlock_t *lock);
-#define spin_unlock_wait(x)     do { barrier(); } while(((volatile spinlock_t *)(x))->lock == 0)
-
-#endif
-
-#endif /* __PARISC_SPINLOCK_T_H */
diff -Naur linux-2.4.23-rc5-pa17/include/asm-parisc/system.h linux-2.4.23-rc5-pa17-bp/include/asm-parisc/system.h
--- linux-2.4.23-rc5-pa17/include/asm-parisc/system.h	2003-11-29 14:11:16.000000000 +0100
+++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/system.h	2003-11-30 17:12:37.000000000 +0100
@@ -5,10 +5,6 @@
 #include <asm/psw.h>
 #include <asm/system_irqsave.h>
 
-#ifdef CONFIG_SMP
-#include <asm/spinlock_t.h>
-#endif
-
 /* The program status word as bitfields.  */
 struct pa_psw {
 	unsigned int y:1;
@@ -69,7 +65,7 @@
 #define save_and_cli(x) do { save_flags(x); cli(); } while(0);
 #define save_and_sti(x) do { save_flags(x); sti(); } while(0);
 
-#else
+#else /* CONFIG_SMP */
 
 #define cli() __cli()
 #define sti() __sti()
@@ -78,7 +74,7 @@
 #define save_and_cli(x) __save_and_cli(x)
 #define save_and_sti(x) __save_and_sti(x)
 
-#endif
+#endif /* CONFIG_SMP */
 
 
 #define mfctl(reg)	({		\
@@ -147,4 +143,51 @@
 
 #define set_mb(var, value) do { var = value; mb(); } while (0)
 
+/* LDCW, the only atomic read-write operation PA-RISC has. *sigh*.
+ *
+ * Note that PA-RISC has to use `1' to mean unlocked and `0' to mean locked
+ * since it only has load-and-zero.
+ */
+#ifdef CONFIG_PA20
+/* 
+> From: "Jim Hull" <jim.hull of hp.com>
+> Delivery-date: Wed, 29 Jan 2003 13:57:05 -0500
+> I've attached a summary of the change, but basically, for PA 2.0, as
+> long as the ",CO" (coherent operation) completer is specified, then the
+> 16-byte alignment requirement for ldcw and ldcd is relaxed, and instead
+> they only require "natural" alignment (4-byte for ldcw, 8-byte for
+> ldcd).
+*/
+
+#define __ldcw(a) ({ \
+	unsigned __ret; \
+	__asm__ __volatile__("ldcw,co 0(%1),%0" : "=r" (__ret) : "r" (a)); \
+	__ret; \
+})
+#else /* CONFIG_PA20 */
+#define __ldcw(a) ({ \
+	unsigned __ret; \
+	__asm__ __volatile__("ldcw 0(%1),%0" : "=r" (__ret) : "r" (a)); \
+	__ret; \
+})
+#endif /* CONFIG_PA20 */
+
+#ifdef CONFIG_SMP
+/*
+ * Your basic SMP spinlocks, allowing only a single CPU anywhere
+ */
+
+typedef struct {
+#ifdef CONFIG_PA20
+	volatile unsigned int lock;
+#else
+	volatile unsigned int __attribute__((aligned(16))) lock;
 #endif
+#ifdef CONFIG_DEBUG_SPINLOCK
+	volatile unsigned long owner_pc;
+	volatile unsigned long owner_cpu;
+#endif
+} spinlock_t;
+#endif /* CONFIG_SMP */
+
+#endif /* __PARISC_SYSTEM_H */

Re: [parisc-linux] spinlock 2.4 re-organise a la 2.6 [was: [RFC] rewrite kernel spinlock code to work better with gcc]

[Grant Grundler]

On Sun, Nov 30, 2003 at 04:31:49PM +0000, Joel Soete wrote:
> No, i think it is right but I forgot to embrace spinlock_t declaration 
> with "#ifdef CONFIG_SMP ...#endif" in system.h. Done in the new attched 
> patch (tested and run fine on c100 (32bit up) with kernel up and smp)
> My bad in previous test: forgot make distclean ; make mrproper (to be sure).

Use of "make distclean" means you don't have the dependencies correct.
ie whereever "ifdef CONFIG_*" is used, linux/config.h needs to be included.

> If you find some interest can you ci (I would like to test Randolph 
> patch on n4k just to be sure).

I don't. I have a basic problem with this patch.

...
> -#include <asm/spinlock_t.h>
...
> diff -Naur linux-2.4.23-rc5-pa17/include/asm-parisc/spinlock_t.h linux-2.4.23-rc5-pa17-bp/include/asm-parisc/spinlock_t.h
> --- linux-2.4.23-rc5-pa17/include/asm-parisc/spinlock_t.h	2003-11-29 16:13:04.000000000 +0100
> +++ linux-2.4.23-rc5-pa17-bp/include/asm-parisc/spinlock_t.h	1970-01-01 01:00:00.000000000 +0100
> @@ -1,97 +0,0 @@
> -#ifndef __PARISC_SPINLOCK_T_H
> -#define __PARISC_SPINLOCK_T_H
...

Why delete spinlock_t.h?

I added spinlock_t.h to resolve the circular inter-dependency between
asm/system.h, asm/bitops.h, and asm/spinlock.h.  spinlock_t.h depends
on nothing and defines spinlock primitives used by the others.

grant

Re: [parisc-linux] spinlock 2.4 re-organise a la 2.6 [was: [RFC] rewrite kernel spinlock code to work better with gcc]

[Joel Soete]

>I added spinlock_t.h to resolve the circular inter-dependency between
>asm/system.h, asm/bitops.h, and asm/spinlock.h.  spinlock_t.h depends
>on nothing and defines spinlock primitives used by the others.

I see (forgive)

Joel


-------------------------------------------------------------------------
Tiscali ADSL: 12 mois à 29,50 €/mois! L'Internet rapide, c'est pour tout
le monde.
http://reg.tiscali.be/default.asp?lg=fr

RE: [parisc-linux] [RFC] rewrite kernel spinlock code to work better with gcc

[Joel Soete]

Hi Randolph,

(Just in case it would help) I backport your patch in 2.4 (just because the
only system on which I can test it is a b2k on which 2.6 doesn't boot yet)
and build a smp 64bit kernel which boot and run fine (rebuild another kernel)
on this b2k:)

Joel

ps: I don't yet investigate Dave idea :(

>-- Original Message --
>From: Randolph Chung <randolph@tausq.org>
>To: parisc-linux@lists.parisc-linux.org
>Reply-To: Randolph Chung <randolph@tausq.org>
>Date: Tue, 25 Nov 2003 23:07:14 -0800
>Subject: [parisc-linux] [RFC] rewrite kernel spinlock code to work better
>with gcc
>
>
>This has been discussed in bits and pieces several times on the list,
let me summarize:

- On at least some PA processors, addresses passed to ldcw() must be
16-byte aligned
- gcc doesn't guarantee alignment of automatic variables even if the
str
>cture is marked with aligned(16). In gcc-3.0.x, this worked most of
the time because stack alignment was set to 128 bits, but this caused
various problems so the change was reverted in later revisions of gcc.

In glibc, Carlos and Dave implemented 
>auto-aligning" locks by using an
array of 4 ints and doing ldcw on the 16-byte aligned word inside that
array. This makes the code work all the time irregardless of how it is
placed in memory. Here's a patch that implements similar locking
mechanis
>s for the kernel. It compiles, but as SMP still doesn't boot
on 2.6, i haven't really tried to run it. 

there is some concern this will make structures bigger, but at least 
in some situations this actually makes them smaller. e.g.
if you have:

>
struct { int x; spinlock_t lock; };

with the current scheme (using the aligned(16) attribute) the structure
is 32 bytes. with the new scheme it is only 20 bytes. actually i don't
think there are any cases where it will make any structures bigger 
>
than they are now..... 

Any comments?

Index: include/asm-parisc/spinlock.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-parisc/spinlock.h,v
retrieving revision 1.1
diff -u -p -r
>.1 spinlock.h
--- include/asm-parisc/spinlock.h	29 Jul 2003 17:02:04 -0000	1.1
+++ include/asm-parisc/spinlock.h	26 Nov 2003 07:00:12 -0000
@@ -4,35 +4,42 @@
 #include <asm/system.h>
 
 /* Note that PA-RISC has to use `1' to mean unlocked and `0'
>to mean locked
- * since it only has load-and-zero.
+ * since it only has load-and-zero. Moreover, at least on some PA processors,
+ * the semaphore address has to be 16-byte aligned.
  */
 
 #undef SPIN_LOCK_UNLOCKED
-#define SPIN_LOCK_UNLOCKED
>(spinlock_t) { 1 }
+#define SPIN_LOCK_UNLOCKED (spinlock_t) { { 1, 1, 1, 1 } }
 
-#define spin_lock_init(x)	do { (x)->lock = 1; } while(0)
+#define spin_lock_init(x)	do { *(x) = SPIN_LOCK_UNLOCKED; } while(0)
 
-#define spin_is_locked(x) ((x)->lo
>k == 0)
-
-#define spin_unlock_wait(x)	do { barrier(); } while(((volatile spinlock_t
*)(x))->lock == 0)
-
-#if 1
-#define _raw_spin_lock(x) do { \
-	while (__ldcw (&(x)->lock) == 0) \
-		while (((x)->lock) == 0) ; } while (0)
-
-#else
-#defin
> _raw_spin_lock(x) \
-	do { while(__ldcw(&(x)->lock) == 0); } while(0)
-#endif
+static inline int spin_is_locked(spinlock_t *x)
+{
+	volatile unsigned int *a = __ldcw_align(x);
+	return *a == 0;
+}
+
+#define spin_unlock_wait(x)	do { barrier()
> } while(spin_is_locked(x))
+
+static inline void _raw_spin_lock(spinlock_t *x)
+{
+	volatile unsigned int *a = __ldcw_align(x);
+	while (__ldcw(a) == 0)
+		while (*a == 0);
+}
+
+static inline void _raw_spin_unlock(spinlock_t *x)
+{
+	volat
>le unsigned int *a = __ldcw_align(x);
+	*a = 1;
+}
+
+static inline int _raw_spin_trylock(spinlock_t *x)
+{
+	volatile unsigned int *a = __ldcw_align(x);
+	return __ldcw(a) != 0;
+}
 	
-#define _raw_spin_unlock(x) \
-	do { (x)->lock = 1; } w
>ile(0)
-
-#define _raw_spin_trylock(x) (__ldcw(&(x)->lock) != 0)
-
-
-
 /*
  * Read-write spinlocks, allowing multiple readers
  * but only one writer.
@@ -42,7 +49,7 @@ typedef struct {
 	volatile int counter;
 } rwlock_t;
 
-#define RW_L
>CK_UNLOCKED (rwlock_t) { {1}, 0 }
+#define RW_LOCK_UNLOCKED (rwlock_t) { { { 1, 1, 1, 1 } }, 0 }
 
 #define rwlock_init(lp)	do { *(lp) = RW_LOCK_UNLOCKED; } while (0)
 
Index: include/asm-parisc/system.h
==========================================
>========================
RCS file: /var/cvs/linux-2.6/include/asm-parisc/system.h,v
retrieving revision 1.1
diff -u -p -r1.1 system.h
--- include/asm-parisc/system.h	29 Jul 2003 17:02:04 -0000	1.1
+++ include/asm-parisc/system.h	26 Nov 2003 07:00:
>2 -0000
@@ -145,6 +145,19 @@ static inline void set_eiem(unsigned lon
 	__ret; \
 })
 
+/* Because kmalloc only guarantees 8-byte alignment for kmalloc'd data,
+   and GCC only guarantees 8-byte alignment for stack locals, we can't
+   be assure
> of 16-byte alignment for atomic lock data even if we
+   specify "__attribute ((aligned(16)))" in the type declaration.  So,
+   we use a struct containing an array of four ints for the atomic lock
+   type and dynamically select the 16-byte aligne
> int from the array
+   for the semaphore.  */
+#define __PA_LDCW_ALIGNMENT 16
+#define __ldcw_align(a) ({ \
+  unsigned long __ret = (unsigned long) a;                     		\
+  __ret = (__ret + __PA_LDCW_ALIGNMENT - 1) & ~(__PA_LDCW_ALIGNMENT -
>1); \
+  (unsigned int *) __ret;                                              
\
+})
 
 #ifdef CONFIG_SMP
 /*
@@ -152,7 +165,7 @@ static inline void set_eiem(unsigned lon
  */
 
 typedef struct {
-	volatile unsigned int __attribute__((aligned
>16))) lock;
+	volatile unsigned int lock[4];
 } spinlock_t;
 #endif
Index: include/asm-parisc/atomic.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-parisc/atomic.h,v
retrieving revi
>ion 1.5
diff -u -p -r1.5 atomic.h
--- include/asm-parisc/atomic.h	22 Sep 2003 14:28:12 -0000	1.5
+++ include/asm-parisc/atomic.h	26 Nov 2003 07:00:12 -0000
@@ -24,11 +24,18 @@
 
 extern spinlock_t __atomic_hash[ATOMIC_HASH_SIZE];
 /* copied from
><asm/spinlock.h> and modified */
-#  define SPIN_LOCK(x) \
-	do { while(__ldcw(&(x)->lock) == 0); } while(0)
-	
-#  define SPIN_UNLOCK(x) \
-	do { (x)->lock = 1; } while(0)
+static inline void SPIN_LOCK(spinlock_t *x)
+{
+	volatile unsigned int
>*a = __ldcw_align(x);
+	while (__ldcw(a) == 0)
+		while (*a == 0);
+}
+
+static inline void SPIN_UNLOCK(spinlock_t *x)
+{
+	volatile unsigned int *a = __ldcw_align(x);
+	*a = 1;
+}
 #else
 #  define ATOMIC_HASH_SIZE 1
 #  define ATOMIC_HASH
>a)	(0)


randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.or
>/mailman/listinfo/parisc-linux



-------------------------------------------------------------------------
Tiscali ADSL: 12 mois à 29,50 €/mois! L'Internet rapide, c'est pour tout
le monde.
http://reg.tiscali.be/default.asp?lg=fr

Re: [parisc-linux] [RFC] rewrite kernel spinlock code to work better with gcc

[John David Anglin]

> (Just in case it would help) I backport your patch in 2.4 (just because the
> only system on which I can test it is a b2k on which 2.6 doesn't boot yet)
> and build a smp 64bit kernel which boot and run fine (rebuild another kernel)
> on this b2k:)

I'd forgotten about the "ldcw,co" errata for PA 2.0.  You only want/need
self-aligning locks for PA 1.x.  You should be able to test this on a
PA 2.0 machine if you don't define CONFIG_PA20.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [parisc-linux] [RFC] rewrite kernel spinlock code to work better with gcc

[Joel Soete]

Hi Dave,

I trust that I have to restart my test (32b smp, 64b smp; all seems to works
fine in 32b but some worries in 64b). With the new 2.4.23-pa1, I discover
that I let badly commented a small chunk a code (just involved toc) and I
wrongly merge Randolph's patch (just about a small #ifdef CONFIG_PA20 in
2.4: thanks for your comments :) ).

I will advise.

Thanks again,
    Joel

>-- Original Message --
>To: soete.joel@tiscali.be (Joel Soete)
>Date: Mon, 1 Dec 2003 13:30:07 -0500 (EST)
>From: "John David Anglin" <dave@hiauly1.hia.nrc.ca>
>Cc: randolph@tausq.org, parisc-linux@lists.parisc-linux.org
>Subject: Re: [parisc-linux] [RFC] rewrite kernel spinlock code to work better
>with gcc
>
>
>> (Just in case it would help) I backport your patch in 2.4 (just because
the
> only system on which I can test it is a b2k on which 2.6 doesn't boot yet)
> and build a smp 64bit kernel which boot and run fine (rebuild another kernel)
> on this b2k:
>

I'd forgotten about the "ldcw,co" errata for PA 2.0.  You only want/need
self-aligning locks for PA 1.x.  You should be able to test this on a
PA 2.0 machine if you don't define CONFIG_PA20.

Dave
-- 
J. David Anglin                          
>       dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)



-------------------------------------------------------------------------
Tiscali ADSL: 12 mois à 29,50 €/mois! L'Internet rapide, c'est pour tout
le monde.
http://reg.tiscali.be/default.asp?lg=fr