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

Groups > linux.kernel > #1412882 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to  atomic_long_t Ingo Molnar <mingo@kernel.org> - 2016-06-03 10:10 +0200
    Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to  atomic_long_t Peter Zijlstra <peterz@infradead.org> - 2016-06-03 14:10 +0200
      Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to  atomic_long_t Peter Zijlstra <peterz@infradead.org> - 2016-06-03 14:20 +0200
      Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to atomic_long_t Linus Torvalds <torvalds@linux-foundation.org> - 2016-06-04 01:40 +0200
    Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to  atomic_long_t Jason Low <jason.low2@hpe.com> - 2016-06-03 20:20 +0200
      Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to  atomic_long_t Peter Zijlstra <peterz@infradead.org> - 2016-06-03 20:50 +0200
      Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to  atomic_long_t Peter Zijlstra <peterz@infradead.org> - 2016-06-04 00:40 +0200
        Re: [RFC][PATCH 0/7] locking/rwsem: Convert rwsem count to  atomic_long_t Jason Low <jason.low2@hpe.com> - 2016-06-04 01:10 +0200

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

* 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

[toc] | [next] | [standalone]

#1413160

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.

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

#1413172

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.

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

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

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

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

#1413383

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

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

#1413397

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.

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

#1413548

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.

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

#1413569

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?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web