[Intel-gfx] [PATCH 03/55] drm/i915: i915_add_request must not fail

[Chris Wilson]

On Fri, May 29, 2015 at 05:43:24PM +0100, John.C.Harrison at Intel.com wrote:
> From: John Harrison <John.C.Harrison at Intel.com>
> 
> The i915_add_request() function is called to keep track of work that has been
> written to the ring buffer. It adds epilogue commands to track progress (seqno
> updates and such), moves the request structure onto the right list and other
> such house keeping tasks. However, the work itself has already been written to
> the ring and will get executed whether or not the add request call succeeds. So
> no matter what goes wrong, there isn't a whole lot of point in failing the call.
> 
> At the moment, this is fine(ish). If the add request does bail early on and not
> do the housekeeping, the request will still float around in the
> ring->outstanding_lazy_request field and be picked up next time. It means
> multiple pieces of work will be tagged as the same request and driver can't
> actually wait for the first piece of work until something else has been
> submitted. But it all sort of hangs together.
> 
> This patch series is all about removing the OLR and guaranteeing that each piece
> of work gets its own personal request. That means that there is no more
> 'hoovering up of forgotten requests'. If the request does not get tracked then
> it will be leaked. Thus the add request call _must_ not fail. The previous patch
> should have already ensured that it _will_ not fail by removing the potential
> for running out of ring space. This patch enforces the rule by actually removing
> the early exit paths and the return code.
> 
> Note that if something does manage to fail and the epilogue commands don't get
> written to the ring, the driver will still hang together. The request will be
> added to the tracking lists. And as in the old case, any subsequent work will
> generate a new seqno which will suffice for marking the old one as complete.
> 
> v2: Improved WARNings (Tomas Elf review request).

Nak. Daniel please revert this mess.

Even in the current code it has a failure mode it cannot handle.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre