Revert "drm/i915: Skip execlists_dequeue() early if the list is empty"
authorChris Wilson <chris@chris-wilson.co.uk>
Wed, 29 Mar 2017 09:47:46 +0000 (10:47 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Wed, 29 Mar 2017 12:02:24 +0000 (13:02 +0100)
commit18afa288920475a9f95d70920b0c54b43b5256a7
treebae33192230e4538bb0641d02254835906cb3944
parenta79a524e9260d4ffaff88348615e70fb3d393692
Revert "drm/i915: Skip execlists_dequeue() early if the list is empty"

This reverts commit 6c943de6686f ("drm/i915: Skip execlists_dequeue()
early if the list is empty").

The validity of using READ_ONCE there depends upon having a mb to
coordinate the assignment of engine->execlist_first inside
submit_request() and checking prior to taking the spinlock in
execlists_dequeue(). We wrote "the update to TASKLET_SCHED incurs a
memory barrier making this cross-cpu checking safe", but failed to
notice that this mb was *conditional* on the execlists being ready, i.e.
there wasn't the required mb when it was most necessary!

We could install an unconditional memory barrier to fixup the
READ_ONCE():

diff --git a/drivers/gpu/drm/i915/intel_lrc.c
b/drivers/gpu/drm/i915/intel_lrc.c
index 7dd732cb9f57..1ed164b16d44 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -616,6 +616,7 @@ static void execlists_submit_request(struct
drm_i915_gem_request *request)

        if (insert_request(&request->priotree, &engine->execlist_queue))
{
                engine->execlist_first = &request->priotree.node;
+               smp_wmb();
                if (execlists_elsp_ready(engine))

But we have opted to remove the race as it should be rarely effective,
and saves us having to explain the necessary memory barriers which we
quite clearly failed at.

Reported-and-tested-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Fixes: 6c943de6686f ("drm/i915: Skip execlists_dequeue() early if the list is empty")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: MichaƂ Winiarski <michal.winiarski@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170329100052.29505-1-chris@chris-wilson.co.uk
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
drivers/gpu/drm/i915/i915_guc_submission.c
drivers/gpu/drm/i915/intel_lrc.c