From patchwork Fri Nov 27 12:05:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Matthew Auld X-Patchwork-Id: 11935927 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 05343C63777 for ; Fri, 27 Nov 2020 12:11:29 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id ABCEF20665 for ; Fri, 27 Nov 2020 12:11:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org ABCEF20665 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E36AF6EC97; Fri, 27 Nov 2020 12:09:58 +0000 (UTC) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5055C6EC91; Fri, 27 Nov 2020 12:09:41 +0000 (UTC) IronPort-SDR: CJnyTKoBcbTTU1QrPascpLU+JF/lbjpvYfhTh60DSZpXGwTpgtggDeKinKG0qLTmDPslZYrj1M L1sWAAUHlqDA== X-IronPort-AV: E=McAfee;i="6000,8403,9817"; a="172540696" X-IronPort-AV: E=Sophos;i="5.78,374,1599548400"; d="scan'208";a="172540696" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Nov 2020 04:09:40 -0800 IronPort-SDR: w8tilsrL6iiGcfLuHfhluGzliTXbSpLb1UUgnlGPnFLAQeezZP+dq3yE3hNHP9frrlY9OZVIqJ n09z85tb1v4Q== X-IronPort-AV: E=Sophos;i="5.78,374,1599548400"; d="scan'208";a="548029036" Received: from mjgleeso-mobl.ger.corp.intel.com (HELO mwauld-desk1.ger.corp.intel.com) ([10.251.85.2]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Nov 2020 04:09:39 -0800 From: Matthew Auld To: intel-gfx@lists.freedesktop.org Subject: [RFC PATCH 073/162] drm/i915: Reference contending lock objects Date: Fri, 27 Nov 2020 12:05:49 +0000 Message-Id: <20201127120718.454037-74-matthew.auld@intel.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20201127120718.454037-1-matthew.auld@intel.com> References: <20201127120718.454037-1-matthew.auld@intel.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?utf-8?q?Thomas_Hellstr=C3=B6m?= , dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Thomas Hellström When we lock objects in leaf functions, for example during eviction, they may disappear as soon as we unreference them, and the locking context contended pointer then points to a free object. Fix this by taking a reference on that object, and also unlock the contending object as soon as we've done the ww transaction relaxation: The restarted transaction may not even need the contending object, and keeping the lock is not needed to prevent starvation. Keeping that lock will unnecessarily requiring us to reference count all locks on the list and also creates locking confusion around -EALREADY. Signed-off-by: Thomas Hellström Cc: Matthew Auld --- drivers/gpu/drm/i915/gem/i915_gem_object.h | 2 +- drivers/gpu/drm/i915/i915_gem.c | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index d56643b3b518..60e27738c39d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -163,7 +163,7 @@ static inline int __i915_gem_object_lock(struct drm_i915_gem_object *obj, ret = 0; if (ret == -EDEADLK) - ww->contended = obj; + ww->contended = i915_gem_object_get(obj); return ret; } diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ef66c0926af6..2248e65cf5f9 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1370,9 +1370,16 @@ int __must_check i915_gem_ww_ctx_backoff(struct i915_gem_ww_ctx *ww) else dma_resv_lock_slow(ww->contended->base.resv, &ww->ctx); + /* + * Unlocking the contended lock again, as might not need it in + * the retried transaction. This does not increase starvation, + * but it's opening up for a wakeup flood if there are many + * transactions relaxing on this object. + */ if (!ret) - list_add_tail(&ww->contended->obj_link, &ww->obj_list); + dma_resv_unlock(ww->contended->base.resv); + i915_gem_object_put(ww->contended); ww->contended = NULL; return ret;