Re: [PATCH 17/29] drm/i915: plumb VM into bind/unbind code

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Aug 06, 2013 at 08:29:47PM +0200, Daniel Vetter wrote:
> On Wed, Jul 31, 2013 at 05:00:10PM -0700, Ben Widawsky wrote:

[snip]

> > @@ -3643,33 +3672,39 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
> >  		    bool map_and_fenceable,
> >  		    bool nonblocking)
> >  {
> > +	struct i915_vma *vma;
> >  	int ret;
> >  
> >  	if (WARN_ON(obj->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT))
> >  		return -EBUSY;
> >  
> > -	if (i915_gem_obj_ggtt_bound(obj)) {
> > -		if ((alignment && i915_gem_obj_ggtt_offset(obj) & (alignment - 1)) ||
> > +	WARN_ON(map_and_fenceable && !i915_is_ggtt(vm));
> > +
> > +	vma = i915_gem_obj_to_vma(obj, vm);
> > +
> > +	if (vma) {
> > +		if ((alignment &&
> > +		     vma->node.start & (alignment - 1)) ||
> >  		    (map_and_fenceable && !obj->map_and_fenceable)) {
> >  			WARN(obj->pin_count,
> >  			     "bo is already pinned with incorrect alignment:"
> >  			     " offset=%lx, req.alignment=%x, req.map_and_fenceable=%d,"
> >  			     " obj->map_and_fenceable=%d\n",
> > -			     i915_gem_obj_ggtt_offset(obj), alignment,
> > +			     i915_gem_obj_offset(obj, vm), alignment,
> >  			     map_and_fenceable,
> >  			     obj->map_and_fenceable);
> > -			ret = i915_gem_object_unbind(obj);
> > +			ret = i915_vma_unbind(vma);
> 
> If I read this correctly then we wont' call i915_gem_vma_destroy anymore
> and so will leak the vma. Is that correct? If so I guess a new slab for
> vmas could be handy to easily detect such bugs.

On re-reading all seems to be fine here since object_unbind was converted
to vma_unbind and so inherited the call to vma_destroy. So no leak here.
The other stuff isn't really critical, so I'll merge this patch (and the
next one).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch