public inbox for linux-arch@vger.kernel.org
 help / color / mirror / Atom feed
* [patch 0/4] Allow inlined spinlocks again V2
@ 2009-08-11 12:47 Heiko Carstens
  2009-08-11 12:47 ` [patch 1/4] spinlock: move spinlock function bodies to header file Heiko Carstens
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Heiko Carstens @ 2009-08-11 12:47 UTC (permalink / raw)
  To: Andrew Morton, Linus Torvalds, Peter Zijlstra, Ingo Molnar
  Cc: linux-arch, Martin Schwidefsky, Heiko Carstens

This patch set allows to have inlined spinlocks again.

V2: rewritten from scratch - now with readable code

The rationale behind this is that function calls on at least s390 are
expensive.

If one considers that server kernels are usually compiled with
!CONFIG_PREEMPT a simple spin_lock is just a compare and swap loop.
The extra overhead for a function call is significant.
With inlined spinlocks overall cpu usage gets reduced by 1%-5% on s390.
These numbers were taken with some network benchmarks. However I expect
any workload that calls frequently into the kernel and which grabs a few
locks to perform better.

The implementation is straight forward: move the function bodies of the
locking functions to static inline functions and place them in a header
file.
Dependent on CONFIG_SPINLOCK_INLINE generate out-of-line or inlined
locking functions.

The patches should be self explaining.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [patch 1/4] spinlock: move spinlock function bodies to header file
  2009-08-11 12:47 [patch 0/4] Allow inlined spinlocks again V2 Heiko Carstens
@ 2009-08-11 12:47 ` Heiko Carstens
  2009-08-11 12:47 ` [patch 2/4] spinlock: add macro to generate out-of-line variants Heiko Carstens
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Heiko Carstens @ 2009-08-11 12:47 UTC (permalink / raw)
  To: Andrew Morton, Linus Torvalds, Peter Zijlstra, Ingo Molnar
  Cc: linux-arch, Martin Schwidefsky, Heiko Carstens

[-- Attachment #1: 01_spinlock_move.diff --]
[-- Type: text/plain, Size: 17056 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Move spinlock function bodies to header file by creating a
static inline version of each variant.
Use the inline version in the out-of-line code. This shouldn't
make any difference besides that the locking code can now
be used to generate inlined code.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 include/linux/spinlock.h         |   18 +-
 include/linux/spinlock_api_smp.h |  263 +++++++++++++++++++++++++++++++++++++++
 kernel/spinlock.c                |  174 ++++---------------------
 3 files changed, 300 insertions(+), 155 deletions(-)

Index: linux-2.6/include/linux/spinlock_api_smp.h
===================================================================
--- linux-2.6.orig/include/linux/spinlock_api_smp.h
+++ linux-2.6/include/linux/spinlock_api_smp.h
@@ -60,4 +60,267 @@ void __lockfunc _read_unlock_irqrestore(
 void __lockfunc _write_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
 							__releases(lock);
 
+static inline int __spin_trylock(spinlock_t *lock)
+{
+	preempt_disable();
+	if (_raw_spin_trylock(lock)) {
+		spin_acquire(&lock->dep_map, 0, 1, _RET_IP_);
+		return 1;
+	}
+	preempt_enable();
+	return 0;
+}
+
+static inline int __read_trylock(rwlock_t *lock)
+{
+	preempt_disable();
+	if (_raw_read_trylock(lock)) {
+		rwlock_acquire_read(&lock->dep_map, 0, 1, _RET_IP_);
+		return 1;
+	}
+	preempt_enable();
+	return 0;
+}
+
+static inline int __write_trylock(rwlock_t *lock)
+{
+	preempt_disable();
+	if (_raw_write_trylock(lock)) {
+		rwlock_acquire(&lock->dep_map, 0, 1, _RET_IP_);
+		return 1;
+	}
+	preempt_enable();
+	return 0;
+}
+
+/*
+ * If lockdep is enabled then we use the non-preemption spin-ops
+ * even on CONFIG_PREEMPT, because lockdep assumes that interrupts are
+ * not re-enabled during lock-acquire (which the preempt-spin-ops do):
+ */
+#if !defined(CONFIG_GENERIC_LOCKBREAK) || defined(CONFIG_DEBUG_LOCK_ALLOC)
+
+static inline void __read_lock(rwlock_t *lock)
+{
+	preempt_disable();
+	rwlock_acquire_read(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED(lock, _raw_read_trylock, _raw_read_lock);
+}
+
+static inline unsigned long __spin_lock_irqsave(spinlock_t *lock)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	preempt_disable();
+	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
+	/*
+	 * On lockdep we dont want the hand-coded irq-enable of
+	 * _raw_spin_lock_flags() code, because lockdep assumes
+	 * that interrupts are not re-enabled during lock-acquire:
+	 */
+#ifdef CONFIG_LOCKDEP
+	LOCK_CONTENDED(lock, _raw_spin_trylock, _raw_spin_lock);
+#else
+	_raw_spin_lock_flags(lock, &flags);
+#endif
+	return flags;
+}
+
+static inline void __spin_lock_irq(spinlock_t *lock)
+{
+	local_irq_disable();
+	preempt_disable();
+	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED(lock, _raw_spin_trylock, _raw_spin_lock);
+}
+
+static inline void __spin_lock_bh(spinlock_t *lock)
+{
+	local_bh_disable();
+	preempt_disable();
+	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED(lock, _raw_spin_trylock, _raw_spin_lock);
+}
+
+static inline unsigned long __read_lock_irqsave(rwlock_t *lock)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	preempt_disable();
+	rwlock_acquire_read(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED_FLAGS(lock, _raw_read_trylock, _raw_read_lock,
+			     _raw_read_lock_flags, &flags);
+	return flags;
+}
+
+static inline void __read_lock_irq(rwlock_t *lock)
+{
+	local_irq_disable();
+	preempt_disable();
+	rwlock_acquire_read(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED(lock, _raw_read_trylock, _raw_read_lock);
+}
+
+static inline void __read_lock_bh(rwlock_t *lock)
+{
+	local_bh_disable();
+	preempt_disable();
+	rwlock_acquire_read(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED(lock, _raw_read_trylock, _raw_read_lock);
+}
+
+static inline unsigned long __write_lock_irqsave(rwlock_t *lock)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	preempt_disable();
+	rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED_FLAGS(lock, _raw_write_trylock, _raw_write_lock,
+			     _raw_write_lock_flags, &flags);
+	return flags;
+}
+
+static inline void __write_lock_irq(rwlock_t *lock)
+{
+	local_irq_disable();
+	preempt_disable();
+	rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED(lock, _raw_write_trylock, _raw_write_lock);
+}
+
+static inline void __write_lock_bh(rwlock_t *lock)
+{
+	local_bh_disable();
+	preempt_disable();
+	rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED(lock, _raw_write_trylock, _raw_write_lock);
+}
+
+static inline void __spin_lock(spinlock_t *lock)
+{
+	preempt_disable();
+	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED(lock, _raw_spin_trylock, _raw_spin_lock);
+}
+
+static inline void __write_lock(rwlock_t *lock)
+{
+	preempt_disable();
+	rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED(lock, _raw_write_trylock, _raw_write_lock);
+}
+
+#endif /* CONFIG_PREEMPT */
+
+static inline void __spin_unlock(spinlock_t *lock)
+{
+	spin_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_spin_unlock(lock);
+	preempt_enable();
+}
+
+static inline void __write_unlock(rwlock_t *lock)
+{
+	rwlock_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_write_unlock(lock);
+	preempt_enable();
+}
+
+static inline void __read_unlock(rwlock_t *lock)
+{
+	rwlock_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_read_unlock(lock);
+	preempt_enable();
+}
+
+static inline void __spin_unlock_irqrestore(spinlock_t *lock,
+					    unsigned long flags)
+{
+	spin_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_spin_unlock(lock);
+	local_irq_restore(flags);
+	preempt_enable();
+}
+
+static inline void __spin_unlock_irq(spinlock_t *lock)
+{
+	spin_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_spin_unlock(lock);
+	local_irq_enable();
+	preempt_enable();
+}
+
+static inline void __spin_unlock_bh(spinlock_t *lock)
+{
+	spin_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_spin_unlock(lock);
+	preempt_enable_no_resched();
+	local_bh_enable_ip((unsigned long)__builtin_return_address(0));
+}
+
+static inline void __read_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
+{
+	rwlock_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_read_unlock(lock);
+	local_irq_restore(flags);
+	preempt_enable();
+}
+
+static inline void __read_unlock_irq(rwlock_t *lock)
+{
+	rwlock_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_read_unlock(lock);
+	local_irq_enable();
+	preempt_enable();
+}
+
+static inline void __read_unlock_bh(rwlock_t *lock)
+{
+	rwlock_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_read_unlock(lock);
+	preempt_enable_no_resched();
+	local_bh_enable_ip((unsigned long)__builtin_return_address(0));
+}
+
+static inline void __write_unlock_irqrestore(rwlock_t *lock,
+					     unsigned long flags)
+{
+	rwlock_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_write_unlock(lock);
+	local_irq_restore(flags);
+	preempt_enable();
+}
+
+static inline void __write_unlock_irq(rwlock_t *lock)
+{
+	rwlock_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_write_unlock(lock);
+	local_irq_enable();
+	preempt_enable();
+}
+
+static inline void __write_unlock_bh(rwlock_t *lock)
+{
+	rwlock_release(&lock->dep_map, 1, _RET_IP_);
+	_raw_write_unlock(lock);
+	preempt_enable_no_resched();
+	local_bh_enable_ip((unsigned long)__builtin_return_address(0));
+}
+
+static inline int __spin_trylock_bh(spinlock_t *lock)
+{
+	local_bh_disable();
+	preempt_disable();
+	if (_raw_spin_trylock(lock)) {
+		spin_acquire(&lock->dep_map, 0, 1, _RET_IP_);
+		return 1;
+	}
+	preempt_enable_no_resched();
+	local_bh_enable_ip((unsigned long)__builtin_return_address(0));
+	return 0;
+}
+
 #endif /* __LINUX_SPINLOCK_API_SMP_H */
Index: linux-2.6/include/linux/spinlock.h
===================================================================
--- linux-2.6.orig/include/linux/spinlock.h
+++ linux-2.6/include/linux/spinlock.h
@@ -143,15 +143,6 @@ static inline void smp_mb__after_lock(vo
  */
 #define spin_unlock_wait(lock)	__raw_spin_unlock_wait(&(lock)->raw_lock)
 
-/*
- * Pull the _spin_*()/_read_*()/_write_*() functions/declarations:
- */
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-# include <linux/spinlock_api_smp.h>
-#else
-# include <linux/spinlock_api_up.h>
-#endif
-
 #ifdef CONFIG_DEBUG_SPINLOCK
  extern void _raw_spin_lock(spinlock_t *lock);
 #define _raw_spin_lock_flags(lock, flags) _raw_spin_lock(lock)
@@ -380,4 +371,13 @@ extern int _atomic_dec_and_lock(atomic_t
  */
 #define spin_can_lock(lock)	(!spin_is_locked(lock))
 
+/*
+ * Pull the _spin_*()/_read_*()/_write_*() functions/declarations:
+ */
+#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
+# include <linux/spinlock_api_smp.h>
+#else
+# include <linux/spinlock_api_up.h>
+#endif
+
 #endif /* __LINUX_SPINLOCK_H */
Index: linux-2.6/kernel/spinlock.c
===================================================================
--- linux-2.6.orig/kernel/spinlock.c
+++ linux-2.6/kernel/spinlock.c
@@ -23,40 +23,19 @@
 
 int __lockfunc _spin_trylock(spinlock_t *lock)
 {
-	preempt_disable();
-	if (_raw_spin_trylock(lock)) {
-		spin_acquire(&lock->dep_map, 0, 1, _RET_IP_);
-		return 1;
-	}
-	
-	preempt_enable();
-	return 0;
+	return __spin_trylock(lock);
 }
 EXPORT_SYMBOL(_spin_trylock);
 
 int __lockfunc _read_trylock(rwlock_t *lock)
 {
-	preempt_disable();
-	if (_raw_read_trylock(lock)) {
-		rwlock_acquire_read(&lock->dep_map, 0, 1, _RET_IP_);
-		return 1;
-	}
-
-	preempt_enable();
-	return 0;
+	return __read_trylock(lock);
 }
 EXPORT_SYMBOL(_read_trylock);
 
 int __lockfunc _write_trylock(rwlock_t *lock)
 {
-	preempt_disable();
-	if (_raw_write_trylock(lock)) {
-		rwlock_acquire(&lock->dep_map, 0, 1, _RET_IP_);
-		return 1;
-	}
-
-	preempt_enable();
-	return 0;
+	return __write_trylock(lock);
 }
 EXPORT_SYMBOL(_write_trylock);
 
@@ -69,129 +48,74 @@ EXPORT_SYMBOL(_write_trylock);
 
 void __lockfunc _read_lock(rwlock_t *lock)
 {
-	preempt_disable();
-	rwlock_acquire_read(&lock->dep_map, 0, 0, _RET_IP_);
-	LOCK_CONTENDED(lock, _raw_read_trylock, _raw_read_lock);
+	__read_lock(lock);
 }
 EXPORT_SYMBOL(_read_lock);
 
 unsigned long __lockfunc _spin_lock_irqsave(spinlock_t *lock)
 {
-	unsigned long flags;
-
-	local_irq_save(flags);
-	preempt_disable();
-	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
-	/*
-	 * On lockdep we dont want the hand-coded irq-enable of
-	 * _raw_spin_lock_flags() code, because lockdep assumes
-	 * that interrupts are not re-enabled during lock-acquire:
-	 */
-#ifdef CONFIG_LOCKDEP
-	LOCK_CONTENDED(lock, _raw_spin_trylock, _raw_spin_lock);
-#else
-	_raw_spin_lock_flags(lock, &flags);
-#endif
-	return flags;
+	return __spin_lock_irqsave(lock);
 }
 EXPORT_SYMBOL(_spin_lock_irqsave);
 
 void __lockfunc _spin_lock_irq(spinlock_t *lock)
 {
-	local_irq_disable();
-	preempt_disable();
-	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
-	LOCK_CONTENDED(lock, _raw_spin_trylock, _raw_spin_lock);
+	__spin_lock_irq(lock);
 }
 EXPORT_SYMBOL(_spin_lock_irq);
 
 void __lockfunc _spin_lock_bh(spinlock_t *lock)
 {
-	local_bh_disable();
-	preempt_disable();
-	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
-	LOCK_CONTENDED(lock, _raw_spin_trylock, _raw_spin_lock);
+	__spin_lock_bh(lock);
 }
 EXPORT_SYMBOL(_spin_lock_bh);
 
 unsigned long __lockfunc _read_lock_irqsave(rwlock_t *lock)
 {
-	unsigned long flags;
-
-	local_irq_save(flags);
-	preempt_disable();
-	rwlock_acquire_read(&lock->dep_map, 0, 0, _RET_IP_);
-	LOCK_CONTENDED_FLAGS(lock, _raw_read_trylock, _raw_read_lock,
-			     _raw_read_lock_flags, &flags);
-	return flags;
+	return __read_lock_irqsave(lock);
 }
 EXPORT_SYMBOL(_read_lock_irqsave);
 
 void __lockfunc _read_lock_irq(rwlock_t *lock)
 {
-	local_irq_disable();
-	preempt_disable();
-	rwlock_acquire_read(&lock->dep_map, 0, 0, _RET_IP_);
-	LOCK_CONTENDED(lock, _raw_read_trylock, _raw_read_lock);
+	__read_lock_irq(lock);
 }
 EXPORT_SYMBOL(_read_lock_irq);
 
 void __lockfunc _read_lock_bh(rwlock_t *lock)
 {
-	local_bh_disable();
-	preempt_disable();
-	rwlock_acquire_read(&lock->dep_map, 0, 0, _RET_IP_);
-	LOCK_CONTENDED(lock, _raw_read_trylock, _raw_read_lock);
+	__read_lock_bh(lock);
 }
 EXPORT_SYMBOL(_read_lock_bh);
 
 unsigned long __lockfunc _write_lock_irqsave(rwlock_t *lock)
 {
-	unsigned long flags;
-
-	local_irq_save(flags);
-	preempt_disable();
-	rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_);
-	LOCK_CONTENDED_FLAGS(lock, _raw_write_trylock, _raw_write_lock,
-			     _raw_write_lock_flags, &flags);
-	return flags;
+	return __write_lock_irqsave(lock);
 }
 EXPORT_SYMBOL(_write_lock_irqsave);
 
 void __lockfunc _write_lock_irq(rwlock_t *lock)
 {
-	local_irq_disable();
-	preempt_disable();
-	rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_);
-	LOCK_CONTENDED(lock, _raw_write_trylock, _raw_write_lock);
+	__write_lock_irq(lock);
 }
 EXPORT_SYMBOL(_write_lock_irq);
 
 void __lockfunc _write_lock_bh(rwlock_t *lock)
 {
-	local_bh_disable();
-	preempt_disable();
-	rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_);
-	LOCK_CONTENDED(lock, _raw_write_trylock, _raw_write_lock);
+	__write_lock_bh(lock);
 }
 EXPORT_SYMBOL(_write_lock_bh);
 
 void __lockfunc _spin_lock(spinlock_t *lock)
 {
-	preempt_disable();
-	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
-	LOCK_CONTENDED(lock, _raw_spin_trylock, _raw_spin_lock);
+	__spin_lock(lock);
 }
-
 EXPORT_SYMBOL(_spin_lock);
 
 void __lockfunc _write_lock(rwlock_t *lock)
 {
-	preempt_disable();
-	rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_);
-	LOCK_CONTENDED(lock, _raw_write_trylock, _raw_write_lock);
+	__write_lock(lock);
 }
-
 EXPORT_SYMBOL(_write_lock);
 
 #else /* CONFIG_PREEMPT: */
@@ -320,121 +244,79 @@ EXPORT_SYMBOL(_spin_lock_nest_lock);
 
 void __lockfunc _spin_unlock(spinlock_t *lock)
 {
-	spin_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_spin_unlock(lock);
-	preempt_enable();
+	__spin_unlock(lock);
 }
 EXPORT_SYMBOL(_spin_unlock);
 
 void __lockfunc _write_unlock(rwlock_t *lock)
 {
-	rwlock_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_write_unlock(lock);
-	preempt_enable();
+	__write_unlock(lock);
 }
 EXPORT_SYMBOL(_write_unlock);
 
 void __lockfunc _read_unlock(rwlock_t *lock)
 {
-	rwlock_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_read_unlock(lock);
-	preempt_enable();
+	__read_unlock(lock);
 }
 EXPORT_SYMBOL(_read_unlock);
 
 void __lockfunc _spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags)
 {
-	spin_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_spin_unlock(lock);
-	local_irq_restore(flags);
-	preempt_enable();
+	__spin_unlock_irqrestore(lock, flags);
 }
 EXPORT_SYMBOL(_spin_unlock_irqrestore);
 
 void __lockfunc _spin_unlock_irq(spinlock_t *lock)
 {
-	spin_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_spin_unlock(lock);
-	local_irq_enable();
-	preempt_enable();
+	__spin_unlock_irq(lock);
 }
 EXPORT_SYMBOL(_spin_unlock_irq);
 
 void __lockfunc _spin_unlock_bh(spinlock_t *lock)
 {
-	spin_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_spin_unlock(lock);
-	preempt_enable_no_resched();
-	local_bh_enable_ip((unsigned long)__builtin_return_address(0));
+	__spin_unlock_bh(lock);
 }
 EXPORT_SYMBOL(_spin_unlock_bh);
 
 void __lockfunc _read_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
 {
-	rwlock_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_read_unlock(lock);
-	local_irq_restore(flags);
-	preempt_enable();
+	__read_unlock_irqrestore(lock, flags);
 }
 EXPORT_SYMBOL(_read_unlock_irqrestore);
 
 void __lockfunc _read_unlock_irq(rwlock_t *lock)
 {
-	rwlock_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_read_unlock(lock);
-	local_irq_enable();
-	preempt_enable();
+	__read_unlock_irq(lock);
 }
 EXPORT_SYMBOL(_read_unlock_irq);
 
 void __lockfunc _read_unlock_bh(rwlock_t *lock)
 {
-	rwlock_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_read_unlock(lock);
-	preempt_enable_no_resched();
-	local_bh_enable_ip((unsigned long)__builtin_return_address(0));
+	__read_unlock_bh(lock);
 }
 EXPORT_SYMBOL(_read_unlock_bh);
 
 void __lockfunc _write_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
 {
-	rwlock_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_write_unlock(lock);
-	local_irq_restore(flags);
-	preempt_enable();
+	__write_unlock_irqrestore(lock, flags);
 }
 EXPORT_SYMBOL(_write_unlock_irqrestore);
 
 void __lockfunc _write_unlock_irq(rwlock_t *lock)
 {
-	rwlock_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_write_unlock(lock);
-	local_irq_enable();
-	preempt_enable();
+	__write_unlock_irq(lock);
 }
 EXPORT_SYMBOL(_write_unlock_irq);
 
 void __lockfunc _write_unlock_bh(rwlock_t *lock)
 {
-	rwlock_release(&lock->dep_map, 1, _RET_IP_);
-	_raw_write_unlock(lock);
-	preempt_enable_no_resched();
-	local_bh_enable_ip((unsigned long)__builtin_return_address(0));
+	__write_unlock_bh(lock);
 }
 EXPORT_SYMBOL(_write_unlock_bh);
 
 int __lockfunc _spin_trylock_bh(spinlock_t *lock)
 {
-	local_bh_disable();
-	preempt_disable();
-	if (_raw_spin_trylock(lock)) {
-		spin_acquire(&lock->dep_map, 0, 1, _RET_IP_);
-		return 1;
-	}
-
-	preempt_enable_no_resched();
-	local_bh_enable_ip((unsigned long)__builtin_return_address(0));
-	return 0;
+	return __spin_trylock_bh(lock);
 }
 EXPORT_SYMBOL(_spin_trylock_bh);
 

-- 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [patch 2/4] spinlock: add macro to generate out-of-line variants
  2009-08-11 12:47 [patch 0/4] Allow inlined spinlocks again V2 Heiko Carstens
  2009-08-11 12:47 ` [patch 1/4] spinlock: move spinlock function bodies to header file Heiko Carstens
@ 2009-08-11 12:47 ` Heiko Carstens
  2009-08-11 13:25   ` Arnd Bergmann
  2009-08-11 12:47 ` [patch 3/4] spinlock: allow inlined spinlocks Heiko Carstens
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Heiko Carstens @ 2009-08-11 12:47 UTC (permalink / raw)
  To: Andrew Morton, Linus Torvalds, Peter Zijlstra, Ingo Molnar
  Cc: linux-arch, Martin Schwidefsky, Heiko Carstens

[-- Attachment #1: 02_spinlock_macros.diff --]
[-- Type: text/plain, Size: 7639 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Since the bodies of the spinlock functions are in a header
file most functions in spinlock.c look like this:

int __lockfunc _spin_trylock(spinlock_t *lock)
{
	return __spin_trylock(lock);
}
EXPORT_SYMBOL(_spin_trylock);

That's just a simple wrapper. Its the same for spin-,
read- and write-lock. So add an extra macro and generate
all versions automatically like it is already done for
the preemption friendly locks.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 kernel/spinlock.c |  233 +++++++++++++++---------------------------------------
 1 file changed, 68 insertions(+), 165 deletions(-)

Index: linux-2.6/kernel/spinlock.c
===================================================================
--- linux-2.6.orig/kernel/spinlock.c
+++ linux-2.6/kernel/spinlock.c
@@ -21,23 +21,37 @@
 #include <linux/debug_locks.h>
 #include <linux/module.h>
 
-int __lockfunc _spin_trylock(spinlock_t *lock)
-{
-	return __spin_trylock(lock);
-}
-EXPORT_SYMBOL(_spin_trylock);
-
-int __lockfunc _read_trylock(rwlock_t *lock)
-{
-	return __read_trylock(lock);
-}
-EXPORT_SYMBOL(_read_trylock);
-
-int __lockfunc _write_trylock(rwlock_t *lock)
-{
-	return __write_trylock(lock);
-}
-EXPORT_SYMBOL(_write_trylock);
+#define BUILD_LOCK_OPS_COMMON(op, locktype)				\
+int __lockfunc _##op##_trylock(locktype##_t *lock)			\
+{									\
+	return __##op##_trylock(lock);					\
+}									\
+EXPORT_SYMBOL(_##op##_trylock);						\
+									\
+void __lockfunc _##op##_unlock(locktype##_t *lock)			\
+{									\
+	__##op##_unlock(lock);						\
+}									\
+EXPORT_SYMBOL(_##op##_unlock);						\
+									\
+void __lockfunc _##op##_unlock_irq(locktype##_t *lock)			\
+{									\
+	__##op##_unlock_irq(lock);					\
+}									\
+EXPORT_SYMBOL(_##op##_unlock_irq);					\
+									\
+void __lockfunc _##op##_unlock_bh(locktype##_t *lock)			\
+{									\
+	__##op##_unlock_bh(lock);					\
+}									\
+EXPORT_SYMBOL(_##op##_unlock_bh);					\
+									\
+void __lockfunc _##op##_unlock_irqrestore(locktype##_t *lock,		\
+					  unsigned long flags)		\
+{									\
+	__##op##_unlock_irqrestore(lock, flags);			\
+}									\
+EXPORT_SYMBOL(_##op##_unlock_irqrestore)
 
 /*
  * If lockdep is enabled then we use the non-preemption spin-ops
@@ -46,77 +60,30 @@ EXPORT_SYMBOL(_write_trylock);
  */
 #if !defined(CONFIG_GENERIC_LOCKBREAK) || defined(CONFIG_DEBUG_LOCK_ALLOC)
 
-void __lockfunc _read_lock(rwlock_t *lock)
-{
-	__read_lock(lock);
-}
-EXPORT_SYMBOL(_read_lock);
-
-unsigned long __lockfunc _spin_lock_irqsave(spinlock_t *lock)
-{
-	return __spin_lock_irqsave(lock);
-}
-EXPORT_SYMBOL(_spin_lock_irqsave);
-
-void __lockfunc _spin_lock_irq(spinlock_t *lock)
-{
-	__spin_lock_irq(lock);
-}
-EXPORT_SYMBOL(_spin_lock_irq);
-
-void __lockfunc _spin_lock_bh(spinlock_t *lock)
-{
-	__spin_lock_bh(lock);
-}
-EXPORT_SYMBOL(_spin_lock_bh);
-
-unsigned long __lockfunc _read_lock_irqsave(rwlock_t *lock)
-{
-	return __read_lock_irqsave(lock);
-}
-EXPORT_SYMBOL(_read_lock_irqsave);
-
-void __lockfunc _read_lock_irq(rwlock_t *lock)
-{
-	__read_lock_irq(lock);
-}
-EXPORT_SYMBOL(_read_lock_irq);
-
-void __lockfunc _read_lock_bh(rwlock_t *lock)
-{
-	__read_lock_bh(lock);
-}
-EXPORT_SYMBOL(_read_lock_bh);
-
-unsigned long __lockfunc _write_lock_irqsave(rwlock_t *lock)
-{
-	return __write_lock_irqsave(lock);
-}
-EXPORT_SYMBOL(_write_lock_irqsave);
-
-void __lockfunc _write_lock_irq(rwlock_t *lock)
-{
-	__write_lock_irq(lock);
-}
-EXPORT_SYMBOL(_write_lock_irq);
-
-void __lockfunc _write_lock_bh(rwlock_t *lock)
-{
-	__write_lock_bh(lock);
-}
-EXPORT_SYMBOL(_write_lock_bh);
-
-void __lockfunc _spin_lock(spinlock_t *lock)
-{
-	__spin_lock(lock);
-}
-EXPORT_SYMBOL(_spin_lock);
-
-void __lockfunc _write_lock(rwlock_t *lock)
-{
-	__write_lock(lock);
-}
-EXPORT_SYMBOL(_write_lock);
+#define BUILD_LOCK_OPS_DEP(op, locktype)				\
+void __lockfunc _##op##_lock(locktype##_t *lock)			\
+{									\
+	__##op##_lock(lock);						\
+}									\
+EXPORT_SYMBOL(_##op##_lock);						\
+									\
+unsigned long __lockfunc _##op##_lock_irqsave(locktype##_t *lock)	\
+{									\
+	return __##op##_lock_irqsave(lock);				\
+}									\
+EXPORT_SYMBOL(_##op##_lock_irqsave);					\
+									\
+void __lockfunc _##op##_lock_irq(locktype##_t *lock)			\
+{									\
+	__##op##_lock_irq(lock);					\
+}									\
+EXPORT_SYMBOL(_##op##_lock_irq);					\
+									\
+void __lockfunc _##op##_lock_bh(locktype##_t *lock)			\
+{									\
+	__##op##_lock_bh(lock);						\
+}									\
+EXPORT_SYMBOL(_##op##_lock_bh)
 
 #else /* CONFIG_PREEMPT: */
 
@@ -128,7 +95,7 @@ EXPORT_SYMBOL(_write_lock);
  * (We do this in a function because inlining it would be excessive.)
  */
 
-#define BUILD_LOCK_OPS(op, locktype)					\
+#define BUILD_LOCK_OPS_DEP(op, locktype)				\
 void __lockfunc _##op##_lock(locktype##_t *lock)			\
 {									\
 	for (;;) {							\
@@ -193,21 +160,29 @@ void __lockfunc _##op##_lock_bh(locktype
 									\
 EXPORT_SYMBOL(_##op##_lock_bh)
 
+#endif /* CONFIG_PREEMPT */
+
+#define BUILD_LOCK_OPS(op, locktype)		\
+	BUILD_LOCK_OPS_COMMON(op, locktype);	\
+	BUILD_LOCK_OPS_DEP(op, locktype);
+
 /*
- * Build preemption-friendly versions of the following
- * lock-spinning functions:
+ * Build versions of the following lock-spinning functions:
  *
  *         _[spin|read|write]_lock()
  *         _[spin|read|write]_lock_irq()
  *         _[spin|read|write]_lock_irqsave()
  *         _[spin|read|write]_lock_bh()
+ *         _[spin|read|write]_trylock()
+ *         _[spin|read|write]_unlock()
+ *         _[spin|read|write]_unlock_irq()
+ *         _[spin|read|write]_unlock_irqrestore()
+ *         _[spin|read|write]_unlock_bh()
  */
 BUILD_LOCK_OPS(spin, spinlock);
 BUILD_LOCK_OPS(read, rwlock);
 BUILD_LOCK_OPS(write, rwlock);
 
-#endif /* CONFIG_PREEMPT */
-
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 
 void __lockfunc _spin_lock_nested(spinlock_t *lock, int subclass)
@@ -242,78 +217,6 @@ EXPORT_SYMBOL(_spin_lock_nest_lock);
 
 #endif
 
-void __lockfunc _spin_unlock(spinlock_t *lock)
-{
-	__spin_unlock(lock);
-}
-EXPORT_SYMBOL(_spin_unlock);
-
-void __lockfunc _write_unlock(rwlock_t *lock)
-{
-	__write_unlock(lock);
-}
-EXPORT_SYMBOL(_write_unlock);
-
-void __lockfunc _read_unlock(rwlock_t *lock)
-{
-	__read_unlock(lock);
-}
-EXPORT_SYMBOL(_read_unlock);
-
-void __lockfunc _spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags)
-{
-	__spin_unlock_irqrestore(lock, flags);
-}
-EXPORT_SYMBOL(_spin_unlock_irqrestore);
-
-void __lockfunc _spin_unlock_irq(spinlock_t *lock)
-{
-	__spin_unlock_irq(lock);
-}
-EXPORT_SYMBOL(_spin_unlock_irq);
-
-void __lockfunc _spin_unlock_bh(spinlock_t *lock)
-{
-	__spin_unlock_bh(lock);
-}
-EXPORT_SYMBOL(_spin_unlock_bh);
-
-void __lockfunc _read_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
-{
-	__read_unlock_irqrestore(lock, flags);
-}
-EXPORT_SYMBOL(_read_unlock_irqrestore);
-
-void __lockfunc _read_unlock_irq(rwlock_t *lock)
-{
-	__read_unlock_irq(lock);
-}
-EXPORT_SYMBOL(_read_unlock_irq);
-
-void __lockfunc _read_unlock_bh(rwlock_t *lock)
-{
-	__read_unlock_bh(lock);
-}
-EXPORT_SYMBOL(_read_unlock_bh);
-
-void __lockfunc _write_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
-{
-	__write_unlock_irqrestore(lock, flags);
-}
-EXPORT_SYMBOL(_write_unlock_irqrestore);
-
-void __lockfunc _write_unlock_irq(rwlock_t *lock)
-{
-	__write_unlock_irq(lock);
-}
-EXPORT_SYMBOL(_write_unlock_irq);
-
-void __lockfunc _write_unlock_bh(rwlock_t *lock)
-{
-	__write_unlock_bh(lock);
-}
-EXPORT_SYMBOL(_write_unlock_bh);
-
 int __lockfunc _spin_trylock_bh(spinlock_t *lock)
 {
 	return __spin_trylock_bh(lock);

-- 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [patch 3/4] spinlock: allow inlined spinlocks
  2009-08-11 12:47 [patch 0/4] Allow inlined spinlocks again V2 Heiko Carstens
  2009-08-11 12:47 ` [patch 1/4] spinlock: move spinlock function bodies to header file Heiko Carstens
  2009-08-11 12:47 ` [patch 2/4] spinlock: add macro to generate out-of-line variants Heiko Carstens
@ 2009-08-11 12:47 ` Heiko Carstens
  2009-08-11 12:48 ` [patch 4/4] spinlock: allow inline spinlocks on s390 Heiko Carstens
  2009-08-11 13:00 ` [patch 0/4] Allow inlined spinlocks again V2 Peter Zijlstra
  4 siblings, 0 replies; 9+ messages in thread
From: Heiko Carstens @ 2009-08-11 12:47 UTC (permalink / raw)
  To: Andrew Morton, Linus Torvalds, Peter Zijlstra, Ingo Molnar
  Cc: linux-arch, Martin Schwidefsky, Heiko Carstens

[-- Attachment #1: 03_spinlock_inline.diff --]
[-- Type: text/plain, Size: 4620 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Add new config option SPINLOCK_INLINE and some defines which depend
on it in order to generate inlined spinlock code instead of out-of-line
code.
Avoiding function calls for spinlocks gives 1%-5% less cpu usage on
network benchmarks on s390.

Architectures must select HAVE_SPINLOCK_INLINE_SUPPORT to enable this
config option.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 include/linux/spinlock_api_smp.h |   35 +++++++++++++++++++++++++++++++++++
 kernel/spinlock.c                |    4 ++++
 lib/Kconfig.debug                |   14 ++++++++++++++
 3 files changed, 53 insertions(+)

Index: linux-2.6/include/linux/spinlock_api_smp.h
===================================================================
--- linux-2.6.orig/include/linux/spinlock_api_smp.h
+++ linux-2.6/include/linux/spinlock_api_smp.h
@@ -19,6 +19,8 @@ int in_lock_functions(unsigned long addr
 
 #define assert_spin_locked(x)	BUG_ON(!spin_is_locked(x))
 
+#ifndef CONFIG_SPINLOCK_INLINE
+
 void __lockfunc _spin_lock(spinlock_t *lock)		__acquires(lock);
 void __lockfunc _spin_lock_nested(spinlock_t *lock, int subclass)
 							__acquires(lock);
@@ -60,6 +62,39 @@ void __lockfunc _read_unlock_irqrestore(
 void __lockfunc _write_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
 							__releases(lock);
 
+#else /* CONFIG_HAVE_SPINLOCK_INLINE_SUPPORT */
+
+#define _spin_trylock(lock)	  __spin_trylock(lock)
+#define _read_trylock(lock)	  __read_trylock(lock)
+#define _write_trylock(lock)	  __write_trylock(lock)
+#define _read_lock(lock)	  __read_lock(lock)
+#define _spin_lock_irqsave(lock)  __spin_lock_irqsave(lock)
+#define _spin_lock_irq(lock)	  __spin_lock_irq(lock)
+#define _spin_lock_bh(lock)	  __spin_lock_bh(lock)
+#define _read_lock_irqsave(lock)  __read_lock_irqsave(lock)
+#define _read_lock_irq(lock)	  __read_lock_irq(lock)
+#define _read_lock_bh(lock)	  __read_lock_bh(lock)
+#define _write_lock_irqsave(lock) __write_lock_irqsave(lock)
+#define _write_lock_irq(lock)	  __write_lock_irq(lock)
+#define _write_lock_bh(lock)	  __write_lock_bh(lock)
+#define _spin_lock(lock)	  __spin_lock(lock)
+#define _write_lock(lock)	  __write_lock(lock)
+#define _spin_unlock(lock)	  __spin_unlock(lock)
+#define _write_unlock(lock)	  __write_unlock(lock)
+#define _read_unlock(lock)	  __read_unlock(lock)
+#define _spin_unlock_irq(lock)	  __spin_unlock_irq(lock)
+#define _spin_unlock_bh(lock)	  __spin_unlock_bh(lock)
+#define _read_unlock_irq(lock)	  __read_unlock_irq(lock)
+#define _read_unlock_bh(lock)	  __read_unlock_bh(lock)
+#define _write_unlock_irq(lock)	  __write_unlock_irq(lock)
+#define _write_unlock_bh(lock)	  __write_unlock_bh(lock)
+#define _spin_trylock_bh(lock)	  __spin_trylock_bh(lock)
+#define _spin_unlock_irqrestore(lock, flags)  __spin_unlock_irqrestore(lock, flags)
+#define _read_unlock_irqrestore(lock, flags)  __read_unlock_irqrestore(lock, flags)
+#define _write_unlock_irqrestore(lock, flags) __write_unlock_irqrestore(lock, flags)
+
+#endif /* CONFIG_HAVE_SPINLOCK_INLINE_SUPPORT */
+
 static inline int __spin_trylock(spinlock_t *lock)
 {
 	preempt_disable();
Index: linux-2.6/lib/Kconfig.debug
===================================================================
--- linux-2.6.orig/lib/Kconfig.debug
+++ linux-2.6/lib/Kconfig.debug
@@ -879,6 +879,20 @@ config SYSCTL_SYSCALL_CHECK
 	  to properly maintain and use. This enables checks that help
 	  you to keep things correct.
 
+config HAVE_SPINLOCK_INLINE_SUPPORT
+	bool
+
+config SPINLOCK_INLINE
+	bool "Inline spinlock code"
+	depends on HAVE_SPINLOCK_INLINE_SUPPORT
+	depends on !DEBUG_SPINLOCK
+	depends on SMP && !PREEMPT
+	help
+	  Select this option if you want to have inline spinlocks instead of
+	  an out of line implementation.
+	  This will generate a larger kernel image. On some architectures this
+	  increases performance.
+
 source mm/Kconfig.debug
 source kernel/trace/Kconfig
 
Index: linux-2.6/kernel/spinlock.c
===================================================================
--- linux-2.6.orig/kernel/spinlock.c
+++ linux-2.6/kernel/spinlock.c
@@ -21,6 +21,8 @@
 #include <linux/debug_locks.h>
 #include <linux/module.h>
 
+#ifndef CONFIG_SPINLOCK_INLINE
+
 #define BUILD_LOCK_OPS_COMMON(op, locktype)				\
 int __lockfunc _##op##_trylock(locktype##_t *lock)			\
 {									\
@@ -223,6 +225,8 @@ int __lockfunc _spin_trylock_bh(spinlock
 }
 EXPORT_SYMBOL(_spin_trylock_bh);
 
+#endif /* CONFIG_HAVE_SPINLOCK_INLINE_SUPPORT */
+
 notrace int in_lock_functions(unsigned long addr)
 {
 	/* Linker adds these: start and end of __lockfunc functions */

-- 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [patch 4/4] spinlock: allow inline spinlocks on s390
  2009-08-11 12:47 [patch 0/4] Allow inlined spinlocks again V2 Heiko Carstens
                   ` (2 preceding siblings ...)
  2009-08-11 12:47 ` [patch 3/4] spinlock: allow inlined spinlocks Heiko Carstens
@ 2009-08-11 12:48 ` Heiko Carstens
  2009-08-11 13:00 ` [patch 0/4] Allow inlined spinlocks again V2 Peter Zijlstra
  4 siblings, 0 replies; 9+ messages in thread
From: Heiko Carstens @ 2009-08-11 12:48 UTC (permalink / raw)
  To: Andrew Morton, Linus Torvalds, Peter Zijlstra, Ingo Molnar
  Cc: linux-arch, Martin Schwidefsky, Heiko Carstens

[-- Attachment #1: 04_spinlock_s390.diff --]
[-- Type: text/plain, Size: 590 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 arch/s390/Kconfig |    1 +
 1 file changed, 1 insertion(+)

Index: linux-2.6/arch/s390/Kconfig
===================================================================
--- linux-2.6.orig/arch/s390/Kconfig
+++ linux-2.6/arch/s390/Kconfig
@@ -93,6 +93,7 @@ config S390
 	select HAVE_KRETPROBES
 	select HAVE_KVM if 64BIT
 	select HAVE_ARCH_TRACEHOOK
+	select HAVE_SPINLOCK_INLINE_SUPPORT
 	select INIT_ALL_POSSIBLE
 	select HAVE_PERF_COUNTERS
 	select GENERIC_ATOMIC64 if !64BIT

-- 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch 0/4] Allow inlined spinlocks again V2
  2009-08-11 12:47 [patch 0/4] Allow inlined spinlocks again V2 Heiko Carstens
                   ` (3 preceding siblings ...)
  2009-08-11 12:48 ` [patch 4/4] spinlock: allow inline spinlocks on s390 Heiko Carstens
@ 2009-08-11 13:00 ` Peter Zijlstra
  4 siblings, 0 replies; 9+ messages in thread
From: Peter Zijlstra @ 2009-08-11 13:00 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Andrew Morton, Linus Torvalds, Ingo Molnar, linux-arch,
	Martin Schwidefsky, David Miller

On Tue, 2009-08-11 at 14:47 +0200, Heiko Carstens wrote:
> This patch set allows to have inlined spinlocks again.
> 
> V2: rewritten from scratch - now with readable code
> 
> The rationale behind this is that function calls on at least s390 are
> expensive.
> 
> If one considers that server kernels are usually compiled with
> !CONFIG_PREEMPT a simple spin_lock is just a compare and swap loop.
> The extra overhead for a function call is significant.
> With inlined spinlocks overall cpu usage gets reduced by 1%-5% on s390.
> These numbers were taken with some network benchmarks. However I expect
> any workload that calls frequently into the kernel and which grabs a few
> locks to perform better.
> 
> The implementation is straight forward: move the function bodies of the
> locking functions to static inline functions and place them in a header
> file.
> Dependent on CONFIG_SPINLOCK_INLINE generate out-of-line or inlined
> locking functions.
> 
> The patches should be self explaining.

These look lots better than the previous series ;-)

Given that you've got a significant performance gain from this and it
doesn't look too horrible anymore,

Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

IIRC sparc64 also has a funny calling convention, so it might be of
interest to DaveM as well.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch 2/4] spinlock: add macro to generate out-of-line variants
  2009-08-11 12:47 ` [patch 2/4] spinlock: add macro to generate out-of-line variants Heiko Carstens
@ 2009-08-11 13:25   ` Arnd Bergmann
  2009-08-11 13:35     ` Peter Zijlstra
  2009-08-11 16:56     ` Heiko Carstens
  0 siblings, 2 replies; 9+ messages in thread
From: Arnd Bergmann @ 2009-08-11 13:25 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Andrew Morton, Linus Torvalds, Peter Zijlstra, Ingo Molnar,
	linux-arch, Martin Schwidefsky

On Tuesday 11 August 2009, Heiko Carstens wrote:
> Since the bodies of the spinlock functions are in a header
> file most functions in spinlock.c look like this:
> 
> int __lockfunc _spin_trylock(spinlock_t *lock)
> {
>         return __spin_trylock(lock);
> }
> EXPORT_SYMBOL(_spin_trylock);
> 
> That's just a simple wrapper. Its the same for spin-,
> read- and write-lock. So add an extra macro and generate
> all versions automatically like it is already done for
> the preemption friendly locks.
> 
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>

If you generate function definitions from macros, you break
ctags support for following the call chain, which is rather
bad when someone tries to understand what the code does.

I would just leave out this patch, AFAICT there are no
dependencies between this and the following patches,
and the object code remains identical.

Alternatively, you could perhaps change scripts/tags.sh so
that the tags file points to the macro location for
each _spin_* function.

The other patches look good.

	Arnd <><

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch 2/4] spinlock: add macro to generate out-of-line variants
  2009-08-11 13:25   ` Arnd Bergmann
@ 2009-08-11 13:35     ` Peter Zijlstra
  2009-08-11 16:56     ` Heiko Carstens
  1 sibling, 0 replies; 9+ messages in thread
From: Peter Zijlstra @ 2009-08-11 13:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiko Carstens, Andrew Morton, Linus Torvalds, Ingo Molnar,
	linux-arch, Martin Schwidefsky

On Tue, 2009-08-11 at 15:25 +0200, Arnd Bergmann wrote:
> On Tuesday 11 August 2009, Heiko Carstens wrote:
> > Since the bodies of the spinlock functions are in a header
> > file most functions in spinlock.c look like this:
> > 
> > int __lockfunc _spin_trylock(spinlock_t *lock)
> > {
> >         return __spin_trylock(lock);
> > }
> > EXPORT_SYMBOL(_spin_trylock);
> > 
> > That's just a simple wrapper. Its the same for spin-,
> > read- and write-lock. So add an extra macro and generate
> > all versions automatically like it is already done for
> > the preemption friendly locks.
> > 
> > Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> 
> If you generate function definitions from macros, you break
> ctags support for following the call chain, which is rather
> bad when someone tries to understand what the code does.
> 
> I would just leave out this patch, AFAICT there are no
> dependencies between this and the following patches,
> and the object code remains identical.
> 
> Alternatively, you could perhaps change scripts/tags.sh so
> that the tags file points to the macro location for
> each _spin_* function.
> 
> The other patches look good.

Ah, good point, I wish someone would teach that ctags script about the
paravirt crap, but I'm afraid paravirt is too inconsistent to begin
with.

/me itches again to send a patch removing it all-together ;-)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch 2/4] spinlock: add macro to generate out-of-line variants
  2009-08-11 13:25   ` Arnd Bergmann
  2009-08-11 13:35     ` Peter Zijlstra
@ 2009-08-11 16:56     ` Heiko Carstens
  1 sibling, 0 replies; 9+ messages in thread
From: Heiko Carstens @ 2009-08-11 16:56 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, Linus Torvalds, Peter Zijlstra, Ingo Molnar,
	linux-arch, Martin Schwidefsky

On Tue, Aug 11, 2009 at 03:25:26PM +0200, Arnd Bergmann wrote:
> On Tuesday 11 August 2009, Heiko Carstens wrote:
> > Since the bodies of the spinlock functions are in a header
> > file most functions in spinlock.c look like this:
> > 
> > int __lockfunc _spin_trylock(spinlock_t *lock)
> > {
> >         return __spin_trylock(lock);
> > }
> > EXPORT_SYMBOL(_spin_trylock);
> > 
> > That's just a simple wrapper. Its the same for spin-,
> > read- and write-lock. So add an extra macro and generate
> > all versions automatically like it is already done for
> > the preemption friendly locks.
> > 
> > Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> 
> If you generate function definitions from macros, you break
> ctags support for following the call chain, which is rather
> bad when someone tries to understand what the code does.
> 
> I would just leave out this patch, AFAICT there are no
> dependencies between this and the following patches,
> and the object code remains identical.

Ok, I just drop this patch. I wasn't aware this could cause problems,
since I don't use tags.

Thanks!

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2009-08-11 16:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-11 12:47 [patch 0/4] Allow inlined spinlocks again V2 Heiko Carstens
2009-08-11 12:47 ` [patch 1/4] spinlock: move spinlock function bodies to header file Heiko Carstens
2009-08-11 12:47 ` [patch 2/4] spinlock: add macro to generate out-of-line variants Heiko Carstens
2009-08-11 13:25   ` Arnd Bergmann
2009-08-11 13:35     ` Peter Zijlstra
2009-08-11 16:56     ` Heiko Carstens
2009-08-11 12:47 ` [patch 3/4] spinlock: allow inlined spinlocks Heiko Carstens
2009-08-11 12:48 ` [patch 4/4] spinlock: allow inline spinlocks on s390 Heiko Carstens
2009-08-11 13:00 ` [patch 0/4] Allow inlined spinlocks again V2 Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox