[Intel-gfx] [PATCH] drm/i915/execlists: Hold reference while on pqueue

[Chris Wilson]

Quoting Chris Wilson (2020-01-22 10:53:19)
> Since the introduction of preempt-to-busy, we leave the request on the
> HW as we process the preemption request. This means that the request may
> complete while it is on the submission queue, and once completed it may
> be retired. We assumed that a single reference for the construction to
> retirement lifetime would suffice to keep the request alive while it is
> on the hardware, but with preempt-to-busy that is no longer the case and
> we need to explicitly hold the reference while it is being managed by
> execlists.
> 
> Reported-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/997
> Fixes: 22b7a426bbe1 ("drm/i915/execlists: Preempt-to-busy")

Scratch this...

> References: b647c7df01b7 ("drm/i915: Fixup preempt-to-busy vs resubmission of a virtual request")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 3a30767ff0c4..f47f55228fee 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -984,6 +984,7 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
>                         }
>                         GEM_BUG_ON(RB_EMPTY_ROOT(&engine->execlists.queue.rb_root));
>  
> +                       i915_request_get(rq);
>                         list_move(&rq->sched.link, pl);
>                         set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
>  
> @@ -2066,6 +2067,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>                                 submit = true;
>                                 last = rq;
>                         }
> +                       i915_request_put(rq);
>                 }
>  
>                 rb_erase_cached(&p->node, &execlists->queue);
> @@ -2735,6 +2737,8 @@ static void execlists_submit_request(struct i915_request *request)
>         struct intel_engine_cs *engine = request->engine;
>         unsigned long flags;
>  
> +       i915_request_get(request); /* hold a reference for the pqueue */
> +
>         /* Will be called from irq-context when using foreign fences. */
>         spin_lock_irqsave(&engine->active.lock, flags);

It's not quite so simple. In fact, as we remove the link during
completion we are reference safe. Ok, the problem is elsewhere...
-Chris