rwsem: check the lock before cpmxchg in down_write_trylock

Cmpxchg will cause the cacheline bouning when do the value checking, that cause scalability issue in a large machine (like a 80 core box).

So a lock pre-read can relief this contention.

Signed-off-by Alex Shi <[email protected]>

The following line should be added head of commit log on patches 1~4. :)

From: Alex Shi <[email protected]>

Quoted 6 lines

Cmpxchg will cause the cacheline bouning when do the value checking, that cause scalability issue in a large machine (like a 80 core box).

So a lock pre-read can relief this contention.

Signed-off-by Alex Shi <[email protected]>

We want to add optimistic spinning to rwsems because we've noticed that the writer rwsem does not perform as well as mutexes. Tim noticed that for exim (mail server) workloads, when reverting commit 4fc3f1d6 and Davidlohr noticed it when converting the i_mmap_mutex to a rwsem in some aim7 workloads. We've noticed that the biggest difference, in a nutshell, is when we fail to acquire a mutex in the fastpath, optimistic spinning comes in to play and we can avoid a large amount of unnecessary sleeping and wait queue overhead.

For rwsems on the other hand, upon entering the writer slowpath in rwsem_down_write_failed(), we just acquire the ->wait_lock, add ourselves to the wait_queue and blocking until we get the lock.

Reviewed-by Peter Zijlstra <[email protected]>

Reviewed-by Peter Hurley <[email protected]>

Signed-off-by Tim Chen <[email protected]>

Signed-off-by Davidlohr Bueso <[email protected]>

---
 include/linux/rwsem.h |    3 +
 init/Kconfig          |    9 +++
 kernel/rwsem.c        |   29 +++++++++-
 lib/rwsem.c           |  150 +++++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 179 insertions(+), 12 deletions(-)

include/linux/rwsem.h +3 -0

index 0616ffe..0c5933b 100644
@@ -29,6 +29,9 @@ struct rw_semaphore {
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 	struct lockdep_map	dep_map;
 #endif
+#ifdef CONFIG_RWSEM_SPIN_ON_WRITE_OWNER
+	struct task_struct	*owner;
+#endif
 };
 
 extern struct rw_semaphore *rwsem_down_read_failed(struct rw_semaphore *sem);

init/Kconfig +0 -0

index 9d3a788..1c582d1 100644
@@ -1595,6 +1595,15 @@ config TRACEPOINTS
 
 source "arch/Kconfig"

init/Kconfig +0 -0

 endmenu		# General setup
 
 config HAVE_GENERIC_DMA_COHERENT

kernel/rwsem.c +28 -1

index cfff143..a32990a 100644
@@ -12,6 +12,26 @@
 
 #include <linux/atomic.h>
 
+#ifdef CONFIG_RWSEM_SPIN_ON_WRITE_OWNER
+static inline void rwsem_set_owner(struct rw_semaphore *sem)
+{
+	sem->owner = current;
+}
+
+static inline void rwsem_clear_owner(struct rw_semaphore *sem)
+{
+	sem->owner = NULL;
+}
+#else
+static inline void rwsem_set_owner(struct rw_semaphore *sem)
+{
+}
+
+static inline void rwsem_clear_owner(struct rw_semaphore *sem)
+{
+}
+#endif
+
 /*
  * lock for reading
  */

@@ -48,6 +68,7 @@ void __sched down_write(struct rw_semaphore *sem)
 	rwsem_acquire(&sem->dep_map, 0, 0, _RET_IP_);
 
 	LOCK_CONTENDED(sem, __down_write_trylock, __down_write);
+	rwsem_set_owner(sem);
 }
 
 EXPORT_SYMBOL(down_write);

@@ -59,8 +80,10 @@ int down_write_trylock(struct rw_semaphore *sem)
 {
 	int ret = __down_write_trylock(sem);
 
-	if (ret == 1)
+	if (ret == 1) {
 		rwsem_acquire(&sem->dep_map, 0, 1, _RET_IP_);
+		rwsem_set_owner(sem);
+	}
 	return ret;
 }
 

@@ -86,6 +109,7 @@ void up_write(struct rw_semaphore *sem)
 	rwsem_release(&sem->dep_map, 1, _RET_IP_);
 
 	__up_write(sem);
+	rwsem_clear_owner(sem);
 }
 
 EXPORT_SYMBOL(up_write);

@@ -100,6 +124,7 @@ void downgrade_write(struct rw_semaphore *sem)
 	 * dependency.
 	 */
 	__downgrade_write(sem);
+	rwsem_clear_owner(sem);
 }
 
 EXPORT_SYMBOL(downgrade_write);

@@ -122,6 +147,7 @@ void _down_write_nest_lock(struct rw_semaphore *sem, struct lockdep_map *nest)
 	rwsem_acquire_nest(&sem->dep_map, 0, 0, nest, _RET_IP_);
 
 	LOCK_CONTENDED(sem, __down_write_trylock, __down_write);
+	rwsem_set_owner(sem);
 }
 
 EXPORT_SYMBOL(_down_write_nest_lock);

@@ -141,6 +167,7 @@ void down_write_nested(struct rw_semaphore *sem, int subclass)
 	rwsem_acquire(&sem->dep_map, subclass, 0, _RET_IP_);
 
 	LOCK_CONTENDED(sem, __down_write_trylock, __down_write);
+	rwsem_set_owner(sem);
 }
 
 EXPORT_SYMBOL(down_write_nested);

lib/rwsem.c +133 -11

index 1d6e6e8..b7fd8ff 100644
@@ -8,6 +8,7 @@
  */
 #include <linux/rwsem.h>
 #include <linux/sched.h>
+#include <linux/sched/rt.h>
 #include <linux/init.h>
 #include <linux/export.h>
 

@@ -27,6 +28,9 @@ void __init_rwsem(struct rw_semaphore *sem, const char *name,
 	sem->count = RWSEM_UNLOCKED_VALUE;
 	raw_spin_lock_init(&sem->wait_lock);
 	INIT_LIST_HEAD(&sem->wait_list);
+#ifdef CONFIG_RWSEM_SPIN_ON_WRITE_OWNER
+	sem->owner = NULL;
+#endif
 }
 
 EXPORT_SYMBOL(__init_rwsem);

@@ -194,6 +198,130 @@ struct rw_semaphore __sched *rwsem_down_read_failed(struct rw_semaphore *sem)
 	return sem;
 }
 
+static inline int rwsem_try_write_lock(long count, bool need_lock,
+	struct rw_semaphore *sem)
+{
+	if (!(count & RWSEM_ACTIVE_MASK)) {
+		/* Try acquiring the write lock. */
+		if (sem->count == RWSEM_WAITING_BIAS &&
+		    cmpxchg(&sem->count, RWSEM_WAITING_BIAS,
+			    RWSEM_ACTIVE_WRITE_BIAS) == RWSEM_WAITING_BIAS) {
+			if (need_lock)
+				raw_spin_lock_irq(&sem->wait_lock);
+			if (!list_is_singular(&sem->wait_list))
+				rwsem_atomic_update(RWSEM_WAITING_BIAS, sem);
+			return 1;
+		}
+	}
+	return 0;
+}
+
+#ifdef CONFIG_RWSEM_SPIN_ON_WRITE_OWNER
+static inline bool rwsem_can_spin_on_owner(struct rw_semaphore *sem)
+{
+	int retval;
+	struct task_struct *owner;
+
+	rcu_read_lock();
+	owner = ACCESS_ONCE(sem->owner);
+
+	/* Spin only if active writer running */
+	if (owner)
+		retval = owner->on_cpu;
+	else
+		retval = false;
+
+	rcu_read_unlock();
+	/*
+	 * if lock->owner is not set, the sem owner may have just acquired
+	 * it and not set the owner yet, or the sem has been released, or
+	 * reader active.
+	 */
+	return retval;
+}
+
+static inline bool owner_running(struct rw_semaphore *lock,
+				struct task_struct *owner)
+{
+	if (lock->owner != owner)
+		return false;
+
+	/*
+	 * Ensure we emit the owner->on_cpu, dereference _after_ checking
+	 * lock->owner still matches owner, if that fails, owner might
+	 * point to free()d memory, if it still matches, the rcu_read_lock()
+	 * ensures the memory stays valid.
+	 */
+	barrier();
+
+	return owner->on_cpu;
+}
+
+static noinline
+int rwsem_spin_on_owner(struct rw_semaphore *lock, struct task_struct *owner)
+{
+	rcu_read_lock();
+	while (owner_running(lock, owner)) {
+		if (need_resched())
+			break;
+
+		arch_mutex_cpu_relax();
+	}
+	rcu_read_unlock();
+
+	/*
+	 * We break out the loop above on need_resched() and when the
+	 * owner changed, which is a sign for heavy contention. Return
+	 * success only when lock->owner is NULL.
+	 */
+	return lock->owner == NULL;
+}
+
+int rwsem_optimistic_spin(struct rw_semaphore *sem)
+{
+	struct	task_struct	*owner;
+	int	ret = 0;
+
+	/* sem->wait_lock should not be held when doing optimistic spinning */
+	if (!rwsem_can_spin_on_owner(sem))
+		return ret;
+
+	preempt_disable();
+	for (;;) {
+		owner = ACCESS_ONCE(sem->owner);
+		if (owner && !rwsem_spin_on_owner(sem, owner))
+			break;
+
+		/* wait_lock will be acquired if write_lock is obtained */
+		if (rwsem_try_write_lock(sem->count, true, sem)) {
+			ret = 1;
+			break;
+		}
+
+		/*
+		 * When there's no owner, we might have preempted between the
+		 * owner acquiring the lock and setting the owner field. If
+		 * we're an RT task that will live-lock because we won't let
+		 * the owner complete.
+		 */
+		if (!owner && (need_resched() || rt_task(current)))
+			break;
+
+		/*
+		 * The cpu_relax() call is a compiler barrier which forces
+		 * everything in this loop to be re-loaded. We don't need
+		 * memory barriers as we'll eventually observe the right
+		 * values at the cost of a few extra spins.
+		 */
+		arch_mutex_cpu_relax();
+
+	}
+
+	preempt_enable();
+	return ret;
+}
+#endif
+
 /*
  * wait until we successfully acquire the write lock
  */

@@ -202,6 +330,9 @@ struct rw_semaphore __sched *rwsem_down_write_failed(struct rw_semaphore *sem)
 	long count, adjustment = -RWSEM_ACTIVE_WRITE_BIAS;
 	struct rwsem_waiter waiter;
 	struct task_struct *tsk = current;
+#ifdef CONFIG_RWSEM_SPIN_ON_WRITE_OWNER
+	bool try_optimistic_spin = true;
+#endif
 
 	/* set up my own style of waitqueue */
 	waiter.task = tsk;

@@ -225,20 +356,17 @@ struct rw_semaphore __sched *rwsem_down_write_failed(struct rw_semaphore *sem)
 	/* wait until we successfully acquire the lock */
 	set_task_state(tsk, TASK_UNINTERRUPTIBLE);
 	while (true) {
-		if (!(count & RWSEM_ACTIVE_MASK)) {
-			/* Try acquiring the write lock. */
-			count = RWSEM_ACTIVE_WRITE_BIAS;
-			if (!list_is_singular(&sem->wait_list))
-				count += RWSEM_WAITING_BIAS;
-
-			if (sem->count == RWSEM_WAITING_BIAS &&
-			    cmpxchg(&sem->count, RWSEM_WAITING_BIAS, count) ==
-							RWSEM_WAITING_BIAS)
-				break;
-		}
+		if (rwsem_try_write_lock(count, false, sem))
+			break;
 
 		raw_spin_unlock_irq(&sem->wait_lock);

Quoted 45 lines · Tim Chen

We want to add optimistic spinning to rwsems because we've noticed that the writer rwsem does not perform as well as mutexes. Tim noticed that for exim (mail server) workloads, when reverting commit 4fc3f1d6 and Davidlohr noticed it when converting the i_mmap_mutex to a rwsem in some aim7 workloads. We've noticed that the biggest difference, in a nutshell, is when we fail to acquire a mutex in the fastpath, optimistic spinning comes in to play and we can avoid a large amount of unnecessary sleeping and wait queue overhead.

For rwsems on the other hand, upon entering the writer slowpath in rwsem_down_write_failed(), we just acquire the ->wait_lock, add ourselves to the wait_queue and blocking until we get the lock.

Reviewed-by Peter Zijlstra <[email protected]>

Reviewed-by Peter Hurley <[email protected]>

Signed-off-by Tim Chen <[email protected]>

Signed-off-by Davidlohr Bueso <[email protected]>

---
 include/linux/rwsem.h |    3 +
 init/Kconfig          |    9 +++
 kernel/rwsem.c        |   29 +++++++++-
 lib/rwsem.c           |  150 +++++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 179 insertions(+), 12 deletions(-)

include/linux/rwsem.h +3 -0

index 0616ffe..0c5933b 100644
@@ -29,6 +29,9 @@ struct rw_semaphore {
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 	struct lockdep_map	dep_map;
 #endif
+#ifdef CONFIG_RWSEM_SPIN_ON_WRITE_OWNER
+	struct task_struct	*owner;
+#endif
 };
 
 extern struct rw_semaphore *rwsem_down_read_failed(struct rw_semaphore *sem);

init/Kconfig +0 -0

index 9d3a788..1c582d1 100644
@@ -1595,6 +1595,15 @@ config TRACEPOINTS
 
 source "arch/Kconfig"

init/Kconfig config TRACEPOINTS · line 1606

Quoted 10 lines

init/Kconfig +9 -0

 
+config RWSEM_SPIN_ON_WRITE_OWNER
+	bool "Optimistic spin write acquisition for writer owned rw-sem"
+	default n
+	depends on SMP
+	help
+	  Allows a writer to perform optimistic spinning if another writer own
+	  the read write semaphore.  This gives a greater chance for writer to
+	  acquire a semaphore before blocking it and putting it to sleep.
+

Quoting from kernel/mutex.c:

"The rationale is that if the lock owner is running, it is likely to release the lock soon."

It would be good to add that to the Kconfig.