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

Mika Kuoppala
2018-03-09T13:22:43Z

Chris Wilson <chris@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@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@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?

-Mika

> list_for_each_entry(target, &ring->request_list, ring_link) {
> /* Would completion of this request free enough space? */
> if (bytes <= __intel_ring_space(target->postfix,
> --
> 2.16.2