[Intel-gfx] [PATCH] drm/i915: Fix ordering of unbind vs unpin pages

[Ben Widawsky]

On Mon, Dec 02, 2013 at 10:08:02AM +0000, Chris Wilson wrote:
> It is useful to assert that if the object is bound, then it must have
> its pages pinned to prevent the shrinker from reaping its backing store.
> This is even more useful with the introduction of real-ppgtt whereupon
> we may have the object bound into several vma, with each instance
> pinning the backing store. This assertion breaks down during unbind
> where we unpinned the backing store before decoupling the vma binding.
> This can be fixed with a trivial reording of the unbind sequence, which
> reinforces the
> 
>    pin pages
>    bind to vma
>    ...
>    unbind from vma
>    unpin pages
> 
> concept.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Ben Widawsky <ben at bwidawsk.net>

A comment in the code why the page unpinning happens last would be nice,
but not mandatory.

Reviewed-by: Ben Widawsky <ben at bwidawsk.net>
Tested-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6312d61f5198..5fef29a5d6af 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2755,7 +2755,6 @@ int i915_vma_unbind(struct i915_vma *vma)
>  	vma->unbind_vma(vma);
>  
>  	i915_gem_gtt_finish_object(obj);
> -	i915_gem_object_unpin_pages(obj);
>  
>  	list_del(&vma->mm_list);
>  	/* Avoid an unnecessary call to unbind on rebind. */
> @@ -2763,7 +2762,6 @@ int i915_vma_unbind(struct i915_vma *vma)
>  		obj->map_and_fenceable = true;
>  
>  	drm_mm_remove_node(&vma->node);
> -
>  	i915_gem_vma_destroy(vma);
>  
>  	/* Since the unbound list is global, only move to that list if
> @@ -2771,6 +2769,8 @@ int i915_vma_unbind(struct i915_vma *vma)
>  	if (list_empty(&obj->vma_list))
>  		list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list);
>  
> +	i915_gem_object_unpin_pages(obj);
> +
>  	return 0;
>  }
>  
> -- 
> 1.8.5
> 

-- 
Ben Widawsky, Intel Open Source Technology Center