Re: [PATCH] drm/i915/guc: Fix a false alert of memory leak when free LRC

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Oct 19, 2015 at 03:05:46PM -0700, yu.dai@intel.com wrote:
> From: Alex Dai <yu.dai@intel.com>
> 
> There is a memory leak warning message from i915_gem_context_clean
> when GuC submission is enabled. The reason is that the request (so
> the LRC associated with it) is freed early than moving the vma list
> to inactive. When retire a gem object, this patch moves its vma
> list to inactive first to avoid the false alert of memory leak.
> 
> We are not seeing this in ExecList (non-GuC) mode because the gem
> request is moved to execlist_retired_req_list queue. The management
> of this queue, therefore free of LRC, happens after retire of vma
> list (i915_gem_retire_requests_ring).

Instead of hacking up the core active tracking code can we just fix lrc
context object tracking instead? This patch here seems to be supremely
fragile, and I really don't want it.
-Daniel

> 
> Signed-off-by: Alex Dai <yu.dai@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 7d6b0c8..a903d45 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2377,28 +2377,31 @@ i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring)
>  	RQ_BUG_ON(obj->last_read_req[ring] == NULL);
>  	RQ_BUG_ON(!(obj->active & (1 << ring)));
>  
> +	obj->active &= ~(1 << ring);
> +	if (!obj->active) {
> +		/* Bump our place on the bound list to keep it roughly in LRU
> +		 * order so that we don't steal from recently used but inactive
> +		 * objects (unless we are forced to ofc!)
> +		 */
> +		list_move_tail(&obj->global_list,
> +				&to_i915(obj->base.dev)->mm.bound_list);
> +
> +		list_for_each_entry(vma, &obj->vma_list, vma_link) {
> +			if (!list_empty(&vma->mm_list))
> +				list_move_tail(&vma->mm_list,
> +						&vma->vm->inactive_list);
> +		}
> +	}
> +
>  	list_del_init(&obj->ring_list[ring]);
>  	i915_gem_request_assign(&obj->last_read_req[ring], NULL);
>  
>  	if (obj->last_write_req && obj->last_write_req->ring->id == ring)
>  		i915_gem_object_retire__write(obj);
>  
> -	obj->active &= ~(1 << ring);
>  	if (obj->active)
>  		return;
>  
> -	/* Bump our place on the bound list to keep it roughly in LRU order
> -	 * so that we don't steal from recently used but inactive objects
> -	 * (unless we are forced to ofc!)
> -	 */
> -	list_move_tail(&obj->global_list,
> -		       &to_i915(obj->base.dev)->mm.bound_list);
> -
> -	list_for_each_entry(vma, &obj->vma_list, vma_link) {
> -		if (!list_empty(&vma->mm_list))
> -			list_move_tail(&vma->mm_list, &vma->vm->inactive_list);
> -	}
> -
>  	i915_gem_request_assign(&obj->last_fenced_req, NULL);
>  	drm_gem_object_unreference(&obj->base);
>  }
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx