Skip to contentSkip to conversation

Allow inlined spinlocks again V2

PATCH In review Heiko Carstens 3 participants 9 messages Aug 11, 2009

Subsystems (4)
Library code s390 Generic include/asm header files Locking primitives
lore
Share & tools
lore

Heiko Carstens

author
#0 lore raw

cover Allow inlined spinlocks again V2

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.

author
#1 lore raw

PATCH 1/4 spinlock: move spinlock function bodies to header file

From: Heiko Carstens <[email protected]>

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 <[email protected]>
---
 include/linux/spinlock.h         |   18 +-
 include/linux/spinlock_api_smp.h |  263 +++++++++++++++++++++++++++++++++++++++
 kernel/spinlock.c                |  174 ++++---------------------
 3 files changed, 300 insertions(+), 155 deletions(-)
include/linux/spinlock_api_smp.h +263 -0
Index: 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 */
include/linux/spinlock.h +9 -9
Index: 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 */
kernel/spinlock.c +28 -146

318 lines not on this page · raw message

author
#2 lore raw

PATCH 2/4 spinlock: add macro to generate out-of-line variants

In reply to Heiko Carstens

From: Heiko Carstens <[email protected]>

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 <[email protected]>
---
 kernel/spinlock.c |  233 +++++++++++++++---------------------------------------
 1 file changed, 68 insertions(+), 165 deletions(-)
kernel/spinlock.c +68 -165
Index: 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);
maintainer
#3 lore raw

Re: [patch 2/4] spinlock: add macro to generate out-of-line variants

Quoted 7 lines · Heiko Carstens

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);
}
Quoted 8 lines

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 <[email protected]>

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 <><
#4 lore raw

Re: [patch 2/4] spinlock: add macro to generate out-of-line variants

Quoted 25 lines · Arnd Bergmann
Quoted 7 lines · Heiko Carstens

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);
}
Quoted 8 lines

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 <[email protected]>

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.

Quoted 5 lines

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 ;-)

author
#5 lore raw

Re: [patch 2/4] spinlock: add macro to generate out-of-line variants

In reply to Arnd Bergmann
Quoted 17 lines · Arnd Bergmann
Quoted 7 lines · Heiko Carstens

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);
}
Quoted 8 lines

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 <[email protected]>
Quoted 7 lines

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!

author
#6 lore raw

PATCH 3/4 spinlock: allow inlined spinlocks

In reply to Heiko Carstens

From: Heiko Carstens <[email protected]>

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 <[email protected]>
---
 include/linux/spinlock_api_smp.h |   35 +++++++++++++++++++++++++++++++++++
 kernel/spinlock.c                |    4 ++++
 lib/Kconfig.debug                |   14 ++++++++++++++
 3 files changed, 53 insertions(+)
include/linux/spinlock_api_smp.h +35 -0
Index: 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();
lib/Kconfig.debug +14 -0
Index: 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
kernel/spinlock.c +4 -0
Index: 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 */
author
#7 lore raw

PATCH 4/4 spinlock: allow inline spinlocks on s390

In reply to Heiko Carstens

From: Heiko Carstens <[email protected]>

Signed-off-by Heiko Carstens <[email protected]>
---
 arch/s390/Kconfig |    1 +
 1 file changed, 1 insertion(+)
arch/s390/Kconfig +1 -0
Index: 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
Acked-by on the v1 series
#8 lore raw
In reply to Heiko Carstens
Quoted 15 lines · 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.

Quoted 7 lines

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 <[email protected]>

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