| Message ID | 20200122105319.451215-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/execlists: Hold reference while on pqueue | expand |
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
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);
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(+)