On Thu, 11 Jun 2015, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote: > On 06/11/2015 08:06 AM, Chris Wilson wrote: >> With the introduction of multiple views of an obj in the same vm, each >> vma was taught to cache its copy of the pages (so that different views >> could have different page arrangements). However, this missed decoupling >> those vma->ggtt_view.pages when the vma released its reference on the >> obj->pages. As we don't always free the vma, this leads to a possible >> scenario (e.g. execbuffer interrupted by the shrinker) where the vma >> points to a stale obj->pages, and explodes. >> >> Fixes regression from commit fe14d5f4e5468c5b80a24f1a64abcbe116143670 >> Author: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> >> Date: Wed Dec 10 17:27:58 2014 +0000 >> >> drm/i915: Infrastructure for supporting different GGTT views per object >> >> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1227892 >> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> >> Cc: Michel Thierry <michel.thierry@xxxxxxxxx> >> Cc: stable@xxxxxxxxxxxxxxx >> --- >> drivers/gpu/drm/i915/i915_gem.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index 9ae98b00ff56..377a6da31a1c 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -3214,8 +3214,8 @@ int i915_vma_unbind(struct i915_vma *vma) >> } else if (vma->ggtt_view.pages) { >> sg_free_table(vma->ggtt_view.pages); >> kfree(vma->ggtt_view.pages); >> - vma->ggtt_view.pages = NULL; >> } >> + vma->ggtt_view.pages = NULL; >> } >> >> drm_mm_remove_node(&vma->node); > > Nasty, thanks for fixing this. > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > If someone else will be confused how this can happen, key is the > reservation execbuffer path. That puts the VMA on the exec_list which > prevents i915_vma_unbind and i915_gem_vma_destroy from fully destroying > the VMA. So the VMA is left existing as an empty object in the list - > unbound and disassociated with the backing store. Kind of a cached > memory object. And then re-using it needs to clear the cached pages > pointer which is fixed above. Pushed to drm-intel-fixes with the above text added to commit message. Thanks for the patch and review. BR, Jani. > > Regards, > > Tvrtko > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
