Re: [PATCH 1/4] [v2] drm/i915: Remove node only when allocated

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Aug 14, 2013 at 10:06:30AM +0200, Daniel Vetter wrote:
> On Tue, Aug 13, 2013 at 06:09:06PM -0700, Ben Widawsky wrote:
> > VMAs can be created and not bound. One may think of it as lazy cleanup,
> > and safely gloss over the conditions which manufacture it. In either
> > case, when the object backing the i915 vma is destroyed, we must cleanup
> > the vma without stumbling into a bunch of pitfalls that assume the vma
> > is bound.
> > 
> > NOTE: I was pretty certain the above condition could only happen when we
> > introduced the use of VMAs being looked up at execbuf, and already
> > existing. Paulo has hit this though, so I must be missing something. As
> > I believe the patch is correct anyway, therefore I won't scratch my head
> > too hard.
> 
> If we end up calling evict_everything from i915_gem_object_bind_to_vm then
> we'll hit this. One more reason for a testcase here ;-) I'll amend the
> commit message and merge this. I've also applied a tiny bikeshed I've
> created while reviewing existing vma_create/destroy callsites.

Actually evict_everything isn't in the callpath, and there's no memory
allocation where the shrinker might play havoc. Furthermore the pages are
pinned so the shrinker shouldn't be able to sneak in at all. This is a bit
unsettling, I need to think more about this.

I'll wait with merging this for now.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch