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.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
