drm/i915: Wake up the bottom-half if we steal their interrupt
authorChris Wilson <chris@chris-wilson.co.uk>
Wed, 6 Jul 2016 11:39:01 +0000 (12:39 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Wed, 6 Jul 2016 11:47:31 +0000 (12:47 +0100)
Following on from the scenario Tvrtko envisioned to explain a hard-to-hit
race with multiple first waiters, we could also then race in the
__i915_request_irq_complete() and the bottom-half may miss the vital
irq-seqno barrier and so go to sleep not noticing their seqno is
complete.

v2: unlock, not double lock the rcu_read_lock.

Fixes: 3d5564e91025 ("drm/i915: Only apply one barrier after...")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467805142-22219-2-git-send-email-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/i915_drv.h

index c269e0ad4057711cdac6041f4dcb9ef9e06edc73..11e9769411e9db35e65348122c3213cf13f42063 100644 (file)
@@ -3998,7 +3998,10 @@ static inline bool __i915_request_irq_complete(struct drm_i915_gem_request *req)
         * is woken.
         */
        if (engine->irq_seqno_barrier &&
+           READ_ONCE(engine->breadcrumbs.tasklet) == current &&
            cmpxchg_relaxed(&engine->irq_posted, 1, 0)) {
+               struct task_struct *tsk;
+
                /* The ordering of irq_posted versus applying the barrier
                 * is crucial. The clearing of the current irq_posted must
                 * be visible before we perform the barrier operation,
@@ -4012,6 +4015,25 @@ static inline bool __i915_request_irq_complete(struct drm_i915_gem_request *req)
                 * seqno update.
                 */
                engine->irq_seqno_barrier(engine);
+
+               /* If we consume the irq, but we are no longer the bottom-half,
+                * the real bottom-half may not have serialised their own
+                * seqno check with the irq-barrier (i.e. may have inspected
+                * the seqno before we believe it coherent since they see
+                * irq_posted == false but we are still running).
+                */
+               rcu_read_lock();
+               tsk = READ_ONCE(engine->breadcrumbs.tasklet);
+               if (tsk && tsk != current)
+                       /* Note that if the bottom-half is changed as we
+                        * are sending the wake-up, the new bottom-half will
+                        * be woken by whomever made the change. We only have
+                        * to worry about when we steal the irq-posted for
+                        * ourself.
+                        */
+                       wake_up_process(tsk);
+               rcu_read_unlock();
+
                if (i915_gem_request_completed(req))
                        return true;
        }