[Intel-gfx] [PATCH 2/6] drm/i915: Reset ring space estimate after unwinding the request

[Chris Wilson]

Quoting Mika Kuoppala (2018-03-09 13:17:09)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > With a series of unusual events (a sequence of interrupted request
> > allocations), we could gradually leak the ring->space estimate by
> > unwinding the ring back to the start of the request, but not return the
> > used space back to the ring. Eventually and with great misfortune, it
> > would be possible to trigger ring->space exhaustion with no requests on
> > the ring.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_request.c     | 1 +
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> > index d437beac3969..efa9ee557f31 100644
> > --- a/drivers/gpu/drm/i915/i915_request.c
> > +++ b/drivers/gpu/drm/i915/i915_request.c
> > @@ -798,6 +798,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx)
> >  
> >  err_unwind:
> >       rq->ring->emit = rq->head;
> > +     intel_ring_update_space(rq->ring);
> >  
> >       /* Make sure we didn't add ourselves to external state before freeing */
> >       GEM_BUG_ON(!list_empty(&rq->active_list));
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 1d599524a759..88eeb64041ae 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -1593,6 +1593,7 @@ static noinline int wait_for_space(struct intel_ring *ring, unsigned int bytes)
> >       if (intel_ring_update_space(ring) >= bytes)
> >               return 0;
> >  
> > +     GEM_BUG_ON(list_empty(&ring->request_list));
> 
> At some point, after long series of misfortunate events, we
> will add a first request and need actually wait a space for this
> and then we kaboom in here?

That's the experience. However, I don't think this patch helps because
we do the intel_ring_update_space() here inside wait_for_space anyway,
so it should come out in the wash so long as we aren't corrupting
the ring space calculation.
-Chris