Re: [PATCH 12/16] drm/i915: Reorder ctx unref on ppgtt cleanup

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jul 01, 2014 at 11:17:47AM -0700, Ben Widawsky wrote:
> The comment [which was mine] is wrong. The context object can never be
> bound in a PPGTT because it is only capable of living in the Global GTT.
> So, remove the comment, and reorder the unref. What's nice about the
> latter is it keeps the context object alive past the PPGTT. This makes
> the destroy ordering symmetric with the creation ordering.
> 
> Create:
> 1. Create context
> 2. Create PPGTT
> 
> Destroy:
> 1. Destroy PPGTT
> 2. Destroy context
> 
> As far as I know, this does not fix a bug. The code previously kept the
> context data structure, only the object was gone. As the code was,
> nothing tried to use the object after this point.
> 
> NOTE: If in the future we have cases where the PPGTT can/should outlive
> the context (which doesn't occur today, but the code permits it), this
> ordering does not matter. Even if this occurs, as it stands now, we do
> not expect that to be the normal case, and having this order makes
> debugging a bit easier if we're tracking object lifetimes for the
> context vs ppgtt
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Queued for -next, thanks for the patch.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index b6803b3..8d106d9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -185,14 +185,12 @@ void i915_gem_context_free(struct kref *ctx_ref)
>  		/* We refcount even the aliasing PPGTT to keep the code symmetric */
>  		if (USES_PPGTT(ctx->obj->base.dev))
>  			ppgtt = ctx_to_ppgtt(ctx);
> -
> -		/* XXX: Free up the object before tearing down the address space, in
> -		 * case we're bound in the PPGTT */
> -		drm_gem_object_unreference(&ctx->obj->base);
>  	}
>  
>  	if (ppgtt)
>  		kref_put(&ppgtt->ref, ppgtt_release);
> +	if (ctx->obj)
> +		drm_gem_object_unreference(&ctx->obj->base);
>  	list_del(&ctx->link);
>  	kfree(ctx);
>  }
> -- 
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch