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

[John Harrison]

On 23/06/2015 11:16, Chris Wilson wrote:
> 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
>

Can you explain further?

Is this a new failure mode? This patch was originally posted back in 
March and there were no comments then to say that is was not the right 
direction to take. The discussion was that this might not be the 
prettiest way to do things but it is the best solution given where the 
driver is at and what we are trying to do.

John.