From patchwork Fri Nov 27 12:07:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Matthew Auld X-Patchwork-Id: 11936081 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=ham 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 876C2C64E7A for ; Fri, 27 Nov 2020 12:14:53 +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 346C0208D5 for ; Fri, 27 Nov 2020 12:14:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 346C0208D5 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 A85226EDA0; Fri, 27 Nov 2020 12:12:24 +0000 (UTC) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTPS id B93046ED98; Fri, 27 Nov 2020 12:12:22 +0000 (UTC) IronPort-SDR: EVEnetAMC9EV5klCgK7fUHaLtcZH+EAOdqmCn4GmerNrR/A4AFISe0PBzhXrATt9NKjeB6agnU ItAH+PT1i1Nw== X-IronPort-AV: E=McAfee;i="6000,8403,9817"; a="168883870" X-IronPort-AV: E=Sophos;i="5.78,374,1599548400"; d="scan'208";a="168883870" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Nov 2020 04:12:22 -0800 IronPort-SDR: VLw7Kyboc/CaW6U9kXljsLS4vvyHwGYoGFI2tw8X7f2c7ToIfy12CnV7qVZzOwuwqWcAfknh3j Pk9xEYmyzihw== X-IronPort-AV: E=Sophos;i="5.78,374,1599548400"; d="scan'208";a="548030032" 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:12:20 -0800 From: Matthew Auld To: intel-gfx@lists.freedesktop.org Subject: [RFC PATCH 153/162] drm/i915: Implement eviction locking v2 Date: Fri, 27 Nov 2020 12:07:09 +0000 Message-Id: <20201127120718.454037-154-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: dri-devel@lists.freedesktop.org, =?utf-8?q?Thomas_Hellstr=C3=B6m?= Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Maarten Lankhorst Use a separate acquire context list and a separate locking function for objects that are locked for eviction. These objects are then properly referenced while on the list and can be unlocked early in the ww transaction. Co-developed-by: Thomas Hellström Signed-off-by: Thomas Hellström Signed-off-by: Maarten Lankhorst Cc: Matthew Auld --- drivers/gpu/drm/i915/gem/i915_gem_object.h | 67 +++++++++++++++++-- .../gpu/drm/i915/gem/i915_gem_object_types.h | 5 ++ drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 14 +++- drivers/gpu/drm/i915/i915_gem_ww.c | 51 ++++++++++---- drivers/gpu/drm/i915/i915_gem_ww.h | 3 + 5 files changed, 122 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 52a36b4052f0..e237b0fb0e79 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -158,6 +158,32 @@ static inline void assert_object_held_shared(struct drm_i915_gem_object *obj) assert_object_held(obj); } +static inline int +i915_gem_object_lock_to_evict(struct drm_i915_gem_object *obj, + struct i915_gem_ww_ctx *ww) +{ + int ret; + + if (ww->intr) + ret = dma_resv_lock_interruptible(obj->base.resv, &ww->ctx); + else + ret = dma_resv_lock(obj->base.resv, &ww->ctx); + + if (!ret) { + list_add_tail(&obj->obj_link, &ww->eviction_list); + i915_gem_object_get(obj); + obj->evict_locked = true; + } + + GEM_WARN_ON(ret == -EALREADY); + if (ret == -EDEADLK) { + ww->contended_evict = true; + ww->contended = i915_gem_object_get(obj); + } + + return ret; +} + static inline int __i915_gem_object_lock(struct drm_i915_gem_object *obj, struct i915_gem_ww_ctx *ww, bool intr) @@ -169,13 +195,25 @@ static inline int __i915_gem_object_lock(struct drm_i915_gem_object *obj, else ret = dma_resv_lock(obj->base.resv, ww ? &ww->ctx : NULL); - if (!ret && ww) + if (!ret && ww) { list_add_tail(&obj->obj_link, &ww->obj_list); - if (ret == -EALREADY) - ret = 0; + obj->evict_locked = false; + } - if (ret == -EDEADLK) + if (ret == -EALREADY) { + ret = 0; + /* We've already evicted an object needed for this batch. */ + if (obj->evict_locked) { + list_move_tail(&obj->obj_link, &ww->obj_list); + i915_gem_object_put(obj); + obj->evict_locked = false; + } + } + + if (ret == -EDEADLK) { + ww->contended_evict = false; ww->contended = i915_gem_object_get(obj); + } return ret; } @@ -580,6 +618,27 @@ i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj, __i915_gem_object_invalidate_frontbuffer(obj, origin); } +/** + * i915_gem_get_locking_ctx - Get the locking context of a locked object + * if any. + * + * @obj: The object to get the locking ctx from + * + * RETURN: The locking context if the object was locked using a context. + * NULL otherwise. + */ +static inline struct i915_gem_ww_ctx * +i915_gem_get_locking_ctx(const struct drm_i915_gem_object *obj) +{ + struct ww_acquire_ctx *ctx; + + ctx = obj->base.resv->lock.ctx; + if (!ctx) + return NULL; + + return container_of(ctx, struct i915_gem_ww_ctx, ctx); +} + #ifdef CONFIG_MMU_NOTIFIER static inline bool i915_gem_object_is_userptr(struct drm_i915_gem_object *obj) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 331d113f7d5b..c42c0d3d5d67 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -142,6 +142,11 @@ struct drm_i915_gem_object { */ struct list_head obj_link; + /** + * @evict_locked: Whether @obj_link sits on the eviction_list + */ + bool evict_locked; + /** Stolen memory for this object, instead of being backed by shmem. */ struct drm_mm_node *stolen; union { diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c index 27674048f17d..59d0f14b90ea 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c @@ -100,6 +100,7 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, unsigned long *nr_scanned, unsigned int shrink) { + struct drm_i915_gem_object *obj; const struct { struct list_head *list; unsigned int bit; @@ -164,7 +165,6 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, */ for (phase = phases; phase->list; phase++) { struct list_head still_in_list; - struct drm_i915_gem_object *obj; unsigned long flags; if ((shrink & phase->bit) == 0) @@ -197,6 +197,10 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, if (!can_release_pages(obj)) continue; + /* Already locked this object? */ + if (ww && ww == i915_gem_get_locking_ctx(obj)) + continue; + if (!kref_get_unless_zero(&obj->base.refcount)) continue; @@ -209,7 +213,11 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, if (!i915_gem_object_trylock(obj)) goto skip; } else { - err = i915_gem_object_lock(obj, ww); + err = i915_gem_object_lock_to_evict(obj, ww); + if (err == -EALREADY) { + err = 0; + goto skip; + } if (err) goto skip; } @@ -235,6 +243,8 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, if (err) return err; } + if (ww) + i915_gem_ww_ctx_unlock_evictions(ww); if (shrink & I915_SHRINK_BOUND) intel_runtime_pm_put(&i915->runtime_pm, wakeref); diff --git a/drivers/gpu/drm/i915/i915_gem_ww.c b/drivers/gpu/drm/i915/i915_gem_ww.c index 43960d8595eb..811bf7677d78 100644 --- a/drivers/gpu/drm/i915/i915_gem_ww.c +++ b/drivers/gpu/drm/i915/i915_gem_ww.c @@ -10,24 +10,45 @@ void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ww, bool intr) { ww_acquire_init(&ww->ctx, &reservation_ww_class); INIT_LIST_HEAD(&ww->obj_list); + INIT_LIST_HEAD(&ww->eviction_list); ww->intr = intr; ww->contended = NULL; + ww->contended_evict = false; +} + +void i915_gem_ww_ctx_unlock_evictions(struct i915_gem_ww_ctx *ww) +{ + struct drm_i915_gem_object *obj, *next; + + list_for_each_entry_safe(obj, next, &ww->eviction_list, obj_link) { + list_del(&obj->obj_link); + GEM_WARN_ON(!obj->evict_locked); + i915_gem_object_unlock(obj); + i915_gem_object_put(obj); + } } static void i915_gem_ww_ctx_unlock_all(struct i915_gem_ww_ctx *ww) { - struct drm_i915_gem_object *obj; + struct drm_i915_gem_object *obj, *next; - while ((obj = list_first_entry_or_null(&ww->obj_list, struct drm_i915_gem_object, obj_link))) { + list_for_each_entry_safe(obj, next, &ww->obj_list, obj_link) { list_del(&obj->obj_link); + GEM_WARN_ON(obj->evict_locked); i915_gem_object_unlock(obj); } + + i915_gem_ww_ctx_unlock_evictions(ww); } void i915_gem_ww_unlock_single(struct drm_i915_gem_object *obj) { + bool evict_locked = obj->evict_locked; + list_del(&obj->obj_link); i915_gem_object_unlock(obj); + if (evict_locked) + i915_gem_object_put(obj); } void i915_gem_ww_ctx_fini(struct i915_gem_ww_ctx *ww) @@ -39,27 +60,33 @@ void i915_gem_ww_ctx_fini(struct i915_gem_ww_ctx *ww) int __must_check i915_gem_ww_ctx_backoff(struct i915_gem_ww_ctx *ww) { + struct drm_i915_gem_object *obj = ww->contended; int ret = 0; - if (WARN_ON(!ww->contended)) + if (WARN_ON(!obj)) return -EINVAL; i915_gem_ww_ctx_unlock_all(ww); if (ww->intr) - ret = dma_resv_lock_slow_interruptible(ww->contended->base.resv, &ww->ctx); + ret = dma_resv_lock_slow_interruptible(obj->base.resv, &ww->ctx); else - dma_resv_lock_slow(ww->contended->base.resv, &ww->ctx); + dma_resv_lock_slow(obj->base.resv, &ww->ctx); + if (ret) + goto out; /* - * 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. + * Unlocking the contended lock again, if it was locked for eviction. + * We will most likely not need it in the retried transaction. */ - if (!ret) - dma_resv_unlock(ww->contended->base.resv); + if (ww->contended_evict) { + dma_resv_unlock(obj->base.resv); + } else { + obj->evict_locked = false; + list_add_tail(&obj->obj_link, &ww->obj_list); + } - i915_gem_object_put(ww->contended); +out: + i915_gem_object_put(obj); ww->contended = NULL; return ret; diff --git a/drivers/gpu/drm/i915/i915_gem_ww.h b/drivers/gpu/drm/i915/i915_gem_ww.h index f6b1a796667b..11793b170cc2 100644 --- a/drivers/gpu/drm/i915/i915_gem_ww.h +++ b/drivers/gpu/drm/i915/i915_gem_ww.h @@ -10,15 +10,18 @@ struct i915_gem_ww_ctx { struct ww_acquire_ctx ctx; struct list_head obj_list; + struct list_head eviction_list; struct drm_i915_gem_object *contended; unsigned short intr; unsigned short loop; + unsigned short contended_evict; }; void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ctx, bool intr); void i915_gem_ww_ctx_fini(struct i915_gem_ww_ctx *ctx); int __must_check i915_gem_ww_ctx_backoff(struct i915_gem_ww_ctx *ctx); void i915_gem_ww_unlock_single(struct drm_i915_gem_object *obj); +void i915_gem_ww_ctx_unlock_evictions(struct i915_gem_ww_ctx *ww); /* Internal functions used by the inlines! Don't use. */ static inline int __i915_gem_ww_fini(struct i915_gem_ww_ctx *ww, int err)