drm/i915/execlists: Hold reference while on pqueue
diff mbox series

Message ID 20200122105319.451215-1-chris@chris-wilson.co.uk
State New
Headers show
Series
  • drm/i915/execlists: Hold reference while on pqueue
Related show

Commit Message

Chris Wilson Jan. 22, 2020, 10:53 a.m. UTC
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@intel.com>
Closes: https://gitlab.freedesktop.org/drm/intel/issues/997
Fixes: 22b7a426bbe1 ("drm/i915/execlists: Preempt-to-busy")
References: b647c7df01b7 ("drm/i915: Fixup preempt-to-busy vs resubmission of a virtual request")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Chris Wilson Jan. 22, 2020, 11:03 a.m. UTC | #1
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@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@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@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

Patch
diff mbox series

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);