[PATCH 1/1] Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire callbacks"

Kai-Heng Feng kai.heng.feng at canonical.com
Fri Nov 13 04:47:31 UTC 2020


BugLink: https://bugs.launchpad.net/bugs/1896091

This reverts commit c1484c993957fe6b366ab453fbb7b50c0d2916de.

Signed-off-by: Kai-Heng Feng <kai.heng.feng at canonical.com>
---
 drivers/gpu/drm/i915/i915_active.c       | 52 ++++--------------------
 drivers/gpu/drm/i915/i915_active.h       | 10 +++--
 drivers/gpu/drm/i915/i915_active_types.h |  2 -
 3 files changed, 14 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 768713657e85..d960d0be5bd2 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -148,22 +148,8 @@ __active_retire(struct i915_active *ref)
 	spin_unlock_irqrestore(&ref->tree_lock, flags);
 
 	/* After the final retire, the entire struct may be freed */
-	if (ref->retire) {
-		if (ref->active) {
-			bool freed = false;
-
-			/* Don't race with the active callback, and avoid UaF */
-			down_write(&ref->rwsem);
-			ref->freed = &freed;
-			ref->retire(ref);
-			if (!freed) {
-				ref->freed = NULL;
-				up_write(&ref->rwsem);
-			}
-		} else {
-			ref->retire(ref);
-		}
-	}
+	if (ref->retire)
+		ref->retire(ref);
 
 	/* ... except if you wait on it, you must manage your own references! */
 	wake_up_var(ref);
@@ -293,8 +279,7 @@ void __i915_active_init(struct i915_active *ref,
 			int (*active)(struct i915_active *ref),
 			void (*retire)(struct i915_active *ref),
 			struct lock_class_key *mkey,
-			struct lock_class_key *wkey,
-			struct lock_class_key *rkey)
+			struct lock_class_key *wkey)
 {
 	unsigned long bits;
 
@@ -303,13 +288,8 @@ void __i915_active_init(struct i915_active *ref,
 	ref->flags = 0;
 	ref->active = active;
 	ref->retire = ptr_unpack_bits(retire, &bits, 2);
-	ref->freed = NULL;
-	if (ref->active && ref->retire) {
-		__init_rwsem(&ref->rwsem, "i915_active.rwsem", rkey);
+	if (bits & I915_ACTIVE_MAY_SLEEP)
 		ref->flags |= I915_ACTIVE_RETIRE_SLEEPS;
-	} else if (bits & I915_ACTIVE_MAY_SLEEP) {
-		ref->flags |= I915_ACTIVE_RETIRE_SLEEPS;
-	}
 
 	spin_lock_init(&ref->tree_lock);
 	ref->tree = RB_ROOT;
@@ -448,20 +428,8 @@ int i915_active_acquire(struct i915_active *ref)
 		return err;
 
 	if (likely(!i915_active_acquire_if_busy(ref))) {
-		if (ref->active) {
-			if (ref->retire) {
-				/*
-				 * This can be a recursive call, and the mutex
-				 * above already protects from concurrent active
-				 * callbacks, so a read lock fits best.
-				 */
-				down_read(&ref->rwsem);
-				err = ref->active(ref);
-				up_read(&ref->rwsem);
-			} else {
-				err = ref->active(ref);
-			}
-		}
+		if (ref->active)
+			err = ref->active(ref);
 		if (!err) {
 			spin_lock_irq(&ref->tree_lock); /* __active_retire() */
 			debug_active_activate(ref);
@@ -683,20 +651,16 @@ int i915_sw_fence_await_active(struct i915_sw_fence *fence,
 	return await_active(ref, flags, sw_await_fence, fence, fence);
 }
 
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
 void i915_active_fini(struct i915_active *ref)
 {
-	if (ref->freed) {
-		*ref->freed = true;
-		up_write(&ref->rwsem);
-	}
-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
 	debug_active_fini(ref);
 	GEM_BUG_ON(atomic_read(&ref->count));
 	GEM_BUG_ON(work_pending(&ref->work));
 	GEM_BUG_ON(!RB_EMPTY_ROOT(&ref->tree));
 	mutex_destroy(&ref->mutex);
-#endif
 }
+#endif
 
 static inline bool is_idle_barrier(struct active_node *node, u64 idx)
 {
diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
index a2e3742b1090..cf4058150966 100644
--- a/drivers/gpu/drm/i915/i915_active.h
+++ b/drivers/gpu/drm/i915/i915_active.h
@@ -153,16 +153,14 @@ void __i915_active_init(struct i915_active *ref,
 			int (*active)(struct i915_active *ref),
 			void (*retire)(struct i915_active *ref),
 			struct lock_class_key *mkey,
-			struct lock_class_key *wkey,
-			struct lock_class_key *rkey);
+			struct lock_class_key *wkey);
 
 /* Specialise each class of i915_active to avoid impossible lockdep cycles. */
 #define i915_active_init(ref, active, retire) do {		\
 	static struct lock_class_key __mkey;				\
 	static struct lock_class_key __wkey;				\
-	static struct lock_class_key __rkey;				\
 									\
-	__i915_active_init(ref, active, retire, &__mkey, &__wkey, &__rkey);	\
+	__i915_active_init(ref, active, retire, &__mkey, &__wkey);	\
 } while (0)
 
 int i915_active_ref(struct i915_active *ref,
@@ -215,7 +213,11 @@ i915_active_is_idle(const struct i915_active *ref)
 	return !atomic_read(&ref->count);
 }
 
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
 void i915_active_fini(struct i915_active *ref);
+#else
+static inline void i915_active_fini(struct i915_active *ref) { }
+#endif
 
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 					    struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
index aaee2548cb19..6360c3e4b765 100644
--- a/drivers/gpu/drm/i915/i915_active_types.h
+++ b/drivers/gpu/drm/i915/i915_active_types.h
@@ -32,8 +32,6 @@ struct active_node;
 struct i915_active {
 	atomic_t count;
 	struct mutex mutex;
-	struct rw_semaphore rwsem;
-	bool *freed;
 
 	spinlock_t tree_lock;
 	struct active_node *cache;
-- 
2.28.0




More information about the kernel-team mailing list