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@xxxxxxxxxxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Christian König <christian.koenig@xxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> ---
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
