Skip to content

Commit

Permalink
drm/i915: Fix ordering of unbind vs unpin pages
Browse files Browse the repository at this point in the history
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.

v2: Bonus comment

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Tested-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
  • Loading branch information
Chris Wilson authored and Daniel Vetter committed Dec 4, 2013
1 parent 5135d64 commit 70903c3
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions drivers/gpu/drm/i915/i915_gem.c
Original file line number Diff line number Diff line change
Expand Up @@ -2746,22 +2746,26 @@ int i915_vma_unbind(struct i915_vma *vma)
obj->has_aliasing_ppgtt_mapping = 0;
}
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. */
if (i915_is_ggtt(vma->vm))
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
* no more VMAs exist. */
if (list_empty(&obj->vma_list))
list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list);

/* And finally now the object is completely decoupled from this vma,
* we can drop its hold on the backing storage and allow it to be
* reaped by the shrinker.
*/
i915_gem_object_unpin_pages(obj);

return 0;
}

Expand Down

0 comments on commit 70903c3

Please sign in to comment.