Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1401831 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t Jason Low <jason.low2@hpe.com> - 2016-05-17 02:50 +0200
    [RFC][PATCH 6/7] locking,s390: Remove s390 rwsem add and rwsem update Jason Low <jason.low2@hpe.com> - 2016-05-17 02:50 +0200
    [RFC][PATCH 7/7] locking,asm-generic: Remove generic rwsem add and rwsem update definitions Jason Low <jason.low2@hpe.com> - 2016-05-17 02:50 +0200
    [RFC][PATCH 5/7] locking,ia64: Remove ia64 rwsem add and rwsem update Jason Low <jason.low2@hpe.com> - 2016-05-17 02:50 +0200
    [RFC][PATCH 1/7] locking/rwsem: Optimize write lock by reducing operations in slowpath Jason Low <jason.low2@hpe.com> - 2016-05-17 02:50 +0200
    [RFC][PATCH 3/7] locking,x86: Remove x86 rwsem add and rwsem update Jason Low <jason.low2@hpe.com> - 2016-05-17 02:50 +0200
    [RFC][PATCH 2/7] locking/rwsem: Convert sem->count to atomic_long_t Jason Low <jason.low2@hpe.com> - 2016-05-17 02:50 +0200
    Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t Linus Torvalds <torvalds@linux-foundation.org> - 2016-05-17 03:20 +0200
      Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to  atomic_long_t Peter Zijlstra <peterz@infradead.org> - 2016-05-17 13:20 +0200
        Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to  atomic_long_t Jason Low <jason.low2@hpe.com> - 2016-05-17 19:10 +0200
        Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to  atomic_long_t Davidlohr Bueso <dave@stgolabs.net> - 2016-05-20 08:30 +0200

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

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

[toc] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

#1401835 — [RFC][PATCH 1/7] 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>
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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

#1401837 — [RFC][PATCH 2/7] locking/rwsem: Convert sem->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>
---
 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

[toc] | [prev] | [next] | [standalone]

#1401851

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

[toc] | [prev] | [next] | [standalone]

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

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 */

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web