From: Ingo Molnar The first fix is that there was no compiler warning on x86 because it uses macros - i fixed this by changing the spinlock field to be '->slock'. (we could also use inline functions to get type protection, i chose this solution because it was the easiest to do.) The second fix is to split rwlock_is_locked() into two functions: +/** + * read_is_locked - would read_trylock() fail? + * @lock: the rwlock in question. + */ +#define read_is_locked(x) (atomic_read((atomic_t *)&(x)->lock) <= 0) + +/** + * write_is_locked - would write_trylock() fail? + * @lock: the rwlock in question. + */ +#define write_is_locked(x) ((x)->lock != RW_LOCK_BIAS) This canonical naming of them also enabled the elimination of the newly added 'is_locked_fn' argument to the BUILD_LOCK_OPS macro. The third change was to change the other user of rwlock_is_locked(), and to put a migration helper there: architectures that dont have read/write_is_locked defined yet will get a #warning message but the build will succeed. (except if PREEMPT is enabled - there we really need.) The fourth change is to make spin_yield() a generic function, with some related namespace cleanups. Compile and boot-tested on x86, on SMP and UP, PREEMPT and !PREEMPT. Non-x86 architectures should work fine, except PREEMPT+SMP builds which will need the read_is_locked()/write_is_locked() definitions. !PREEMPT+SMP builds will work fine and will produce a #warning. Signed-off-by: Ingo Molnar Signed-off-by: Andrew Morton --- 25-akpm/arch/ppc64/lib/locks.c | 6 +++--- 25-akpm/include/asm-generic/spinlock.h | 11 +++++++++++ 25-akpm/include/asm-i386/spinlock.h | 28 ++++++++++++++++++++-------- 25-akpm/include/asm-ia64/spinlock.h | 2 ++ 25-akpm/include/asm-ppc64/spinlock.h | 16 ++++++++-------- 25-akpm/include/asm-x86_64/spinlock.h | 2 ++ 25-akpm/include/linux/spinlock.h | 6 +++++- 25-akpm/kernel/exit.c | 6 +++++- 25-akpm/kernel/spinlock.c | 24 ++++++++++++------------ 9 files changed, 68 insertions(+), 33 deletions(-) diff -puN arch/ppc64/lib/locks.c~spinlocking-fixes arch/ppc64/lib/locks.c --- 25/arch/ppc64/lib/locks.c~spinlocking-fixes Wed Jan 19 15:24:55 2005 +++ 25-akpm/arch/ppc64/lib/locks.c Wed Jan 19 15:24:55 2005 @@ -23,7 +23,7 @@ /* waiting for a spinlock... */ #if defined(CONFIG_PPC_SPLPAR) || defined(CONFIG_PPC_ISERIES) -void __spin_yield(spinlock_t *lock) +void spinlock_yield(spinlock_t *lock) { unsigned int lock_value, holder_cpu, yield_count; struct paca_struct *holder_paca; @@ -54,7 +54,7 @@ void __spin_yield(spinlock_t *lock) * This turns out to be the same for read and write locks, since * we only know the holder if it is write-locked. */ -void __rw_yield(rwlock_t *rw) +void rwlock_yield(rwlock_t *rw) { int lock_value; unsigned int holder_cpu, yield_count; @@ -87,7 +87,7 @@ void spin_unlock_wait(spinlock_t *lock) while (lock->lock) { HMT_low(); if (SHARED_PROCESSOR) - __spin_yield(lock); + spinlock_yield(lock); } HMT_medium(); } diff -puN /dev/null include/asm-generic/spinlock.h --- /dev/null Thu Apr 11 07:25:15 2002 +++ 25-akpm/include/asm-generic/spinlock.h Wed Jan 19 15:24:55 2005 @@ -0,0 +1,11 @@ +#ifndef _ASM_GENERIC_SPINLOCK_H +#define _ASM_GENERIC_SPINLOCK_H + +/* + * Virtual platforms might use these to + * yield to specific virtual CPUs: + */ +#define spinlock_yield(lock) cpu_relax() +#define rwlock_yield(lock) cpu_relax() + +#endif /* _ASM_GENERIC_SPINLOCK_H */ diff -puN include/asm-i386/spinlock.h~spinlocking-fixes include/asm-i386/spinlock.h --- 25/include/asm-i386/spinlock.h~spinlocking-fixes Wed Jan 19 15:24:55 2005 +++ 25-akpm/include/asm-i386/spinlock.h Wed Jan 19 15:24:55 2005 @@ -7,6 +7,8 @@ #include #include +#include + asmlinkage int printk(const char * fmt, ...) __attribute__ ((format (printf, 1, 2))); @@ -15,7 +17,7 @@ asmlinkage int printk(const char * fmt, */ typedef struct { - volatile unsigned int lock; + volatile unsigned int slock; #ifdef CONFIG_DEBUG_SPINLOCK unsigned magic; #endif @@ -43,7 +45,7 @@ typedef struct { * We make no fairness assumptions. They have a cost. */ -#define spin_is_locked(x) (*(volatile signed char *)(&(x)->lock) <= 0) +#define spin_is_locked(x) (*(volatile signed char *)(&(x)->slock) <= 0) #define spin_unlock_wait(x) do { barrier(); } while(spin_is_locked(x)) #define spin_lock_string \ @@ -83,7 +85,7 @@ typedef struct { #define spin_unlock_string \ "movb $1,%0" \ - :"=m" (lock->lock) : : "memory" + :"=m" (lock->slock) : : "memory" static inline void _raw_spin_unlock(spinlock_t *lock) @@ -101,7 +103,7 @@ static inline void _raw_spin_unlock(spin #define spin_unlock_string \ "xchgb %b0, %1" \ - :"=q" (oldval), "=m" (lock->lock) \ + :"=q" (oldval), "=m" (lock->slock) \ :"0" (oldval) : "memory" static inline void _raw_spin_unlock(spinlock_t *lock) @@ -123,7 +125,7 @@ static inline int _raw_spin_trylock(spin char oldval; __asm__ __volatile__( "xchgb %b0,%1" - :"=q" (oldval), "=m" (lock->lock) + :"=q" (oldval), "=m" (lock->slock) :"0" (0) : "memory"); return oldval > 0; } @@ -138,7 +140,7 @@ static inline void _raw_spin_lock(spinlo #endif __asm__ __volatile__( spin_lock_string - :"=m" (lock->lock) : : "memory"); + :"=m" (lock->slock) : : "memory"); } static inline void _raw_spin_lock_flags (spinlock_t *lock, unsigned long flags) @@ -151,7 +153,7 @@ static inline void _raw_spin_lock_flags #endif __asm__ __volatile__( spin_lock_string_flags - :"=m" (lock->lock) : "r" (flags) : "memory"); + :"=m" (lock->slock) : "r" (flags) : "memory"); } /* @@ -186,7 +188,17 @@ typedef struct { #define rwlock_init(x) do { *(x) = RW_LOCK_UNLOCKED; } while(0) -#define rwlock_is_locked(x) ((x)->lock != RW_LOCK_BIAS) +/** + * read_is_locked - would read_trylock() fail? + * @lock: the rwlock in question. + */ +#define read_is_locked(x) (atomic_read((atomic_t *)&(x)->lock) <= 0) + +/** + * write_is_locked - would write_trylock() fail? + * @lock: the rwlock in question. + */ +#define write_is_locked(x) ((x)->lock != RW_LOCK_BIAS) /* * On x86, we implement read-write locks as a 32-bit counter diff -puN include/asm-ia64/spinlock.h~spinlocking-fixes include/asm-ia64/spinlock.h --- 25/include/asm-ia64/spinlock.h~spinlocking-fixes Wed Jan 19 15:24:55 2005 +++ 25-akpm/include/asm-ia64/spinlock.h Wed Jan 19 15:24:55 2005 @@ -17,6 +17,8 @@ #include #include +#include + typedef struct { volatile unsigned int lock; #ifdef CONFIG_PREEMPT diff -puN include/asm-ppc64/spinlock.h~spinlocking-fixes include/asm-ppc64/spinlock.h --- 25/include/asm-ppc64/spinlock.h~spinlocking-fixes Wed Jan 19 15:24:55 2005 +++ 25-akpm/include/asm-ppc64/spinlock.h Wed Jan 19 15:24:55 2005 @@ -64,11 +64,11 @@ static __inline__ void _raw_spin_unlock( #if defined(CONFIG_PPC_SPLPAR) || defined(CONFIG_PPC_ISERIES) /* We only yield to the hypervisor if we are in shared processor mode */ #define SHARED_PROCESSOR (get_paca()->lppaca.shared_proc) -extern void __spin_yield(spinlock_t *lock); -extern void __rw_yield(rwlock_t *lock); +extern void spinlock_yield(spinlock_t *lock); +extern void rwlock_yield(rwlock_t *lock); #else /* SPLPAR || ISERIES */ -#define __spin_yield(x) barrier() -#define __rw_yield(x) barrier() +#define spinlock_yield(x) barrier() +#define rwlock_yield(x) barrier() #define SHARED_PROCESSOR 0 #endif extern void spin_unlock_wait(spinlock_t *lock); @@ -109,7 +109,7 @@ static void __inline__ _raw_spin_lock(sp do { HMT_low(); if (SHARED_PROCESSOR) - __spin_yield(lock); + spinlock_yield(lock); } while (likely(lock->lock != 0)); HMT_medium(); } @@ -127,7 +127,7 @@ static void __inline__ _raw_spin_lock_fl do { HMT_low(); if (SHARED_PROCESSOR) - __spin_yield(lock); + spinlock_yield(lock); } while (likely(lock->lock != 0)); HMT_medium(); local_irq_restore(flags_dis); @@ -201,7 +201,7 @@ static void __inline__ _raw_read_lock(rw do { HMT_low(); if (SHARED_PROCESSOR) - __rw_yield(rw); + rwlock_yield(rw); } while (likely(rw->lock < 0)); HMT_medium(); } @@ -258,7 +258,7 @@ static void __inline__ _raw_write_lock(r do { HMT_low(); if (SHARED_PROCESSOR) - __rw_yield(rw); + rwlock_yield(rw); } while (likely(rw->lock != 0)); HMT_medium(); } diff -puN include/asm-x86_64/spinlock.h~spinlocking-fixes include/asm-x86_64/spinlock.h --- 25/include/asm-x86_64/spinlock.h~spinlocking-fixes Wed Jan 19 15:24:55 2005 +++ 25-akpm/include/asm-x86_64/spinlock.h Wed Jan 19 15:24:55 2005 @@ -6,6 +6,8 @@ #include #include +#include + extern int printk(const char * fmt, ...) __attribute__ ((format (printf, 1, 2))); diff -puN include/linux/spinlock.h~spinlocking-fixes include/linux/spinlock.h --- 25/include/linux/spinlock.h~spinlocking-fixes Wed Jan 19 15:24:55 2005 +++ 25-akpm/include/linux/spinlock.h Wed Jan 19 15:24:55 2005 @@ -202,10 +202,12 @@ typedef struct { #define _raw_spin_lock(lock) do { (void)(lock); } while(0) #define spin_is_locked(lock) ((void)(lock), 0) #define _raw_spin_trylock(lock) (((void)(lock), 1)) -#define spin_unlock_wait(lock) (void)(lock); +#define spin_unlock_wait(lock) (void)(lock) #define _raw_spin_unlock(lock) do { (void)(lock); } while(0) #endif /* CONFIG_DEBUG_SPINLOCK */ +#define spinlock_yield(lock) (void)(lock) + /* RW spinlocks: No debug version */ #if (__GNUC__ > 2) @@ -224,6 +226,8 @@ typedef struct { #define _raw_read_trylock(lock) ({ (void)(lock); (1); }) #define _raw_write_trylock(lock) ({ (void)(lock); (1); }) +#define rwlock_yield(lock) (void)(lock) + #define _spin_trylock(lock) ({preempt_disable(); _raw_spin_trylock(lock) ? \ 1 : ({preempt_enable(); 0;});}) diff -puN kernel/exit.c~spinlocking-fixes kernel/exit.c --- 25/kernel/exit.c~spinlocking-fixes Wed Jan 19 15:24:55 2005 +++ 25-akpm/kernel/exit.c Wed Jan 19 15:24:55 2005 @@ -861,8 +861,12 @@ task_t fastcall *next_thread(const task_ #ifdef CONFIG_SMP if (!p->sighand) BUG(); +#ifndef write_is_locked +# warning please implement read_is_locked()/write_is_locked()! +# define write_is_locked rwlock_is_locked +#endif if (!spin_is_locked(&p->sighand->siglock) && - !rwlock_is_locked(&tasklist_lock)) + !write_is_locked(&tasklist_lock)) BUG(); #endif return pid_task(p->pids[PIDTYPE_TGID].pid_list.next, PIDTYPE_TGID); diff -puN kernel/spinlock.c~spinlocking-fixes kernel/spinlock.c --- 25/kernel/spinlock.c~spinlocking-fixes Wed Jan 19 15:24:55 2005 +++ 25-akpm/kernel/spinlock.c Wed Jan 19 15:24:55 2005 @@ -173,8 +173,8 @@ EXPORT_SYMBOL(_write_lock); * (We do this in a function because inlining it would be excessive.) */ -#define BUILD_LOCK_OPS(op, locktype, is_locked_fn) \ -void __lockfunc _##op##_lock(locktype *lock) \ +#define BUILD_LOCK_OPS(op, locktype) \ +void __lockfunc _##op##_lock(locktype##_t *lock) \ { \ preempt_disable(); \ for (;;) { \ @@ -183,15 +183,15 @@ void __lockfunc _##op##_lock(locktype *l preempt_enable(); \ if (!(lock)->break_lock) \ (lock)->break_lock = 1; \ - while (is_locked_fn(lock) && (lock)->break_lock) \ - cpu_relax(); \ + while (op##_is_locked(lock) && (lock)->break_lock) \ + locktype##_yield(lock); \ preempt_disable(); \ } \ } \ \ EXPORT_SYMBOL(_##op##_lock); \ \ -unsigned long __lockfunc _##op##_lock_irqsave(locktype *lock) \ +unsigned long __lockfunc _##op##_lock_irqsave(locktype##_t *lock) \ { \ unsigned long flags; \ \ @@ -205,8 +205,8 @@ unsigned long __lockfunc _##op##_lock_ir preempt_enable(); \ if (!(lock)->break_lock) \ (lock)->break_lock = 1; \ - while (is_locked_fn(lock) && (lock)->break_lock) \ - cpu_relax(); \ + while (op##_is_locked(lock) && (lock)->break_lock) \ + locktype##_yield(lock); \ preempt_disable(); \ } \ return flags; \ @@ -214,14 +214,14 @@ unsigned long __lockfunc _##op##_lock_ir \ EXPORT_SYMBOL(_##op##_lock_irqsave); \ \ -void __lockfunc _##op##_lock_irq(locktype *lock) \ +void __lockfunc _##op##_lock_irq(locktype##_t *lock) \ { \ _##op##_lock_irqsave(lock); \ } \ \ EXPORT_SYMBOL(_##op##_lock_irq); \ \ -void __lockfunc _##op##_lock_bh(locktype *lock) \ +void __lockfunc _##op##_lock_bh(locktype##_t *lock) \ { \ unsigned long flags; \ \ @@ -246,9 +246,9 @@ EXPORT_SYMBOL(_##op##_lock_bh) * _[spin|read|write]_lock_irqsave() * _[spin|read|write]_lock_bh() */ -BUILD_LOCK_OPS(spin, spinlock_t, spin_is_locked); -BUILD_LOCK_OPS(read, rwlock_t, rwlock_is_locked); -BUILD_LOCK_OPS(write, rwlock_t, spin_is_locked); +BUILD_LOCK_OPS(spin, spinlock); +BUILD_LOCK_OPS(read, rwlock); +BUILD_LOCK_OPS(write, rwlock); #endif /* CONFIG_PREEMPT */ _