diff mbox series

[RFC,073/162] drm/i915: Reference contending lock objects

Message ID 20201127120718.454037-74-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show
Series DG1 + LMEM enabling | expand

Commit Message

Matthew Auld Nov. 27, 2020, 12:05 p.m. UTC 
From: Thomas Hellström <thomas.hellstrom@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@intel.com>
Cc: Matthew Auld <matthew.auld@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 mbox series

Patch

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;