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

Groups > linux.kernel > #1231800 > unrolled thread

[PATCH v2] locking/rtmutex: Use acquire/release semantics

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

  [PATCH v2] locking/rtmutex: Use acquire/release semantics Davidlohr Bueso <dave@stgolabs.net> - 2015-09-24 03:10 +0200
    Re: [PATCH v2] locking/rtmutex: Use acquire/release semantics Thomas Gleixner <tglx@linutronix.de> - 2015-09-27 12:40 +0200
      Re: [PATCH v2] locking/rtmutex: Use acquire/release semantics Davidlohr Bueso <dave@stgolabs.net> - 2015-09-28 22:00 +0200
        Re: [PATCH v2] locking/rtmutex: Use acquire/release semantics Thomas Gleixner <tglx@linutronix.de> - 2015-09-29 23:00 +0200

#1231800 — [PATCH v2] locking/rtmutex: Use acquire/release semantics

From: Davidlohr Bueso <dave@stgolabs.net>

As such, weakly ordered archs can benefit from more relaxed use
of barriers when locking/unlocking.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---

Changes from v1:
  - fix bogus acquire in unlock_rt_mutex_safe() (tglx)

  kernel/locking/rtmutex.c | 30 +++++++++++++++++++++---------
  1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 35e9bfc..8251e75 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -74,14 +74,23 @@ static void fixup_rt_mutex_waiters(struct rt_mutex *lock)
   * set up.
   */
  #ifndef CONFIG_DEBUG_RT_MUTEXES
-# define rt_mutex_cmpxchg(l,c,n)	(cmpxchg(&l->owner, c, n) == c)
+# define rt_mutex_cmpxchg_relaxed(l,c,n) (cmpxchg_relaxed(&l->owner, c, n) == c)
+# define rt_mutex_cmpxchg_acquire(l,c,n) (cmpxchg_acquire(&l->owner, c, n) == c)
+# define rt_mutex_cmpxchg_release(l,c,n) (cmpxchg_release(&l->owner, c, n) == c)
+
+/*
+ * Callers must hold the ->wait_lock -- which is the whole purpose as we force
+ * all future threads that attempt to [Rmw] the lock to the slowpath. As such
+ * relaxed semantics suffice.
+ */
  static inline void mark_rt_mutex_waiters(struct rt_mutex *lock)
  {
  	unsigned long owner, *p = (unsigned long *) &lock->owner;
  
  	do {
  		owner = *p;
-	} while (cmpxchg(p, owner, owner | RT_MUTEX_HAS_WAITERS) != owner);
+	} while (cmpxchg_relaxed(p, owner,
+				 owner | RT_MUTEX_HAS_WAITERS) != owner);
  }
  
  /*
@@ -121,11 +130,14 @@ static inline bool unlock_rt_mutex_safe(struct rt_mutex *lock)
  	 *					lock(wait_lock);
  	 *					acquire(lock);
  	 */
-	return rt_mutex_cmpxchg(lock, owner, NULL);
+	return rt_mutex_cmpxchg_release(lock, owner, NULL);
  }
  
  #else
-# define rt_mutex_cmpxchg(l,c,n)	(0)
+# define rt_mutex_cmpxchg_relaxed(l,c,n)	(0)
+# define rt_mutex_cmpxchg_acquire(l,c,n)	(0)
+# define rt_mutex_cmpxchg_release(l,c,n)	(0)
+
  static inline void mark_rt_mutex_waiters(struct rt_mutex *lock)
  {
  	lock->owner = (struct task_struct *)
@@ -1322,7 +1334,7 @@ rt_mutex_fastlock(struct rt_mutex *lock, int state,
  				struct hrtimer_sleeper *timeout,
  				enum rtmutex_chainwalk chwalk))
  {
-	if (likely(rt_mutex_cmpxchg(lock, NULL, current))) {
+	if (likely(rt_mutex_cmpxchg_acquire(lock, NULL, current))) {
  		rt_mutex_deadlock_account_lock(lock, current);
  		return 0;
  	} else
@@ -1338,7 +1350,7 @@ rt_mutex_timed_fastlock(struct rt_mutex *lock, int state,
  				      enum rtmutex_chainwalk chwalk))
  {
  	if (chwalk == RT_MUTEX_MIN_CHAINWALK &&
-	    likely(rt_mutex_cmpxchg(lock, NULL, current))) {
+	    likely(rt_mutex_cmpxchg_acquire(lock, NULL, current))) {
  		rt_mutex_deadlock_account_lock(lock, current);
  		return 0;
  	} else
@@ -1349,7 +1361,7 @@ static inline int
  rt_mutex_fasttrylock(struct rt_mutex *lock,
  		     int (*slowfn)(struct rt_mutex *lock))
  {
-	if (likely(rt_mutex_cmpxchg(lock, NULL, current))) {
+	if (likely(rt_mutex_cmpxchg_acquire(lock, NULL, current))) {
  		rt_mutex_deadlock_account_lock(lock, current);
  		return 1;
  	}
@@ -1363,7 +1375,7 @@ rt_mutex_fastunlock(struct rt_mutex *lock,
  {
  	WAKE_Q(wake_q);
  
-	if (likely(rt_mutex_cmpxchg(lock, current, NULL))) {
+	if (likely(rt_mutex_cmpxchg_release(lock, current, NULL))) {
  		rt_mutex_deadlock_account_unlock(current);
  
  	} else {
@@ -1485,7 +1497,7 @@ EXPORT_SYMBOL_GPL(rt_mutex_unlock);
  bool __sched rt_mutex_futex_unlock(struct rt_mutex *lock,
  				   struct wake_q_head *wqh)
  {
-	if (likely(rt_mutex_cmpxchg(lock, current, NULL))) {
+	if (likely(rt_mutex_cmpxchg_release(lock, current, NULL))) {
  		rt_mutex_deadlock_account_unlock(current);
  		return false;
  	}
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1233643

On Wed, 23 Sep 2015, Davidlohr Bueso wrote:

> From: Davidlohr Bueso <dave@stgolabs.net>
> 
> As such, weakly ordered archs can benefit from more relaxed use
> of barriers when locking/unlocking.

That changelog is not really helpful if someone is going to look at it
half a year from now who doesn't have the background of the discussion
leading to these changes.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1234398

On Sun, 27 Sep 2015, Thomas Gleixner wrote:

>On Wed, 23 Sep 2015, Davidlohr Bueso wrote:
>
>> From: Davidlohr Bueso <dave@stgolabs.net>
>>
>> As such, weakly ordered archs can benefit from more relaxed use
>> of barriers when locking/unlocking.
>
>That changelog is not really helpful if someone is going to look at it
>half a year from now who doesn't have the background of the discussion
>leading to these changes.

Ok, how does the following sound?

""
As of 654672d4ba1 (locking/atomics: Add _{acquire|release|relaxed}() variants
of some atomic operations) and 6d79ef2d30e (locking, asm-generic: Add
_{relaxed|acquire|release}() variants for 'atomic_long_t'), weakly ordered
archs can benefit from more relaxed use of barriers when locking and unlocking,
instead of regular full barrier semantics. While currently only arm64 supports
such optimizations, updating corresponding locking primitives serves for other
archs to immediately benefit as well, once the necessary machinery is implemented
of course.
""

It's not _that_ different from the original changelong, but it should allow
future readers to at least be able to easily see the context of where the
changes come from. I don't think it's of much use going into the actual code
changes as they are pretty obvious -- and the ones that aren't (ie relaxed)
have the justification in the comments.

Thanks,
Davidlohr
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1235547

On Mon, 28 Sep 2015, Davidlohr Bueso wrote:
> On Sun, 27 Sep 2015, Thomas Gleixner wrote:
> > That changelog is not really helpful if someone is going to look at it
> > half a year from now who doesn't have the background of the discussion
> > leading to these changes.
> 
> Ok, how does the following sound?
> 
> ""
> As of 654672d4ba1 (locking/atomics: Add _{acquire|release|relaxed}() variants
> of some atomic operations) and 6d79ef2d30e (locking, asm-generic: Add
> _{relaxed|acquire|release}() variants for 'atomic_long_t'), weakly ordered
> archs can benefit from more relaxed use of barriers when locking and
> unlocking,
> instead of regular full barrier semantics. While currently only arm64 supports
> such optimizations, updating corresponding locking primitives serves for other
> archs to immediately benefit as well, once the necessary machinery is
> implemented
> of course.
> ""
> 
> It's not _that_ different from the original changelong, but it should allow
> future readers to at least be able to easily see the context of where the
> changes come from. I don't think it's of much use going into the actual code
> changes as they are pretty obvious -- and the ones that aren't (ie relaxed)
> have the justification in the comments.

Ok, care to resend the whole thing?

Thanks,

	tglx


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web