[Intel-gfx] [RFC PATCH 073/162] drm/i915: Reference contending lock objects

Matthew Auld matthew.auld at intel.com
Fri Nov 27 12:05:49 UTC 2020


From: Thomas Hellström <thomas.hellstrom at intel.com>

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 <thomas.hellstrom at intel.com>
Cc: Matthew Auld <matthew.auld at intel.com>
---
 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;
-- 
2.26.2



More information about the Intel-gfx mailing list