[RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Jason Low]

The first patch contains an optimization for acquiring the rwsem write lock
in the slowpath.

This rest of the series converts the rwsem count variable to an atomic_long_t
since it is used it as an atomic variable. This allows us to also remove
the rwsem_atomic_{add,update} abstraction and reduce 100+ lines of code.

 arch/alpha/include/asm/rwsem.h | 42 --------------------------------
 arch/ia64/include/asm/rwsem.h  |  7 ------
 arch/s390/include/asm/rwsem.h  | 37 -----------------------------
 arch/x86/include/asm/rwsem.h   | 18 --------------
 include/asm-generic/rwsem.h    | 16 -------------
 include/linux/rwsem.h          |  6 ++---
 kernel/locking/rwsem-xadd.c    | 54 ++++++++++++++++++++++++++----------------
 7 files changed, 36 insertions(+), 144 deletions(-)

-- 
2.1.4

[RFC][PATCH 1/7] locking/rwsem: Optimize write lock by reducing operations in slowpath

[Jason Low]

When acquiring the rwsem write lock in the slowpath, we first try
to set count to RWSEM_WAITING_BIAS. When that is successful,
we then atomically add the RWSEM_WAITING_BIAS in cases where
there are other tasks on the wait list. This causes write lock
operations to often issue multiple atomic operations.

We can instead make the list_is_singular() check first, and then
set the count accordingly, so that we issue at most 1 atomic
operation when acquiring the write lock and reduce unnecessary
cacheline contention.

Signed-off-by: Jason Low <jason.low2@hpe.com>
Acked-by: Waiman Long<Waiman.Long@hpe.com>
Acked-by: Davidlohr Bueso <dave@stgolabs.net>
---
 kernel/locking/rwsem-xadd.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index 09e30c6..296d421 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -255,17 +255,28 @@ struct rw_semaphore __sched *rwsem_down_read_failed(struct rw_semaphore *sem)
 }
 EXPORT_SYMBOL(rwsem_down_read_failed);
 
+/*
+ * This function must be called with the sem->wait_lock held to prevent
+ * race conditions between checking the rwsem wait list and setting the
+ * sem->count accordingly.
+ */
 static inline bool rwsem_try_write_lock(long count, struct rw_semaphore *sem)
 {
 	/*
-	 * Try acquiring the write lock. Check count first in order
-	 * to reduce unnecessary expensive cmpxchg() operations.
+	 * Avoid trying to acquire write lock if count isn't RWSEM_WAITING_BIAS.
 	 */
-	if (count == RWSEM_WAITING_BIAS &&
-	    cmpxchg_acquire(&sem->count, RWSEM_WAITING_BIAS,
-		    RWSEM_ACTIVE_WRITE_BIAS) == RWSEM_WAITING_BIAS) {
-		if (!list_is_singular(&sem->wait_list))
-			rwsem_atomic_update(RWSEM_WAITING_BIAS, sem);
+	if (count != RWSEM_WAITING_BIAS)
+		return false;
+
+	/*
+	 * Acquire the lock by trying to set it to ACTIVE_WRITE_BIAS. If there
+	 * are other tasks on the wait list, we need to add on WAITING_BIAS.
+	 */
+	count = list_is_singular(&sem->wait_list) ?
+			RWSEM_ACTIVE_WRITE_BIAS :
+			RWSEM_ACTIVE_WRITE_BIAS + RWSEM_WAITING_BIAS;
+
+	if (cmpxchg_acquire(&sem->count, RWSEM_WAITING_BIAS, count) == RWSEM_WAITING_BIAS) {
 		rwsem_set_owner(sem);
 		return true;
 	}
-- 
2.1.4

[RFC][PATCH 2/7] locking/rwsem: Convert sem->count to atomic_long_t

[Jason Low]

Convert the rwsem count variable to an atomic_long_t since we use it
as an atomic variable. This also allows us to remove the
rwsem_atomic_{add,update} "abstraction" which would now be an unnecesary
level of indirection. In follow up patches, we also remove the
rwsem_atomic_{add,update} definitions across the various architectures.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jason Low <jason.low2@hpe.com>
---
 include/linux/rwsem.h       |  6 +++---
 kernel/locking/rwsem-xadd.c | 31 ++++++++++++++++---------------
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h
index d1c12d1..e3d5a00 100644
--- a/include/linux/rwsem.h
+++ b/include/linux/rwsem.h
@@ -26,7 +26,7 @@ struct rw_semaphore;
 #else
 /* All arch specific implementations share the same struct */
 struct rw_semaphore {
-	long count;
+	atomic_long_t count;
 	struct list_head wait_list;
 	raw_spinlock_t wait_lock;
 #ifdef CONFIG_RWSEM_SPIN_ON_OWNER
@@ -54,7 +54,7 @@ extern struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem);
 /* In all implementations count != 0 means locked */
 static inline int rwsem_is_locked(struct rw_semaphore *sem)
 {
-	return sem->count != 0;
+	return atomic_long_read(&sem->count) != 0;
 }
 
 #endif
@@ -74,7 +74,7 @@ static inline int rwsem_is_locked(struct rw_semaphore *sem)
 #endif
 
 #define __RWSEM_INITIALIZER(name)				\
-	{ .count = RWSEM_UNLOCKED_VALUE,			\
+	{ .count = ATOMIC_LONG_INIT(RWSEM_UNLOCKED_VALUE),	\
 	  .wait_list = LIST_HEAD_INIT((name).wait_list),	\
 	  .wait_lock = __RAW_SPIN_LOCK_UNLOCKED(name.wait_lock)	\
 	  __RWSEM_OPT_INIT(name)				\
diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index 296d421..d5ecec3 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -80,7 +80,7 @@ void __init_rwsem(struct rw_semaphore *sem, const char *name,
 	debug_check_no_locks_freed((void *)sem, sizeof(*sem));
 	lockdep_init_map(&sem->dep_map, name, key, 0);
 #endif
-	sem->count = RWSEM_UNLOCKED_VALUE;
+	atomic_long_set(&sem->count, RWSEM_UNLOCKED_VALUE);
 	raw_spin_lock_init(&sem->wait_lock);
 	INIT_LIST_HEAD(&sem->wait_list);
 #ifdef CONFIG_RWSEM_SPIN_ON_OWNER
@@ -146,10 +146,11 @@ __rwsem_do_wake(struct rw_semaphore *sem, enum rwsem_wake_type wake_type)
 	if (wake_type != RWSEM_WAKE_READ_OWNED) {
 		adjustment = RWSEM_ACTIVE_READ_BIAS;
  try_reader_grant:
-		oldcount = rwsem_atomic_update(adjustment, sem) - adjustment;
+		oldcount = atomic_long_add_return(adjustment, &sem->count) - adjustment;
+
 		if (unlikely(oldcount < RWSEM_WAITING_BIAS)) {
 			/* A writer stole the lock. Undo our reader grant. */
-			if (rwsem_atomic_update(-adjustment, sem) &
+			if (atomic_long_sub_return(adjustment, &sem->count) &
 						RWSEM_ACTIVE_MASK)
 				goto out;
 			/* Last active locker left. Retry waking readers. */
@@ -179,7 +180,7 @@ __rwsem_do_wake(struct rw_semaphore *sem, enum rwsem_wake_type wake_type)
 		adjustment -= RWSEM_WAITING_BIAS;
 
 	if (adjustment)
-		rwsem_atomic_add(adjustment, sem);
+		atomic_long_add(adjustment, &sem->count);
 
 	next = sem->wait_list.next;
 	loop = woken;
@@ -228,7 +229,7 @@ struct rw_semaphore __sched *rwsem_down_read_failed(struct rw_semaphore *sem)
 	list_add_tail(&waiter.list, &sem->wait_list);
 
 	/* we're now waiting on the lock, but no longer actively locking */
-	count = rwsem_atomic_update(adjustment, sem);
+	count = atomic_long_add_return(adjustment, &sem->count);
 
 	/* If there are no active locks, wake the front queued process(es).
 	 *
@@ -276,7 +277,8 @@ static inline bool rwsem_try_write_lock(long count, struct rw_semaphore *sem)
 			RWSEM_ACTIVE_WRITE_BIAS :
 			RWSEM_ACTIVE_WRITE_BIAS + RWSEM_WAITING_BIAS;
 
-	if (cmpxchg_acquire(&sem->count, RWSEM_WAITING_BIAS, count) == RWSEM_WAITING_BIAS) {
+	if (atomic_long_cmpxchg_acquire(&sem->count, RWSEM_WAITING_BIAS, count)
+							== RWSEM_WAITING_BIAS) {
 		rwsem_set_owner(sem);
 		return true;
 	}
@@ -290,13 +292,13 @@ static inline bool rwsem_try_write_lock(long count, struct rw_semaphore *sem)
  */
 static inline bool rwsem_try_write_lock_unqueued(struct rw_semaphore *sem)
 {
-	long old, count = READ_ONCE(sem->count);
+	long old, count = atomic_long_read(&sem->count);
 
 	while (true) {
 		if (!(count == 0 || count == RWSEM_WAITING_BIAS))
 			return false;
 
-		old = cmpxchg_acquire(&sem->count, count,
+		old = atomic_long_cmpxchg_acquire(&sem->count, count,
 				      count + RWSEM_ACTIVE_WRITE_BIAS);
 		if (old == count) {
 			rwsem_set_owner(sem);
@@ -318,7 +320,7 @@ static inline bool rwsem_can_spin_on_owner(struct rw_semaphore *sem)
 	rcu_read_lock();
 	owner = READ_ONCE(sem->owner);
 	if (!owner) {
-		long count = READ_ONCE(sem->count);
+		long count = atomic_long_read(&sem->count);
 		/*
 		 * If sem->owner is not set, yet we have just recently entered the
 		 * slowpath with the lock being active, then there is a possibility
@@ -369,7 +371,7 @@ bool rwsem_spin_on_owner(struct rw_semaphore *sem, struct task_struct *owner)
 	 * held by readers. Check the counter to verify the
 	 * state.
 	 */
-	count = READ_ONCE(sem->count);
+	count = atomic_long_read(&sem->count);
 	return (count == 0 || count == RWSEM_WAITING_BIAS);
 }
 
@@ -453,7 +455,7 @@ __rwsem_down_write_failed_common(struct rw_semaphore *sem, int state)
 	struct rw_semaphore *ret = sem;
 
 	/* undo write bias from down_write operation, stop active locking */
-	count = rwsem_atomic_update(-RWSEM_ACTIVE_WRITE_BIAS, sem);
+	count = atomic_long_sub_return(RWSEM_ACTIVE_WRITE_BIAS, &sem->count);
 
 	/* do optimistic spinning and steal lock if possible */
 	if (rwsem_optimistic_spin(sem))
@@ -476,7 +478,7 @@ __rwsem_down_write_failed_common(struct rw_semaphore *sem, int state)
 
 	/* we're now waiting on the lock, but no longer actively locking */
 	if (waiting) {
-		count = READ_ONCE(sem->count);
+		count = atomic_long_read(&sem->count);
 
 		/*
 		 * If there were already threads queued before us and there are
@@ -485,9 +487,8 @@ __rwsem_down_write_failed_common(struct rw_semaphore *sem, int state)
 		 */
 		if (count > RWSEM_WAITING_BIAS)
 			sem = __rwsem_do_wake(sem, RWSEM_WAKE_READERS);
-
 	} else
-		count = rwsem_atomic_update(RWSEM_WAITING_BIAS, sem);
+		count = atomic_long_add_return(RWSEM_WAITING_BIAS, &sem->count);
 
 	/* wait until we successfully acquire the lock */
 	set_current_state(state);
@@ -503,7 +504,7 @@ __rwsem_down_write_failed_common(struct rw_semaphore *sem, int state)
 
 			schedule();
 			set_current_state(state);
-		} while ((count = sem->count) & RWSEM_ACTIVE_MASK);
+		} while ((count = atomic_long_read(&sem->count)) & RWSEM_ACTIVE_MASK);
 
 		raw_spin_lock_irq(&sem->wait_lock);
 	}
-- 
2.1.4

[RFC][PATCH 3/7] locking,x86: Remove x86 rwsem add and rwsem update

[Jason Low]

The rwsem count has been converted to an atomic variable and the rwsem
code now directly uses atomic_long_add() and atomic_long_add_return(),
so we can remove the x86 implementation of rwsem_atomic_add() and
rwsem_atomic_update().

Signed-off-by: Jason Low <jason.low2@hpe.com>
---
 arch/x86/include/asm/rwsem.h | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/arch/x86/include/asm/rwsem.h b/arch/x86/include/asm/rwsem.h
index d2f8d10..91cf42c 100644
--- a/arch/x86/include/asm/rwsem.h
+++ b/arch/x86/include/asm/rwsem.h
@@ -215,23 +215,5 @@ static inline void __downgrade_write(struct rw_semaphore *sem)
 		     : "memory", "cc");
 }
 
-/*
- * implement atomic add functionality
- */
-static inline void rwsem_atomic_add(long delta, struct rw_semaphore *sem)
-{
-	asm volatile(LOCK_PREFIX _ASM_ADD "%1,%0"
-		     : "+m" (sem->count)
-		     : "er" (delta));
-}
-
-/*
- * implement exchange and add functionality
- */
-static inline long rwsem_atomic_update(long delta, struct rw_semaphore *sem)
-{
-	return delta + xadd(&sem->count, delta);
-}
-
 #endif /* __KERNEL__ */
 #endif /* _ASM_X86_RWSEM_H */
-- 
2.1.4

[RFC][PATCH 4/7] locking,alpha: Remove Alpha rwsem add and rwsem update

[Jason Low]

The rwsem count has been converted to an atomic variable and the rwsem
code now directly uses atomic_long_add() and atomic_long_add_return(),
so we can remove the alpha implementation of rwsem_atomic_add() and
rwsem_atomic_update().

Signed-off-by: Jason Low <jason.low2@hpe.com>
---
 arch/alpha/include/asm/rwsem.h | 42 ------------------------------------------
 1 file changed, 42 deletions(-)

diff --git a/arch/alpha/include/asm/rwsem.h b/arch/alpha/include/asm/rwsem.h
index 0131a70..a217bf8 100644
--- a/arch/alpha/include/asm/rwsem.h
+++ b/arch/alpha/include/asm/rwsem.h
@@ -191,47 +191,5 @@ static inline void __downgrade_write(struct rw_semaphore *sem)
 		rwsem_downgrade_wake(sem);
 }
 
-static inline void rwsem_atomic_add(long val, struct rw_semaphore *sem)
-{
-#ifndef	CONFIG_SMP
-	sem->count += val;
-#else
-	long temp;
-	__asm__ __volatile__(
-	"1:	ldq_l	%0,%1\n"
-	"	addq	%0,%2,%0\n"
-	"	stq_c	%0,%1\n"
-	"	beq	%0,2f\n"
-	".subsection 2\n"
-	"2:	br	1b\n"
-	".previous"
-	:"=&r" (temp), "=m" (sem->count)
-	:"Ir" (val), "m" (sem->count));
-#endif
-}
-
-static inline long rwsem_atomic_update(long val, struct rw_semaphore *sem)
-{
-#ifndef	CONFIG_SMP
-	sem->count += val;
-	return sem->count;
-#else
-	long ret, temp;
-	__asm__ __volatile__(
-	"1:	ldq_l	%0,%1\n"
-	"	addq 	%0,%3,%2\n"
-	"	addq	%0,%3,%0\n"
-	"	stq_c	%2,%1\n"
-	"	beq	%2,2f\n"
-	".subsection 2\n"
-	"2:	br	1b\n"
-	".previous"
-	:"=&r" (ret), "=m" (sem->count), "=&r" (temp)
-	:"Ir" (val), "m" (sem->count));
-
-	return ret;
-#endif
-}
-
 #endif /* __KERNEL__ */
 #endif /* _ALPHA_RWSEM_H */
-- 
2.1.4

[RFC][PATCH 5/7] locking,ia64: Remove ia64 rwsem add and rwsem update

[Jason Low]

The rwsem count has been converted to an atomic variable and the rwsem
code now directly uses atomic_long_add() and atomic_long_add_return(),
so we can remove the ia64 implementation of rwsem_atomic_add() and
rwsem_atomic_update().

Signed-off-by: Jason Low <jason.low2@hpe.com>
---
 arch/ia64/include/asm/rwsem.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/ia64/include/asm/rwsem.h b/arch/ia64/include/asm/rwsem.h
index 8b23e07..dfd5895 100644
--- a/arch/ia64/include/asm/rwsem.h
+++ b/arch/ia64/include/asm/rwsem.h
@@ -151,11 +151,4 @@ __downgrade_write (struct rw_semaphore *sem)
 		rwsem_downgrade_wake(sem);
 }
 
-/*
- * Implement atomic add functionality.  These used to be "inline" functions, but GCC v3.1
- * doesn't quite optimize this stuff right and ends up with bad calls to fetchandadd.
- */
-#define rwsem_atomic_add(delta, sem)	atomic64_add(delta, (atomic64_t *)(&(sem)->count))
-#define rwsem_atomic_update(delta, sem)	atomic64_add_return(delta, (atomic64_t *)(&(sem)->count))
-
 #endif /* _ASM_IA64_RWSEM_H */
-- 
2.1.4

[RFC][PATCH 6/7] locking,s390: Remove s390 rwsem add and rwsem update

[Jason Low]

The rwsem count has been converted to an atomic variable and the rwsem
code now directly uses atomic_long_add() and atomic_long_add_return(),
so we can remove the s390 implementation of rwsem_atomic_add() and
rwsem_atomic_update().

Signed-off-by: Jason Low <jason.low2@hpe.com>
---
 arch/s390/include/asm/rwsem.h | 37 -------------------------------------
 1 file changed, 37 deletions(-)

diff --git a/arch/s390/include/asm/rwsem.h b/arch/s390/include/asm/rwsem.h
index c75e447..597e7e9 100644
--- a/arch/s390/include/asm/rwsem.h
+++ b/arch/s390/include/asm/rwsem.h
@@ -207,41 +207,4 @@ static inline void __downgrade_write(struct rw_semaphore *sem)
 		rwsem_downgrade_wake(sem);
 }
 
-/*
- * implement atomic add functionality
- */
-static inline void rwsem_atomic_add(long delta, struct rw_semaphore *sem)
-{
-	signed long old, new;
-
-	asm volatile(
-		"	lg	%0,%2\n"
-		"0:	lgr	%1,%0\n"
-		"	agr	%1,%4\n"
-		"	csg	%0,%1,%2\n"
-		"	jl	0b"
-		: "=&d" (old), "=&d" (new), "=Q" (sem->count)
-		: "Q" (sem->count), "d" (delta)
-		: "cc", "memory");
-}
-
-/*
- * implement exchange and add functionality
- */
-static inline long rwsem_atomic_update(long delta, struct rw_semaphore *sem)
-{
-	signed long old, new;
-
-	asm volatile(
-		"	lg	%0,%2\n"
-		"0:	lgr	%1,%0\n"
-		"	agr	%1,%4\n"
-		"	csg	%0,%1,%2\n"
-		"	jl	0b"
-		: "=&d" (old), "=&d" (new), "=Q" (sem->count)
-		: "Q" (sem->count), "d" (delta)
-		: "cc", "memory");
-	return new;
-}
-
 #endif /* _S390_RWSEM_H */
-- 
2.1.4

[RFC][PATCH 7/7] locking,asm-generic: Remove generic rwsem add and rwsem update definitions

[Jason Low]

The rwsem count has been converted to an atomic variable and we
now directly use atomic_long_add() and atomic_long_add_return()
on the count, so we can remove the asm-generic implementation of
rwsem_atomic_add() and rwsem_atomic_update().

Signed-off-by: Jason Low <jason.low2@hpe.com>
---
 include/asm-generic/rwsem.h | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/include/asm-generic/rwsem.h b/include/asm-generic/rwsem.h
index 3fc94a0..dd9db88 100644
--- a/include/asm-generic/rwsem.h
+++ b/include/asm-generic/rwsem.h
@@ -107,14 +107,6 @@ static inline void __up_write(struct rw_semaphore *sem)
 }
 
 /*
- * implement atomic add functionality
- */
-static inline void rwsem_atomic_add(long delta, struct rw_semaphore *sem)
-{
-	atomic_long_add(delta, (atomic_long_t *)&sem->count);
-}
-
-/*
  * downgrade write lock to read lock
  */
 static inline void __downgrade_write(struct rw_semaphore *sem)
@@ -134,13 +126,5 @@ static inline void __downgrade_write(struct rw_semaphore *sem)
 		rwsem_downgrade_wake(sem);
 }
 
-/*
- * implement exchange and add functionality
- */
-static inline long rwsem_atomic_update(long delta, struct rw_semaphore *sem)
-{
-	return atomic_long_add_return(delta, (atomic_long_t *)&sem->count);
-}
-
 #endif	/* __KERNEL__ */
 #endif	/* _ASM_GENERIC_RWSEM_H */
-- 
2.1.4

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Linus Torvalds]

On Mon, May 16, 2016 at 5:37 PM, Jason Low <jason.low2@hpe.com> wrote:
>
> This rest of the series converts the rwsem count variable to an atomic_long_t
> since it is used it as an atomic variable. This allows us to also remove
> the rwsem_atomic_{add,update} abstraction and reduce 100+ lines of code.

I would suggest you merge all the "remove rwsem_atomic_{add,update}"
patches into a single patch.

I don't see the advantage to splitting those up by architecture, and
it does add noise to the series.

Other than that it all looks fine to me.

                Linus

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Peter Zijlstra]

On Mon, May 16, 2016 at 06:12:25PM -0700, Linus Torvalds wrote:
> On Mon, May 16, 2016 at 5:37 PM, Jason Low <jason.low2@hpe.com> wrote:
> >
> > This rest of the series converts the rwsem count variable to an atomic_long_t
> > since it is used it as an atomic variable. This allows us to also remove
> > the rwsem_atomic_{add,update} abstraction and reduce 100+ lines of code.
> 
> I would suggest you merge all the "remove rwsem_atomic_{add,update}"
> patches into a single patch.
> 
> I don't see the advantage to splitting those up by architecture, and
> it does add noise to the series.
> 
> Other than that it all looks fine to me.

OK, done.

---
Subject: locking,rwsem: Remove rwsem_atomic_add() and rwsem_atomic_update()
From: Jason Low <jason.low2@hpe.com>
Date: Mon, 16 May 2016 17:38:02 -0700

The rwsem-xadd count has been converted to an atomic variable and the
rwsem code now directly uses atomic_long_add() and
atomic_long_add_return(), so we can remove the arch implementations of
rwsem_atomic_add() and rwsem_atomic_update().

Cc: Waiman Long <Waiman.Long@hpe.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Jason Low <jason.low2@hp.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Terry Rudd <terry.rudd@hpe.com>
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Matt Turner <mattst88@gmail.com>
Signed-off-by: Jason Low <jason.low2@hpe.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/alpha/include/asm/rwsem.h |   42 -----------------------------------------
 arch/ia64/include/asm/rwsem.h  |    7 ------
 arch/s390/include/asm/rwsem.h  |   37 ------------------------------------
 arch/x86/include/asm/rwsem.h   |   18 -----------------
 include/asm-generic/rwsem.h    |   16 ---------------
 5 files changed, 120 deletions(-)

--- a/arch/alpha/include/asm/rwsem.h
+++ b/arch/alpha/include/asm/rwsem.h
@@ -191,47 +191,5 @@ static inline void __downgrade_write(str
 		rwsem_downgrade_wake(sem);
 }
 
-static inline void rwsem_atomic_add(long val, struct rw_semaphore *sem)
-{
-#ifndef	CONFIG_SMP
-	sem->count += val;
-#else
-	long temp;
-	__asm__ __volatile__(
-	"1:	ldq_l	%0,%1\n"
-	"	addq	%0,%2,%0\n"
-	"	stq_c	%0,%1\n"
-	"	beq	%0,2f\n"
-	".subsection 2\n"
-	"2:	br	1b\n"
-	".previous"
-	:"=&r" (temp), "=m" (sem->count)
-	:"Ir" (val), "m" (sem->count));
-#endif
-}
-
-static inline long rwsem_atomic_update(long val, struct rw_semaphore *sem)
-{
-#ifndef	CONFIG_SMP
-	sem->count += val;
-	return sem->count;
-#else
-	long ret, temp;
-	__asm__ __volatile__(
-	"1:	ldq_l	%0,%1\n"
-	"	addq 	%0,%3,%2\n"
-	"	addq	%0,%3,%0\n"
-	"	stq_c	%2,%1\n"
-	"	beq	%2,2f\n"
-	".subsection 2\n"
-	"2:	br	1b\n"
-	".previous"
-	:"=&r" (ret), "=m" (sem->count), "=&r" (temp)
-	:"Ir" (val), "m" (sem->count));
-
-	return ret;
-#endif
-}
-
 #endif /* __KERNEL__ */
 #endif /* _ALPHA_RWSEM_H */
--- a/arch/ia64/include/asm/rwsem.h
+++ b/arch/ia64/include/asm/rwsem.h
@@ -151,11 +151,4 @@ __downgrade_write (struct rw_semaphore *
 		rwsem_downgrade_wake(sem);
 }
 
-/*
- * Implement atomic add functionality.  These used to be "inline" functions, but GCC v3.1
- * doesn't quite optimize this stuff right and ends up with bad calls to fetchandadd.
- */
-#define rwsem_atomic_add(delta, sem)	atomic64_add(delta, (atomic64_t *)(&(sem)->count))
-#define rwsem_atomic_update(delta, sem)	atomic64_add_return(delta, (atomic64_t *)(&(sem)->count))
-
 #endif /* _ASM_IA64_RWSEM_H */
--- a/arch/s390/include/asm/rwsem.h
+++ b/arch/s390/include/asm/rwsem.h
@@ -207,41 +207,4 @@ static inline void __downgrade_write(str
 		rwsem_downgrade_wake(sem);
 }
 
-/*
- * implement atomic add functionality
- */
-static inline void rwsem_atomic_add(long delta, struct rw_semaphore *sem)
-{
-	signed long old, new;
-
-	asm volatile(
-		"	lg	%0,%2\n"
-		"0:	lgr	%1,%0\n"
-		"	agr	%1,%4\n"
-		"	csg	%0,%1,%2\n"
-		"	jl	0b"
-		: "=&d" (old), "=&d" (new), "=Q" (sem->count)
-		: "Q" (sem->count), "d" (delta)
-		: "cc", "memory");
-}
-
-/*
- * implement exchange and add functionality
- */
-static inline long rwsem_atomic_update(long delta, struct rw_semaphore *sem)
-{
-	signed long old, new;
-
-	asm volatile(
-		"	lg	%0,%2\n"
-		"0:	lgr	%1,%0\n"
-		"	agr	%1,%4\n"
-		"	csg	%0,%1,%2\n"
-		"	jl	0b"
-		: "=&d" (old), "=&d" (new), "=Q" (sem->count)
-		: "Q" (sem->count), "d" (delta)
-		: "cc", "memory");
-	return new;
-}
-
 #endif /* _S390_RWSEM_H */
--- a/arch/x86/include/asm/rwsem.h
+++ b/arch/x86/include/asm/rwsem.h
@@ -215,23 +215,5 @@ static inline void __downgrade_write(str
 		     : "memory", "cc");
 }
 
-/*
- * implement atomic add functionality
- */
-static inline void rwsem_atomic_add(long delta, struct rw_semaphore *sem)
-{
-	asm volatile(LOCK_PREFIX _ASM_ADD "%1,%0"
-		     : "+m" (sem->count)
-		     : "er" (delta));
-}
-
-/*
- * implement exchange and add functionality
- */
-static inline long rwsem_atomic_update(long delta, struct rw_semaphore *sem)
-{
-	return delta + xadd(&sem->count, delta);
-}
-
 #endif /* __KERNEL__ */
 #endif /* _ASM_X86_RWSEM_H */
--- a/include/asm-generic/rwsem.h
+++ b/include/asm-generic/rwsem.h
@@ -107,14 +107,6 @@ static inline void __up_write(struct rw_
 }
 
 /*
- * implement atomic add functionality
- */
-static inline void rwsem_atomic_add(long delta, struct rw_semaphore *sem)
-{
-	atomic_long_add(delta, (atomic_long_t *)&sem->count);
-}
-
-/*
  * downgrade write lock to read lock
  */
 static inline void __downgrade_write(struct rw_semaphore *sem)
@@ -134,13 +126,5 @@ static inline void __downgrade_write(str
 		rwsem_downgrade_wake(sem);
 }
 
-/*
- * implement exchange and add functionality
- */
-static inline long rwsem_atomic_update(long delta, struct rw_semaphore *sem)
-{
-	return atomic_long_add_return(delta, (atomic_long_t *)&sem->count);
-}
-
 #endif	/* __KERNEL__ */
 #endif	/* _ASM_GENERIC_RWSEM_H */

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Jason Low]

On Tue, 2016-05-17 at 13:09 +0200, Peter Zijlstra wrote:
> On Mon, May 16, 2016 at 06:12:25PM -0700, Linus Torvalds wrote:
> > On Mon, May 16, 2016 at 5:37 PM, Jason Low <jason.low2@hpe.com> wrote:
> > >
> > > This rest of the series converts the rwsem count variable to an atomic_long_t
> > > since it is used it as an atomic variable. This allows us to also remove
> > > the rwsem_atomic_{add,update} abstraction and reduce 100+ lines of code.
> > 
> > I would suggest you merge all the "remove rwsem_atomic_{add,update}"
> > patches into a single patch.
> > 
> > I don't see the advantage to splitting those up by architecture, and
> > it does add noise to the series.
> > 
> > Other than that it all looks fine to me.
> 
> OK, done.

Right, they all fit under the same category of "Removing
rwsem_atomic_{add,update}", so it makes sense to fold them into one
patch.

Thanks,
Jason

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Davidlohr Bueso]

On Tue, 17 May 2016, Peter Zijlstra wrote:

>Subject: locking,rwsem: Remove rwsem_atomic_add() and rwsem_atomic_update()
>From: Jason Low <jason.low2@hpe.com>
>Date: Mon, 16 May 2016 17:38:02 -0700
>
>The rwsem-xadd count has been converted to an atomic variable and the
>rwsem code now directly uses atomic_long_add() and
>atomic_long_add_return(), so we can remove the arch implementations of
>rwsem_atomic_add() and rwsem_atomic_update().
>
>Cc: Waiman Long <Waiman.Long@hpe.com>
>Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
>Cc: Jason Low <jason.low2@hp.com>
>Cc: Richard Henderson <rth@twiddle.net>
>Cc: Ingo Molnar <mingo@redhat.com>
>Cc: Fenghua Yu <fenghua.yu@intel.com>
>Cc: Andrew Morton <akpm@linux-foundation.org>
>Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
>Cc: Linus Torvalds <torvalds@linux-foundation.org>
>Cc: Tim Chen <tim.c.chen@linux.intel.com>
>Cc: Christoph Lameter <cl@linux.com>
>Cc: Thomas Gleixner <tglx@linutronix.de>
>Cc: Arnd Bergmann <arnd@arndb.de>
>Cc: Terry Rudd <terry.rudd@hpe.com>
>Cc: Peter Hurley <peter@hurleysoftware.com>
>Cc: Davidlohr Bueso <dave@stgolabs.net>

Acked-by: Davidlohr Bueso <dave@stgolabs.net>

>Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
>Cc: Tony Luck <tony.luck@intel.com>
>Cc: Matt Turner <mattst88@gmail.com>
>Signed-off-by: Jason Low <jason.low2@hpe.com>
>Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>---
> arch/alpha/include/asm/rwsem.h |   42 -----------------------------------------
> arch/ia64/include/asm/rwsem.h  |    7 ------
> arch/s390/include/asm/rwsem.h  |   37 ------------------------------------
> arch/x86/include/asm/rwsem.h   |   18 -----------------
> include/asm-generic/rwsem.h    |   16 ---------------
> 5 files changed, 120 deletions(-)

Nice. This, along with Michal's work getting rid of a lot of superfluous
implementations, have gotten rid of plenty of rwsem code in arch/*

Thanks,
Davidlohr

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Mon, May 16, 2016 at 06:12:25PM -0700, Linus Torvalds wrote:
> > On Mon, May 16, 2016 at 5:37 PM, Jason Low <jason.low2@hpe.com> wrote:
> > >
> > > This rest of the series converts the rwsem count variable to an atomic_long_t
> > > since it is used it as an atomic variable. This allows us to also remove
> > > the rwsem_atomic_{add,update} abstraction and reduce 100+ lines of code.
> > 
> > I would suggest you merge all the "remove rwsem_atomic_{add,update}"
> > patches into a single patch.
> > 
> > I don't see the advantage to splitting those up by architecture, and
> > it does add noise to the series.
> > 
> > Other than that it all looks fine to me.
> 
> OK, done.
> 
> ---
> Subject: locking,rwsem: Remove rwsem_atomic_add() and rwsem_atomic_update()
> From: Jason Low <jason.low2@hpe.com>
> Date: Mon, 16 May 2016 17:38:02 -0700

So I tried to pick up this series, and it broke the Alpha and IA64 builds:

/home/mingo/tip/arch/ia64/include/asm/rwsem.h: In function '___down_write':
/home/mingo/tip/arch/ia64/include/asm/rwsem.h:58:7: error: incompatible types when 
assigning to type 'long int' from type 'atomic_long_t'
   old = sem->count;
       ^

home/mingo/tip/arch/alpha/include/asm/rwsem.h: In function '__down_read':
/home/mingo/tip/arch/alpha/include/asm/rwsem.h:28:11: error: incompatible types 
when assigning to type 'long int' from type 'atomic_long_t'
  oldcount = sem->count;
           ^

etc.

btw., for some reason I don't have the mails from Jason in my mbox, perhaps GMail 
spam filtering ate it?

Thanks,

	Ingo

[tip:locking/core] locking/rwsem: Optimize write lock by reducing operations in slowpath

[tip-bot for Jason Low]

Commit-ID:  c0fcb6c2d332041256dc55d8a1ec3c0a2d0befb8
Gitweb:     http://git.kernel.org/tip/c0fcb6c2d332041256dc55d8a1ec3c0a2d0befb8
Author:     Jason Low <jason.low2@hpe.com>
AuthorDate: Mon, 16 May 2016 17:38:00 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 3 Jun 2016 09:47:13 +0200

locking/rwsem: Optimize write lock by reducing operations in slowpath

When acquiring the rwsem write lock in the slowpath, we first try
to set count to RWSEM_WAITING_BIAS. When that is successful,
we then atomically add the RWSEM_WAITING_BIAS in cases where
there are other tasks on the wait list. This causes write lock
operations to often issue multiple atomic operations.

We can instead make the list_is_singular() check first, and then
set the count accordingly, so that we issue at most 1 atomic
operation when acquiring the write lock and reduce unnecessary
cacheline contention.

Signed-off-by: Jason Low <jason.low2@hpe.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Waiman Long<Waiman.Long@hpe.com>
Acked-by: Davidlohr Bueso <dave@stgolabs.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Christoph Lameter <cl@linux.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Jason Low <jason.low2@hp.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Terry Rudd <terry.rudd@hpe.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Link: http://lkml.kernel.org/r/1463445486-16078-2-git-send-email-jason.low2@hpe.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/locking/rwsem-xadd.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index fcbf75a..b957da7 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -261,17 +261,28 @@ struct rw_semaphore __sched *rwsem_down_read_failed(struct rw_semaphore *sem)
 }
 EXPORT_SYMBOL(rwsem_down_read_failed);
 
+/*
+ * This function must be called with the sem->wait_lock held to prevent
+ * race conditions between checking the rwsem wait list and setting the
+ * sem->count accordingly.
+ */
 static inline bool rwsem_try_write_lock(long count, struct rw_semaphore *sem)
 {
 	/*
-	 * Try acquiring the write lock. Check count first in order
-	 * to reduce unnecessary expensive cmpxchg() operations.
+	 * Avoid trying to acquire write lock if count isn't RWSEM_WAITING_BIAS.
 	 */
-	if (count == RWSEM_WAITING_BIAS &&
-	    cmpxchg_acquire(&sem->count, RWSEM_WAITING_BIAS,
-		    RWSEM_ACTIVE_WRITE_BIAS) == RWSEM_WAITING_BIAS) {
-		if (!list_is_singular(&sem->wait_list))
-			rwsem_atomic_update(RWSEM_WAITING_BIAS, sem);
+	if (count != RWSEM_WAITING_BIAS)
+		return false;
+
+	/*
+	 * Acquire the lock by trying to set it to ACTIVE_WRITE_BIAS. If there
+	 * are other tasks on the wait list, we need to add on WAITING_BIAS.
+	 */
+	count = list_is_singular(&sem->wait_list) ?
+			RWSEM_ACTIVE_WRITE_BIAS :
+			RWSEM_ACTIVE_WRITE_BIAS + RWSEM_WAITING_BIAS;
+
+	if (cmpxchg_acquire(&sem->count, RWSEM_WAITING_BIAS, count) == RWSEM_WAITING_BIAS) {
 		rwsem_set_owner(sem);
 		return true;
 	}

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Peter Zijlstra]

On Fri, Jun 03, 2016 at 10:04:07AM +0200, Ingo Molnar wrote:
> So I tried to pick up this series, and it broke the Alpha and IA64 builds:
> 
> /home/mingo/tip/arch/ia64/include/asm/rwsem.h: In function '___down_write':
> /home/mingo/tip/arch/ia64/include/asm/rwsem.h:58:7: error: incompatible types when 
> assigning to type 'long int' from type 'atomic_long_t'
>    old = sem->count;
>        ^
> 
> home/mingo/tip/arch/alpha/include/asm/rwsem.h: In function '__down_read':
> /home/mingo/tip/arch/alpha/include/asm/rwsem.h:28:11: error: incompatible types 
> when assigning to type 'long int' from type 'atomic_long_t'
>   oldcount = sem->count;
>            ^

Yeah, that's intermediate borkage, the easiest fix would be to just fold
the two patches. Let me do that and verify it builds properly.

> 
> btw., for some reason I don't have the mails from Jason in my mbox, perhaps GMail 
> spam filtering ate it?

Linus has been complaining about a lot of lost email due to gmail's idea
of what constitutes spam, so that's entirely possible.

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Peter Zijlstra]

On Fri, Jun 03, 2016 at 02:00:42PM +0200, Peter Zijlstra wrote:
> On Fri, Jun 03, 2016 at 10:04:07AM +0200, Ingo Molnar wrote:
> > So I tried to pick up this series, and it broke the Alpha and IA64 builds:
> > 
> > /home/mingo/tip/arch/ia64/include/asm/rwsem.h: In function '___down_write':
> > /home/mingo/tip/arch/ia64/include/asm/rwsem.h:58:7: error: incompatible types when 
> > assigning to type 'long int' from type 'atomic_long_t'
> >    old = sem->count;
> >        ^
> > 
> > home/mingo/tip/arch/alpha/include/asm/rwsem.h: In function '__down_read':
> > /home/mingo/tip/arch/alpha/include/asm/rwsem.h:28:11: error: incompatible types 
> > when assigning to type 'long int' from type 'atomic_long_t'
> >   oldcount = sem->count;
> >            ^
> 
> Yeah, that's intermediate borkage, the easiest fix would be to just fold
> the two patches. Let me do that and verify it builds properly.

Oh, no, its !SMP build, lemme go fix that up.

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Jason Low]

On Fri, 2016-06-03 at 10:04 +0200, Ingo Molnar wrote:
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Mon, May 16, 2016 at 06:12:25PM -0700, Linus Torvalds wrote:
> > > On Mon, May 16, 2016 at 5:37 PM, Jason Low <jason.low2@hpe.com> wrote:
> > > >
> > > > This rest of the series converts the rwsem count variable to an atomic_long_t
> > > > since it is used it as an atomic variable. This allows us to also remove
> > > > the rwsem_atomic_{add,update} abstraction and reduce 100+ lines of code.
> > > 
> > > I would suggest you merge all the "remove rwsem_atomic_{add,update}"
> > > patches into a single patch.
> > > 
> > > I don't see the advantage to splitting those up by architecture, and
> > > it does add noise to the series.
> > > 
> > > Other than that it all looks fine to me.
> > 
> > OK, done.
> > 
> > ---
> > Subject: locking,rwsem: Remove rwsem_atomic_add() and rwsem_atomic_update()
> > From: Jason Low <jason.low2@hpe.com>
> > Date: Mon, 16 May 2016 17:38:02 -0700
> 
> So I tried to pick up this series, and it broke the Alpha and IA64 builds:
> 
> /home/mingo/tip/arch/ia64/include/asm/rwsem.h: In function '___down_write':
> /home/mingo/tip/arch/ia64/include/asm/rwsem.h:58:7: error: incompatible types when 
> assigning to type 'long int' from type 'atomic_long_t'
>    old = sem->count;
>        ^
> 
> home/mingo/tip/arch/alpha/include/asm/rwsem.h: In function '__down_read':
> /home/mingo/tip/arch/alpha/include/asm/rwsem.h:28:11: error: incompatible types 
> when assigning to type 'long int' from type 'atomic_long_t'
>   oldcount = sem->count;

Right, we would also need to convert the sem->count usages in those
architectures to use atomic_long_read(), ect...

---
Subject: [PATCH] locking/rwsem: Convert rwsem->count to atomic_long_t

Convert the rwsem count variable to an atomic_long_t since we use it
as an atomic variable. This also allows us to remove the
rwsem_atomic_{add,update} "abstraction" which would now be an unnecesary
level of indirection. In follow up patches, we also remove the
rwsem_atomic_{add,update} definitions across the various architectures.
 
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jason Low <jason.low2@hpe.com>
---
 arch/alpha/include/asm/rwsem.h | 26 +++++++++++++-------------
 arch/ia64/include/asm/rwsem.h  | 20 ++++++++++----------
 include/asm-generic/rwsem.h    |  6 +++---
 include/linux/rwsem.h          |  6 +++---
 kernel/locking/rwsem-xadd.c    | 32 +++++++++++++++++---------------
 5 files changed, 46 insertions(+), 44 deletions(-)

diff --git a/arch/alpha/include/asm/rwsem.h b/arch/alpha/include/asm/rwsem.h
index 0131a70..9b66688 100644
--- a/arch/alpha/include/asm/rwsem.h
+++ b/arch/alpha/include/asm/rwsem.h
@@ -25,8 +25,8 @@ static inline void __down_read(struct rw_semaphore *sem)
 {
 	long oldcount;
 #ifndef	CONFIG_SMP
-	oldcount = sem->count;
-	sem->count += RWSEM_ACTIVE_READ_BIAS;
+	oldcount = atomic_long_read(&sem->count);
+	atomic_long_add(RWSEM_ACTIVE_READ_BIAS, &sem->count);
 #else
 	long temp;
 	__asm__ __volatile__(
@@ -52,13 +52,13 @@ static inline int __down_read_trylock(struct rw_semaphore *sem)
 {
 	long old, new, res;
 
-	res = sem->count;
+	res = atomic_long_read(&sem->count);
 	do {
 		new = res + RWSEM_ACTIVE_READ_BIAS;
 		if (new <= 0)
 			break;
 		old = res;
-		res = cmpxchg(&sem->count, old, new);
+		res = atomic_long_cmpxchg(&sem->count, old, new);
 	} while (res != old);
 	return res >= 0 ? 1 : 0;
 }
@@ -67,8 +67,8 @@ static inline long ___down_write(struct rw_semaphore *sem)
 {
 	long oldcount;
 #ifndef	CONFIG_SMP
-	oldcount = sem->count;
-	sem->count += RWSEM_ACTIVE_WRITE_BIAS;
+	oldcount = atomic_long_read(&sem->count);
+	atomic_long_add(RWSEM_ACTIVE_WRITE_BIAS, &sem->count);
 #else
 	long temp;
 	__asm__ __volatile__(
@@ -106,7 +106,7 @@ static inline int __down_write_killable(struct rw_semaphore *sem)
  */
 static inline int __down_write_trylock(struct rw_semaphore *sem)
 {
-	long ret = cmpxchg(&sem->count, RWSEM_UNLOCKED_VALUE,
+	long ret = atomic_long_cmpxchg(&sem->count, RWSEM_UNLOCKED_VALUE,
 			   RWSEM_ACTIVE_WRITE_BIAS);
 	if (ret == RWSEM_UNLOCKED_VALUE)
 		return 1;
@@ -117,8 +117,8 @@ static inline void __up_read(struct rw_semaphore *sem)
 {
 	long oldcount;
 #ifndef	CONFIG_SMP
-	oldcount = sem->count;
-	sem->count -= RWSEM_ACTIVE_READ_BIAS;
+	oldcount = atomic_long_read(&sem->count);
+	atomic_long_add(-RWSEM_ACTIVE_READ_BIAS, &sem->count);
 #else
 	long temp;
 	__asm__ __volatile__(
@@ -142,8 +142,8 @@ static inline void __up_write(struct rw_semaphore *sem)
 {
 	long count;
 #ifndef	CONFIG_SMP
-	sem->count -= RWSEM_ACTIVE_WRITE_BIAS;
-	count = sem->count;
+	atomic_long_add(-RWSEM_ACTIVE_WRITE_BIAS, &sem->count);
+	count = atomic_long_read(&sem->count);
 #else
 	long temp;
 	__asm__ __volatile__(
@@ -171,8 +171,8 @@ static inline void __downgrade_write(struct rw_semaphore *sem)
 {
 	long oldcount;
 #ifndef	CONFIG_SMP
-	oldcount = sem->count;
-	sem->count -= RWSEM_WAITING_BIAS;
+	oldcount = atomic_long_read(&sem->count);
+	atomic_long_add(-RWSEM_WAITING_BIAS, &sem->count);
 #else
 	long temp;
 	__asm__ __volatile__(
diff --git a/arch/ia64/include/asm/rwsem.h b/arch/ia64/include/asm/rwsem.h
index 8b23e07..72c37bb 100644
--- a/arch/ia64/include/asm/rwsem.h
+++ b/arch/ia64/include/asm/rwsem.h
@@ -55,9 +55,9 @@ ___down_write (struct rw_semaphore *sem)
 	long old, new;
 
 	do {
-		old = sem->count;
+		old = atomic_long_read(&sem->count);
 		new = old + RWSEM_ACTIVE_WRITE_BIAS;
-	} while (cmpxchg_acq(&sem->count, old, new) != old);
+	} while (atomic_long_cmpxchg_acquire(&sem->count, old, new) != old);
 
 	return old;
 }
@@ -100,9 +100,9 @@ __up_write (struct rw_semaphore *sem)
 	long old, new;
 
 	do {
-		old = sem->count;
+		old = atomic_long_read(&sem->count);
 		new = old - RWSEM_ACTIVE_WRITE_BIAS;
-	} while (cmpxchg_rel(&sem->count, old, new) != old);
+	} while (atomic_long_cmpxchg_release(&sem->count, old, new) != old);
 
 	if (new < 0 && (new & RWSEM_ACTIVE_MASK) == 0)
 		rwsem_wake(sem);
@@ -115,8 +115,8 @@ static inline int
 __down_read_trylock (struct rw_semaphore *sem)
 {
 	long tmp;
-	while ((tmp = sem->count) >= 0) {
-		if (tmp == cmpxchg_acq(&sem->count, tmp, tmp+1)) {
+	while ((tmp = atomic_long_read(&sem->count)) >= 0) {
+		if (tmp == atomic_long_cmpxchg_acquire(&sem->count, tmp, tmp+1)) {
 			return 1;
 		}
 	}
@@ -129,8 +129,8 @@ __down_read_trylock (struct rw_semaphore *sem)
 static inline int
 __down_write_trylock (struct rw_semaphore *sem)
 {
-	long tmp = cmpxchg_acq(&sem->count, RWSEM_UNLOCKED_VALUE,
-			      RWSEM_ACTIVE_WRITE_BIAS);
+	long tmp = atomic_long_cmpxchg_acquire(&sem->count,
+			RWSEM_UNLOCKED_VALUE, RWSEM_ACTIVE_WRITE_BIAS);
 	return tmp == RWSEM_UNLOCKED_VALUE;
 }
 
@@ -143,9 +143,9 @@ __downgrade_write (struct rw_semaphore *sem)
 	long old, new;
 
 	do {
-		old = sem->count;
+		old = atomic_long_read(&sem->count);
 		new = old - RWSEM_WAITING_BIAS;
-	} while (cmpxchg_rel(&sem->count, old, new) != old);
+	} while (atomic_long_cmpxchg_release(&sem->count, old, new) != old);
 
 	if (old < 0)
 		rwsem_downgrade_wake(sem);
diff --git a/include/asm-generic/rwsem.h b/include/asm-generic/rwsem.h
index 3fc94a0..a3a93ec 100644
--- a/include/asm-generic/rwsem.h
+++ b/include/asm-generic/rwsem.h
@@ -41,8 +41,8 @@ static inline int __down_read_trylock(struct rw_semaphore *sem)
 {
 	long tmp;
 
-	while ((tmp = sem->count) >= 0) {
-		if (tmp == cmpxchg_acquire(&sem->count, tmp,
+	while ((tmp = atomic_long_read(&sem->count)) >= 0) {
+		if (tmp == atomic_long_cmpxchg_acquire(&sem->count, tmp,
 				   tmp + RWSEM_ACTIVE_READ_BIAS)) {
 			return 1;
 		}
@@ -79,7 +79,7 @@ static inline int __down_write_trylock(struct rw_semaphore *sem)
 {
 	long tmp;
 
-	tmp = cmpxchg_acquire(&sem->count, RWSEM_UNLOCKED_VALUE,
+	tmp = atomic_long_cmpxchg_acquire(&sem->count, RWSEM_UNLOCKED_VALUE,
 		      RWSEM_ACTIVE_WRITE_BIAS);
 	return tmp == RWSEM_UNLOCKED_VALUE;
 }
diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h
index d37fbb3..cb4d4aa 100644
--- a/include/linux/rwsem.h
+++ b/include/linux/rwsem.h
@@ -26,7 +26,7 @@ struct rw_semaphore;
 #else
 /* All arch specific implementations share the same struct */
 struct rw_semaphore {
-	long count;
+	atomic_long_t count;
 	struct list_head wait_list;
 	raw_spinlock_t wait_lock;
 #ifdef CONFIG_RWSEM_SPIN_ON_OWNER
@@ -54,7 +54,7 @@ extern struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem);
 /* In all implementations count != 0 means locked */
 static inline int rwsem_is_locked(struct rw_semaphore *sem)
 {
-	return sem->count != 0;
+	return atomic_long_read(&sem->count) != 0;
 }
 
 #endif
@@ -74,7 +74,7 @@ static inline int rwsem_is_locked(struct rw_semaphore *sem)
 #endif
 
 #define __RWSEM_INITIALIZER(name)				\
-	{ .count = RWSEM_UNLOCKED_VALUE,			\
+	{ .count = ATOMIC_LONG_INIT(RWSEM_UNLOCKED_VALUE),	\
 	  .wait_list = LIST_HEAD_INIT((name).wait_list),	\
 	  .wait_lock = __RAW_SPIN_LOCK_UNLOCKED(name.wait_lock)	\
 	  __RWSEM_OPT_INIT(name)				\
diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index b957da7..63b40a5 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -80,7 +80,7 @@ void __init_rwsem(struct rw_semaphore *sem, const char *name,
 	debug_check_no_locks_freed((void *)sem, sizeof(*sem));
 	lockdep_init_map(&sem->dep_map, name, key, 0);
 #endif
-	sem->count = RWSEM_UNLOCKED_VALUE;
+	atomic_long_set(&sem->count, RWSEM_UNLOCKED_VALUE);
 	raw_spin_lock_init(&sem->wait_lock);
 	INIT_LIST_HEAD(&sem->wait_list);
 #ifdef CONFIG_RWSEM_SPIN_ON_OWNER
@@ -153,10 +153,11 @@ __rwsem_mark_wake(struct rw_semaphore *sem,
 	if (wake_type != RWSEM_WAKE_READ_OWNED) {
 		adjustment = RWSEM_ACTIVE_READ_BIAS;
  try_reader_grant:
-		oldcount = rwsem_atomic_update(adjustment, sem) - adjustment;
+		oldcount = atomic_long_add_return(adjustment, &sem->count) - adjustment;
+
 		if (unlikely(oldcount < RWSEM_WAITING_BIAS)) {
 			/* A writer stole the lock. Undo our reader grant. */
-			if (rwsem_atomic_update(-adjustment, sem) &
+			if (atomic_long_sub_return(adjustment, &sem->count) &
 						RWSEM_ACTIVE_MASK)
 				goto out;
 			/* Last active locker left. Retry waking readers. */
@@ -186,7 +187,7 @@ __rwsem_mark_wake(struct rw_semaphore *sem,
 		adjustment -= RWSEM_WAITING_BIAS;
 
 	if (adjustment)
-		rwsem_atomic_add(adjustment, sem);
+		atomic_long_add(adjustment, &sem->count);
 
 	next = sem->wait_list.next;
 	loop = woken;
@@ -233,7 +234,7 @@ struct rw_semaphore __sched *rwsem_down_read_failed(struct rw_semaphore *sem)
 	list_add_tail(&waiter.list, &sem->wait_list);
 
 	/* we're now waiting on the lock, but no longer actively locking */
-	count = rwsem_atomic_update(adjustment, sem);
+	count = atomic_long_add_return(adjustment, &sem->count);
 
 	/* If there are no active locks, wake the front queued process(es).
 	 *
@@ -282,7 +283,8 @@ static inline bool rwsem_try_write_lock(long count, struct rw_semaphore *sem)
 			RWSEM_ACTIVE_WRITE_BIAS :
 			RWSEM_ACTIVE_WRITE_BIAS + RWSEM_WAITING_BIAS;
 
-	if (cmpxchg_acquire(&sem->count, RWSEM_WAITING_BIAS, count) == RWSEM_WAITING_BIAS) {
+	if (atomic_long_cmpxchg_acquire(&sem->count, RWSEM_WAITING_BIAS, count)
+							== RWSEM_WAITING_BIAS) {
 		rwsem_set_owner(sem);
 		return true;
 	}
@@ -296,13 +298,13 @@ static inline bool rwsem_try_write_lock(long count, struct rw_semaphore *sem)
  */
 static inline bool rwsem_try_write_lock_unqueued(struct rw_semaphore *sem)
 {
-	long old, count = READ_ONCE(sem->count);
+	long old, count = atomic_long_read(&sem->count);
 
 	while (true) {
 		if (!(count == 0 || count == RWSEM_WAITING_BIAS))
 			return false;
 
-		old = cmpxchg_acquire(&sem->count, count,
+		old = atomic_long_cmpxchg_acquire(&sem->count, count,
 				      count + RWSEM_ACTIVE_WRITE_BIAS);
 		if (old == count) {
 			rwsem_set_owner(sem);
@@ -324,7 +326,7 @@ static inline bool rwsem_can_spin_on_owner(struct rw_semaphore *sem)
 	rcu_read_lock();
 	owner = READ_ONCE(sem->owner);
 	if (!owner) {
-		long count = READ_ONCE(sem->count);
+		long count = atomic_long_read(&sem->count);
 		/*
 		 * If sem->owner is not set, yet we have just recently entered the
 		 * slowpath with the lock being active, then there is a possibility
@@ -375,7 +377,7 @@ bool rwsem_spin_on_owner(struct rw_semaphore *sem, struct task_struct *owner)
 	 * held by readers. Check the counter to verify the
 	 * state.
 	 */
-	count = READ_ONCE(sem->count);
+	count = atomic_long_read(&sem->count);
 	return (count == 0 || count == RWSEM_WAITING_BIAS);
 }
 
@@ -460,7 +462,7 @@ __rwsem_down_write_failed_common(struct rw_semaphore *sem, int state)
 	WAKE_Q(wake_q);
 
 	/* undo write bias from down_write operation, stop active locking */
-	count = rwsem_atomic_update(-RWSEM_ACTIVE_WRITE_BIAS, sem);
+	count = atomic_long_sub_return(RWSEM_ACTIVE_WRITE_BIAS, &sem->count);
 
 	/* do optimistic spinning and steal lock if possible */
 	if (rwsem_optimistic_spin(sem))
@@ -483,7 +485,7 @@ __rwsem_down_write_failed_common(struct rw_semaphore *sem, int state)
 
 	/* we're now waiting on the lock, but no longer actively locking */
 	if (waiting) {
-		count = READ_ONCE(sem->count);
+		count = atomic_long_read(&sem->count);
 
 		/*
 		 * If there were already threads queued before us and there are
@@ -505,7 +507,7 @@ __rwsem_down_write_failed_common(struct rw_semaphore *sem, int state)
 		}
 
 	} else
-		count = rwsem_atomic_update(RWSEM_WAITING_BIAS, sem);
+		count = atomic_long_add_return(RWSEM_WAITING_BIAS, &sem->count);
 
 	/* wait until we successfully acquire the lock */
 	set_current_state(state);
@@ -521,7 +523,7 @@ __rwsem_down_write_failed_common(struct rw_semaphore *sem, int state)
 
 			schedule();
 			set_current_state(state);
-		} while ((count = sem->count) & RWSEM_ACTIVE_MASK);
+		} while ((count = atomic_long_read(&sem->count)) & RWSEM_ACTIVE_MASK);
 
 		raw_spin_lock_irq(&sem->wait_lock);
 	}
@@ -536,7 +538,7 @@ out_nolock:
 	raw_spin_lock_irq(&sem->wait_lock);
 	list_del(&waiter.list);
 	if (list_empty(&sem->wait_list))
-		rwsem_atomic_update(-RWSEM_WAITING_BIAS, sem);
+		atomic_long_add(-RWSEM_WAITING_BIAS, &sem->count);
 	else
 		__rwsem_mark_wake(sem, RWSEM_WAKE_ANY, &wake_q);
 	raw_spin_unlock_irq(&sem->wait_lock);
-- 
2.1.4

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Peter Zijlstra]

On Fri, Jun 03, 2016 at 11:09:54AM -0700, Jason Low wrote:

>  arch/alpha/include/asm/rwsem.h | 26 +++++++++++++-------------
>  arch/ia64/include/asm/rwsem.h  | 20 ++++++++++----------

So I was looking at rm arch/*/include/asm/rwsem.h :-) Because at the end
of the day you're reimplementing atomic bits.

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Peter Zijlstra]

On Fri, Jun 03, 2016 at 11:09:54AM -0700, Jason Low wrote:
> --- a/arch/alpha/include/asm/rwsem.h
> +++ b/arch/alpha/include/asm/rwsem.h
> @@ -25,8 +25,8 @@ static inline void __down_read(struct rw_semaphore *sem)
>  {
>  	long oldcount;
>  #ifndef	CONFIG_SMP
> -	oldcount = sem->count;
> -	sem->count += RWSEM_ACTIVE_READ_BIAS;
> +	oldcount = atomic_long_read(&sem->count);
> +	atomic_long_add(RWSEM_ACTIVE_READ_BIAS, &sem->count);
>  #else

That _completely_ misses the point of the whole !SMP code.

Something like:

diff --git a/arch/alpha/include/asm/rwsem.h b/arch/alpha/include/asm/rwsem.h
index a217bf8..77873d0 100644
--- a/arch/alpha/include/asm/rwsem.h
+++ b/arch/alpha/include/asm/rwsem.h
@@ -25,8 +25,8 @@ static inline void __down_read(struct rw_semaphore *sem)
 {
 	long oldcount;
 #ifndef	CONFIG_SMP
-	oldcount = sem->count;
-	sem->count += RWSEM_ACTIVE_READ_BIAS;
+	oldcount = sem->count.counter;
+	sem->count.counter += RWSEM_ACTIVE_READ_BIAS;
 #else
 	long temp;
 	__asm__ __volatile__(

preserves the intent.

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Jason Low]

On Sat, 2016-06-04 at 00:36 +0200, Peter Zijlstra wrote:
> On Fri, Jun 03, 2016 at 11:09:54AM -0700, Jason Low wrote:
> > --- a/arch/alpha/include/asm/rwsem.h
> > +++ b/arch/alpha/include/asm/rwsem.h
> > @@ -25,8 +25,8 @@ static inline void __down_read(struct rw_semaphore *sem)
> >  {
> >  	long oldcount;
> >  #ifndef	CONFIG_SMP
> > -	oldcount = sem->count;
> > -	sem->count += RWSEM_ACTIVE_READ_BIAS;
> > +	oldcount = atomic_long_read(&sem->count);
> > +	atomic_long_add(RWSEM_ACTIVE_READ_BIAS, &sem->count);
> >  #else
> 
> That _completely_ misses the point of the whole !SMP code.

Oh right, this defeats the purpose of the additional code path. Might as
well have removed the code entirely  :\

I can send out a new patch assuming we're still keeping all the arch/*/
rwsem.h?

Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t

[Linus Torvalds]

On Fri, Jun 3, 2016 at 5:00 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Jun 03, 2016 at 10:04:07AM +0200, Ingo Molnar wrote:
>>
>> btw., for some reason I don't have the mails from Jason in my mbox, perhaps GMail
>> spam filtering ate it?
>
> Linus has been complaining about a lot of lost email due to gmail's idea
> of what constitutes spam, so that's entirely possible.

What has happened lately is that gmail has become a _lot_ stricter
about bad email setups. That should not affect the emails from Jason
as far as I can see.

Both broadcom.com and microsoft.com end up having DMARC records, and
email from those domains should have a DKIM signature. And people
inside broadcom and microsoft that had mis-configured smtp server
setups would send out email without that, and gmail seems to have
become much less lax about this, and almost automatically marks them
as spam.

Gmail also seems to notice when some smtp servers send out a lot of
bulk email, and will have a rule that such sites need to also do
proper SPF or DKIM. This hit the tip-bot, for example, which does
neither.

And I can't really say that I'm "complaining" about it. I've
complained about the gmail spam filtering in the past, but while these
problems have been a bit of an annoyance, I think gmail is at least
trying to do the right thing. It's not that I love SPF or DKIM, but I
do think that badly configured smtp servers are a real problem too.

Jason's emails at least have SPF, so they should not be hated upon by
gmail (although spf and mailing lists don't work so well together, but
Ingo was on the direct recipients list, so..).

That said, I've made it a habit of just having one browser tab open
with the spambox, and going through it every day. I've also tried to
let people know when their smtp setup is bogus, although with both
broadcom and microsoft there were several engieers involved and I
ended up instead just asking them to try to educate their cow-orkers
internally, because I couldn't be arsed to remember who I had already
notified..

             Linus