Re: [PATCH 09/11] drm/i915: Release vma when the handle is closed

Tvrtko Ursulin
2015-12-17T14:32:49Z

On 17/12/15 14:21, Chris Wilson wrote:
> On Thu, Dec 17, 2015 at 01:46:58PM +0000, Tvrtko Ursulin wrote:
>>> list_for_each_entry_safe(vma, next, &obj->vma_list, obj_link) {
>>> - int ret;
>>> -
>>> vma->pin_count = 0;
>>> - ret = i915_vma_unbind(vma);
>>> - if (WARN_ON(ret == -ERESTARTSYS)) {
>>> - bool was_interruptible;
>>> -
>>> - was_interruptible = dev_priv->mm.interruptible;
>>> - dev_priv->mm.interruptible = false;
>>> -
>>> - WARN_ON(i915_vma_unbind(vma));
>>> -
>>> - dev_priv->mm.interruptible = was_interruptible;
>>> - }
>>> + i915_vma_close(vma);
>>
>> In what circumstances can there be any VMAs still left unclosed at
>> this point? I thought i915_gem_close_object would had closed them
>> all.
>
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 688162703070..edfa5ebc4e77 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3857,7 +3857,14 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
>
> trace_i915_gem_object_destroy(obj);
>
> + /* All file-owned VMA should have been released by this point (through
> + * i915_gem_close_object). However, the object may also be bound into
> + * the global GTT (e.g. older GPUs without per-process support, or
> + * for direct access through the GTT either for the user or for
> + * scanout). Those VMA still need to unbound now.
> + */
> list_for_each_entry_safe(vma, next, &obj->vma_list, obj_link) {
> + RQ_BUG_ON(!i915_is_ggtt(vma->vm));
> RQ_BUG_ON(vma->active);
> vma->pin_count = 0;
> i915_vma_close(vma);
>

Ah yes, I've missed that detail. Very good to have it in a comment (and
assert).

Regards,

Tvrtko