We do not hold a reference to rq->engine, and so if it is a virtual engine it may have already been freed by the time we free the request. The last reference we hold on the virtual engine is via rq->context, and that is released on request retirement. So if we find ourselves retiring a virtual request, redirect it to a real sibling. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1906 Fixes: 43acd6516ca9 ("drm/i915: Keep a per-engine request pool") Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> --- drivers/gpu/drm/i915/i915_request.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 31ef683d27b4..a816218cc693 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -242,9 +242,26 @@ static void remove_from_engine(struct i915_request *rq) spin_lock(&engine->active.lock); locked = engine; } + list_del_init(&rq->sched.link); clear_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); clear_bit(I915_FENCE_FLAG_HOLD, &rq->fence.flags); + + /* + * During i915_fence_release we stash one request on the + * rq->engine for use as an emergency reserve. However, we + * neither want to keep a request on a virtual engine, nor do + * we hold a reference to a virtual engine at that point. So + * if rq->engine is virtual, replace it with a real one. Which + * one is immaterial at this point as the request has been + * retired, and if it was a virtual engine will not have any + * signaling or other related paraphernalia. + * + * However, it would be nice if we didn't have to... + */ + if (intel_engine_is_virtual(rq->engine)) + rq->engine = intel_virtual_engine_get_sibling(rq->engine, 0); + spin_unlock_irq(&locked->active.lock); } -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
