From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753604AbcEZKhf (ORCPT ); Thu, 26 May 2016 06:37:35 -0400 Received: from mga09.intel.com ([134.134.136.24]:51584 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753112AbcEZKhd (ORCPT ); Thu, 26 May 2016 06:37:33 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,367,1459839600"; d="scan'208";a="110851036" Subject: Re: [PATCH] mutex: Do not spin/queue before performing ww_mutex deadlock avoidance To: Chris Wilson , Peter Zijlstra , Ingo Molnar References: <1464251487-23778-1-git-send-email-chris@chris-wilson.co.uk> Cc: intel-gfx@lists.freedesktop.org, =?UTF-8?Q?Christian_K=c3=b6nig?= , linux-kernel@vger.kernel.org From: Maarten Lankhorst Message-ID: Date: Thu, 26 May 2016 12:37:30 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <1464251487-23778-1-git-send-email-chris@chris-wilson.co.uk> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Cc: Peter Zijlstra > Cc: Ingo Molnar > Cc: Christian König > Cc: Maarten Lankhorst > 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