Re: [PATCH 3/3] drm/i915: Remove the vma from the drm_mm if binding fails

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On la, 2017-02-25 at 23:25 +0000, Chris Wilson wrote:
> As we track whether a vma has been inserted into the drm_mm using the
> vma->flags, if we fail to bind the vma into the GTT we do not update
> those bits and will attempt to reinsert the vma into the drm_mm on
> future passes. To prevent that, we want to unwind i915_vma_insert() if
> we fail in our attempt to bind.
> 
> Fixes: 59bfa1248e22 ("drm/i915: Start passing around i915_vma from execbuffer")
> Testcase: igt/drv_selftest/live_gtt
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v4.9+

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

One note below.

> @@ -541,7 +566,12 @@ int __i915_vma_do_pin(struct i915_vma *vma,
>  	GEM_BUG_ON(i915_vma_misplaced(vma, size, alignment, flags));
>  	return 0;
>  
> -err:
> +err_remove:
> +	if ((vma->flags & I915_VMA_BIND_MASK) == 0) {

This condition could be more symmetric.

Regards, Joonas

> +		GEM_BUG_ON(vma->pages);
> +		i915_vma_remove(vma);
> +	}
> +err_unpin:
>  	__i915_vma_unpin(vma);
>  	return ret;
>  }

-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation