From: Jason Low <jason.low2@hpe.com>
To: imre.deak@intel.com
Cc: jason.low2@hpe.com, Peter Zijlstra <peterz@infradead.org>,
linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
Chris Wilson <chris@chris-wilson.co.uk>,
Daniel Vetter <daniel.vetter@intel.com>,
Waiman Long <Waiman.Long@hpe.com>,
Davidlohr Bueso <dave@stgolabs.net>,
jason.low2@hp.com
Subject: Re: [RFC] Avoid mutex starvation when optimistic spinning is disabled
Date: Tue, 19 Jul 2016 21:39:16 -0700 [thread overview]
Message-ID: <1468989556.10247.22.camel@j-VirtualBox> (raw)
In-Reply-To: <1468969470.10247.15.camel@j-VirtualBox>
On Tue, 2016-07-19 at 16:04 -0700, Jason Low wrote:
> Hi Imre,
>
> Here is a patch which prevents a thread from spending too much "time"
> waiting for a mutex in the !CONFIG_MUTEX_SPIN_ON_OWNER case.
>
> Would you like to try this out and see if this addresses the mutex
> starvation issue you are seeing in your workload when optimistic
> spinning is disabled?
Although it looks like it didn't take care of the 'lock stealing' case
in the slowpath. Here is the updated fixed version:
---
Signed-off-by: Jason Low <jason.low2@hpe.com>
---
include/linux/mutex.h | 2 ++
kernel/locking/mutex.c | 65 ++++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index 2cb7531..c1ca68d 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h
@@ -57,6 +57,8 @@ struct mutex {
#endif
#ifdef CONFIG_MUTEX_SPIN_ON_OWNER
struct optimistic_spin_queue osq; /* Spinner MCS lock */
+#else
+ bool yield_to_waiter; /* Prevent starvation when spinning disabled */
#endif
#ifdef CONFIG_DEBUG_MUTEXES
void *magic;
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index a70b90d..6c915ca 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -55,6 +55,8 @@ __mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key)
mutex_clear_owner(lock);
#ifdef CONFIG_MUTEX_SPIN_ON_OWNER
osq_lock_init(&lock->osq);
+#else
+ lock->yield_to_waiter = false;
#endif
debug_mutex_init(lock, name, key);
@@ -71,6 +73,9 @@ EXPORT_SYMBOL(__mutex_init);
*/
__visible void __sched __mutex_lock_slowpath(atomic_t *lock_count);
+
+static inline bool need_yield_to_waiter(struct mutex *lock);
+
/**
* mutex_lock - acquire the mutex
* @lock: the mutex to be acquired
@@ -95,11 +100,15 @@ __visible void __sched __mutex_lock_slowpath(atomic_t *lock_count);
void __sched mutex_lock(struct mutex *lock)
{
might_sleep();
+
/*
* The locking fastpath is the 1->0 transition from
* 'unlocked' into 'locked' state.
*/
- __mutex_fastpath_lock(&lock->count, __mutex_lock_slowpath);
+ if (!need_yield_to_waiter(lock))
+ __mutex_fastpath_lock(&lock->count, __mutex_lock_slowpath);
+ else
+ __mutex_lock_slowpath(&lock->count);
mutex_set_owner(lock);
}
@@ -398,12 +407,39 @@ done:
return false;
}
+
+static inline void do_yield_to_waiter(struct mutex *lock, int loops)
+{
+ return;
+}
+
+static inline bool need_yield_to_waiter(struct mutex *lock)
+{
+ return false;
+}
+
#else
static bool mutex_optimistic_spin(struct mutex *lock,
struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx)
{
return false;
}
+
+#define MUTEX_MAX_WAIT 32
+
+static inline void do_yield_to_waiter(struct mutex *lock, int loops)
+{
+ if (loops < MUTEX_MAX_WAIT)
+ return;
+
+ if (lock->yield_to_waiter != true)
+ lock->yield_to_waiter =true;
+}
+
+static inline bool need_yield_to_waiter(struct mutex *lock)
+{
+ return lock->yield_to_waiter;
+}
#endif
__visible __used noinline
@@ -510,6 +546,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
struct mutex_waiter waiter;
unsigned long flags;
int ret;
+ int loop = 0;
if (use_ww_ctx) {
struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
@@ -532,7 +569,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
* Once more, try to acquire the lock. Only try-lock the mutex if
* it is unlocked to reduce unnecessary xchg() operations.
*/
- if (!mutex_is_locked(lock) &&
+ if (!need_yield_to_waiter(lock) && !mutex_is_locked(lock) &&
(atomic_xchg_acquire(&lock->count, 0) == 1))
goto skip_wait;
@@ -546,6 +583,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
lock_contended(&lock->dep_map, ip);
for (;;) {
+ loop++;
+
/*
* Lets try to take the lock again - this is needed even if
* we get here for the first time (shortly after failing to
@@ -556,7 +595,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
* other waiters. We only attempt the xchg if the count is
* non-negative in order to avoid unnecessary xchg operations:
*/
- if (atomic_read(&lock->count) >= 0 &&
+ if ((!need_yield_to_waiter(lock) || loop > 1) &&
+ atomic_read(&lock->count) >= 0 &&
(atomic_xchg_acquire(&lock->count, -1) == 1))
break;
@@ -581,6 +621,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
spin_unlock_mutex(&lock->wait_lock, flags);
schedule_preempt_disabled();
spin_lock_mutex(&lock->wait_lock, flags);
+ do_yield_to_waiter(lock, loop);
}
__set_task_state(task, TASK_RUNNING);
@@ -590,6 +631,10 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
atomic_set(&lock->count, 0);
debug_mutex_free_waiter(&waiter);
+#ifndef CONFIG_MUTEX_SPIN_ON_OWNER
+ lock->yield_to_waiter = false;
+#endif
+
skip_wait:
/* got the lock - cleanup and rejoice! */
lock_acquired(&lock->dep_map, ip);
@@ -789,10 +834,13 @@ __mutex_lock_interruptible_slowpath(struct mutex *lock);
*/
int __sched mutex_lock_interruptible(struct mutex *lock)
{
- int ret;
+ int ret = 1;
might_sleep();
- ret = __mutex_fastpath_lock_retval(&lock->count);
+
+ if (!need_yield_to_waiter(lock))
+ ret = __mutex_fastpath_lock_retval(&lock->count);
+
if (likely(!ret)) {
mutex_set_owner(lock);
return 0;
@@ -804,10 +852,13 @@ EXPORT_SYMBOL(mutex_lock_interruptible);
int __sched mutex_lock_killable(struct mutex *lock)
{
- int ret;
+ int ret = 1;
might_sleep();
- ret = __mutex_fastpath_lock_retval(&lock->count);
+
+ if (!need_yield_to_waiter(lock))
+ ret = __mutex_fastpath_lock_retval(&lock->count);
+
if (likely(!ret)) {
mutex_set_owner(lock);
return 0;
--
2.1.4
next prev parent reply other threads:[~2016-07-20 4:42 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-18 16:16 [RFC] locking/mutex: Fix starvation of sleeping waiters Imre Deak
2016-07-18 17:15 ` Peter Zijlstra
2016-07-18 17:47 ` Jason Low
2016-07-19 16:53 ` Imre Deak
2016-07-19 22:57 ` Jason Low
2016-07-19 23:04 ` [RFC] Avoid mutex starvation when optimistic spinning is disabled Jason Low
2016-07-20 4:39 ` Jason Low [this message]
2016-07-20 13:29 ` Imre Deak
2016-07-21 20:57 ` Jason Low
2016-07-22 17:55 ` Waiman Long
2016-07-22 18:03 ` Davidlohr Bueso
2016-07-22 18:29 ` Imre Deak
2016-07-22 19:26 ` Davidlohr Bueso
2016-07-22 19:53 ` Imre Deak
2016-07-20 18:37 ` Waiman Long
2016-07-21 22:29 ` Jason Low
2016-07-22 9:34 ` Imre Deak
2016-07-22 18:44 ` Jason Low
2016-07-22 18:01 ` Waiman Long
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1468989556.10247.22.camel@j-VirtualBox \
--to=jason.low2@hpe.com \
--cc=Waiman.Long@hpe.com \
--cc=chris@chris-wilson.co.uk \
--cc=daniel.vetter@intel.com \
--cc=dave@stgolabs.net \
--cc=imre.deak@intel.com \
--cc=jason.low2@hp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).