[Mesa-dev] [PATCH 3/4] i965: Prevent infinite finish_batch recursion

[Kenneth Graunke]

On Saturday, February 28, 2015 01:19:28 PM Ben Widawsky wrote:
> On Sat, Feb 28, 2015 at 12:31:59AM -0800, Kenneth Graunke wrote:
> > On Friday, February 27, 2015 10:22:10 AM Ben Widawsky wrote:
> > > From the comments in the code:
> > > 
> > >    Called from intel_batchbuffer_flush before emitting MI_BATCHBUFFER_END and
> > >    sending it off.
> > > 
> > > This fixes a possible, unlikely infinite recursion in our batch flush path. More
> > > importantly it allows me to add some code here.
> > > 
> > > The relevant part of the call chain for flush
> > > intel_batchbuffer_flush()->brw_finish_batch(). The problem arises if anything in
> > > the time from intel_batchbuffer_flush, until brw_finish_batch ends up calling
> > > itself. This can happen as a result of a call to
> > > intel_batchbuffer_begin()->intel_batchbuffer_require_space()->intel_batchbuffer_flush().
> > > 
> > > There are two possible cases today which can spawn this recursion.
> > > 1. There is a ring switch occurring (impossible, see below)
> > > 2. The ring is out of space (fairly unlikely)
> > 
> > NAK.  brw_finish_batch is for putting any last-minute commands in the
> > batchbuffer before sending it off.
> > 
> > Such commands must fit in the BATCH_RESERVED space (which is probably
> > already large enough since SNB has a zillion workarounds).  This
> > eliminates reason #2.
> > 
> > #1 means that you're trying to emit commands for the wrong ring, i.e.
> > trying to do RCS workarounds in a BLT batch.  This isn't what you want.
> 
> Sorry you wasted your time looking at this. I had fixed both of what you
> mentioned, but hadn't sent it out yet. It's fixed with a better commit
> message here:
> http://cgit.freedesktop.org/~bwidawsk/mesa/commit/?h=workarounds&id=f87c17e34eeac9d5e3f54c9b182090f8aede0284

I still don't understand the point - I see nothing in your code that
would trigger a batch flush from brw_finish_batch.  Any such code would
be most certainly wrong.  I think you can simply drop this and the rest
of your updated series will continue working just fine.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150228/f9a4fcb9/attachment.sig>