Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: intel-xe@lists.freedesktop.org
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Somalapuram Amaranath" <Amaranath.Somalapuram@amd.com>,
	"Matthew Brost" <matthew.brost@intel.com>,
	dri-devel@lists.freedesktop.org
Subject: [RFC PATCH v3 16/21] drm/exec: Introduce an evict mode
Date: Tue, 21 May 2024 09:16:34 +0200	[thread overview]
Message-ID: <20240521071639.77614-17-thomas.hellstrom@linux.intel.com> (raw)
In-Reply-To: <20240521071639.77614-1-thomas.hellstrom@linux.intel.com>

Locking for eviction is in some way different from locking for
submission:

1) We can't lock objects that are already locked for submission,
hence DRM_EXEC_IGNORE_DUPLICATES must be unset.
2) We must be able to re-lock objects locked for eviction,
either for submission or for yet another eviction, in
particular objects sharing a single resv must be considered.
3) There is no point to keep a contending object after the
transaction restart. We don't know whether we actually want to use
it again.

So introduce a drm_exec evict mode, and for now instead of
explicitly setting it using a function call or implement separate
locking functions that use evict mode, assume evict mode if
there is a snapshot registered. This can easily be changed later.

To keep track of resvs locked for eviction, use a pointer set
implemented by an xarray. This is probably not the most efficient
data structure but used as an easy-to-implement first approach.
If the set is empty (evict mode never used), the performance-
and memory usage impact will be very small.

TODO: Probably want to implement the set using an open addressing
hash table.

Cc: Christian König <christian.koenig@amd.com>
Cc: Somalapuram Amaranath <Amaranath.Somalapuram@amd.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: <dri-devel@lists.freedesktop.org>
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/drm_exec.c | 77 ++++++++++++++++++++++++++++++++++----
 include/drm/drm_exec.h     | 15 ++++++++
 2 files changed, 85 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c
index 9eea5d0d3a98..ea79d96f5439 100644
--- a/drivers/gpu/drm/drm_exec.c
+++ b/drivers/gpu/drm/drm_exec.c
@@ -65,6 +65,10 @@ static void drm_exec_unlock_all(struct drm_exec *exec)
 
 	drm_gem_object_put(exec->prelocked);
 	exec->prelocked = NULL;
+
+	/* garbage collect */
+	xa_destroy(&exec->resv_set);
+	xa_init(&exec->resv_set);
 }
 
 /**
@@ -92,6 +96,8 @@ void drm_exec_init(struct drm_exec *exec, u32 flags, unsigned nr)
 	exec->contended = DRM_EXEC_DUMMY;
 	exec->prelocked = NULL;
 	exec->snap = NULL;
+	exec->drop_contended = false;
+	xa_init(&exec->resv_set);
 }
 EXPORT_SYMBOL(drm_exec_init);
 
@@ -110,6 +116,7 @@ void drm_exec_fini(struct drm_exec *exec)
 		drm_gem_object_put(exec->contended);
 		ww_acquire_fini(&exec->ticket);
 	}
+	xa_destroy(&exec->resv_set);
 }
 EXPORT_SYMBOL(drm_exec_fini);
 
@@ -139,6 +146,30 @@ bool drm_exec_cleanup(struct drm_exec *exec)
 }
 EXPORT_SYMBOL(drm_exec_cleanup);
 
+static unsigned long drm_exec_resv_to_key(const struct dma_resv *resv)
+{
+	return (unsigned long)resv / __alignof__(typeof(*resv));
+}
+
+static void
+drm_exec_resv_set_erase(struct drm_exec *exec, unsigned long key)
+{
+	if (xa_load(&exec->resv_set, key))
+		xa_erase(&exec->resv_set, key);
+}
+
+static bool drm_exec_in_evict_mode(struct drm_exec *exec)
+{
+	return !!exec->snap;
+}
+
+static void drm_exec_set_evict_mode(struct drm_exec *exec,
+				    struct drm_exec_snapshot *snap)
+{
+	exec->snap = snap;
+	exec->flags &= ~DRM_EXEC_IGNORE_DUPLICATES;
+}
+
 /* Track the locked object in the array */
 static int drm_exec_obj_locked(struct drm_exec *exec,
 			       struct drm_gem_object *obj,
@@ -161,6 +192,14 @@ static int drm_exec_obj_locked(struct drm_exec *exec,
 	drm_gem_object_get(obj);
 	exec->objects[exec->num_objects++] = obj;
 
+	/*
+	 * Errors here are not fatal, It means the object we locked
+	 * for eviction can't be locked again. If that is problematic
+	 * we may need to reconsider this.
+	 */
+	if (drm_exec_in_evict_mode(exec))
+		(void)xa_store(&exec->resv_set, drm_exec_resv_to_key(obj->resv),
+			       obj->resv, gfp | __GFP_NOWARN);
 	return 0;
 }
 
@@ -184,6 +223,9 @@ static int drm_exec_lock_contended(struct drm_exec *exec)
 		dma_resv_lock_slow(obj->resv, &exec->ticket);
 	}
 
+	if (exec->drop_contended)
+		goto error_unlock;
+
 	ret = drm_exec_obj_locked(exec, obj, GFP_KERNEL);
 	if (unlikely(ret))
 		goto error_unlock;
@@ -245,10 +287,19 @@ int drm_exec_trylock_obj(struct drm_exec *exec, struct drm_gem_object *obj)
 	}
 
 	if (!dma_resv_trylock_ctx(obj->resv, &exec->ticket)) {
-		if (dma_resv_locking_ctx(obj->resv) == &exec->ticket)
-			return (exec->flags & DRM_EXEC_IGNORE_DUPLICATES) ? 0 : -EALREADY;
-		else
+		if (dma_resv_locking_ctx(obj->resv) == &exec->ticket) {
+			unsigned long key = drm_exec_resv_to_key(obj->resv);
+
+			if (exec->flags & DRM_EXEC_IGNORE_DUPLICATES ||
+			    xa_load(&exec->resv_set, key)) {
+				if (!drm_exec_in_evict_mode(exec))
+					drm_exec_resv_set_erase(exec, key);
+				return 0;
+			}
+			return -EALREADY;
+		} else {
 			return -EBUSY;
+		}
 	}
 
 	ret = drm_exec_obj_locked(exec, obj, GFP_ATOMIC | __GFP_NOWARN);
@@ -288,12 +339,20 @@ int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj)
 	if (unlikely(ret == -EDEADLK)) {
 		drm_gem_object_get(obj);
 		exec->contended = obj;
+		exec->drop_contended = drm_exec_in_evict_mode(exec);
 		return -EDEADLK;
 	}
 
-	if (unlikely(ret == -EALREADY) &&
-	    exec->flags & DRM_EXEC_IGNORE_DUPLICATES)
-		return 0;
+	if (unlikely(ret == -EALREADY)) {
+		unsigned long key = drm_exec_resv_to_key(obj->resv);
+
+		if (exec->flags & DRM_EXEC_IGNORE_DUPLICATES ||
+		    xa_load(&exec->resv_set, key)) {
+			if (!drm_exec_in_evict_mode(exec))
+				drm_exec_resv_set_erase(exec, key);
+			return 0;
+		}
+	}
 
 	if (unlikely(ret))
 		return ret;
@@ -324,6 +383,7 @@ void drm_exec_unlock_obj(struct drm_exec *exec, struct drm_gem_object *obj)
 
 	for (i = exec->num_objects; i--;) {
 		if (exec->objects[i] == obj) {
+			drm_exec_resv_set_erase(exec, drm_exec_resv_to_key(obj->resv));
 			dma_resv_unlock(obj->resv);
 			for (++i; i < exec->num_objects; ++i)
 				exec->objects[i - 1] = exec->objects[i];
@@ -415,12 +475,14 @@ void drm_exec_restore(struct drm_exec *exec, struct drm_exec_snapshot *snap)
 		if (index + 1 == snap->num_locked)
 			break;
 
+		xa_erase(&exec->resv_set, drm_exec_resv_to_key(obj->resv));
 		dma_resv_unlock(obj->resv);
 		drm_gem_object_put(obj);
 		exec->objects[index] = NULL;
 	}
 
 	exec->num_objects = snap->num_locked;
+	exec->flags = snap->flags;
 
 	if (!exec->prelocked)
 		exec->prelocked = snap->prelocked;
@@ -443,8 +505,9 @@ void drm_exec_snapshot(struct drm_exec *exec, struct drm_exec_snapshot *snap)
 	snap->prelocked = exec->prelocked;
 	if (snap->prelocked)
 		drm_gem_object_get(snap->prelocked);
+	snap->flags = exec->flags;
 	snap->saved_snap = exec->snap;
-	exec->snap = snap;
+	drm_exec_set_evict_mode(exec, snap);
 }
 EXPORT_SYMBOL(drm_exec_snapshot);
 
diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
index 0ce4d749511b..0b6d5ac0c092 100644
--- a/include/drm/drm_exec.h
+++ b/include/drm/drm_exec.h
@@ -5,6 +5,7 @@
 
 #include <linux/compiler.h>
 #include <linux/ww_mutex.h>
+#include <linux/xarray.h>
 
 #define DRM_EXEC_INTERRUPTIBLE_WAIT	BIT(0)
 #define DRM_EXEC_IGNORE_DUPLICATES	BIT(1)
@@ -53,6 +54,17 @@ struct drm_exec {
 	 * @snap: Pointer to the last snapshot taken or NULL if none.
 	 */
 	struct drm_exec_snapshot *snap;
+
+	/**
+	 * @resv_set: Set of pointers to locked objects in evict mode.
+	 */
+	struct xarray resv_set;
+
+	/**
+	 * @drop_contended: Drop the contended object after WW transaction
+	 * relaxation.
+	 */
+	bool drop_contended;
 };
 
 /**
@@ -67,6 +79,9 @@ struct drm_exec_snapshot {
 
 	/** @num_locked: Number of locked objects at snapshot time. */
 	unsigned long num_locked;
+
+	/** @flags: The drm_exec flags at snapshot time. */
+	u32 flags;
 };
 
 int drm_exec_handle_contended(struct drm_exec *exec);
-- 
2.44.0


  parent reply	other threads:[~2024-05-21  7:18 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-21  7:16 [PATCH v3 00/21] TTM shrinker helpers and xe buffer object shrinker Thomas Hellström
2024-05-21  7:16 ` [PATCH v3 01/21] drm/ttm: Allow TTM LRU list nodes of different types Thomas Hellström
2024-05-21 13:12   ` Matthew Brost
2024-05-28  9:16   ` Christian König
2024-05-21  7:16 ` [PATCH v3 02/21] drm/ttm: Slightly clean up LRU list iteration Thomas Hellström
2024-05-21 15:39   ` Matthew Brost
2024-05-28  9:19   ` Christian König
2024-05-21  7:16 ` [PATCH v3 03/21] drm/ttm: Use LRU hitches Thomas Hellström
2024-05-21 16:09   ` Matthew Brost
2024-05-21  7:16 ` [PATCH v3 04/21] drm/ttm, drm/amdgpu, drm/xe: Consider hitch moves within bulk sublist moves Thomas Hellström
2024-05-21  7:16 ` [PATCH v3 05/21] drm/ttm: Provide a generic LRU walker helper Thomas Hellström
2024-05-21  7:16 ` [PATCH v3 06/21] drm/ttm: Use the LRU walker helper for swapping Thomas Hellström
2024-05-21  7:16 ` [PATCH v3 07/21] drm/ttm: Use the LRU walker for eviction Thomas Hellström
2024-05-21  7:16 ` [PATCH v3 08/21] drm/ttm: Add a virtual base class for graphics memory backup Thomas Hellström
2024-05-21  7:16 ` [PATCH v3 09/21] drm/ttm/pool: Provide a helper to shrink pages Thomas Hellström
2024-05-21  7:16 ` [PATCH v3 10/21] drm/ttm: Use fault-injection to test error paths Thomas Hellström
2024-05-21  7:16 ` [PATCH v3 11/21] drm/ttm, drm/xe: Add a shrinker for xe bos Thomas Hellström
2024-05-21  7:16 ` [RFC PATCH v3 12/21] dma-buf/dma-resv: Introduce dma_resv_trylock_ctx() Thomas Hellström
2024-05-21  7:16 ` [RFC PATCH v3 13/21] drm/exec: Rework contended locking Thomas Hellström
2024-05-22  5:52   ` Christian König
2024-05-22 14:32     ` Thomas Hellström
2024-05-22 16:52       ` Christian König
2024-05-22 17:42         ` Thomas Hellström
2024-05-28  6:36           ` Thomas Hellström
2024-05-28  6:51             ` Christian König
2024-05-28  8:07               ` Thomas Hellström
2024-05-28 11:03                 ` Christian König
2024-05-29  7:18                   ` Thomas Hellström
2024-05-21  7:16 ` [RFC PATCH v3 14/21] drm/exec: Introduce a drm_exec_trylock_obj() function Thomas Hellström
2024-05-21  7:16 ` [RFC PATCH v3 15/21] drm/exec: Add a snapshot capability Thomas Hellström
2024-05-22 11:27   ` Christian König
2024-05-22 13:54     ` Thomas Hellström
2024-05-22 14:41       ` Thomas Hellström
2024-05-21  7:16 ` Thomas Hellström [this message]
2024-05-22 13:28   ` [RFC PATCH v3 16/21] drm/exec: Introduce an evict mode Christian König
2024-05-22 13:44     ` Thomas Hellström
2024-05-21  7:16 ` [RFC PATCH v3 17/21] drm/ttm: Support drm_exec locking for eviction and swapping Thomas Hellström
2024-05-21  7:16 ` [RFC PATCH v3 18/21] drm/ttm: Convert ttm vm to using drm_exec Thomas Hellström
2024-05-21  7:16 ` [RFC PATCH v3 19/21] drm/xe: Use drm_exec for fault locking Thomas Hellström
2024-05-21  7:16 ` [RFC PATCH v3 20/21] drm/ttm: Use drm_exec_trylock for bo initialization Thomas Hellström
2024-05-21  7:16 ` [RFC PATCH v3 21/21] drm/xe: Initial support for drm exec locking during validate Thomas Hellström
2024-05-21  7:22 ` [PATCH v3 00/21] TTM shrinker helpers and xe buffer object shrinker Thomas Hellström
2024-05-21  7:23 ` ✓ CI.Patch_applied: success for TTM shrinker helpers and xe buffer object shrinker (rev3) Patchwork
2024-05-21  7:24 ` ✗ CI.checkpatch: warning " Patchwork
2024-05-21  7:25 ` ✓ CI.KUnit: success " Patchwork
2024-05-21  7:37 ` ✓ CI.Build: " Patchwork
2024-05-21  7:39 ` ✗ CI.Hooks: failure " Patchwork
2024-05-21  7:41 ` ✗ CI.checksparse: warning " Patchwork
2024-05-21  8:03 ` ✓ CI.BAT: success " Patchwork
2024-05-21  9:09 ` ✗ CI.FULL: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240521071639.77614-17-thomas.hellstrom@linux.intel.com \
    --to=thomas.hellstrom@linux.intel.com \
    --cc=Amaranath.Somalapuram@amd.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.brost@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox