* [RFC PATCH-queue/locking/rfc 1/2] locking/mutex: Simplify some ww_mutex code in __mutex_lock_common()
@ 2016-08-26 23:35 Waiman Long
2016-08-26 23:35 ` [RFC PATCH-queue/locking/rfc 2/2] locking/mutex: Enable optimistic spinning of woken waiter Waiman Long
2016-10-25 10:29 ` [tip:locking/core] locking/mutex: Simplify some ww_mutex code in __mutex_lock_common() tip-bot for Waiman Long
0 siblings, 2 replies; 6+ messages in thread
From: Waiman Long @ 2016-08-26 23:35 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar
Cc: linux-kernel, Linus Torvalds, Ding Tianhong, Jason Low,
Davidlohr Bueso, Paul E. McKenney, Thomas Gleixner, Will Deacon,
Tim Chen, Imre Deak, Waiman Long
This patch removes some of the redundant ww_mutex code in
__mutex_lock_common().
Signed-off-by: Waiman Long <Waiman.Long@hpe.com>
---
kernel/locking/mutex.c | 13 ++++---------
1 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 0f8a3e6..4a0e16e 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -559,10 +559,11 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
struct task_struct *task = current;
struct mutex_waiter waiter;
unsigned long flags;
+ struct ww_mutex *ww;
int ret;
if (use_ww_ctx) {
- struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
+ ww = container_of(lock, struct ww_mutex, base);
if (unlikely(ww_ctx == READ_ONCE(ww->ctx)))
return -EALREADY;
}
@@ -573,12 +574,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
if (__mutex_trylock(lock) || mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx)) {
/* got the lock, yay! */
lock_acquired(&lock->dep_map, ip);
- if (use_ww_ctx) {
- struct ww_mutex *ww;
- ww = container_of(lock, struct ww_mutex, base);
-
+ if (use_ww_ctx)
ww_mutex_set_context_fastpath(ww, ww_ctx);
- }
preempt_enable();
return 0;
}
@@ -649,10 +646,8 @@ skip_wait:
/* got the lock - cleanup and rejoice! */
lock_acquired(&lock->dep_map, ip);
- if (use_ww_ctx) {
- struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
+ if (use_ww_ctx)
ww_mutex_set_context_slowpath(ww, ww_ctx);
- }
spin_unlock_mutex(&lock->wait_lock, flags);
preempt_enable();
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH-queue/locking/rfc 2/2] locking/mutex: Enable optimistic spinning of woken waiter
2016-08-26 23:35 [RFC PATCH-queue/locking/rfc 1/2] locking/mutex: Simplify some ww_mutex code in __mutex_lock_common() Waiman Long
@ 2016-08-26 23:35 ` Waiman Long
2016-08-30 15:08 ` Peter Zijlstra
2016-10-25 10:29 ` [tip:locking/core] " tip-bot for Waiman Long
2016-10-25 10:29 ` [tip:locking/core] locking/mutex: Simplify some ww_mutex code in __mutex_lock_common() tip-bot for Waiman Long
1 sibling, 2 replies; 6+ messages in thread
From: Waiman Long @ 2016-08-26 23:35 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar
Cc: linux-kernel, Linus Torvalds, Ding Tianhong, Jason Low,
Davidlohr Bueso, Paul E. McKenney, Thomas Gleixner, Will Deacon,
Tim Chen, Imre Deak, Waiman Long
This patch makes the waiter that sets the HANDOFF flag start spinning
instead of sleeping until the handoff is complete or the owner
sleeps. Otherwise, the handoff will cause the optimistic spinners to
abort spinning as the handed-off owner may not be running.
Signed-off-by: Waiman Long <Waiman.Long@hpe.com>
---
kernel/locking/mutex.c | 104 ++++++++++++++++++++++++++++++++----------------
1 files changed, 70 insertions(+), 34 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 4a0e16e..064dd61 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -380,22 +380,38 @@ static inline int mutex_can_spin_on_owner(struct mutex *lock)
*
* Returns true when the lock was taken, otherwise false, indicating
* that we need to jump to the slowpath and sleep.
+ *
+ * The waiter flag is set to true if the spinner is a waiter in the wait
+ * queue. The waiter-spinner will spin on the lock directly and concurrently
+ * with the spinner at the head of the OSQ, if present, until the owner is
+ * changed to itself.
*/
static bool mutex_optimistic_spin(struct mutex *lock,
- struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx)
+ struct ww_acquire_ctx *ww_ctx,
+ const bool use_ww_ctx, bool waiter)
{
struct task_struct *task = current;
+ bool acquired = false;
- if (!mutex_can_spin_on_owner(lock))
- goto done;
+ if (!waiter) {
+ /*
+ * The purpose of the mutex_can_spin_on_owner() function is
+ * to eliminate the overhead of osq_lock() and osq_unlock()
+ * in case spinning isn't possible. As a waiter-spinner
+ * is not going to take OSQ lock anyway, there is no need
+ * to call mutex_can_spin_on_owner().
+ */
+ if (!mutex_can_spin_on_owner(lock))
+ goto done;
- /*
- * In order to avoid a stampede of mutex spinners trying to
- * acquire the mutex all at once, the spinners need to take a
- * MCS (queued) lock first before spinning on the owner field.
- */
- if (!osq_lock(&lock->osq))
- goto done;
+ /*
+ * In order to avoid a stampede of mutex spinners trying to
+ * acquire the mutex all at once, the spinners need to take a
+ * MCS (queued) lock first before spinning on the owner field.
+ */
+ if (!osq_lock(&lock->osq))
+ goto done;
+ }
while (true) {
struct task_struct *owner;
@@ -421,23 +437,24 @@ static bool mutex_optimistic_spin(struct mutex *lock,
* release the lock or go to sleep.
*/
owner = __mutex_owner(lock);
- if (owner && !mutex_spin_on_owner(lock, owner))
- break;
- /* Try to acquire the mutex if it is unlocked. */
- if (__mutex_trylock(lock)) {
- osq_unlock(&lock->osq);
- return true;
+ if (owner == task)
+ goto gotlock;
+
+ if (owner) {
+ if (!mutex_spin_on_owner(lock, owner))
+ break;
+ /*
+ * For waiter-spinner, recheck the owner field
+ * as it may have been changed to itself.
+ */
+ if (waiter && (__mutex_owner(lock) == task))
+ goto gotlock;
}
- /*
- * When there's no owner, we might have preempted between the
- * owner acquiring the lock and setting the owner field. If
- * we're an RT task that will live-lock because we won't let
- * the owner complete.
- */
- if (!owner && (need_resched() || rt_task(task)))
- break;
+ /* Try to acquire the mutex if it is unlocked. */
+ if (__mutex_trylock(lock))
+ goto gotlock;
/*
* The cpu_relax() call is a compiler barrier which forces
@@ -446,16 +463,21 @@ static bool mutex_optimistic_spin(struct mutex *lock,
* values at the cost of a few extra spins.
*/
cpu_relax_lowlatency();
+ continue;
+gotlock:
+ acquired = true;
+ break;
}
- osq_unlock(&lock->osq);
+ if (!waiter)
+ osq_unlock(&lock->osq);
done:
/*
* If we fell out of the spin path because of need_resched(),
* reschedule now, before we try-lock the mutex. This avoids getting
* scheduled out right after we obtained the mutex.
*/
- if (need_resched()) {
+ if (!acquired && need_resched()) {
/*
* We _should_ have TASK_RUNNING here, but just in case
* we do not, make it so, otherwise we might get stuck.
@@ -464,11 +486,12 @@ done:
schedule_preempt_disabled();
}
- return false;
+ return acquired;
}
#else
static bool mutex_optimistic_spin(struct mutex *lock,
- struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx)
+ struct ww_acquire_ctx *ww_ctx,
+ const bool use_ww_ctx, bool waiter)
{
return false;
}
@@ -561,6 +584,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
unsigned long flags;
struct ww_mutex *ww;
int ret;
+ bool acquired;
if (use_ww_ctx) {
ww = container_of(lock, struct ww_mutex, base);
@@ -571,7 +595,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
preempt_disable();
mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip);
- if (__mutex_trylock(lock) || mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx)) {
+ if (__mutex_trylock(lock) ||
+ mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, false)) {
/* got the lock, yay! */
lock_acquired(&lock->dep_map, ip);
if (use_ww_ctx)
@@ -603,7 +628,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
lock_contended(&lock->dep_map, ip);
- for (;;) {
+ for (acquired = false; !acquired; ) {
/*
* got a signal? (This code gets eliminated in the
* TASK_UNINTERRUPTIBLE case.)
@@ -624,13 +649,24 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
/* didn't get the lock, go to sleep: */
spin_unlock_mutex(&lock->wait_lock, flags);
schedule_preempt_disabled();
- spin_lock_mutex(&lock->wait_lock, flags);
- if (__mutex_trylock(lock))
- break;
+ /*
+ * Both __mutex_trylock() and __mutex_waiter_is_first()
+ * can be done without the protection of wait_lock.
+ */
+ acquired = __mutex_trylock(lock);
- if (__mutex_waiter_is_first(lock, &waiter))
+ if (!acquired && __mutex_waiter_is_first(lock, &waiter)) {
__mutex_set_flag(lock, MUTEX_FLAG_HANDOFF);
+ /*
+ * Wait until the lock is handed off or the owner
+ * sleeps.
+ */
+ acquired = mutex_optimistic_spin(lock, ww_ctx,
+ use_ww_ctx, true);
+ }
+
+ spin_lock_mutex(&lock->wait_lock, flags);
}
__set_task_state(task, TASK_RUNNING);
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH-queue/locking/rfc 2/2] locking/mutex: Enable optimistic spinning of woken waiter
2016-08-26 23:35 ` [RFC PATCH-queue/locking/rfc 2/2] locking/mutex: Enable optimistic spinning of woken waiter Waiman Long
@ 2016-08-30 15:08 ` Peter Zijlstra
2016-08-30 22:58 ` Waiman Long
2016-10-25 10:29 ` [tip:locking/core] " tip-bot for Waiman Long
1 sibling, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2016-08-30 15:08 UTC (permalink / raw)
To: Waiman Long
Cc: Ingo Molnar, linux-kernel, Linus Torvalds, Ding Tianhong,
Jason Low, Davidlohr Bueso, Paul E. McKenney, Thomas Gleixner,
Will Deacon, Tim Chen, Imre Deak
On Fri, Aug 26, 2016 at 07:35:09PM -0400, Waiman Long wrote:
> @@ -624,13 +649,24 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
> /* didn't get the lock, go to sleep: */
> spin_unlock_mutex(&lock->wait_lock, flags);
> schedule_preempt_disabled();
>
> + /*
> + * Both __mutex_trylock() and __mutex_waiter_is_first()
> + * can be done without the protection of wait_lock.
> + */
True, but it took me a little while to figure out why
__mutex_waiter_is_first() is safe without the lock :-)
> + acquired = __mutex_trylock(lock);
>
> + if (!acquired && __mutex_waiter_is_first(lock, &waiter)) {
> __mutex_set_flag(lock, MUTEX_FLAG_HANDOFF);
> + /*
> + * Wait until the lock is handed off or the owner
> + * sleeps.
> + */
> + acquired = mutex_optimistic_spin(lock, ww_ctx,
> + use_ww_ctx, true);
> + }
That said; I think there's a few problems with this. Since we now poke
at the loop termination conditions outside of the wait_lock, it becomes
important where we do the task->state vs wakeup bits.
Specifically, since we still have state==RUNNING here, its possible
we'll fail to acquire the lock _and_ miss the wakeup from
mutex_unlock(). Leaving us stuck forever more.
Also, we should do the __mutex_trylock _after_ we set the handoff,
otherwise its possible we get the lock handed (miss the wakeup as per
the above) and fail to notice, again going back to sleep forever more.
> +
> + spin_lock_mutex(&lock->wait_lock, flags);
> }
> __set_task_state(task, TASK_RUNNING);
I'm thinking something like the below on top of yours will cure things..
have not tested yet...
---
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -394,10 +394,9 @@ static inline int mutex_can_spin_on_owne
*/
static bool mutex_optimistic_spin(struct mutex *lock,
struct ww_acquire_ctx *ww_ctx,
- const bool use_ww_ctx, bool waiter)
+ const bool use_ww_ctx, const bool waiter)
{
struct task_struct *task = current;
- bool acquired = false;
if (!waiter) {
/*
@@ -408,7 +407,7 @@ static bool mutex_optimistic_spin(struct
* to call mutex_can_spin_on_owner().
*/
if (!mutex_can_spin_on_owner(lock))
- goto done;
+ goto fail;
/*
* In order to avoid a stampede of mutex spinners trying to
@@ -416,10 +415,10 @@ static bool mutex_optimistic_spin(struct
* MCS (queued) lock first before spinning on the owner field.
*/
if (!osq_lock(&lock->osq))
- goto done;
+ goto fail;
}
- while (true) {
+ for (;;) {
struct task_struct *owner;
if (use_ww_ctx && ww_ctx->acquired > 0) {
@@ -435,7 +434,7 @@ static bool mutex_optimistic_spin(struct
* performed the optimistic spinning cannot be done.
*/
if (READ_ONCE(ww->ctx))
- break;
+ goto fail_unlock;
}
/*
@@ -443,23 +442,16 @@ static bool mutex_optimistic_spin(struct
* release the lock or go to sleep.
*/
owner = __mutex_owner(lock);
-
- if (owner == task)
- goto gotlock;
-
if (owner) {
+ if (waiter && owner == task)
+ goto gotlock_acquire;
+
if (!mutex_spin_on_owner(lock, owner))
- break;
- /*
- * For waiter-spinner, recheck the owner field
- * as it may have been changed to itself.
- */
- if (waiter && (__mutex_owner(lock) == task))
- goto gotlock;
+ goto fail_unlock;
}
/* Try to acquire the mutex if it is unlocked. */
- if (__mutex_trylock(lock, false))
+ if (__mutex_trylock(lock, waiter))
goto gotlock;
/*
@@ -469,21 +461,28 @@ static bool mutex_optimistic_spin(struct
* values at the cost of a few extra spins.
*/
cpu_relax_lowlatency();
- continue;
-gotlock:
- acquired = true;
- break;
}
+gotlock_acquire:
+ smp_mb(); /* ACQUIRE */
+gotlock:
+ if (!waiter)
+ osq_unlock(&lock->osq);
+
+ return true;
+
+
+fail_unlock:
if (!waiter)
osq_unlock(&lock->osq);
-done:
+
+fail:
/*
* If we fell out of the spin path because of need_resched(),
* reschedule now, before we try-lock the mutex. This avoids getting
* scheduled out right after we obtained the mutex.
*/
- if (!acquired && need_resched()) {
+ if (need_resched()) {
/*
* We _should_ have TASK_RUNNING here, but just in case
* we do not, make it so, otherwise we might get stuck.
@@ -492,12 +491,12 @@ static bool mutex_optimistic_spin(struct
schedule_preempt_disabled();
}
- return acquired;
+ return false;
}
#else
static bool mutex_optimistic_spin(struct mutex *lock,
struct ww_acquire_ctx *ww_ctx,
- const bool use_ww_ctx, bool waiter)
+ const bool use_ww_ctx, const bool waiter)
{
return false;
}
@@ -590,7 +589,6 @@ __mutex_lock_common(struct mutex *lock,
unsigned long flags;
struct ww_mutex *ww;
int ret;
- bool acquired;
if (use_ww_ctx) {
ww = container_of(lock, struct ww_mutex, base);
@@ -606,7 +604,7 @@ __mutex_lock_common(struct mutex *lock,
* possible to allow recursive lock attempts by accident.
*/
if (__mutex_trylock(lock, false) ||
- mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx)) {
+ mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, false)) {
/* got the lock, yay! */
lock_acquired(&lock->dep_map, ip);
if (use_ww_ctx)
@@ -638,7 +636,8 @@ __mutex_lock_common(struct mutex *lock,
lock_contended(&lock->dep_map, ip);
- for (acquired = false; !acquired; ) {
+ set_task_state(task, state);
+ for (;;) {
/*
* got a signal? (This code gets eliminated in the
* TASK_UNINTERRUPTIBLE case.)
@@ -654,30 +653,23 @@ __mutex_lock_common(struct mutex *lock,
goto err;
}
- __set_task_state(task, state);
-
- /* didn't get the lock, go to sleep: */
spin_unlock_mutex(&lock->wait_lock, flags);
schedule_preempt_disabled();
- /*
- * Both __mutex_trylock() and __mutex_waiter_is_first()
- * can be done without the protection of wait_lock.
- */
- acquired = __mutex_trylock(lock, true);
+ set_task_state(task, state);
- if (!acquired && __mutex_waiter_is_first(lock, &waiter)) {
+ if (__mutex_waiter_is_first(lock, &waiter)) {
__mutex_set_flag(lock, MUTEX_FLAG_HANDOFF);
- /*
- * Wait until the lock is handed off or the owner
- * sleeps.
- */
- acquired = mutex_optimistic_spin(lock, ww_ctx,
- use_ww_ctx, true);
+ if (mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, true))
+ break;
}
+ if (__mutex_trylock(lock, true))
+ break;
+
spin_lock_mutex(&lock->wait_lock, flags);
}
+ spin_lock_mutex(&lock->wait_lock, flags);
__set_task_state(task, TASK_RUNNING);
remove_waiter:
@@ -700,6 +692,7 @@ __mutex_lock_common(struct mutex *lock,
return 0;
err:
+ __set_task_state(task, TASK_RUNNING);
mutex_remove_waiter(lock, &waiter, task);
spin_unlock_mutex(&lock->wait_lock, flags);
debug_mutex_free_waiter(&waiter);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH-queue/locking/rfc 2/2] locking/mutex: Enable optimistic spinning of woken waiter
2016-08-30 15:08 ` Peter Zijlstra
@ 2016-08-30 22:58 ` Waiman Long
0 siblings, 0 replies; 6+ messages in thread
From: Waiman Long @ 2016-08-30 22:58 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, linux-kernel, Linus Torvalds, Ding Tianhong,
Jason Low, Davidlohr Bueso, Paul E. McKenney, Thomas Gleixner,
Will Deacon, Tim Chen, Imre Deak
On 08/30/2016 11:08 AM, Peter Zijlstra wrote:
> On Fri, Aug 26, 2016 at 07:35:09PM -0400, Waiman Long wrote:
>
>> @@ -624,13 +649,24 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
>> /* didn't get the lock, go to sleep: */
>> spin_unlock_mutex(&lock->wait_lock, flags);
>> schedule_preempt_disabled();
>>
>> + /*
>> + * Both __mutex_trylock() and __mutex_waiter_is_first()
>> + * can be done without the protection of wait_lock.
>> + */
> True, but it took me a little while to figure out why
> __mutex_waiter_is_first() is safe without the lock :-)
Yes, if you are the first waiter, the condition will not be changed even
when new waiter is being added to the tail of the list.
>
>> + acquired = __mutex_trylock(lock);
>>
>> + if (!acquired&& __mutex_waiter_is_first(lock,&waiter)) {
>> __mutex_set_flag(lock, MUTEX_FLAG_HANDOFF);
>> + /*
>> + * Wait until the lock is handed off or the owner
>> + * sleeps.
>> + */
>> + acquired = mutex_optimistic_spin(lock, ww_ctx,
>> + use_ww_ctx, true);
>> + }
> That said; I think there's a few problems with this. Since we now poke
> at the loop termination conditions outside of the wait_lock, it becomes
> important where we do the task->state vs wakeup bits.
>
> Specifically, since we still have state==RUNNING here, its possible
> we'll fail to acquire the lock _and_ miss the wakeup from
> mutex_unlock(). Leaving us stuck forever more.
>
> Also, we should do the __mutex_trylock _after_ we set the handoff,
> otherwise its possible we get the lock handed (miss the wakeup as per
> the above) and fail to notice, again going back to sleep forever more.
>
Yes, you are right. I am less familiar with the intricacy of the
sleep-wakeup interaction.
> @@ -638,7 +636,8 @@ __mutex_lock_common(struct mutex *lock,
>
> lock_contended(&lock->dep_map, ip);
>
> - for (acquired = false; !acquired; ) {
> + set_task_state(task, state);
> + for (;;) {
> /*
> * got a signal? (This code gets eliminated in the
> * TASK_UNINTERRUPTIBLE case.)
> @@ -654,30 +653,23 @@ __mutex_lock_common(struct mutex *lock,
> goto err;
> }
>
> - __set_task_state(task, state);
> -
> - /* didn't get the lock, go to sleep: */
> spin_unlock_mutex(&lock->wait_lock, flags);
> schedule_preempt_disabled();
>
> - /*
> - * Both __mutex_trylock() and __mutex_waiter_is_first()
> - * can be done without the protection of wait_lock.
> - */
> - acquired = __mutex_trylock(lock, true);
> + set_task_state(task, state);
>
> - if (!acquired&& __mutex_waiter_is_first(lock,&waiter)) {
> + if (__mutex_waiter_is_first(lock,&waiter)) {
> __mutex_set_flag(lock, MUTEX_FLAG_HANDOFF);
> - /*
> - * Wait until the lock is handed off or the owner
> - * sleeps.
> - */
> - acquired = mutex_optimistic_spin(lock, ww_ctx,
> - use_ww_ctx, true);
> + if (mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, true))
> + break;
> }
>
> + if (__mutex_trylock(lock, true))
> + break;
> +
I think the set _task_state() can be moved to just before
__mutex_trylock(). In this way, we can save a smp_mb() if we can get the
lock in the optspin loop. Other than that, I am fine with the other changes.
Cheers,
Longman
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:locking/core] locking/mutex: Simplify some ww_mutex code in __mutex_lock_common()
2016-08-26 23:35 [RFC PATCH-queue/locking/rfc 1/2] locking/mutex: Simplify some ww_mutex code in __mutex_lock_common() Waiman Long
2016-08-26 23:35 ` [RFC PATCH-queue/locking/rfc 2/2] locking/mutex: Enable optimistic spinning of woken waiter Waiman Long
@ 2016-10-25 10:29 ` tip-bot for Waiman Long
1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Waiman Long @ 2016-10-25 10:29 UTC (permalink / raw)
To: linux-tip-commits
Cc: dingtianhong, tglx, jason.low2, paulmck, tim.c.chen, paulmck,
akpm, linux-kernel, dave, hpa, imre.deak, torvalds, Will.Deacon,
mingo, peterz, Waiman.Long
Commit-ID: a40ca56577f628eb3f7af22b484e95edfdd047a2
Gitweb: http://git.kernel.org/tip/a40ca56577f628eb3f7af22b484e95edfdd047a2
Author: Waiman Long <Waiman.Long@hpe.com>
AuthorDate: Fri, 26 Aug 2016 19:35:08 -0400
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 25 Oct 2016 11:31:53 +0200
locking/mutex: Simplify some ww_mutex code in __mutex_lock_common()
This patch removes some of the redundant ww_mutex code in
__mutex_lock_common().
Tested-by: Jason Low <jason.low2@hpe.com>
Signed-off-by: Waiman Long <Waiman.Long@hpe.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Ding Tianhong <dingtianhong@huawei.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Paul E. McKenney <paulmck@us.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Will Deacon <Will.Deacon@arm.com>
Link: http://lkml.kernel.org/r/1472254509-27508-1-git-send-email-Waiman.Long@hpe.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/mutex.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 8bb2304..6c0d304 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -587,10 +587,11 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
struct mutex_waiter waiter;
unsigned long flags;
bool first = false;
+ struct ww_mutex *ww;
int ret;
if (use_ww_ctx) {
- struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
+ ww = container_of(lock, struct ww_mutex, base);
if (unlikely(ww_ctx == READ_ONCE(ww->ctx)))
return -EALREADY;
}
@@ -602,12 +603,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx)) {
/* got the lock, yay! */
lock_acquired(&lock->dep_map, ip);
- if (use_ww_ctx) {
- struct ww_mutex *ww;
- ww = container_of(lock, struct ww_mutex, base);
-
+ if (use_ww_ctx)
ww_mutex_set_context_fastpath(ww, ww_ctx);
- }
preempt_enable();
return 0;
}
@@ -691,10 +688,8 @@ skip_wait:
/* got the lock - cleanup and rejoice! */
lock_acquired(&lock->dep_map, ip);
- if (use_ww_ctx) {
- struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
+ if (use_ww_ctx)
ww_mutex_set_context_slowpath(ww, ww_ctx);
- }
spin_unlock_mutex(&lock->wait_lock, flags);
preempt_enable();
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip:locking/core] locking/mutex: Enable optimistic spinning of woken waiter
2016-08-26 23:35 ` [RFC PATCH-queue/locking/rfc 2/2] locking/mutex: Enable optimistic spinning of woken waiter Waiman Long
2016-08-30 15:08 ` Peter Zijlstra
@ 2016-10-25 10:29 ` tip-bot for Waiman Long
1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Waiman Long @ 2016-10-25 10:29 UTC (permalink / raw)
To: linux-tip-commits
Cc: dave, tglx, dingtianhong, torvalds, peterz, hpa, imre.deak, akpm,
mingo, tim.c.chen, jason.low2, linux-kernel, Will.Deacon,
paulmck, paulmck, Waiman.Long
Commit-ID: b341afb325eb390f707a82cbefd65cda887302ab
Gitweb: http://git.kernel.org/tip/b341afb325eb390f707a82cbefd65cda887302ab
Author: Waiman Long <Waiman.Long@hpe.com>
AuthorDate: Fri, 26 Aug 2016 19:35:09 -0400
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 25 Oct 2016 11:31:54 +0200
locking/mutex: Enable optimistic spinning of woken waiter
This patch makes the waiter that sets the HANDOFF flag start spinning
instead of sleeping until the handoff is complete or the owner
sleeps. Otherwise, the handoff will cause the optimistic spinners to
abort spinning as the handed-off owner may not be running.
Tested-by: Jason Low <jason.low2@hpe.com>
Signed-off-by: Waiman Long <Waiman.Long@hpe.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Ding Tianhong <dingtianhong@huawei.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Paul E. McKenney <paulmck@us.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Will Deacon <Will.Deacon@arm.com>
Link: http://lkml.kernel.org/r/1472254509-27508-2-git-send-email-Waiman.Long@hpe.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/mutex.c | 77 +++++++++++++++++++++++++++++++++++---------------
1 file changed, 54 insertions(+), 23 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 6c0d304..17a88e9 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -416,24 +416,39 @@ static inline int mutex_can_spin_on_owner(struct mutex *lock)
*
* Returns true when the lock was taken, otherwise false, indicating
* that we need to jump to the slowpath and sleep.
+ *
+ * The waiter flag is set to true if the spinner is a waiter in the wait
+ * queue. The waiter-spinner will spin on the lock directly and concurrently
+ * with the spinner at the head of the OSQ, if present, until the owner is
+ * changed to itself.
*/
static bool mutex_optimistic_spin(struct mutex *lock,
- struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx)
+ struct ww_acquire_ctx *ww_ctx,
+ const bool use_ww_ctx, const bool waiter)
{
struct task_struct *task = current;
- if (!mutex_can_spin_on_owner(lock))
- goto done;
+ if (!waiter) {
+ /*
+ * The purpose of the mutex_can_spin_on_owner() function is
+ * to eliminate the overhead of osq_lock() and osq_unlock()
+ * in case spinning isn't possible. As a waiter-spinner
+ * is not going to take OSQ lock anyway, there is no need
+ * to call mutex_can_spin_on_owner().
+ */
+ if (!mutex_can_spin_on_owner(lock))
+ goto fail;
- /*
- * In order to avoid a stampede of mutex spinners trying to
- * acquire the mutex all at once, the spinners need to take a
- * MCS (queued) lock first before spinning on the owner field.
- */
- if (!osq_lock(&lock->osq))
- goto done;
+ /*
+ * In order to avoid a stampede of mutex spinners trying to
+ * acquire the mutex all at once, the spinners need to take a
+ * MCS (queued) lock first before spinning on the owner field.
+ */
+ if (!osq_lock(&lock->osq))
+ goto fail;
+ }
- while (true) {
+ for (;;) {
struct task_struct *owner;
if (use_ww_ctx && ww_ctx->acquired > 0) {
@@ -449,7 +464,7 @@ static bool mutex_optimistic_spin(struct mutex *lock,
* performed the optimistic spinning cannot be done.
*/
if (READ_ONCE(ww->ctx))
- break;
+ goto fail_unlock;
}
/*
@@ -457,15 +472,20 @@ static bool mutex_optimistic_spin(struct mutex *lock,
* release the lock or go to sleep.
*/
owner = __mutex_owner(lock);
- if (owner && !mutex_spin_on_owner(lock, owner))
- break;
+ if (owner) {
+ if (waiter && owner == task) {
+ smp_mb(); /* ACQUIRE */
+ break;
+ }
- /* Try to acquire the mutex if it is unlocked. */
- if (__mutex_trylock(lock, false)) {
- osq_unlock(&lock->osq);
- return true;
+ if (!mutex_spin_on_owner(lock, owner))
+ goto fail_unlock;
}
+ /* Try to acquire the mutex if it is unlocked. */
+ if (__mutex_trylock(lock, waiter))
+ break;
+
/*
* The cpu_relax() call is a compiler barrier which forces
* everything in this loop to be re-loaded. We don't need
@@ -475,8 +495,17 @@ static bool mutex_optimistic_spin(struct mutex *lock,
cpu_relax_lowlatency();
}
- osq_unlock(&lock->osq);
-done:
+ if (!waiter)
+ osq_unlock(&lock->osq);
+
+ return true;
+
+
+fail_unlock:
+ if (!waiter)
+ osq_unlock(&lock->osq);
+
+fail:
/*
* If we fell out of the spin path because of need_resched(),
* reschedule now, before we try-lock the mutex. This avoids getting
@@ -495,7 +524,8 @@ done:
}
#else
static bool mutex_optimistic_spin(struct mutex *lock,
- struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx)
+ struct ww_acquire_ctx *ww_ctx,
+ const bool use_ww_ctx, const bool waiter)
{
return false;
}
@@ -600,7 +630,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip);
if (__mutex_trylock(lock, false) ||
- mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx)) {
+ mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, false)) {
/* got the lock, yay! */
lock_acquired(&lock->dep_map, ip);
if (use_ww_ctx)
@@ -669,7 +699,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
* state back to RUNNING and fall through the next schedule(),
* or we must see its unlock and acquire.
*/
- if (__mutex_trylock(lock, first))
+ if ((first && mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, true)) ||
+ __mutex_trylock(lock, first))
break;
spin_lock_mutex(&lock->wait_lock, flags);
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-25 10:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-26 23:35 [RFC PATCH-queue/locking/rfc 1/2] locking/mutex: Simplify some ww_mutex code in __mutex_lock_common() Waiman Long
2016-08-26 23:35 ` [RFC PATCH-queue/locking/rfc 2/2] locking/mutex: Enable optimistic spinning of woken waiter Waiman Long
2016-08-30 15:08 ` Peter Zijlstra
2016-08-30 22:58 ` Waiman Long
2016-10-25 10:29 ` [tip:locking/core] " tip-bot for Waiman Long
2016-10-25 10:29 ` [tip:locking/core] locking/mutex: Simplify some ww_mutex code in __mutex_lock_common() tip-bot for Waiman Long
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.