[Intel-gfx] [PATCH 25/29] drm/i915: Convert execbuf code to use vmas

[Daniel Vetter]

On Tue, Aug 06, 2013 at 10:43:08PM +0200, Daniel Vetter wrote:
> On Wed, Jul 31, 2013 at 05:00:18PM -0700, Ben Widawsky wrote:
> > This attempts to convert all the execbuf code to speak in vmas. Since
> > the execbuf code is very self contained it was a nice isolated
> > conversion.
> > 
> > The meat of the code is about turning eb_objects into eb_vma, and then
> > wiring up the rest of the code to use vmas instead of obj, vm pairs.
> > 
> > Unfortunately, to do this, we must move the exec_list link from the obj
> > structure. This list is reused in the eviction code, so we must also
> > modify the eviction code to make this work.
> > 
> > v2: Release table lock early, and two a 2 phase vma lookup to avoid
> > having to use a GFP_ATOMIC. (Chris)
> > 
> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> 
> I think the leaking of preallocated vmas if execbuf fails can blow up:
> 1. We call lookup_or_create and create new vmas, linked into the vma_link
> chain.
> 2. Later on execbuf fails somewhere (for an igt the simplest way is
> probaly to use more buffers than what would fit into the gtt) and we bail
> out.
> -> Note that at this point we leak vmas which are on the vma_link list but
> which have no gtt node allocation.
> 3. Userspace dies in flames (or just quits).
> 4. All buffers get their final unref and we call vma_unbind on each vma,
> even the ones that do not have an allocation.
> 5. hilarity ensues since vma_unbind doesn't bail out if
> drm_mm_node_allocated(vma->node) == false.
> 
> We need broken userspace to actually exercise this bug since all normal
> ways for execbuf to bail out involve singals and ioclt restarting. If this
> is a real bug I think we need an igt to exercise it.

Forgotten to mention: Beside that error path the patch looks good. Same
applies to patches 21-24 before.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch