* [PATCH] locking/rtmutex: remove unused cmpxchg_relaxed @ 2020-01-21 8:34 Alex Shi 2020-01-31 17:39 ` Davidlohr Bueso 0 siblings, 1 reply; 7+ messages in thread From: Alex Shi @ 2020-01-21 8:34 UTC (permalink / raw) Cc: Peter Zijlstra, Davidlohr Bueso, Ingo Molnar, Will Deacon, linux-kernel No one use this macro after it was introduced. Better to remove it? Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> Cc: Peter Zijlstra <peterz@infradead.org> 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 | 1 - 1 file changed, 1 deletion(-) diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c index 851bbb10819d..a849964a348d 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) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] locking/rtmutex: remove unused cmpxchg_relaxed 2020-01-21 8:34 [PATCH] locking/rtmutex: remove unused cmpxchg_relaxed Alex Shi @ 2020-01-31 17:39 ` Davidlohr Bueso 2020-01-31 20:23 ` Thomas Gleixner 0 siblings, 1 reply; 7+ messages in thread From: Davidlohr Bueso @ 2020-01-31 17:39 UTC (permalink / raw) To: Alex Shi; +Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, linux-kernel, tglx Cc'ing tglx. On Tue, 21 Jan 2020, Alex Shi wrote: >No one use this macro after it was introduced. Better to remove it? You also need to remove it for the CONFIG_DEBUG_RT_MUTEXES=y case. > >Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> >Cc: Peter Zijlstra <peterz@infradead.org> >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 | 1 - > 1 file changed, 1 deletion(-) > >diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c >index 851bbb10819d..a849964a348d 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) Hmm unrelated, but do we want CCAS for rtmutex fastpath? Ie: (l->owner == c && cmpxchg_acquire(&l->owner, c, n) == c) That would optimize for the contended case and avoid the cmpxchg - it would also help if we ever do the top-waiter spin thing. Thanks, Davidlohr ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] locking/rtmutex: remove unused cmpxchg_relaxed 2020-01-31 17:39 ` Davidlohr Bueso @ 2020-01-31 20:23 ` Thomas Gleixner 2020-02-04 10:18 ` Alex Shi 0 siblings, 1 reply; 7+ messages in thread From: Thomas Gleixner @ 2020-01-31 20:23 UTC (permalink / raw) To: Davidlohr Bueso, Alex Shi Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, linux-kernel Davidlohr Bueso <dave@stgolabs.net> writes: > On Tue, 21 Jan 2020, Alex Shi wrote: Subject: locking/rtmutex: remove unused cmpxchg_relaxed should be Subject: locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed() You're not removing cmpxchg_relaxed, right? >> No one use this macro after it was introduced. Better to remove it? Please make that factual. The macro was never used at all. Remove it. > You also need to remove it for the CONFIG_DEBUG_RT_MUTEXES=y case. Yes. > Hmm unrelated, but do we want CCAS for rtmutex fastpath? Ie: > > (l->owner == c && cmpxchg_acquire(&l->owner, c, n) == c) > > That would optimize for the contended case and avoid the cmpxchg - it would > also help if we ever do the top-waiter spin thing. Not sure if it buys much, but it kinda makes sense. Thanks, tglx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] locking/rtmutex: remove unused cmpxchg_relaxed 2020-01-31 20:23 ` Thomas Gleixner @ 2020-02-04 10:18 ` Alex Shi 2020-02-12 15:07 ` Davidlohr Bueso 2020-02-14 15:17 ` [PATCH 1/2] locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed() Alex Shi 0 siblings, 2 replies; 7+ messages in thread From: Alex Shi @ 2020-02-04 10:18 UTC (permalink / raw) To: Thomas Gleixner, Davidlohr Bueso Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, linux-kernel 在 2020/2/1 上午4:23, Thomas Gleixner 写道: > Davidlohr Bueso <dave@stgolabs.net> writes: >> On Tue, 21 Jan 2020, Alex Shi wrote: > > Subject: locking/rtmutex: remove unused cmpxchg_relaxed > > should be > > Subject: locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed() > > You're not removing cmpxchg_relaxed, right? > >>> No one use this macro after it was introduced. Better to remove it? > > Please make that factual. > > The macro was never used at all. Remove it. > >> You also need to remove it for the CONFIG_DEBUG_RT_MUTEXES=y case. > > Yes. > >> Hmm unrelated, but do we want CCAS for rtmutex fastpath? Ie: >> >> (l->owner == c && cmpxchg_acquire(&l->owner, c, n) == c) >> >> That would optimize for the contended case and avoid the cmpxchg - it would >> also help if we ever do the top-waiter spin thing. > > Not sure if it buys much, but it kinda makes sense. > > Thanks, > > tglx > Thanks Thomas and David! Is this following patch ok? Thanks Alex --- From 4cf9e38a73c67c6894f3addb2ddca26bb51b1a28 Mon Sep 17 00:00:00 2001 From: Alex Shi <alex.shi@linux.alibaba.com> Date: Tue, 21 Jan 2020 15:03:33 +0800 Subject: [PATCH v2] locking/rtmutex: optimize rt_mutex_cmpxchg_xxx series func rt_mutex_cmpxchg_relexed isn't interested by anyone, so remove it. And Davidlohr Bueso suggests check l->owner before cmpxchg to reduce lock contention. 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 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c index 851bbb10819d..eb26f4e57ce4 100644 --- a/kernel/locking/rtmutex.c +++ b/kernel/locking/rtmutex.c @@ -141,9 +141,10 @@ 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) +# 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 @@ -202,7 +203,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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] locking/rtmutex: remove unused cmpxchg_relaxed 2020-02-04 10:18 ` Alex Shi @ 2020-02-12 15:07 ` Davidlohr Bueso 2020-02-14 15:17 ` [PATCH 1/2] locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed() Alex Shi 1 sibling, 0 replies; 7+ messages in thread From: Davidlohr Bueso @ 2020-02-12 15:07 UTC (permalink / raw) To: Alex Shi Cc: Thomas Gleixner, Peter Zijlstra, Ingo Molnar, Will Deacon, linux-kernel On Tue, 04 Feb 2020, Alex Shi wrote: >Thanks Thomas and David! >Is this following patch ok? So if anything, this really wants to be two patches. >--- >From 4cf9e38a73c67c6894f3addb2ddca26bb51b1a28 Mon Sep 17 00:00:00 2001 >From: Alex Shi <alex.shi@linux.alibaba.com> >Date: Tue, 21 Jan 2020 15:03:33 +0800 >Subject: [PATCH v2] locking/rtmutex: optimize rt_mutex_cmpxchg_xxx series func > >rt_mutex_cmpxchg_relexed isn't interested by anyone, so remove it. >And Davidlohr Bueso suggests check l->owner before cmpxchg to reduce >lock contention. > >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 | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > >diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c >index 851bbb10819d..eb26f4e57ce4 100644 >--- a/kernel/locking/rtmutex.c >+++ b/kernel/locking/rtmutex.c >@@ -141,9 +141,10 @@ 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) >+# 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) Thomas, should I resend the top-waiter spin series? Otherwise yeah, I see little point to the CCAS fastpath thing. Thanks, Davidlohr ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed() 2020-02-04 10:18 ` Alex Shi 2020-02-12 15:07 ` Davidlohr Bueso @ 2020-02-14 15:17 ` Alex Shi 2020-02-14 15:17 ` [PATCH 2/2] locking/rtmutex: optimize rt_mutex_cmpxchgs Alex Shi 1 sibling, 1 reply; 7+ messages in thread From: Alex Shi @ 2020-02-14 15:17 UTC (permalink / raw) Cc: Thomas Gleixner, Davidlohr Bueso, Ingo Molnar, Will Deacon, linux-kernel 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 851bbb10819d..7ad22eade1cc 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] locking/rtmutex: optimize rt_mutex_cmpxchgs 2020-02-14 15:17 ` [PATCH 1/2] locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed() Alex Shi @ 2020-02-14 15:17 ` Alex Shi 0 siblings, 0 replies; 7+ messages in thread From: Alex Shi @ 2020-02-14 15:17 UTC (permalink / raw) Cc: Thomas Gleixner, Davidlohr Bueso, Ingo Molnar, Will Deacon, linux-kernel 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 7ad22eade1cc..e8df466af8ab 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-14 15:17 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-21 8:34 [PATCH] locking/rtmutex: remove unused cmpxchg_relaxed Alex Shi 2020-01-31 17:39 ` Davidlohr Bueso 2020-01-31 20:23 ` Thomas Gleixner 2020-02-04 10:18 ` Alex Shi 2020-02-12 15:07 ` Davidlohr Bueso 2020-02-14 15:17 ` [PATCH 1/2] locking/rtmutex: Remove unused rt_mutex_cmpxchg_relaxed() Alex Shi 2020-02-14 15:17 ` [PATCH 2/2] locking/rtmutex: optimize rt_mutex_cmpxchgs Alex Shi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).