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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 17/12/15 12:48, Chris Wilson wrote:
> 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/

Yeah found it later, mail filtering split the thread to different folders.

>>> 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.

No I just gave up on you seeing the light! :>

Point is RQ_BUG_ON is behind CONFIG_DRM_I915_DEBUG_REQUESTS so it is 
wrong to use it for other things.

>>> +	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.

# grep buffer i915_gem.c | grep -v ring | grep -v front | grep -v batch 
| grep -v execbuffer | grep -v scanout | wc -l
8

Ok, there is some precedence for the term.

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx