mutex: Optimize mutex_trylock() fast-path
Subsystems (4)
RFC PATCH 1/1 mutex: Optimize mutex_trylock() fast-path
A while back Viro posted a number of 'interesting' mutex_is_locked() users on IRC, one of those was RCU.
RCU seems to use mutex_is_locked() to avoid doing mutex_trylock(), the regular load before modify pattern.
While the use isn't wrong per se, its curious in that its needed at all, mutex_trylock() should be good enough on its own to avoid the pointless cacheline bounces.
So fix those and remove the mutex_is_locked() (ab)use from RCU.
Reported-by Al Viro <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
--- arch/ia64/include/asm/mutex.h | 2 +- arch/powerpc/include/asm/mutex.h | 2 +- arch/x86/include/asm/mutex_32.h | 2 +- arch/x86/include/asm/mutex_64.h | 6 +++--- include/asm-generic/mutex-dec.h | 2 +- include/asm-generic/mutex-xchg.h | 6 +++++- kernel/rcu/tree.c | 1 - 7 files changed, 12 insertions(+), 9 deletions(-)
arch/ia64/include/asm/mutex.h +1 -1
@@ -82,7 +82,7 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (cmpxchg_acq (count,1 ,0 ) ==1 ) +if (atomic_read (count) ==1 &&cmpxchg_acq (count,1 ,0 ) ==1 )return 1 ;return 0 ; }
arch/powerpc/include/asm/mutex.h +1 -1
@@ -124,7 +124,7 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (likely (__mutex_cmpxchg_lock (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&__mutex_cmpxchg_lock (count,1 ,0 ) ==1 ))return 1 ;return 0 ; }
arch/x86/include/asm/mutex_32.h +1 -1
@@ -101,7 +101,7 @@ static inline int __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *)) {/* cmpxchg because it never induces a false contention state. */ -if (likely (atomic_cmpxchg (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&atomic_cmpxchg (count,1 ,0 ) ==1 ))return 1 ;return 0 ;
arch/x86/include/asm/mutex_64.h +3 -3
@@ -118,10 +118,10 @@ do { \static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (likely (atomic_cmpxchg (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&atomic_cmpxchg (count,1 ,0 ) ==1 ))return 1 ; -else -return 0 ; + +return 0 ; }#endif /* _ASM_X86_MUTEX_64_H */
include/asm-generic/mutex-dec.h +1 -1
@@ -80,7 +80,7 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (likely (atomic_cmpxchg_acquire (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&atomic_cmpxchg_acquire (count,1 ,0 ) ==1 ))return 1 ;return 0 ; }
include/asm-generic/mutex-xchg.h +5 -1
@@ -91,8 +91,12 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -int prev =atomic_xchg_acquire (count,0 ); +int prev; +if (atomic_read (count) !=1 ) +return 0 ; + + prev =atomic_xchg_acquire (count,0 );if (unlikely (prev <0 )) {/* * The lock was marked contended so we must restore that
kernel/rcu/tree.c +0 -1
@@ -3681,7 +3681,6 @@ static bool exp_funnel_lock(struct rcu_state *rsp, unsigned long s)if (ULONG_CMP_LT (READ_ONCE (rnp->exp_seq_rq), s) && (rnp == rnp_root ||ULONG_CMP_LT (READ_ONCE (rnp_root->exp_seq_rq), s)) && - !mutex_is_locked (&rsp->exp_mutex) &&mutex_trylock (&rsp->exp_mutex)) goto fastpath;
Quoted 10 lines · Peter Zijlstra
A while back Viro posted a number of 'interesting'
mutex_is_locked()users on IRC, one of those was RCU.RCU seems to use
mutex_is_locked()to avoid doingmutex_trylock(), the regular load before modify pattern.While the use isn't wrong per se, its curious in that its needed at all,
mutex_trylock()should be good enough on its own to avoid the pointless cacheline bounces.
Yeah, and we use ccas just about everywhere else for mutexes.
Quoted 5 lines
So fix those and remove the
mutex_is_locked()(ab)use from RCU.Reported-by Al Viro <[email protected]>Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Acked-by Davidlohr Bueso <[email protected]>
Quoted 6 lines · Peter Zijlstra
A while back Viro posted a number of 'interesting'
mutex_is_locked()users on IRC, one of those was RCU.RCU seems to use
mutex_is_locked()to avoid doingmutex_trylock(), the regular load before modify pattern.
Definitely -- the old versions of mutex_trylock() looked to take ownership of the cacheline, which most definitely is not what you want if you have a bunch of CPUs doing expedited grace periods concurrently.
Quoted 5 lines
While the use isn't wrong per se, its curious in that its needed at all,
mutex_trylock()should be good enough on its own to avoid the pointless cacheline bounces.So fix those and remove the
mutex_is_locked()(ab)use from RCU.
Given the changes below that push the mutex_is_locked() check into mutex_trylock():
Acked-by Paul E. McKenney <[email protected]>
Or I could pull the changes into -rcu if you prefer.
Thanx, Paul
Quoted 113 lines
Reported-by Al Viro <[email protected]>Signed-off-by Peter Zijlstra (Intel) <[email protected]>--- arch/ia64/include/asm/mutex.h | 2 +- arch/powerpc/include/asm/mutex.h | 2 +- arch/x86/include/asm/mutex_32.h | 2 +- arch/x86/include/asm/mutex_64.h | 6 +++--- include/asm-generic/mutex-dec.h | 2 +- include/asm-generic/mutex-xchg.h | 6 +++++- kernel/rcu/tree.c | 1 - 7 files changed, 12 insertions(+), 9 deletions(-)arch/ia64/include/asm/mutex.h +1
-1@@ -82,7 +82,7 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (cmpxchg_acq (count,1 ,0 ) ==1 ) +if (atomic_read (count) ==1 &&cmpxchg_acq (count,1 ,0 ) ==1 )return 1 ;return 0 ; }arch/powerpc/include/asm/mutex.h +1
-1@@ -124,7 +124,7 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (likely (__mutex_cmpxchg_lock (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&__mutex_cmpxchg_lock (count,1 ,0 ) ==1 ))return 1 ;return 0 ; }arch/x86/include/asm/mutex_32.h +1
-1@@ -101,7 +101,7 @@ static inline int __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *)) {/* cmpxchg because it never induces a false contention state. */ -if (likely (atomic_cmpxchg (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&atomic_cmpxchg (count,1 ,0 ) ==1 ))return 1 ;return 0 ;arch/x86/include/asm/mutex_64.h +3
-3@@ -118,10 +118,10 @@ do { \static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (likely (atomic_cmpxchg (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&atomic_cmpxchg (count,1 ,0 ) ==1 ))return 1 ; -else -return 0 ; + +return 0 ; }#endif /* _ASM_X86_MUTEX_64_H */ include/asm-generic/mutex-dec.h +1
-1@@ -80,7 +80,7 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (likely (atomic_cmpxchg_acquire (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&atomic_cmpxchg_acquire (count,1 ,0 ) ==1 ))return 1 ;return 0 ; }include/asm-generic/mutex-xchg.h +5
-1@@ -91,8 +91,12 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -int prev =atomic_xchg_acquire (count,0 ); +int prev; +if (atomic_read (count) !=1 ) +return 0 ; + + prev =atomic_xchg_acquire (count,0 );if (unlikely (prev <0 )) {/* * The lock was marked contended so we must restore that kernel/rcu/tree.c +0
-1@@ -3681,7 +3681,6 @@ static bool exp_funnel_lock(struct rcu_state *rsp, unsigned long s)if (ULONG_CMP_LT (READ_ONCE (rnp->exp_seq_rq), s) && (rnp == rnp_root ||ULONG_CMP_LT (READ_ONCE (rnp_root->exp_seq_rq), s)) && - !mutex_is_locked (&rsp->exp_mutex) &&mutex_trylock (&rsp->exp_mutex)) goto fastpath;
tip-bot for Peter Zijlstra
merge bot[tip:locking/core] locking/mutex: Optimize mutex_trylock() fast-path
In reply to Peter Zijlstralocking/mutex: Optimize mutex_trylock() fast-path
Commit-ID: 6428671bae97caa7040e24e79e969fd87908f4f3 Gitweb: http://git.kernel.org/tip/6428671bae97caa7040e24e79e969fd87908f4f3 Author: Peter Zijlstra <[email protected]> AuthorDate: Wed, 1 Jun 2016 20:58:15 +0200 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 8 Jun 2016 15:17:01 +0200
locking/mutex: Optimize mutex_trylock() fast-path
A while back Viro posted a number of 'interesting' mutex_is_locked() users on IRC, one of those was RCU.
RCU seems to use mutex_is_locked() to avoid doing mutex_trylock(), the regular load before modify pattern.
While the use isn't wrong per se, its curious in that its needed at all, mutex_trylock() should be good enough on its own to avoid the pointless cacheline bounces.
So fix those and remove the mutex_is_locked() (ab)use from RCU.
Reported-by Al Viro <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Acked-by Paul McKenney <[email protected]>
Acked-by Davidlohr Bueso <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Waiman Long <[email protected]>
Signed-off-by Ingo Molnar <[email protected]>
--- arch/ia64/include/asm/mutex.h | 2 +- arch/powerpc/include/asm/mutex.h | 2 +- arch/x86/include/asm/mutex_32.h | 2 +- arch/x86/include/asm/mutex_64.h | 6 +++--- include/asm-generic/mutex-dec.h | 2 +- include/asm-generic/mutex-xchg.h | 6 +++++- kernel/rcu/tree.c | 1 - 7 files changed, 12 insertions(+), 9 deletions(-)
arch/ia64/include/asm/mutex.h +1 -1
@@ -82,7 +82,7 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (cmpxchg_acq (count,1 ,0 ) ==1 ) +if (atomic_read (count) ==1 &&cmpxchg_acq (count,1 ,0 ) ==1 )return 1 ;return 0 ; }
arch/powerpc/include/asm/mutex.h +1 -1
@@ -124,7 +124,7 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (likely (__mutex_cmpxchg_lock (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&__mutex_cmpxchg_lock (count,1 ,0 ) ==1 ))return 1 ;return 0 ; }
arch/x86/include/asm/mutex_32.h +1 -1
@@ -101,7 +101,7 @@ static inline int __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *)) {/* cmpxchg because it never induces a false contention state. */ -if (likely (atomic_cmpxchg (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&atomic_cmpxchg (count,1 ,0 ) ==1 ))return 1 ;return 0 ;
arch/x86/include/asm/mutex_64.h +3 -3
@@ -118,10 +118,10 @@ do { \static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (likely (atomic_cmpxchg (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&atomic_cmpxchg (count,1 ,0 ) ==1 ))return 1 ; -else -return 0 ; + +return 0 ; }#endif /* _ASM_X86_MUTEX_64_H */
include/asm-generic/mutex-dec.h +1 -1
@@ -80,7 +80,7 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -if (likely (atomic_cmpxchg_acquire (count,1 ,0 ) ==1 )) +if (likely (atomic_read (count) ==1 &&atomic_cmpxchg_acquire (count,1 ,0 ) ==1 ))return 1 ;return 0 ; }
include/asm-generic/mutex-xchg.h +5 -1
@@ -91,8 +91,12 @@ __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))static inline int __mutex_fastpath_trylock (atomic_t *count,int (*fail_fn)(atomic_t *)) { -int prev =atomic_xchg_acquire (count,0 ); +int prev; +if (atomic_read (count) !=1 ) +return 0 ; + + prev =atomic_xchg_acquire (count,0 );if (unlikely (prev <0 )) {/* * The lock was marked contended so we must restore that
kernel/rcu/tree.c +0 -1
@@ -3681,7 +3681,6 @@ static bool exp_funnel_lock(struct rcu_state *rsp, unsigned long s)if (ULONG_CMP_LT (READ_ONCE (rnp->exp_seq_rq), s) && (rnp == rnp_root ||ULONG_CMP_LT (READ_ONCE (rnp_root->exp_seq_rq), s)) && - !mutex_is_locked (&rsp->exp_mutex) &&mutex_trylock (&rsp->exp_mutex)) goto fastpath;