Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1534129
| From | Nicolai Hähnle <nhaehnle@gmail.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop |
| Date | 2016-12-01 15:10 +0100 |
| Message-ID | <sJAdJ-4Rh-45@gated-at.bofh.it> (permalink) |
| References | <sJAdI-4Rh-7@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
From: Nicolai Hähnle <Nicolai.Haehnle@amd.com>
In the following scenario, thread #1 should back off its attempt to lock
ww1 and unlock ww2 (assuming the acquire context stamps are ordered
accordingly).
Thread #0 Thread #1
--------- ---------
successfully lock ww2
set ww1->base.owner
attempt to lock ww1
confirm ww1->ctx == NULL
enter mutex_spin_on_owner
set ww1->ctx
What was likely to happen previously is:
attempt to lock ww2
refuse to spin because
ww2->ctx != NULL
schedule()
detect thread #0 is off CPU
stop optimistic spin
return -EDEADLK
unlock ww2
wakeup thread #0
lock ww2
Now, we are more likely to see:
detect ww1->ctx != NULL
stop optimistic spin
return -EDEADLK
unlock ww2
successfully lock ww2
... because thread #1 will stop its optimistic spin as soon as possible.
The whole scenario is quite unlikely, since it requires thread #1 to get
between thread #0 setting the owner and setting the ctx. But since we're
idling here anyway, the additional check is basically free.
Found by inspection.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Maarten Lankhorst <dev@mblankhorst.nl>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Nicolai Hähnle <Nicolai.Haehnle@amd.com>
---
kernel/locking/mutex.c | 44 ++++++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 9b34961..0afa998 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -350,7 +350,8 @@ ww_mutex_set_context_slowpath(struct ww_mutex *lock,
* 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,
+ bool use_ww_ctx, struct ww_acquire_ctx *ww_ctx)
{
bool ret = true;
@@ -373,6 +374,28 @@ bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
break;
}
+ 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.
+ *
+ * Check this in every inner iteration because we may
+ * be racing against another thread's ww_mutex_lock.
+ */
+ if (READ_ONCE(ww->ctx)) {
+ ret = false;
+ break;
+ }
+ }
+
cpu_relax();
}
rcu_read_unlock();
@@ -460,22 +483,6 @@ static bool mutex_optimistic_spin(struct mutex *lock,
for (;;) {
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))
- goto fail_unlock;
- }
-
/*
* If there's an owner, wait for it to either
* release the lock or go to sleep.
@@ -487,7 +494,8 @@ static bool mutex_optimistic_spin(struct mutex *lock,
break;
}
- if (!mutex_spin_on_owner(lock, owner))
+ if (!mutex_spin_on_owner(lock, owner, use_ww_ctx,
+ ww_ctx))
goto fail_unlock;
}
--
2.7.4
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v2 00/11] locking/ww_mutex: Keep sorted wait list to avoid stampedes Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
[PATCH v2 05/11] locking/ww_mutex: Add waiters in stamp order Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
Re: [PATCH v2 05/11] locking/ww_mutex: Add waiters in stamp order Chris Wilson <chris@chris-wilson.co.uk> - 2016-12-01 17:10 +0100
Re: [PATCH v2 05/11] locking/ww_mutex: Add waiters in stamp order Peter Zijlstra <peterz@infradead.org> - 2016-12-06 16:40 +0100
Re: [PATCH v2 05/11] locking/ww_mutex: Add waiters in stamp order Peter Zijlstra <peterz@infradead.org> - 2016-12-06 18:00 +0100
[PATCH v2 07/11] locking/ww_mutex: Wake at most one waiter for back off when acquiring the lock Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
[PATCH v2 10/11] Documentation/locking/ww_mutex: Update the design document Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
[PATCH v2 09/11] locking/mutex: Initialize mutex_waiter::ww_ctx with poison when debugging Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
[PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop Chris Wilson <chris@chris-wilson.co.uk> - 2016-12-01 15:40 +0100
Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop Peter Zijlstra <peterz@infradead.org> - 2016-12-06 16:10 +0100
Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop Waiman Long <longman@redhat.com> - 2016-12-06 17:10 +0100
Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop Waiman Long <longman@redhat.com> - 2016-12-06 19:50 +0100
Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop Peter Zijlstra <peterz@infradead.org> - 2016-12-06 20:10 +0100
[PATCH v2 01/11] drm/vgem: Use ww_mutex_(un)lock even with a NULL context Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
Re: [PATCH v2 01/11] drm/vgem: Use ww_mutex_(un)lock even with a NULL context Chris Wilson <chris@chris-wilson.co.uk> - 2016-12-01 15:20 +0100
Re: [PATCH v2 01/11] drm/vgem: Use ww_mutex_(un)lock even with a NULL context Daniel Vetter <daniel@ffwll.ch> - 2016-12-01 16:20 +0100
Re: [PATCH v2 01/11] drm/vgem: Use ww_mutex_(un)lock even with a NULL context Peter Zijlstra <peterz@infradead.org> - 2016-12-01 17:30 +0100
[PATCH v2 11/11] [rfc] locking/ww_mutex: Always spin optimistically for the first waiter Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
[PATCH v2 06/11] locking/ww_mutex: Notify waiters that have to back off while adding tasks to wait list Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
csiph-web