* [PATCH] mutex: Do not spin/queue before performing ww_mutex deadlock avoidance
@ 2016-05-26 8:31 ` Chris Wilson
0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2016-05-26 8:31 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar
Cc: intel-gfx, Chris Wilson, Christian König, Maarten Lankhorst,
linux-kernel
The ww_mutex has the property of allowing the lock to detect and report
when it may be used in deadlocking scenarios (to allow the caller to
unwind its locks and avoid the deadlock). This detection needs to be
performed before we queue up for the spin, otherwise we wait on the
osq_lock() for our turn to detect the deadlock that another thread is
spinning on, waiting for us. Otherwise as we are stuck behind our waiter,
throughput plummets.
This can be demonstrated by trying concurrent atomic modesets.
Testcase: igt/kms_cursor_legacy
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
---
kernel/locking/mutex.c | 56 ++++++++++++++++++++++++++++++++------------------
1 file changed, 36 insertions(+), 20 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index e364b424b019..d60f1ba3e64f 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -217,12 +217,35 @@ ww_mutex_set_context_slowpath(struct ww_mutex *lock,
}
#ifdef CONFIG_MUTEX_SPIN_ON_OWNER
+static bool ww_mutex_may_deadlock(struct mutex *lock,
+ struct ww_acquire_ctx *ww_ctx)
+{
+ if (ww_ctx && ww_ctx->acquired > 0) {
+ struct ww_mutex *ww;
+
+ ww = container_of(lock, struct ww_mutex, base);
+ /*
+ * If ww->ctx is set the contents are undefined, only
+ * by acquiring wait_lock there is a guarantee that
+ * they are not invalid when reading.
+ *
+ * As such, when deadlock detection needs to be
+ * performed the optimistic spinning cannot be done.
+ */
+ if (READ_ONCE(ww->ctx))
+ return true;
+ }
+
+ return false;
+}
+
/*
* Look out! "owner" is an entirely speculative pointer
* access and not reliable.
*/
static noinline
-bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
+bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner,
+ struct ww_acquire_ctx *ww_ctx)
{
bool ret = true;
@@ -241,6 +264,11 @@ bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
break;
}
+ if (ww_mutex_may_deadlock(lock, ww_ctx)) {
+ ret = false;
+ break;
+ }
+
cpu_relax_lowlatency();
}
rcu_read_unlock();
@@ -251,7 +279,8 @@ bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
/*
* Initial check for entering the mutex spinning loop
*/
-static inline int mutex_can_spin_on_owner(struct mutex *lock)
+static inline int mutex_can_spin_on_owner(struct mutex *lock,
+ struct ww_acquire_ctx *ww_ctx)
{
struct task_struct *owner;
int retval = 1;
@@ -259,6 +288,9 @@ static inline int mutex_can_spin_on_owner(struct mutex *lock)
if (need_resched())
return 0;
+ if (ww_mutex_may_deadlock(lock, ww_ctx))
+ return 0;
+
rcu_read_lock();
owner = READ_ONCE(lock->owner);
if (owner)
@@ -308,7 +340,7 @@ static bool mutex_optimistic_spin(struct mutex *lock,
{
struct task_struct *task = current;
- if (!mutex_can_spin_on_owner(lock))
+ if (!mutex_can_spin_on_owner(lock, ww_ctx))
goto done;
/*
@@ -322,28 +354,12 @@ static bool mutex_optimistic_spin(struct mutex *lock,
while (true) {
struct task_struct *owner;
- if (use_ww_ctx && ww_ctx->acquired > 0) {
- struct ww_mutex *ww;
-
- ww = container_of(lock, struct ww_mutex, base);
- /*
- * If ww->ctx is set the contents are undefined, only
- * by acquiring wait_lock there is a guarantee that
- * they are not invalid when reading.
- *
- * As such, when deadlock detection needs to be
- * performed the optimistic spinning cannot be done.
- */
- if (READ_ONCE(ww->ctx))
- break;
- }
-
/*
* If there's an owner, wait for it to either
* release the lock or go to sleep.
*/
owner = READ_ONCE(lock->owner);
- if (owner && !mutex_spin_on_owner(lock, owner))
+ if (owner && !mutex_spin_on_owner(lock, owner, ww_ctx))
break;
/* Try to acquire the mutex if it is unlocked. */
--
2.8.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] mutex: Do not spin/queue before performing ww_mutex deadlock avoidance
@ 2016-05-26 8:31 ` Chris Wilson
0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2016-05-26 8:31 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar; +Cc: linux-kernel, intel-gfx, Christian König
The ww_mutex has the property of allowing the lock to detect and report
when it may be used in deadlocking scenarios (to allow the caller to
unwind its locks and avoid the deadlock). This detection needs to be
performed before we queue up for the spin, otherwise we wait on the
osq_lock() for our turn to detect the deadlock that another thread is
spinning on, waiting for us. Otherwise as we are stuck behind our waiter,
throughput plummets.
This can be demonstrated by trying concurrent atomic modesets.
Testcase: igt/kms_cursor_legacy
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
---
kernel/locking/mutex.c | 56 ++++++++++++++++++++++++++++++++------------------
1 file changed, 36 insertions(+), 20 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index e364b424b019..d60f1ba3e64f 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -217,12 +217,35 @@ ww_mutex_set_context_slowpath(struct ww_mutex *lock,
}
#ifdef CONFIG_MUTEX_SPIN_ON_OWNER
+static bool ww_mutex_may_deadlock(struct mutex *lock,
+ struct ww_acquire_ctx *ww_ctx)
+{
+ if (ww_ctx && ww_ctx->acquired > 0) {
+ struct ww_mutex *ww;
+
+ ww = container_of(lock, struct ww_mutex, base);
+ /*
+ * If ww->ctx is set the contents are undefined, only
+ * by acquiring wait_lock there is a guarantee that
+ * they are not invalid when reading.
+ *
+ * As such, when deadlock detection needs to be
+ * performed the optimistic spinning cannot be done.
+ */
+ if (READ_ONCE(ww->ctx))
+ return true;
+ }
+
+ return false;
+}
+
/*
* Look out! "owner" is an entirely speculative pointer
* access and not reliable.
*/
static noinline
-bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
+bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner,
+ struct ww_acquire_ctx *ww_ctx)
{
bool ret = true;
@@ -241,6 +264,11 @@ bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
break;
}
+ if (ww_mutex_may_deadlock(lock, ww_ctx)) {
+ ret = false;
+ break;
+ }
+
cpu_relax_lowlatency();
}
rcu_read_unlock();
@@ -251,7 +279,8 @@ bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
/*
* Initial check for entering the mutex spinning loop
*/
-static inline int mutex_can_spin_on_owner(struct mutex *lock)
+static inline int mutex_can_spin_on_owner(struct mutex *lock,
+ struct ww_acquire_ctx *ww_ctx)
{
struct task_struct *owner;
int retval = 1;
@@ -259,6 +288,9 @@ static inline int mutex_can_spin_on_owner(struct mutex *lock)
if (need_resched())
return 0;
+ if (ww_mutex_may_deadlock(lock, ww_ctx))
+ return 0;
+
rcu_read_lock();
owner = READ_ONCE(lock->owner);
if (owner)
@@ -308,7 +340,7 @@ static bool mutex_optimistic_spin(struct mutex *lock,
{
struct task_struct *task = current;
- if (!mutex_can_spin_on_owner(lock))
+ if (!mutex_can_spin_on_owner(lock, ww_ctx))
goto done;
/*
@@ -322,28 +354,12 @@ static bool mutex_optimistic_spin(struct mutex *lock,
while (true) {
struct task_struct *owner;
- if (use_ww_ctx && ww_ctx->acquired > 0) {
- struct ww_mutex *ww;
-
- ww = container_of(lock, struct ww_mutex, base);
- /*
- * If ww->ctx is set the contents are undefined, only
- * by acquiring wait_lock there is a guarantee that
- * they are not invalid when reading.
- *
- * As such, when deadlock detection needs to be
- * performed the optimistic spinning cannot be done.
- */
- if (READ_ONCE(ww->ctx))
- break;
- }
-
/*
* If there's an owner, wait for it to either
* release the lock or go to sleep.
*/
owner = READ_ONCE(lock->owner);
- if (owner && !mutex_spin_on_owner(lock, owner))
+ if (owner && !mutex_spin_on_owner(lock, owner, ww_ctx))
break;
/* Try to acquire the mutex if it is unlocked. */
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* ✗ Ro.CI.BAT: warning for mutex: Do not spin/queue before performing ww_mutex deadlock avoidance
2016-05-26 8:31 ` Chris Wilson
(?)
@ 2016-05-26 9:02 ` Patchwork
-1 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2016-05-26 9:02 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: mutex: Do not spin/queue before performing ww_mutex deadlock avoidance
URL : https://patchwork.freedesktop.org/series/7788/
State : warning
== Summary ==
Series 7788v1 mutex: Do not spin/queue before performing ww_mutex deadlock avoidance
http://patchwork.freedesktop.org/api/1.0/series/7788/revisions/1/mbox
Test gem_busy:
Subgroup basic-blt:
dmesg-warn -> PASS (ro-skl-i7-6700hq)
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-cmd:
fail -> PASS (fi-byt-n2820)
Test gem_mmap_gtt:
Subgroup basic-read:
dmesg-warn -> PASS (ro-skl-i7-6700hq)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass -> DMESG-WARN (ro-byt-n2820)
Subgroup basic-flip-vs-wf_vblank:
fail -> PASS (ro-bdw-i7-5600u)
skip -> PASS (fi-skl-i5-6260u)
Test kms_psr_sink_crc:
Subgroup psr_basic:
pass -> DMESG-WARN (ro-skl-i7-6700hq)
Test pm_rps:
Subgroup basic-api:
pass -> DMESG-WARN (ro-skl-i7-6700hq)
fi-bdw-i7-5557u total:209 pass:197 dwarn:0 dfail:0 fail:0 skip:12
fi-byt-n2820 total:209 pass:169 dwarn:0 dfail:0 fail:2 skip:38
fi-hsw-i7-4770k total:209 pass:190 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-i7-4770r total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
fi-skl-i5-6260u total:209 pass:198 dwarn:0 dfail:0 fail:0 skip:11
fi-skl-i7-6700k total:209 pass:184 dwarn:0 dfail:0 fail:0 skip:25
fi-snb-i7-2600 total:209 pass:170 dwarn:0 dfail:0 fail:0 skip:39
ro-bdw-i5-5250u total:209 pass:172 dwarn:0 dfail:0 fail:0 skip:37
ro-bdw-i7-5557U total:209 pass:197 dwarn:0 dfail:0 fail:0 skip:12
ro-bdw-i7-5600u total:209 pass:181 dwarn:0 dfail:0 fail:0 skip:28
ro-byt-n2820 total:209 pass:169 dwarn:1 dfail:0 fail:2 skip:37
ro-hsw-i3-4010u total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
ro-hsw-i7-4770r total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
ro-ilk-i7-620lm total:209 pass:146 dwarn:0 dfail:0 fail:1 skip:62
ro-ilk1-i5-650 total:204 pass:146 dwarn:0 dfail:0 fail:1 skip:57
ro-ivb-i7-3770 total:209 pass:177 dwarn:0 dfail:0 fail:0 skip:32
ro-ivb2-i7-3770 total:209 pass:181 dwarn:0 dfail:0 fail:0 skip:28
ro-skl-i7-6700hq total:204 pass:180 dwarn:3 dfail:0 fail:0 skip:21
ro-snb-i7-2620M total:209 pass:170 dwarn:0 dfail:0 fail:1 skip:38
fi-bsw-n3050 failed to connect after reboot
ro-bsw-n3050 failed to connect after reboot
Results at /archive/results/CI_IGT_test/RO_Patchwork_1017/
fc9d741 drm-intel-nightly: 2016y-05m-25d-07h-45m-48s UTC integration manifest
71d575d mutex: Do not spin/queue before performing ww_mutex deadlock avoidance
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Do not spin/queue before performing ww_mutex deadlock avoidance
2016-05-26 8:31 ` Chris Wilson
(?)
(?)
@ 2016-05-26 10:37 ` Maarten Lankhorst
2016-05-26 10:43 ` Chris Wilson
-1 siblings, 1 reply; 20+ messages in thread
From: Maarten Lankhorst @ 2016-05-26 10:37 UTC (permalink / raw)
To: Chris Wilson, Peter Zijlstra, Ingo Molnar
Cc: intel-gfx, Christian König, linux-kernel
Op 26-05-16 om 10:31 schreef Chris Wilson:
> The ww_mutex has the property of allowing the lock to detect and report
> when it may be used in deadlocking scenarios (to allow the caller to
> unwind its locks and avoid the deadlock). This detection needs to be
> performed before we queue up for the spin, otherwise we wait on the
> osq_lock() for our turn to detect the deadlock that another thread is
> spinning on, waiting for us. Otherwise as we are stuck behind our waiter,
> throughput plummets.
>
> This can be demonstrated by trying concurrent atomic modesets.
>
> Testcase: igt/kms_cursor_legacy
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: linux-kernel@vger.kernel.org
> ---
> kernel/locking/mutex.c | 56 ++++++++++++++++++++++++++++++++------------------
> 1 file changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> index e364b424b019..d60f1ba3e64f 100644
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -217,12 +217,35 @@ ww_mutex_set_context_slowpath(struct ww_mutex *lock,
> }
>
> #ifdef CONFIG_MUTEX_SPIN_ON_OWNER
> +static bool ww_mutex_may_deadlock(struct mutex *lock,
> + struct ww_acquire_ctx *ww_ctx)
> +{
> + if (ww_ctx && ww_ctx->acquired > 0) {
> + struct ww_mutex *ww;
> +
> + ww = container_of(lock, struct ww_mutex, base);
> + /*
> + * If ww->ctx is set the contents are undefined, only
> + * by acquiring wait_lock there is a guarantee that
> + * they are not invalid when reading.
> + *
> + * As such, when deadlock detection needs to be
> + * performed the optimistic spinning cannot be done.
> + */
> + if (READ_ONCE(ww->ctx))
> + return true;
> + }
> +
> + return false;
> +}
The check should be at the beginning of __mutex_lock_common,
regardless of spin_on_owner.
This is because -EALREADY was originally designed to be exceptional,
but is used a lot by design in drm/atomic now.
The other check for -EALREADY can be killed, or changed to a
DEBUG_LOCKS_WARN_ON.
The check should also not be for NULL, but for use_ww_ctx.
This way the if check is optimized out for the ww_ctx path, where
ww_ctx is always non-null.
This would also be something for Cc: stable. :)
~Maarten
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Do not spin/queue before performing ww_mutex deadlock avoidance
2016-05-26 10:37 ` [PATCH] " Maarten Lankhorst
@ 2016-05-26 10:43 ` Chris Wilson
0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2016-05-26 10:43 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Peter Zijlstra, Ingo Molnar, intel-gfx, Christian König,
linux-kernel
On Thu, May 26, 2016 at 12:37:30PM +0200, Maarten Lankhorst wrote:
> The check should also not be for NULL, but for use_ww_ctx.
> This way the if check is optimized out for the ww_ctx path, where
> ww_ctx is always non-null.
The compiler can see use_ww_ctx == false => ww_ctx == NULL just as well
to do dead-code elimination, i.e. use_ww_ctx is superflouus and does not
reduce the code size. (gcc 4.7.2, 4.9.1, 5.3.1)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Do not spin/queue before performing ww_mutex deadlock avoidance
@ 2016-05-26 10:43 ` Chris Wilson
0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2016-05-26 10:43 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Peter Zijlstra, intel-gfx, Ingo Molnar, Christian König,
linux-kernel
On Thu, May 26, 2016 at 12:37:30PM +0200, Maarten Lankhorst wrote:
> The check should also not be for NULL, but for use_ww_ctx.
> This way the if check is optimized out for the ww_ctx path, where
> ww_ctx is always non-null.
The compiler can see use_ww_ctx == false => ww_ctx == NULL just as well
to do dead-code elimination, i.e. use_ww_ctx is superflouus and does not
reduce the code size. (gcc 4.7.2, 4.9.1, 5.3.1)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Do not spin/queue before performing ww_mutex deadlock avoidance
2016-05-26 10:43 ` Chris Wilson
(?)
@ 2016-05-26 11:08 ` Maarten Lankhorst
-1 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2016-05-26 11:08 UTC (permalink / raw)
To: Chris Wilson, Peter Zijlstra, Ingo Molnar, intel-gfx,
Christian König, linux-kernel
Op 26-05-16 om 12:43 schreef Chris Wilson:
> On Thu, May 26, 2016 at 12:37:30PM +0200, Maarten Lankhorst wrote:
>> The check should also not be for NULL, but for use_ww_ctx.
>> This way the if check is optimized out for the ww_ctx path, where
>> ww_ctx is always non-null.
> The compiler can see use_ww_ctx == false => ww_ctx == NULL just as well
> to do dead-code elimination, i.e. use_ww_ctx is superflouus and does not
> reduce the code size. (gcc 4.7.2, 4.9.1, 5.3.1)
That's true, but it cannot do the same when use_ww_ctx = true.
In this case the function will always be called with ww_ctx != NULL,
but the compiler can't see that, so it will keep the check even if it's always true.
~Maarten
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] mutex: Report recursive ww_mutex locking early
2016-05-26 8:31 ` Chris Wilson
@ 2016-05-26 20:08 ` Chris Wilson
-1 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2016-05-26 20:08 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar
Cc: intel-gfx, Chris Wilson, Christian König, Maarten Lankhorst,
linux-kernel
Recursive locking for ww_mutexes was originally conceived as an
exception. However, it is heavily used by the DRM atomic modesetting
code. Currently, the recursive deadlock is checked after we have queued
up for a busy-spin and as we never release the lock, we spin until
kicked, whereupon the deadlock is discovered and reported.
A simple solution for the now common problem is to move the recursive
deadlock discovery to the first action when taking the ww_mutex.
Testcase: igt/kms_cursor_legacy
Suggested-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
---
Maarten suggested this as a simpler fix to the immediate problem. Imo,
we still want to perform deadlock detection within the spin in order to
catch more complicated deadlocks without osq_lock() forcing fairness!
-Chris
---
kernel/locking/mutex.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index d60f1ba3e64f..1659398dc8f8 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -502,9 +502,6 @@ __ww_mutex_lock_check_stamp(struct mutex *lock, struct ww_acquire_ctx *ctx)
if (!hold_ctx)
return 0;
- if (unlikely(ctx == hold_ctx))
- return -EALREADY;
-
if (ctx->stamp - hold_ctx->stamp <= LONG_MAX &&
(ctx->stamp != hold_ctx->stamp || ctx > hold_ctx)) {
#ifdef CONFIG_DEBUG_MUTEXES
@@ -530,6 +527,12 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
unsigned long flags;
int ret;
+ if (use_ww_ctx) {
+ struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
+ if (unlikely(ww_ctx == READ_ONCE(ww->ctx)))
+ return -EALREADY;
+ }
+
preempt_disable();
mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip);
--
2.8.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] mutex: Report recursive ww_mutex locking early
@ 2016-05-26 20:08 ` Chris Wilson
0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2016-05-26 20:08 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar; +Cc: linux-kernel, intel-gfx, Christian König
Recursive locking for ww_mutexes was originally conceived as an
exception. However, it is heavily used by the DRM atomic modesetting
code. Currently, the recursive deadlock is checked after we have queued
up for a busy-spin and as we never release the lock, we spin until
kicked, whereupon the deadlock is discovered and reported.
A simple solution for the now common problem is to move the recursive
deadlock discovery to the first action when taking the ww_mutex.
Testcase: igt/kms_cursor_legacy
Suggested-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
---
Maarten suggested this as a simpler fix to the immediate problem. Imo,
we still want to perform deadlock detection within the spin in order to
catch more complicated deadlocks without osq_lock() forcing fairness!
-Chris
---
kernel/locking/mutex.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index d60f1ba3e64f..1659398dc8f8 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -502,9 +502,6 @@ __ww_mutex_lock_check_stamp(struct mutex *lock, struct ww_acquire_ctx *ctx)
if (!hold_ctx)
return 0;
- if (unlikely(ctx == hold_ctx))
- return -EALREADY;
-
if (ctx->stamp - hold_ctx->stamp <= LONG_MAX &&
(ctx->stamp != hold_ctx->stamp || ctx > hold_ctx)) {
#ifdef CONFIG_DEBUG_MUTEXES
@@ -530,6 +527,12 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
unsigned long flags;
int ret;
+ if (use_ww_ctx) {
+ struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
+ if (unlikely(ww_ctx == READ_ONCE(ww->ctx)))
+ return -EALREADY;
+ }
+
preempt_disable();
mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip);
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* ✗ Ro.CI.BAT: warning for mutex: Do not spin/queue before performing ww_mutex deadlock avoidance (rev2)
2016-05-26 8:31 ` Chris Wilson
` (3 preceding siblings ...)
(?)
@ 2016-05-27 5:34 ` Patchwork
-1 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2016-05-27 5:34 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: mutex: Do not spin/queue before performing ww_mutex deadlock avoidance (rev2)
URL : https://patchwork.freedesktop.org/series/7788/
State : warning
== Summary ==
Series 7788v2 mutex: Do not spin/queue before performing ww_mutex deadlock avoidance
http://patchwork.freedesktop.org/api/1.0/series/7788/revisions/2/mbox
Test gem_busy:
Subgroup basic-blt:
pass -> DMESG-WARN (ro-skl-i7-6700hq)
Subgroup basic-vebox:
pass -> DMESG-WARN (ro-skl-i7-6700hq)
Test kms_sink_crc_basic:
skip -> PASS (ro-skl-i7-6700hq)
ro-bdw-i5-5250u total:209 pass:172 dwarn:0 dfail:0 fail:0 skip:37
ro-bdw-i7-5557U total:209 pass:197 dwarn:0 dfail:0 fail:0 skip:12
ro-bdw-i7-5600u total:209 pass:181 dwarn:0 dfail:0 fail:0 skip:28
ro-bsw-n3050 total:209 pass:168 dwarn:0 dfail:0 fail:2 skip:39
ro-byt-n2820 total:209 pass:169 dwarn:0 dfail:0 fail:3 skip:37
ro-hsw-i3-4010u total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
ro-hsw-i7-4770r total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23
ro-ilk-i7-620lm total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
ro-ilk1-i5-650 total:204 pass:146 dwarn:0 dfail:0 fail:1 skip:57
ro-ivb-i7-3770 total:209 pass:177 dwarn:0 dfail:0 fail:0 skip:32
ro-ivb2-i7-3770 total:209 pass:181 dwarn:0 dfail:0 fail:0 skip:28
ro-skl-i7-6700hq total:204 pass:179 dwarn:4 dfail:0 fail:0 skip:21
ro-snb-i7-2620M total:209 pass:170 dwarn:0 dfail:0 fail:1 skip:38
Results at /archive/results/CI_IGT_test/RO_Patchwork_1030/
c17d7e8 drm-intel-nightly: 2016y-05m-26d-15h-04m-52s UTC integration manifest
1e55a3d mutex: Report recursive ww_mutex locking early
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Report recursive ww_mutex locking early
2016-05-26 20:08 ` Chris Wilson
(?)
@ 2016-05-30 7:43 ` Maarten Lankhorst
2016-05-30 9:11 ` Peter Zijlstra
-1 siblings, 1 reply; 20+ messages in thread
From: Maarten Lankhorst @ 2016-05-30 7:43 UTC (permalink / raw)
To: Chris Wilson, Peter Zijlstra, Ingo Molnar
Cc: intel-gfx, Christian König, linux-kernel
Op 26-05-16 om 22:08 schreef Chris Wilson:
> Recursive locking for ww_mutexes was originally conceived as an
> exception. However, it is heavily used by the DRM atomic modesetting
> code. Currently, the recursive deadlock is checked after we have queued
> up for a busy-spin and as we never release the lock, we spin until
> kicked, whereupon the deadlock is discovered and reported.
>
> A simple solution for the now common problem is to move the recursive
> deadlock discovery to the first action when taking the ww_mutex.
>
> Testcase: igt/kms_cursor_legacy
> Suggested-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: linux-kernel@vger.kernel.org
> ---
>
> Maarten suggested this as a simpler fix to the immediate problem. Imo,
> we still want to perform deadlock detection within the spin in order to
> catch more complicated deadlocks without osq_lock() forcing fairness!
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Should this be Cc: stable@vger.kernel.org ?
I think in the normal case things would move forward even with osq_lock,
but you can make a separate patch to add it to mutex_can_spin_on_owner,
with the same comment as in mutex_optimistic_spin.
> ---
> kernel/locking/mutex.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> index d60f1ba3e64f..1659398dc8f8 100644
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -502,9 +502,6 @@ __ww_mutex_lock_check_stamp(struct mutex *lock, struct ww_acquire_ctx *ctx)
> if (!hold_ctx)
> return 0;
>
> - if (unlikely(ctx == hold_ctx))
> - return -EALREADY;
> -
> if (ctx->stamp - hold_ctx->stamp <= LONG_MAX &&
> (ctx->stamp != hold_ctx->stamp || ctx > hold_ctx)) {
> #ifdef CONFIG_DEBUG_MUTEXES
> @@ -530,6 +527,12 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
> unsigned long flags;
> int ret;
>
> + if (use_ww_ctx) {
> + struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
> + if (unlikely(ww_ctx == READ_ONCE(ww->ctx)))
> + return -EALREADY;
> + }
> +
> preempt_disable();
> mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip);
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Report recursive ww_mutex locking early
2016-05-30 7:43 ` Maarten Lankhorst
@ 2016-05-30 9:11 ` Peter Zijlstra
0 siblings, 0 replies; 20+ messages in thread
From: Peter Zijlstra @ 2016-05-30 9:11 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Chris Wilson, Ingo Molnar, intel-gfx, Christian König, linux-kernel
On Mon, May 30, 2016 at 09:43:53AM +0200, Maarten Lankhorst wrote:
> Op 26-05-16 om 22:08 schreef Chris Wilson:
> > Recursive locking for ww_mutexes was originally conceived as an
> > exception. However, it is heavily used by the DRM atomic modesetting
> > code. Currently, the recursive deadlock is checked after we have queued
> > up for a busy-spin and as we never release the lock, we spin until
> > kicked, whereupon the deadlock is discovered and reported.
> >
> > A simple solution for the now common problem is to move the recursive
> > deadlock discovery to the first action when taking the ww_mutex.
> >
> > Testcase: igt/kms_cursor_legacy
I've no idea what this tag is or where to find the actual testcase, so
I've killed it.
> > Suggested-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >
> > Maarten suggested this as a simpler fix to the immediate problem. Imo,
> > we still want to perform deadlock detection within the spin in order to
> > catch more complicated deadlocks without osq_lock() forcing fairness!
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> Should this be Cc: stable@vger.kernel.org ?
Can do; how far back?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Report recursive ww_mutex locking early
@ 2016-05-30 9:11 ` Peter Zijlstra
0 siblings, 0 replies; 20+ messages in thread
From: Peter Zijlstra @ 2016-05-30 9:11 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: linux-kernel, intel-gfx, Ingo Molnar, Christian König
On Mon, May 30, 2016 at 09:43:53AM +0200, Maarten Lankhorst wrote:
> Op 26-05-16 om 22:08 schreef Chris Wilson:
> > Recursive locking for ww_mutexes was originally conceived as an
> > exception. However, it is heavily used by the DRM atomic modesetting
> > code. Currently, the recursive deadlock is checked after we have queued
> > up for a busy-spin and as we never release the lock, we spin until
> > kicked, whereupon the deadlock is discovered and reported.
> >
> > A simple solution for the now common problem is to move the recursive
> > deadlock discovery to the first action when taking the ww_mutex.
> >
> > Testcase: igt/kms_cursor_legacy
I've no idea what this tag is or where to find the actual testcase, so
I've killed it.
> > Suggested-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >
> > Maarten suggested this as a simpler fix to the immediate problem. Imo,
> > we still want to perform deadlock detection within the spin in order to
> > catch more complicated deadlocks without osq_lock() forcing fairness!
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> Should this be Cc: stable@vger.kernel.org ?
Can do; how far back?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Report recursive ww_mutex locking early
2016-05-30 9:11 ` Peter Zijlstra
@ 2016-05-30 9:43 ` Maarten Lankhorst
-1 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2016-05-30 9:43 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Chris Wilson, Ingo Molnar, intel-gfx, Christian König, linux-kernel
Op 30-05-16 om 11:11 schreef Peter Zijlstra:
> On Mon, May 30, 2016 at 09:43:53AM +0200, Maarten Lankhorst wrote:
>> Op 26-05-16 om 22:08 schreef Chris Wilson:
>>> Recursive locking for ww_mutexes was originally conceived as an
>>> exception. However, it is heavily used by the DRM atomic modesetting
>>> code. Currently, the recursive deadlock is checked after we have queued
>>> up for a busy-spin and as we never release the lock, we spin until
>>> kicked, whereupon the deadlock is discovered and reported.
>>>
>>> A simple solution for the now common problem is to move the recursive
>>> deadlock discovery to the first action when taking the ww_mutex.
>>>
>>> Testcase: igt/kms_cursor_legacy
> I've no idea what this tag is or where to find the actual testcase, so
> I've killed it.
https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/
tests/kms_cursor_legacy tries to do as many updates as possible with SCHED_RR..
Patch not applied, SCHED_RR:
# ./kms_cursor_legacy
IGT-Version: 1.14-g9579e5447aa3 (x86_64) (Linux: 4.6.0-patser+ x86_64)
[3] count=86
[2] count=91
[1] count=78
[0] count=104
Subtest stress-bo: SUCCESS (22,372s)
Patch not applied, SCHED_NORMAL:
# ./kms_cursor_legacy
IGT-Version: 1.14-g9579e5447aa3 (x86_64) (Linux: 4.6.0-patser+ x86_64)
[2] count=4713
[0] count=4288
[3] count=4776
[1] count=4521
Subtest stress-bo: SUCCESS (21,492s)
Patch applied, NORMAL + RR give roughly same results:
# nfs/intel-gpu-tools/tests/kms_cursor_legacy
IGT-Version: 1.14-g9579e5447aa3 (x86_64) (Linux: 4.6.0-patser+ x86_64)
[0] count=77631
[1] count=77740
[3] count=77612
[2] count=77666
Subtest stress-bo: SUCCESS (21,487s)
>>> Suggested-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: Christian König <christian.koenig@amd.com>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Cc: linux-kernel@vger.kernel.org
>>> ---
>>>
>>> Maarten suggested this as a simpler fix to the immediate problem. Imo,
>>> we still want to perform deadlock detection within the spin in order to
>>> catch more complicated deadlocks without osq_lock() forcing fairness!
>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>
>> Should this be Cc: stable@vger.kernel.org ?
> Can do; how far back?
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Report recursive ww_mutex locking early
@ 2016-05-30 9:43 ` Maarten Lankhorst
0 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2016-05-30 9:43 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: linux-kernel, intel-gfx, Ingo Molnar, Christian König
Op 30-05-16 om 11:11 schreef Peter Zijlstra:
> On Mon, May 30, 2016 at 09:43:53AM +0200, Maarten Lankhorst wrote:
>> Op 26-05-16 om 22:08 schreef Chris Wilson:
>>> Recursive locking for ww_mutexes was originally conceived as an
>>> exception. However, it is heavily used by the DRM atomic modesetting
>>> code. Currently, the recursive deadlock is checked after we have queued
>>> up for a busy-spin and as we never release the lock, we spin until
>>> kicked, whereupon the deadlock is discovered and reported.
>>>
>>> A simple solution for the now common problem is to move the recursive
>>> deadlock discovery to the first action when taking the ww_mutex.
>>>
>>> Testcase: igt/kms_cursor_legacy
> I've no idea what this tag is or where to find the actual testcase, so
> I've killed it.
https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/
tests/kms_cursor_legacy tries to do as many updates as possible with SCHED_RR..
Patch not applied, SCHED_RR:
# ./kms_cursor_legacy
IGT-Version: 1.14-g9579e5447aa3 (x86_64) (Linux: 4.6.0-patser+ x86_64)
[3] count=86
[2] count=91
[1] count=78
[0] count=104
Subtest stress-bo: SUCCESS (22,372s)
Patch not applied, SCHED_NORMAL:
# ./kms_cursor_legacy
IGT-Version: 1.14-g9579e5447aa3 (x86_64) (Linux: 4.6.0-patser+ x86_64)
[2] count=4713
[0] count=4288
[3] count=4776
[1] count=4521
Subtest stress-bo: SUCCESS (21,492s)
Patch applied, NORMAL + RR give roughly same results:
# nfs/intel-gpu-tools/tests/kms_cursor_legacy
IGT-Version: 1.14-g9579e5447aa3 (x86_64) (Linux: 4.6.0-patser+ x86_64)
[0] count=77631
[1] count=77740
[3] count=77612
[2] count=77666
Subtest stress-bo: SUCCESS (21,487s)
>>> Suggested-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: Christian König <christian.koenig@amd.com>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Cc: linux-kernel@vger.kernel.org
>>> ---
>>>
>>> Maarten suggested this as a simpler fix to the immediate problem. Imo,
>>> we still want to perform deadlock detection within the spin in order to
>>> catch more complicated deadlocks without osq_lock() forcing fairness!
>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>
>> Should this be Cc: stable@vger.kernel.org ?
> Can do; how far back?
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Report recursive ww_mutex locking early
2016-05-30 9:43 ` Maarten Lankhorst
(?)
@ 2016-05-30 10:27 ` Peter Zijlstra
2016-05-30 10:45 ` Chris Wilson
-1 siblings, 1 reply; 20+ messages in thread
From: Peter Zijlstra @ 2016-05-30 10:27 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Chris Wilson, Ingo Molnar, intel-gfx, Christian König, linux-kernel
On Mon, May 30, 2016 at 11:43:31AM +0200, Maarten Lankhorst wrote:
> Patch not applied, SCHED_RR:
ww_mutex isn't RT aware at all; its one of the things I still have on a
todo list. Should I look harder at finding time for this?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Report recursive ww_mutex locking early
2016-05-30 10:27 ` Peter Zijlstra
@ 2016-05-30 10:45 ` Chris Wilson
2016-05-30 11:16 ` Maarten Lankhorst
0 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2016-05-30 10:45 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Maarten Lankhorst, Ingo Molnar, intel-gfx, Christian König,
linux-kernel
On Mon, May 30, 2016 at 12:27:46PM +0200, Peter Zijlstra wrote:
> On Mon, May 30, 2016 at 11:43:31AM +0200, Maarten Lankhorst wrote:
> > Patch not applied, SCHED_RR:
>
> ww_mutex isn't RT aware at all; its one of the things I still have on a
> todo list. Should I look harder at finding time for this?
The RT usage in the test is to just try and starve the kernel threads
that may be used behind the atomic modeset - a problem we have
encountered in the past. Afaik, no one is using ww_mutex from RT in the
wild, calling the atomic modeset from the RT was just a shortcut to
having the system fully populated with RT threads. To be more realistic
we should be using a couple of normal modesetting threads vs a set of RT
cpu hogs.
Otoh, i915.ko always draws the ire of rt-linux so ww_mutex is likely to
be in their sights in the near future (when i915.ko completes its
transition to full atomic modesetting).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Report recursive ww_mutex locking early
2016-05-30 10:45 ` Chris Wilson
@ 2016-05-30 11:16 ` Maarten Lankhorst
0 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2016-05-30 11:16 UTC (permalink / raw)
To: Chris Wilson, Peter Zijlstra, Ingo Molnar, intel-gfx,
Christian König, linux-kernel
Op 30-05-16 om 12:45 schreef Chris Wilson:
> On Mon, May 30, 2016 at 12:27:46PM +0200, Peter Zijlstra wrote:
>> On Mon, May 30, 2016 at 11:43:31AM +0200, Maarten Lankhorst wrote:
>>> Patch not applied, SCHED_RR:
>> ww_mutex isn't RT aware at all; its one of the things I still have on a
>> todo list. Should I look harder at finding time for this?
> The RT usage in the test is to just try and starve the kernel threads
> that may be used behind the atomic modeset - a problem we have
> encountered in the past. Afaik, no one is using ww_mutex from RT in the
> wild, calling the atomic modeset from the RT was just a shortcut to
> having the system fully populated with RT threads. To be more realistic
> we should be using a couple of normal modesetting threads vs a set of RT
> cpu hogs.
Yeah, unfortunately this doesn't work as you intend it to. You'd need to spawn a few more threads at slightly lower priority so when a thread is blocked waiting for acquisition of the mutexes the workqueues still can't run.
ssh is still responsive with the rest running.
> Otoh, i915.ko always draws the ire of rt-linux so ww_mutex is likely to
> be in their sights in the near future (when i915.ko completes its
> transition to full atomic modesetting).
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mutex: Report recursive ww_mutex locking early
@ 2016-05-30 11:16 ` Maarten Lankhorst
0 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2016-05-30 11:16 UTC (permalink / raw)
To: Chris Wilson, Peter Zijlstra, Ingo Molnar, intel-gfx,
Christian König, linux-kernel
Op 30-05-16 om 12:45 schreef Chris Wilson:
> On Mon, May 30, 2016 at 12:27:46PM +0200, Peter Zijlstra wrote:
>> On Mon, May 30, 2016 at 11:43:31AM +0200, Maarten Lankhorst wrote:
>>> Patch not applied, SCHED_RR:
>> ww_mutex isn't RT aware at all; its one of the things I still have on a
>> todo list. Should I look harder at finding time for this?
> The RT usage in the test is to just try and starve the kernel threads
> that may be used behind the atomic modeset - a problem we have
> encountered in the past. Afaik, no one is using ww_mutex from RT in the
> wild, calling the atomic modeset from the RT was just a shortcut to
> having the system fully populated with RT threads. To be more realistic
> we should be using a couple of normal modesetting threads vs a set of RT
> cpu hogs.
Yeah, unfortunately this doesn't work as you intend it to. You'd need to spawn a few more threads at slightly lower priority so when a thread is blocked waiting for acquisition of the mutexes the workqueues still can't run.
ssh is still responsive with the rest running.
> Otoh, i915.ko always draws the ire of rt-linux so ww_mutex is likely to
> be in their sights in the near future (when i915.ko completes its
> transition to full atomic modesetting).
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tip:locking/core] locking/ww_mutex: Report recursive ww_mutex locking early
2016-05-26 20:08 ` Chris Wilson
(?)
(?)
@ 2016-06-03 10:46 ` tip-bot for Chris Wilson
-1 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Chris Wilson @ 2016-06-03 10:46 UTC (permalink / raw)
To: linux-tip-commits
Cc: chris, tglx, linux-kernel, maarten.lankhorst, peterz, akpm, hpa,
torvalds, paulmck, mingo
Commit-ID: 0422e83d84ae24b933e4b0d4c1e0f0b4ae8a0a3b
Gitweb: http://git.kernel.org/tip/0422e83d84ae24b933e4b0d4c1e0f0b4ae8a0a3b
Author: Chris Wilson <chris@chris-wilson.co.uk>
AuthorDate: Thu, 26 May 2016 21:08:17 +0100
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 3 Jun 2016 08:37:26 +0200
locking/ww_mutex: Report recursive ww_mutex locking early
Recursive locking for ww_mutexes was originally conceived as an
exception. However, it is heavily used by the DRM atomic modesetting
code. Currently, the recursive deadlock is checked after we have queued
up for a busy-spin and as we never release the lock, we spin until
kicked, whereupon the deadlock is discovered and reported.
A simple solution for the now common problem is to move the recursive
deadlock discovery to the first action when taking the ww_mutex.
Suggested-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/1464293297-19777-1-git-send-email-chris@chris-wilson.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/mutex.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index e364b42..79d2d76 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -486,9 +486,6 @@ __ww_mutex_lock_check_stamp(struct mutex *lock, struct ww_acquire_ctx *ctx)
if (!hold_ctx)
return 0;
- if (unlikely(ctx == hold_ctx))
- return -EALREADY;
-
if (ctx->stamp - hold_ctx->stamp <= LONG_MAX &&
(ctx->stamp != hold_ctx->stamp || ctx > hold_ctx)) {
#ifdef CONFIG_DEBUG_MUTEXES
@@ -514,6 +511,12 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
unsigned long flags;
int ret;
+ if (use_ww_ctx) {
+ struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
+ if (unlikely(ww_ctx == READ_ONCE(ww->ctx)))
+ return -EALREADY;
+ }
+
preempt_disable();
mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip);
^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-06-03 10:47 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-26 8:31 [PATCH] mutex: Do not spin/queue before performing ww_mutex deadlock avoidance Chris Wilson
2016-05-26 8:31 ` Chris Wilson
2016-05-26 9:02 ` ✗ Ro.CI.BAT: warning for " Patchwork
2016-05-26 10:37 ` [PATCH] " Maarten Lankhorst
2016-05-26 10:43 ` Chris Wilson
2016-05-26 10:43 ` Chris Wilson
2016-05-26 11:08 ` Maarten Lankhorst
2016-05-26 20:08 ` [PATCH] mutex: Report recursive ww_mutex locking early Chris Wilson
2016-05-26 20:08 ` Chris Wilson
2016-05-30 7:43 ` Maarten Lankhorst
2016-05-30 9:11 ` Peter Zijlstra
2016-05-30 9:11 ` Peter Zijlstra
2016-05-30 9:43 ` Maarten Lankhorst
2016-05-30 9:43 ` Maarten Lankhorst
2016-05-30 10:27 ` Peter Zijlstra
2016-05-30 10:45 ` Chris Wilson
2016-05-30 11:16 ` Maarten Lankhorst
2016-05-30 11:16 ` Maarten Lankhorst
2016-06-03 10:46 ` [tip:locking/core] locking/ww_mutex: " tip-bot for Chris Wilson
2016-05-27 5:34 ` ✗ Ro.CI.BAT: warning for mutex: Do not spin/queue before performing ww_mutex deadlock avoidance (rev2) Patchwork
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.