[CI,2/2] drm/i915: Don't disable interrupts independently of the lock
diff mbox series

Message ID 20191017154208.20128-2-chris@chris-wilson.co.uk
State New
Headers show
Series
  • [CI,1/2] drm/i915/execlists: Don't merely skip submission if maybe timeslicing
Related show

Commit Message

Chris Wilson Oct. 17, 2019, 3:42 p.m. UTC
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

The locks (active.lock and rq->lock) need to be taken with disabled
interrupts. This is done in i915_request_retire() by disabling the
interrupts independently of the locks itself.
While local_irq_disable()+spin_lock() equals spin_lock_irq() on vanilla
it does not on PREEMPT_RT.
Chris Wilson confirmed that local_irq_disable() was just introduced as
an optimisation to avoid enabling/disabling interrupts during
lock/unlock combo.

Enable/disable interrupts as part of the locking instruction.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_request.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index f1cadad4e81c..4575f368455d 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -206,14 +206,14 @@  static void remove_from_engine(struct i915_request *rq)
 	 * check that the rq still belongs to the newly locked engine.
 	 */
 	locked = READ_ONCE(rq->engine);
-	spin_lock(&locked->active.lock);
+	spin_lock_irq(&locked->active.lock);
 	while (unlikely(locked != (engine = READ_ONCE(rq->engine)))) {
 		spin_unlock(&locked->active.lock);
 		spin_lock(&engine->active.lock);
 		locked = engine;
 	}
 	list_del(&rq->sched.link);
-	spin_unlock(&locked->active.lock);
+	spin_unlock_irq(&locked->active.lock);
 }
 
 bool i915_request_retire(struct i915_request *rq)
@@ -242,8 +242,6 @@  bool i915_request_retire(struct i915_request *rq)
 				  &i915_request_timeline(rq)->requests));
 	rq->ring->head = rq->postfix;
 
-	local_irq_disable();
-
 	/*
 	 * We only loosely track inflight requests across preemption,
 	 * and so we may find ourselves attempting to retire a _completed_
@@ -252,7 +250,7 @@  bool i915_request_retire(struct i915_request *rq)
 	 */
 	remove_from_engine(rq);
 
-	spin_lock(&rq->lock);
+	spin_lock_irq(&rq->lock);
 	i915_request_mark_complete(rq);
 	if (!i915_request_signaled(rq))
 		dma_fence_signal_locked(&rq->fence);
@@ -267,9 +265,7 @@  bool i915_request_retire(struct i915_request *rq)
 		__notify_execute_cb(rq);
 	}
 	GEM_BUG_ON(!list_empty(&rq->execute_cb));
-	spin_unlock(&rq->lock);
-
-	local_irq_enable();
+	spin_unlock_irq(&rq->lock);
 
 	remove_from_client(rq);
 	list_del(&rq->link);