On Tue, Jun 16, 2015 at 12:03:45PM +0100, Dave Gordon wrote: > On 15/06/15 21:41, Chris Wilson wrote: > > On Mon, Jun 15, 2015 at 07:11:37PM +0100, Dave Gordon wrote: > >>> It still applies. If you submit say 1024 interrupted execbuffers they > >> > >> What is an interrupted execbuffer? AFAICT we hold the struct_mutex while > >> stuffing the ringbuffer so we can only ever be in the process of adding > >> instructions to one ringbuffer at a time, and we don't (now) interleave > >> any flip commands (execlists mode requires mmio flip). Is there still > >> something that just adds random stuff to someone else's OLR? > > > > Write commands to stream, fail to add the request because the wait for > > ring space fails due to a pending signals. Repeat until the entire ring > > is filled by a single pending request. > > -Chris > > Uuuurgh ... I always said there was something wrong with leaving partial > command streams in the ringbuffer :( > > In the Android version we make sure that can't happen by checking that > there's going to be enough space for all the commands generated by the > request before we start writing into the ringbuffer, so no ring_begin() > call thereafter can ever have to wait for more space. Yeah that's the guarantee that the olr removal series should finally put into place by pre-reserving sufficient amounts of ringspace to guarantee we can put the execbuf in fully or not at all. > Of course, having a global lock doesn't help. With multiple ringbuffers > per engine, one should really only need a per-ringbuffer lock for > writing into the ringbuffer ... and we should reset the tail pointer if > writing the request is interrupted or aborted. Maybe someone should look at per-buffer locks before trying to split up the low-level hw locks ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
