v1->v2:
- Remove the MUTEX_SHOW_NO_WAITER() macro.
- Use !mutex_is_locked() instead of introducing a new MUTEX_IS_UNLOCKED() macro.
- Add more information to the changelog of the "Optimize mutex trylock slowpath"
patch
This patchset contains documentation fixes and optimizations for mutex.
Jason Low (4):
mutex: Correct documentation on mutex optimistic spinning
mutex: Delete the MUTEX_SHOW_NO_WAITER macro
mutex: Try to acquire mutex only if it is unlocked
mutex: Optimize mutex trylock slowpath
kernel/locking/mutex.c | 35 ++++++++++++++++++-----------------
1 files changed, 18 insertions(+), 17 deletions(-)
The mutex optimistic spinning documentation states that we spin for
acquisition when we find that there are no pending waiters. However,
in actuality, whether or not there are waiters for the mutex doesn't
determine if we will spin for it.
This patch removes that statement and also adds a comment which
mentions that we spin for the mutex while we don't need to reschedule.
Acked-by: Davidlohr Bueso <[email protected]>
Signed-off-by: Jason Low <[email protected]>
---
kernel/locking/mutex.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index bc73d33..dd26bf6 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -388,12 +388,10 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
/*
* Optimistic spinning.
*
- * We try to spin for acquisition when we find that there are no
- * pending waiters and the lock owner is currently running on a
- * (different) CPU.
- *
- * The rationale is that if the lock owner is running, it is likely to
- * release the lock soon.
+ * We try to spin for acquisition when we find that the lock owner
+ * is currently running on a (different) CPU and while we don't
+ * need to reschedule. The rationale is that if the lock owner is
+ * running, it is likely to release the lock soon.
*
* Since this needs the lock owner, and this mutex implementation
* doesn't track the owner atomically in the lock field, we need to
--
1.7.1
Upon entering the slowpath in __mutex_lock_common(), we try once more to
acquire the mutex. We only try to acquire if (lock->count >= 0). However,
what we actually want here is to try to acquire if the mutex is unlocked
(lock->count == 1).
This patch changes it so that we only try-acquire the mutex upon entering
the slowpath if it is unlocked, rather than if the lock count is non-negative.
This helps further reduce unnecessary atomic xchg() operations.
Furthermore, this patch uses !mutex_is_locked(lock) to do the initial
checks for if the lock is free rather than directly calling atomic_read()
on the lock->count, in order to improve readability.
Signed-off-by: Jason Low <[email protected]>
---
kernel/locking/mutex.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 4bd9546..e4d997b 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -432,7 +432,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
if (owner && !mutex_spin_on_owner(lock, owner))
break;
- if ((atomic_read(&lock->count) == 1) &&
+ /* Try to acquire the mutex if it is unlocked. */
+ if (!mutex_is_locked(lock) &&
(atomic_cmpxchg(&lock->count, 1, 0) == 1)) {
lock_acquired(&lock->dep_map, ip);
if (use_ww_ctx) {
@@ -479,9 +480,9 @@ slowpath:
/*
* Once more, try to acquire the lock. Only try-lock the mutex if
- * lock->count >= 0 to reduce unnecessary xchg operations.
+ * it is unlocked to reduce unnecessary xchg() operations.
*/
- if (atomic_read(&lock->count) >= 0 && (atomic_xchg(&lock->count, 0) == 1))
+ if (!mutex_is_locked(lock) && (atomic_xchg(&lock->count, 0) == 1))
goto skip_wait;
debug_mutex_lock_common(lock, &waiter);
--
1.7.1
v1->v2:
- There were discussions in v1 about a possible mutex_has_waiters()
function. This patch didn't use that function because the places which
used MUTEX_SHOW_NO_WAITER requires checking for lock->count while an
actual mutex_has_waiters() should check for !list_empty(wait_list).
We'll just delete the macro and directly use atomic_read() + comments.
MUTEX_SHOW_NO_WAITER() is a macro which checks for if there are
"no waiters" on a mutex by checking if the lock count is non-negative.
Based on feedback from the discussion in the earlier version of this
patchset, the macro is not very readable.
Furthermore, checking lock->count isn't always the correct way to
determine if there are "no waiters" on a mutex. For example, a negative
count on a mutex really only means that there "potentially" are
waiters. Likewise, there can be waiters on the mutex even if the count is
non-negative. Thus, "MUTEX_SHOW_NO_WAITER" doesn't always do what the name
of the macro suggests.
So this patch deletes the MUTEX_SHOW_NO_WAITERS() macro, directly
use atomic_read() instead of the macro, and adds comments which
elaborate on how the extra atomic_read() checks can help reduce
unnecessary xchg() operations.
Signed-off-by: Jason Low <[email protected]>
---
kernel/locking/mutex.c | 18 ++++++++----------
1 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index dd26bf6..4bd9546 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -46,12 +46,6 @@
# include <asm/mutex.h>
#endif
-/*
- * A negative mutex count indicates that waiters are sleeping waiting for the
- * mutex.
- */
-#define MUTEX_SHOW_NO_WAITER(mutex) (atomic_read(&(mutex)->count) >= 0)
-
void
__mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key)
{
@@ -483,8 +477,11 @@ slowpath:
#endif
spin_lock_mutex(&lock->wait_lock, flags);
- /* once more, can we acquire the lock? */
- if (MUTEX_SHOW_NO_WAITER(lock) && (atomic_xchg(&lock->count, 0) == 1))
+ /*
+ * Once more, try to acquire the lock. Only try-lock the mutex if
+ * lock->count >= 0 to reduce unnecessary xchg operations.
+ */
+ if (atomic_read(&lock->count) >= 0 && (atomic_xchg(&lock->count, 0) == 1))
goto skip_wait;
debug_mutex_lock_common(lock, &waiter);
@@ -504,9 +501,10 @@ slowpath:
* it's unlocked. Later on, if we sleep, this is the
* operation that gives us the lock. We xchg it to -1, so
* that when we release the lock, we properly wake up the
- * other waiters:
+ * other waiters. We only attempt the xchg if the count is
+ * non-negative in order to avoid unnecessary xchg operations:
*/
- if (MUTEX_SHOW_NO_WAITER(lock) &&
+ if (atomic_read(&lock->count) >= 0 &&
(atomic_xchg(&lock->count, -1) == 1))
break;
--
1.7.1
The mutex_trylock() function calls into __mutex_trylock_fastpath() when
trying to obtain the mutex. On 32 bit x86, in the !__HAVE_ARCH_CMPXCHG
case, __mutex_trylock_fastpath() calls directly into __mutex_trylock_slowpath()
regardless of whether or not the mutex is locked.
In __mutex_trylock_slowpath(), we then acquire the wait_lock spinlock, xchg()
lock->count with -1, then set lock->count back to 0 if there are no waiters,
and return true if the prev lock count was 1.
However, if the mutex is already locked, then there isn't much point
in attempting all of the above expensive operations. In this patch, we only
attempt the above trylock operations if the mutex is unlocked.
Signed-off-by: Jason Low <[email protected]>
---
kernel/locking/mutex.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index e4d997b..11b103d 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -820,6 +820,10 @@ static inline int __mutex_trylock_slowpath(atomic_t *lock_count)
unsigned long flags;
int prev;
+ /* No need to trylock if the mutex is locked. */
+ if (mutex_is_locked(lock))
+ return 0;
+
spin_lock_mutex(&lock->wait_lock, flags);
prev = atomic_xchg(&lock->count, -1);
--
1.7.1
On 6/11/2014 2:37 PM, Jason Low wrote:
> v1->v2:
> - There were discussions in v1 about a possible mutex_has_waiters()
> function. This patch didn't use that function because the places which
> used MUTEX_SHOW_NO_WAITER requires checking for lock->count while an
> actual mutex_has_waiters() should check for !list_empty(wait_list).
> We'll just delete the macro and directly use atomic_read() + comments.
>
> MUTEX_SHOW_NO_WAITER() is a macro which checks for if there are
> "no waiters" on a mutex by checking if the lock count is non-negative.
> Based on feedback from the discussion in the earlier version of this
> patchset, the macro is not very readable.
>
> Furthermore, checking lock->count isn't always the correct way to
> determine if there are "no waiters" on a mutex. For example, a negative
> count on a mutex really only means that there "potentially" are
> waiters. Likewise, there can be waiters on the mutex even if the count is
> non-negative. Thus, "MUTEX_SHOW_NO_WAITER" doesn't always do what the name
> of the macro suggests.
>
> So this patch deletes the MUTEX_SHOW_NO_WAITERS() macro, directly
> use atomic_read() instead of the macro, and adds comments which
> elaborate on how the extra atomic_read() checks can help reduce
> unnecessary xchg() operations.
>
> Signed-off-by: Jason Low <[email protected]>
> ---
> kernel/locking/mutex.c | 18 ++++++++----------
> 1 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> index dd26bf6..4bd9546 100644
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -46,12 +46,6 @@
> # include <asm/mutex.h>
> #endif
>
> -/*
> - * A negative mutex count indicates that waiters are sleeping waiting for the
> - * mutex.
> - */
> -#define MUTEX_SHOW_NO_WAITER(mutex) (atomic_read(&(mutex)->count) >= 0)
> -
> void
> __mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key)
> {
> @@ -483,8 +477,11 @@ slowpath:
> #endif
> spin_lock_mutex(&lock->wait_lock, flags);
>
> - /* once more, can we acquire the lock? */
> - if (MUTEX_SHOW_NO_WAITER(lock) && (atomic_xchg(&lock->count, 0) == 1))
> + /*
> + * Once more, try to acquire the lock. Only try-lock the mutex if
> + * lock->count >= 0 to reduce unnecessary xchg operations.
> + */
> + if (atomic_read(&lock->count) >= 0 && (atomic_xchg(&lock->count, 0) == 1))
> goto skip_wait;
>
> debug_mutex_lock_common(lock, &waiter);
> @@ -504,9 +501,10 @@ slowpath:
> * it's unlocked. Later on, if we sleep, this is the
> * operation that gives us the lock. We xchg it to -1, so
> * that when we release the lock, we properly wake up the
> - * other waiters:
> + * other waiters. We only attempt the xchg if the count is
> + * non-negative in order to avoid unnecessary xchg operations:
> */
> - if (MUTEX_SHOW_NO_WAITER(lock) &&
> + if (atomic_read(&lock->count) >= 0 &&
> (atomic_xchg(&lock->count, -1) == 1))
> break;
>
Acked-by: Waiman Long <[email protected]>
On 6/11/2014 2:37 PM, Jason Low wrote:
> Upon entering the slowpath in __mutex_lock_common(), we try once more to
> acquire the mutex. We only try to acquire if (lock->count >= 0). However,
> what we actually want here is to try to acquire if the mutex is unlocked
> (lock->count == 1).
>
> This patch changes it so that we only try-acquire the mutex upon entering
> the slowpath if it is unlocked, rather than if the lock count is non-negative.
> This helps further reduce unnecessary atomic xchg() operations.
>
> Furthermore, this patch uses !mutex_is_locked(lock) to do the initial
> checks for if the lock is free rather than directly calling atomic_read()
> on the lock->count, in order to improve readability.
>
> Signed-off-by: Jason Low <[email protected]>
> ---
> kernel/locking/mutex.c | 7 ++++---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> index 4bd9546..e4d997b 100644
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -432,7 +432,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
> if (owner && !mutex_spin_on_owner(lock, owner))
> break;
>
> - if ((atomic_read(&lock->count) == 1) &&
> + /* Try to acquire the mutex if it is unlocked. */
> + if (!mutex_is_locked(lock) &&
> (atomic_cmpxchg(&lock->count, 1, 0) == 1)) {
> lock_acquired(&lock->dep_map, ip);
> if (use_ww_ctx) {
> @@ -479,9 +480,9 @@ slowpath:
>
> /*
> * Once more, try to acquire the lock. Only try-lock the mutex if
> - * lock->count >= 0 to reduce unnecessary xchg operations.
> + * it is unlocked to reduce unnecessary xchg() operations.
> */
> - if (atomic_read(&lock->count) >= 0 && (atomic_xchg(&lock->count, 0) == 1))
> + if (!mutex_is_locked(lock) && (atomic_xchg(&lock->count, 0) == 1))
> goto skip_wait;
>
> debug_mutex_lock_common(lock, &waiter);
Acked-by: Waiman Long <[email protected]>
On Wed, 2014-06-11 at 11:37 -0700, Jason Low wrote:
> The mutex_trylock() function calls into __mutex_trylock_fastpath() when
> trying to obtain the mutex. On 32 bit x86, in the !__HAVE_ARCH_CMPXCHG
> case, __mutex_trylock_fastpath() calls directly into __mutex_trylock_slowpath()
> regardless of whether or not the mutex is locked.
>
> In __mutex_trylock_slowpath(), we then acquire the wait_lock spinlock, xchg()
> lock->count with -1, then set lock->count back to 0 if there are no waiters,
> and return true if the prev lock count was 1.
>
> However, if the mutex is already locked, then there isn't much point
> in attempting all of the above expensive operations. In this patch, we only
> attempt the above trylock operations if the mutex is unlocked.
>
> Signed-off-by: Jason Low <[email protected]>
This is significantly cleaner than the v1 patch.
Reviewed-by: Davidlohr Bueso <[email protected]>
On Wed, 2014-06-11 at 11:37 -0700, Jason Low wrote:
> Upon entering the slowpath in __mutex_lock_common(), we try once more to
> acquire the mutex. We only try to acquire if (lock->count >= 0). However,
> what we actually want here is to try to acquire if the mutex is unlocked
> (lock->count == 1).
>
> This patch changes it so that we only try-acquire the mutex upon entering
> the slowpath if it is unlocked, rather than if the lock count is non-negative.
> This helps further reduce unnecessary atomic xchg() operations.
>
> Furthermore, this patch uses !mutex_is_locked(lock) to do the initial
> checks for if the lock is free rather than directly calling atomic_read()
> on the lock->count, in order to improve readability.
I think this patch can be merged in 2/4, like you had in v1. Otherwise
looks good.
On Thu, 2014-06-12 at 12:37 -0700, Davidlohr Bueso wrote:
> On Wed, 2014-06-11 at 11:37 -0700, Jason Low wrote:
> > Upon entering the slowpath in __mutex_lock_common(), we try once more to
> > acquire the mutex. We only try to acquire if (lock->count >= 0). However,
> > what we actually want here is to try to acquire if the mutex is unlocked
> > (lock->count == 1).
> >
> > This patch changes it so that we only try-acquire the mutex upon entering
> > the slowpath if it is unlocked, rather than if the lock count is non-negative.
> > This helps further reduce unnecessary atomic xchg() operations.
> >
> > Furthermore, this patch uses !mutex_is_locked(lock) to do the initial
> > checks for if the lock is free rather than directly calling atomic_read()
> > on the lock->count, in order to improve readability.
>
> I think this patch can be merged in 2/4, like you had in v1. Otherwise
> looks good.
Ah, I was thinking that removing the macro would be considered a
separate change whereas this 3/4 patch was more of an "optimization".
But yes, those 2 patches could also have been kept as 1 patch as well.
Thanks for the reviews David and Waiman.
Commit-ID: 0c3c0f0d6e56422cef60a33726d062e9923005c3
Gitweb: http://git.kernel.org/tip/0c3c0f0d6e56422cef60a33726d062e9923005c3
Author: Jason Low <[email protected]>
AuthorDate: Wed, 11 Jun 2014 11:37:20 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 5 Jul 2014 11:25:41 +0200
locking/mutexes: Correct documentation on mutex optimistic spinning
The mutex optimistic spinning documentation states that we spin for
acquisition when we find that there are no pending waiters. However,
in actuality, whether or not there are waiters for the mutex doesn't
determine if we will spin for it.
This patch removes that statement and also adds a comment which
mentions that we spin for the mutex while we don't need to reschedule.
Signed-off-by: Jason Low <[email protected]>
Acked-by: Davidlohr Bueso <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Linus Torvalds <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/locking/mutex.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index bc73d33..dd26bf6de 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -388,12 +388,10 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
/*
* Optimistic spinning.
*
- * We try to spin for acquisition when we find that there are no
- * pending waiters and the lock owner is currently running on a
- * (different) CPU.
- *
- * The rationale is that if the lock owner is running, it is likely to
- * release the lock soon.
+ * We try to spin for acquisition when we find that the lock owner
+ * is currently running on a (different) CPU and while we don't
+ * need to reschedule. The rationale is that if the lock owner is
+ * running, it is likely to release the lock soon.
*
* Since this needs the lock owner, and this mutex implementation
* doesn't track the owner atomically in the lock field, we need to
Commit-ID: 1e820c9608eace237e2c519d8fd9074aec479d81
Gitweb: http://git.kernel.org/tip/1e820c9608eace237e2c519d8fd9074aec479d81
Author: Jason Low <[email protected]>
AuthorDate: Wed, 11 Jun 2014 11:37:21 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 5 Jul 2014 11:25:41 +0200
locking/mutexes: Delete the MUTEX_SHOW_NO_WAITER macro
MUTEX_SHOW_NO_WAITER() is a macro which checks for if there are
"no waiters" on a mutex by checking if the lock count is non-negative.
Based on feedback from the discussion in the earlier version of this
patchset, the macro is not very readable.
Furthermore, checking lock->count isn't always the correct way to
determine if there are "no waiters" on a mutex. For example, a negative
count on a mutex really only means that there "potentially" are
waiters. Likewise, there can be waiters on the mutex even if the count is
non-negative. Thus, "MUTEX_SHOW_NO_WAITER" doesn't always do what the name
of the macro suggests.
So this patch deletes the MUTEX_SHOW_NO_WAITERS() macro, directly
use atomic_read() instead of the macro, and adds comments which
elaborate on how the extra atomic_read() checks can help reduce
unnecessary xchg() operations.
Signed-off-by: Jason Low <[email protected]>
Acked-by: Waiman Long <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Linus Torvalds <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/locking/mutex.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index dd26bf6de..4bd9546 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -46,12 +46,6 @@
# include <asm/mutex.h>
#endif
-/*
- * A negative mutex count indicates that waiters are sleeping waiting for the
- * mutex.
- */
-#define MUTEX_SHOW_NO_WAITER(mutex) (atomic_read(&(mutex)->count) >= 0)
-
void
__mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key)
{
@@ -483,8 +477,11 @@ slowpath:
#endif
spin_lock_mutex(&lock->wait_lock, flags);
- /* once more, can we acquire the lock? */
- if (MUTEX_SHOW_NO_WAITER(lock) && (atomic_xchg(&lock->count, 0) == 1))
+ /*
+ * Once more, try to acquire the lock. Only try-lock the mutex if
+ * lock->count >= 0 to reduce unnecessary xchg operations.
+ */
+ if (atomic_read(&lock->count) >= 0 && (atomic_xchg(&lock->count, 0) == 1))
goto skip_wait;
debug_mutex_lock_common(lock, &waiter);
@@ -504,9 +501,10 @@ slowpath:
* it's unlocked. Later on, if we sleep, this is the
* operation that gives us the lock. We xchg it to -1, so
* that when we release the lock, we properly wake up the
- * other waiters:
+ * other waiters. We only attempt the xchg if the count is
+ * non-negative in order to avoid unnecessary xchg operations:
*/
- if (MUTEX_SHOW_NO_WAITER(lock) &&
+ if (atomic_read(&lock->count) >= 0 &&
(atomic_xchg(&lock->count, -1) == 1))
break;
Commit-ID: 0d968dd8c6aced585b86fa7ba8ce4573bf19e848
Gitweb: http://git.kernel.org/tip/0d968dd8c6aced585b86fa7ba8ce4573bf19e848
Author: Jason Low <[email protected]>
AuthorDate: Wed, 11 Jun 2014 11:37:22 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 5 Jul 2014 11:25:42 +0200
locking/mutexes: Try to acquire mutex only if it is unlocked
Upon entering the slowpath in __mutex_lock_common(), we try once more to
acquire the mutex. We only try to acquire if (lock->count >= 0). However,
what we actually want here is to try to acquire if the mutex is unlocked
(lock->count == 1).
This patch changes it so that we only try-acquire the mutex upon entering
the slowpath if it is unlocked, rather than if the lock count is non-negative.
This helps further reduce unnecessary atomic xchg() operations.
Furthermore, this patch uses !mutex_is_locked(lock) to do the initial
checks for if the lock is free rather than directly calling atomic_read()
on the lock->count, in order to improve readability.
Signed-off-by: Jason Low <[email protected]>
Acked-by: Waiman Long <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Linus Torvalds <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/locking/mutex.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 4bd9546..e4d997b 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -432,7 +432,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
if (owner && !mutex_spin_on_owner(lock, owner))
break;
- if ((atomic_read(&lock->count) == 1) &&
+ /* Try to acquire the mutex if it is unlocked. */
+ if (!mutex_is_locked(lock) &&
(atomic_cmpxchg(&lock->count, 1, 0) == 1)) {
lock_acquired(&lock->dep_map, ip);
if (use_ww_ctx) {
@@ -479,9 +480,9 @@ slowpath:
/*
* Once more, try to acquire the lock. Only try-lock the mutex if
- * lock->count >= 0 to reduce unnecessary xchg operations.
+ * it is unlocked to reduce unnecessary xchg() operations.
*/
- if (atomic_read(&lock->count) >= 0 && (atomic_xchg(&lock->count, 0) == 1))
+ if (!mutex_is_locked(lock) && (atomic_xchg(&lock->count, 0) == 1))
goto skip_wait;
debug_mutex_lock_common(lock, &waiter);
Commit-ID: 72d5305dcb3637913c2c37e847a4de9028e49244
Gitweb: http://git.kernel.org/tip/72d5305dcb3637913c2c37e847a4de9028e49244
Author: Jason Low <[email protected]>
AuthorDate: Wed, 11 Jun 2014 11:37:23 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 5 Jul 2014 11:25:42 +0200
locking/mutexes: Optimize mutex trylock slowpath
The mutex_trylock() function calls into __mutex_trylock_fastpath() when
trying to obtain the mutex. On 32 bit x86, in the !__HAVE_ARCH_CMPXCHG
case, __mutex_trylock_fastpath() calls directly into __mutex_trylock_slowpath()
regardless of whether or not the mutex is locked.
In __mutex_trylock_slowpath(), we then acquire the wait_lock spinlock, xchg()
lock->count with -1, then set lock->count back to 0 if there are no waiters,
and return true if the prev lock count was 1.
However, if the mutex is already locked, then there isn't much point
in attempting all of the above expensive operations. In this patch, we only
attempt the above trylock operations if the mutex is unlocked.
Signed-off-by: Jason Low <[email protected]>
Reviewed-by: Davidlohr Bueso <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Linus Torvalds <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/locking/mutex.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index e4d997b..11b103d 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -820,6 +820,10 @@ static inline int __mutex_trylock_slowpath(atomic_t *lock_count)
unsigned long flags;
int prev;
+ /* No need to trylock if the mutex is locked. */
+ if (mutex_is_locked(lock))
+ return 0;
+
spin_lock_mutex(&lock->wait_lock, flags);
prev = atomic_xchg(&lock->count, -1);