On Tue, Dec 22, 2015 at 05:14:34PM +0000, Tvrtko Ursulin wrote:
>
> 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..
Please only BUG_ON if imminent kernel death in the next few instructions
is a certainty. Otherwise I think it's much better to carry on and give
the user a change to grab logs and other useful information before the
system keels over completely.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
