[Intel-gfx] [PATCH 11/12] drm/i915: Introduce dedicated safe object VMA iterator

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Tue Feb 2 11:06:29 UTC 2016


From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Purpose is to catch places which iterate the object VMA list
without holding the big lock.

Implemented by open coding list_for_each_entry_safe to make
the macro compatible with existing call sites.

v2: Replace WARN_ON with lockdep_assert_held. (Chris Wilson, Daniel Vetter)
v3: Moved under dedicated CONFIG_DRM_I915_DEBUG and back to WARN_ON.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h          | 10 ++++++++++
 drivers/gpu/drm/i915/i915_gem.c          |  6 +++---
 drivers/gpu/drm/i915/i915_gem_shrinker.c |  3 +--
 drivers/gpu/drm/i915/i915_gem_userptr.c  |  2 +-
 4 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 05ef750386df..4ad025210416 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2867,9 +2867,19 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
 	     vma = list_first_entry(&(obj)->vma_list, typeof(*vma), vma_link);\
 	     &vma->vma_link != (&(obj)->vma_list); \
 	     vma = list_next_entry(vma, vma_link))
+
+  #define i915_gem_obj_for_each_vma_safe(vma, next, obj) \
+	for (WARN_ON_ONCE(!mutex_is_locked(&(obj)->base.dev->struct_mutex)), \
+	     vma = list_first_entry(&(obj)->vma_list, typeof(*vma), vma_link), \
+	     next = list_next_entry(vma, vma_link); \
+	     &vma->vma_link != (&(obj)->vma_list); \
+	     vma = next, next = list_next_entry(next, vma_link))
 #else
   #define i915_gem_obj_for_each_vma(vma, obj) \
 		list_for_each_entry((vma), &(obj)->vma_list, vma_link)
+
+  #define i915_gem_obj_for_each_vma_safe(vma, next, obj) \
+	    list_for_each_entry_safe((vma), (next), &(obj)->vma_list, vma_link)
 #endif
 
 /* Flags used by pin/bind&friends. */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ce9d0544b42c..d75062d7aa6a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -272,7 +272,7 @@ drop_pages(struct drm_i915_gem_object *obj)
 	int ret;
 
 	drm_gem_object_reference(&obj->base);
-	list_for_each_entry_safe(vma, next, &obj->vma_list, vma_link)
+	i915_gem_obj_for_each_vma_safe(vma, next, obj)
 		if (i915_vma_unbind(vma))
 			break;
 
@@ -3810,7 +3810,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 	 * catch the issue of the CS prefetch crossing page boundaries and
 	 * reading an invalid PTE on older architectures.
 	 */
-	list_for_each_entry_safe(vma, next, &obj->vma_list, vma_link) {
+	i915_gem_obj_for_each_vma_safe(vma, next, obj) {
 		if (!drm_mm_node_allocated(&vma->node))
 			continue;
 
@@ -4556,7 +4556,7 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
 
 	trace_i915_gem_object_destroy(obj);
 
-	list_for_each_entry_safe(vma, next, &obj->vma_list, vma_link) {
+	i915_gem_obj_for_each_vma_safe(vma, next, obj) {
 		int ret;
 
 		vma->pin_count = 0;
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 4c89bd45dbde..ae13a9f636fa 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -175,8 +175,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
 			drm_gem_object_reference(&obj->base);
 
 			/* For the unbound phase, this should be a no-op! */
-			list_for_each_entry_safe(vma, v,
-						 &obj->vma_list, vma_link)
+			i915_gem_obj_for_each_vma_safe(vma, v, obj)
 				if (i915_vma_unbind(vma))
 					break;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 74a4d1714879..a2430b8c4aba 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -78,7 +78,7 @@ static void cancel_userptr(struct work_struct *work)
 		was_interruptible = dev_priv->mm.interruptible;
 		dev_priv->mm.interruptible = false;
 
-		list_for_each_entry_safe(vma, tmp, &obj->vma_list, vma_link) {
+		i915_gem_obj_for_each_vma_safe(vma, tmp, obj) {
 			int ret = i915_vma_unbind(vma);
 			WARN_ON(ret && ret != -EIO);
 		}
-- 
1.9.1



More information about the Intel-gfx mailing list