[Intel-gfx] [PATCH v2] drm/i915: recheck ringspace after wait+retire

Dave Gordon david.s.gordon at intel.com
Thu Dec 18 10:09:18 PST 2014


On 18/12/14 17:18, Chris Wilson wrote:
> On Thu, Dec 18, 2014 at 05:03:41PM +0000, Dave Gordon wrote:
>> In {intel,logical}_ring_wait_request(), we try to find a request
>> whose completion will release the amount of ring space required.
>> If we find such a request, we wait for it, and then retire it, in
>> the expectation that we will now have at least the required amount
>> of free space in the ring. But it's good to check that this has
>> actually happened, so we can back out with a meaningful error
>> code if something unexpected has happened, such as wait_request
>> returning early.
> 
> It's pretty pointless. It's a programming bug, if anything it should
> WARN if it happens.
> -Chris

I don't regard preventing the propagation of errors as pointless,
whether or not it's a programming bug. If this case arose, then without
this check we would go ahead and trample over whatever was next in the
ring. Quite likely TAIL would overtake HEAD, and then all sorts of
bizarre things would happen some time later.

In particular, this may be able to catch bugs introduced by /future
features/, such as TDR, scheduling, and preemption, all of which are in
preparation now and all of which have the potential to disrupt the
predictable flow of requests. So it's good to be resilient to upcoming
changes and make it as easy as possible to catch bugs at the earliest
opportunity.

This is also a step towards the deduplication of the ringbuffer and LRC
paths by making sure that they are as similar as possible, and thus
simple to merge at some later opportunity.

.Dave.


More information about the Intel-gfx mailing list