[PATCH 1/2] locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed()

[PATCH 1/2] locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed()

[Alex Shi]

This macro isn't interested by anyone, so remove it.

Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Will Deacon <will@kernel.org>
Cc: linux-kernel@vger.kernel.org
---
 kernel/locking/rtmutex.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index c9f090d64f00..cfdd5b93264d 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -141,7 +141,6 @@ static void fixup_rt_mutex_waiters(struct rt_mutex *lock)
  * set up.
  */
 #ifndef CONFIG_DEBUG_RT_MUTEXES
-# 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)
 
@@ -202,7 +201,6 @@ static inline bool unlock_rt_mutex_safe(struct rt_mutex *lock,
 }
 
 #else
-# 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)
 
-- 
1.8.3.1

[PATCH 2/2] locking/rtmutex: optimize rt_mutex_cmpxchgs

[Alex Shi]

Checking l->owner first to skip time cost cmpxchgs.

Suggested-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Will Deacon <will@kernel.org>
Cc: linux-kernel@vger.kernel.org
---
 kernel/locking/rtmutex.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index cfdd5b93264d..232727a4a220 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -141,8 +141,10 @@ static void fixup_rt_mutex_waiters(struct rt_mutex *lock)
  * set up.
  */
 #ifndef CONFIG_DEBUG_RT_MUTEXES
-# 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)
+# define rt_mutex_cmpxchg_acquire(l, c, n)	\
+			(l->owner == c && cmpxchg_acquire(&l->owner, c, n) == c)
+# define rt_mutex_cmpxchg_release(l, c, n)	\
+			(l->owner == c && cmpxchg_release(&l->owner, c, n) == c)
 
 /*
  * Callers must hold the ->wait_lock -- which is the whole purpose as we force
-- 
1.8.3.1

Re: [PATCH 1/2] locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed()

[Alex Shi]

This 2 small patches may missed for a few time.

Is it ok to be merged?

Thanks
Alex

在 2020/4/17 下午10:50, Alex Shi 写道:
> This macro isn't interested by anyone, so remove it.
> 
> Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-kernel@vger.kernel.org
> ---
>  kernel/locking/rtmutex.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
> index c9f090d64f00..cfdd5b93264d 100644
> --- a/kernel/locking/rtmutex.c
> +++ b/kernel/locking/rtmutex.c
> @@ -141,7 +141,6 @@ static void fixup_rt_mutex_waiters(struct rt_mutex *lock)
>   * set up.
>   */
>  #ifndef CONFIG_DEBUG_RT_MUTEXES
> -# 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)
>  
> @@ -202,7 +201,6 @@ static inline bool unlock_rt_mutex_safe(struct rt_mutex *lock,
>  }
>  
>  #else
> -# 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)
>  
> 

Re: [PATCH 2/2] locking/rtmutex: optimize rt_mutex_cmpxchgs

[Thomas Gleixner]

Alex Shi <alex.shi@linux.alibaba.com> writes:

> Checking l->owner first to skip time cost cmpxchgs.

I don't see what that buys.

It actually adds an extra conditional in the non-contended case, which
is the case we are optimizing for.

In the contended case, i.e. when l->owner != c the cmpxchg cost is
completely irrelevant compared to the slowpath costs.

>  #ifndef CONFIG_DEBUG_RT_MUTEXES
> -# 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)
> +# define rt_mutex_cmpxchg_acquire(l, c, n)	\
> +			(l->owner == c && cmpxchg_acquire(&l->owner, c, n) == c)
> +# define rt_mutex_cmpxchg_release(l, c, n)	\
> +			(l->owner == c && cmpxchg_release(&l->owner, c, n) == c)

This kind of micro-optimizing is more than dubious especially w/o
numbers backing up the benefit.

Thanks,

        tglx

[tip: locking/core] locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed()

[tip-bot2 for Alex Shi]

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     2f879e9f05f7806d0885b57532831176204e411f
Gitweb:        https://git.kernel.org/tip/2f879e9f05f7806d0885b57532831176204e411f
Author:        Alex Shi <alex.shi@linux.alibaba.com>
AuthorDate:    Fri, 17 Apr 2020 22:50:31 +08:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Sun, 26 Apr 2020 21:58:42 +02:00

locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed()

Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/1587135032-188866-1-git-send-email-alex.shi@linux.alibaba.com

---
 kernel/locking/rtmutex.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 851bbb1..7ad22ea 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -141,7 +141,6 @@ static void fixup_rt_mutex_waiters(struct rt_mutex *lock)
  * set up.
  */
 #ifndef CONFIG_DEBUG_RT_MUTEXES
-# 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)
 
@@ -202,7 +201,6 @@ static inline bool unlock_rt_mutex_safe(struct rt_mutex *lock,
 }
 
 #else
-# 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)
 

[tip: locking/core] locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed()

[tip-bot2 for Alex Shi]

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     23b5ae2e8e1326c91b5dfdbb6ebcd5a6820074ae
Gitweb:        https://git.kernel.org/tip/23b5ae2e8e1326c91b5dfdbb6ebcd5a6820074ae
Author:        Alex Shi <alex.shi@linux.alibaba.com>
AuthorDate:    Fri, 17 Apr 2020 22:50:31 +08:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 27 Apr 2020 12:26:40 +02:00

locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed()

Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/1587135032-188866-1-git-send-email-alex.shi@linux.alibaba.com

---
 kernel/locking/rtmutex.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index c9f090d..cfdd5b9 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -141,7 +141,6 @@ static void fixup_rt_mutex_waiters(struct rt_mutex *lock)
  * set up.
  */
 #ifndef CONFIG_DEBUG_RT_MUTEXES
-# 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)
 
@@ -202,7 +201,6 @@ static inline bool unlock_rt_mutex_safe(struct rt_mutex *lock,
 }
 
 #else
-# 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)