On Thu, 02 Jan 2014, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
> In very rare cases (such as a memory failure stress test) it is possible
> to fill the entire ring without emitting a request. Under this
> circumstance, the outstanding request is flushed and waited upon. After
> space on the ring is cleared, we return to emitting the new command -
> except that we just cleared the seqno allocated for this operation and
> trigger the sanity check that a request is only ever emitted with a
> valid seqno. The fix is to rearrange the code to make sure the
> allocation of the seqno for this operation is after any required flushes
> of outstanding operations.
>
> The bug exists since the preallocation was introduced in
> commit 9d7730914f4cd496e356acfab95b41075aa8eae8
> Author: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Date: Tue Nov 27 16:22:52 2012 +0000
>
> drm/i915: Preallocate next seqno before touching the ring
>
I can't claim to be an expert in the area, but the explanation and code
make sense.
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 5744841669e4..6bb914e49bf0 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1610,8 +1610,8 @@ intel_ring_alloc_seqno(struct intel_ring_buffer *ring)
> return i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno);
> }
>
> -static int __intel_ring_begin(struct intel_ring_buffer *ring,
> - int bytes)
> +static int __intel_ring_prepare(struct intel_ring_buffer *ring,
> + int bytes)
> {
> int ret;
>
> @@ -1627,7 +1627,6 @@ static int __intel_ring_begin(struct intel_ring_buffer *ring,
> return ret;
> }
>
> - ring->space -= bytes;
> return 0;
> }
>
> @@ -1642,12 +1641,17 @@ int intel_ring_begin(struct intel_ring_buffer *ring,
> if (ret)
> return ret;
>
> + ret = __intel_ring_prepare(ring, num_dwords * sizeof(uint32_t));
> + if (ret)
> + return ret;
> +
> /* Preallocate the olr before touching the ring */
> ret = intel_ring_alloc_seqno(ring);
> if (ret)
> return ret;
>
> - return __intel_ring_begin(ring, num_dwords * sizeof(uint32_t));
> + ring->space -= num_dwords * sizeof(uint32_t);
> + return 0;
> }
>
> void intel_ring_init_seqno(struct intel_ring_buffer *ring, u32 seqno)
> --
> 1.8.5.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html