[Intel-gfx] [PATCH v2] drm/i915: Avoid lock dropping between rescheduling

[Chris Wilson]

On Wed, Mar 29, 2017 at 10:33:47AM +0100, Tvrtko Ursulin wrote:
> 
> On 27/03/2017 21:21, Chris Wilson wrote:
> >Unlocking is dangerous. In this case we combine an early update to the
> >out-of-queue request, because we know that it will be inserted into the
> >correct FIFO priority-ordered slot when it becomes ready in the future.
> >However, given sufficient enthusiasm, it may become ready as we are
> >continuing to reschedule, and so may gazump the FIFO if we have since
> >dropped its spinlock. The result is that it may be executed too early,
> >before its dependencies.
> >
> >v2: Move all work into the second phase over the topological sort. This
> >removes the shortcut on the out-of-rbtree request to ensure that we only
> >adjust its priority after adjusting all of its dependencies.
> >
> >Fixes: 20311bd35060 ("drm/i915/scheduler: Execute requests in order of priorities")
> >Testcase: igt/gem_exec_whisper
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >Cc: <stable at vger.kernel.org> # v4.10+
> >---
> > drivers/gpu/drm/i915/intel_lrc.c | 44 ++++++++++++++++++----------------------
> > 1 file changed, 20 insertions(+), 24 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >index b0c3a029b592..91e38e80a095 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -665,8 +665,8 @@ pt_lock_engine(struct i915_priotree *pt, struct intel_engine_cs *locked)
> > 			      priotree)->engine;
> > 	if (engine != locked) {
> > 		if (locked)
> 
> Could replace "if (locked)" with a GEM_BUG_ON(!locked) now.
> 
> >-			spin_unlock_irq(&locked->timeline->lock);
> >-		spin_lock_irq(&engine->timeline->lock);
> >+			spin_unlock(&locked->timeline->lock);
> >+		spin_lock(&engine->timeline->lock);
> > 	}
> >
> > 	return engine;

[snip]

> Looks OK to me. Preferably with the tidy in pt_lock_engine:
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Pushed with the suggested improvement, thanks.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre