drm/i915: Don't disable interrupts for intel_engine_breadcrumbs_irq()

The function intel_engine_breadcrumbs_irq() is always invoked from an interrupt
handler and for that reason it invokes (as an optimisation) only spin_lock()
for locking assuming that the interrupts are already disabled. The
function intel_engine_signal_breadcrumbs() is provided to disable
interrupts while the former function is invoked so that assumption is
also true for callers from preemptible context.

On PREEMPT_RT local_irq_disable() really disables interrupts and this
forbids to invoke spin_lock() which becomes a sleeping spinlock.

This is also problematic with `threadirqs' in conjunction with
irq_work. With force threading the interrupt handler, the handler is
invoked with disabled BH but with interrupts enabled. This is okay and
the lock itself is never acquired in IRQ context. This changes with
irq_work (signal_irq_work()) which _still_ invokes
intel_engine_breadcrumbs_irq() from IRQ context. Lockdep should see this
and complain.

Acquire the locks in intel_engine_breadcrumbs_irq() with _irqsave()
suffix and let all callers invoke intel_engine_breadcrumbs_irq()
directly instead using intel_engine_signal_breadcrumbs().

Reported-by: Clark Williams <williams@redhat.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190926105644.16703-2-bigeasy@linutronix.de

diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index 0c6a1b11e25941862403e1a52ebc6fdadf1c16e5..55317081d48b0aaf8ff4aeba36d0a53515126678 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -133,9 +133,10 @@ void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine)
        const ktime_t timestamp = ktime_get();
        struct intel_context *ce, *cn;
        struct list_head *pos, *next;
+       unsigned long flags;
        LIST_HEAD(signal);
 
-       spin_lock(&b->irq_lock);
+       spin_lock_irqsave(&b->irq_lock, flags);
 
        if (b->irq_armed && list_empty(&b->signalers))
                __intel_breadcrumbs_disarm_irq(b);
@@ -181,30 +182,23 @@ void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine)
                }
        }
 
-       spin_unlock(&b->irq_lock);
+       spin_unlock_irqrestore(&b->irq_lock, flags);
 
        list_for_each_safe(pos, next, &signal) {
                struct i915_request *rq =
                        list_entry(pos, typeof(*rq), signal_link);
                struct list_head cb_list;
 
-               spin_lock(&rq->lock);
+               spin_lock_irqsave(&rq->lock, flags);
                list_replace(&rq->fence.cb_list, &cb_list);
                __dma_fence_signal__timestamp(&rq->fence, timestamp);
                __dma_fence_signal__notify(&rq->fence, &cb_list);
-               spin_unlock(&rq->lock);
+               spin_unlock_irqrestore(&rq->lock, flags);
 
                i915_request_put(rq);
        }
 }
 
-void intel_engine_signal_breadcrumbs(struct intel_engine_cs *engine)
-{
-       local_irq_disable();
-       intel_engine_breadcrumbs_irq(engine);
-       local_irq_enable();
-}
-
 static void signal_irq_work(struct irq_work *work)
 {
        struct intel_engine_cs *engine =

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index d3c6993f4f46f8c71f89d14759ded4f10ca23e11..c9e8c8ccbd47f354d4eb699a056aaec59bb1e264 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -335,7 +335,6 @@ void intel_engine_init_execlists(struct intel_engine_cs *engine);
 void intel_engine_init_breadcrumbs(struct intel_engine_cs *engine);
 void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine);
 
-void intel_engine_signal_breadcrumbs(struct intel_engine_cs *engine);
 void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine);
 
 static inline void

diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
index 40f62f780be5ce9b737d3f2200eb1ea344ede3cf..9814b18b32adccbc0e66bbb2fb8c9ba19cd368af 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
@@ -284,7 +284,7 @@ static void hangcheck_elapsed(struct work_struct *work)
        for_each_engine(engine, gt->i915, id) {
                struct hangcheck hc;
 
-               intel_engine_signal_breadcrumbs(engine);
+               intel_engine_breadcrumbs_irq(engine);
 
                hangcheck_load_sample(engine, &hc);
                hangcheck_accumulate_sample(engine, &hc);

diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index ec978b4d1be34ed7789360f20c2505b1576c205a..d08226f5bea59c49fc0ff2a16d2ab391444d230e 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -711,7 +711,7 @@ static void reset_finish_engine(struct intel_engine_cs *engine)
        engine->reset.finish(engine);
        intel_uncore_forcewake_put(engine->uncore, FORCEWAKE_ALL);
 
-       intel_engine_signal_breadcrumbs(engine);
+       intel_engine_breadcrumbs_irq(engine);
 }
 
 static void reset_finish(struct intel_gt *gt, intel_engine_mask_t awake)