[Intel-gfx] [PATCH] drm/i915: Prevent recursion by retiring requests when the ring is full

[Chris Wilson]

On Tue, Jan 28, 2014 at 09:11:17AM +0100, Daniel Vetter wrote:
> On Mon, Jan 27, 2014 at 10:43:07PM +0000, Chris Wilson wrote:
> > As the VM do not track activity of objects and instead use a large
> > hammer to forcibly idle and evict all of their associated objects when
> > one is released, it is possible for that to cause a recursion when we
> > need to wait for free space on a ring and call retire requests.
> > (intel_ring_begin -> intel_ring_wait_request ->
> > i915_gem_retire_requests_ring -> i915_gem_context_free ->
> > i915_gem_evict_vm -> i915_gpu_idle -> intel_ring_begin etc)
> 
> Is there no way to get rid of the gpu_idle? Imo doing a gpu idle as part
> of the cleanup of a retired request is just not great and has a good
> chance to trip us up all over the place since it's _really_ unexpected.

Yes, that gpu-idle is only an artifact of lacking activity counting on
the vm.
 
> So if we can do it (and iirc we've agreed that all the ppgtt cleanup is
> mostly fluff) then I prefer we remove the root-cause instead of just
> chopping off another head of the hydra.

Look again, I think this is a welcome simplicity to that piece of code.
We lose the optimisation of having the most accurate ring->space
following the wait in exchange for having a straightforward wait&update.
Plus retiring_requests at random points of time has turned out to be a
bad idea - wait long enough and I can probably find several other code
paths that could explode here.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre