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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Thomas Hellström <thomas.hellstrom@xxxxxxxxx>

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@xxxxxxxxx>
Cc: Matthew Auld <matthew.auld@xxxxxxxxx>
---
 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

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux