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

Daniel Vetter
2013-08-06T18:54:46Z

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