Re: [PATCH 10/11] drm/i915: Mark the context and address space as closed

Chris Wilson
2015-12-17T12:48:14Z

On Thu, Dec 17, 2015 at 12:37:13PM +0000, Tvrtko Ursulin wrote:
> >-static void i915_vma_close(struct i915_vma *vma)
>
> Can't find this in this series?

Should be the previous patch (9/11: Release vma when the handle is
closed) that hooks in gem_object_close to mark each vma as closed if it
is owned by the file.

http://patchwork.freedesktop.org/patch/68086/

> >diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> >index c4a8a64cd1b2..9669547c7c2d 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_context.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_context.c
> >@@ -153,6 +153,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
> > struct intel_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
> >
> > trace_i915_context_free(ctx);
> >+ RQ_BUG_ON(!ctx->closed);
>
> Normal BUG_ON I think.

You want a BUG_ON! :-p Just enable them.

> >+ for (phase = phases; *phase; phase++) {
> >+ struct i915_vma *vma, *vn;
> >+
> >+ list_for_each_entry_safe(vma, vn, *phase, vm_link)
> >+ i915_vma_close(vma);
>
> Can't really carry on since I don't see the implementation of this.
>
> Does it wait for retirement?

No. i915_vma_close() uses vma tracking to defer the unbind until idle.

> >+ /**
> >+ * List of vma that have been unbound.
> >+ *
> >+ * A reference is not held on the buffer while on this list.
>
> s/buffer/object/

They are buffer objects! The comment was cut'n'paste. I don't think it
is entirely apt to be talking about the object level active reference
here anyway. But I didn't feel inclined to write something that was even
more confusing.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre