[Intel-gfx] [PATCH] drm/i915: Always use kref tracking for contexts.

[Chris Wilson]

On Sun, Mar 02, 2014 at 03:58:09PM -0800, Ben Widawsky wrote:
> On Fri, Feb 28, 2014 at 08:06:50PM +0000, Chris Wilson wrote:
> >  	ctx->obj = i915_gem_object_create_stolen(dev, dev_priv->hw_context_size);
> 		  How this working for you ^ ?

Fine. It helps detect cases where we try to load uninitialised contexts,
but other than that I have not noticed any difference.

> > +	if (HAS_HW_CONTEXTS(dev)) {
> > +		idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
> > +		idr_destroy(&file_priv->context_idr);
> >  	}
> >  
> > -	idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
> >  	i915_gem_context_unreference(file_priv->private_default_ctx);
> > -	idr_destroy(&file_priv->context_idr);
> >  }
> 
> I don't see too much harm in just initializing the idr regardless to
> keep the close path simpler. Then stick a WARN in context_idr_cleanup if
> it actually does anything, fake_context_idr_cleanup();

Agreed I didn't see any harm in making that change as well. I was just
trying to keep the set of changes small, and targetted towards removing
that ctx->obj dereference.
 
> I have some recollection of hitting a really tricky thing while
> developing PPGTT where the order of the unref in the above sequence
> really mattered. Looking again though, I don't see anything familiar
> - but my suggestion would put me completely at ease.

Sure, but I think you've squashed that bug already. :)
 
> In either case:
> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

Ok, let's go with a second patch to converge the fake context with the
hw context later.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre