On 22/12/15 17:02, Chris Wilson wrote:
On Tue, Dec 22, 2015 at 12:33:41PM +0000, Tvrtko Ursulin wrote:
On 22/12/15 06:20, ankitprasad.r.sharma@xxxxxxxxx wrote:
From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
+ /* Recreate any pinned binding with pointers to the new storage */
+ if (!list_empty(&obj->vma_list)) {
+ ret = i915_gem_object_get_pages_gtt(obj);
+ if (ret) {
+ obj->pages = stolen_pages;
+ goto err_file;
+ }
+
+ obj->get_page.sg = obj->pages->sgl;
+ obj->get_page.last = 0;
+
+ list_for_each_entry(vma, &obj->vma_list, vma_link) {
+ if (!drm_mm_node_allocated(&vma->node))
+ continue;
+
+ WARN_ON(i915_vma_bind(vma,
+ obj->cache_level,
+ PIN_UPDATE));
It looks like this should also fail (and restore) the migration.
Otherwise if it fails it leaves GTT mappings to pages which will be
released below.
Or a big fat comment explaining why it cannot fail, ever.
It is an impossible error, fortunately. The failure handling case would
have to redo the previous rebindings which are then subject to exactly
the same error.
I take it WARN_ON isn't enough, you would rather we document impossible
failure conditions with BUG_ON? And since this does leave hardware
pointing into stolen, it should really be a full BUG_ON.
Ok ok BUG_ON for this one sounds better.
One day I'll try and sketch my I915_BUG_ON I mentioned a few times in
the past..
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
