From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [Intel-gfx] [PATCH] drm/ivb: Flush outstanding requests before allocating new seqno Date: Tue, 07 Jan 2014 13:14:57 +0200 Message-ID: <87a9f8caby.fsf@intel.com> References: <1388673155-10235-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: In-Reply-To: <1388673155-10235-1-git-send-email-chris@chris-wilson.co.uk> Sender: stable-owner@vger.kernel.org To: Chris Wilson , intel-gfx@lists.freedesktop.org Cc: Daniel Vetter , Mika Kuoppala , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 02 Jan 2014, Chris Wilson 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 > 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 > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Cc: Daniel Vetter > Cc: stable@vger.kernel.org > Signed-off-by: Chris Wilson > --- > 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center