From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] drm/i915: Always use kref tracking for contexts. Date: Mon, 3 Mar 2014 07:19:19 +0000 Message-ID: <20140303071919.GB2154@nuc-i3427.alporthouse.com> References: <1393618010-24070-1-git-send-email-chris@chris-wilson.co.uk> <20140302235808.GA1791@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from fireflyinternet.com (mail.fireflyinternet.com [87.106.93.118]) by gabe.freedesktop.org (Postfix) with ESMTP id 167DBFAF03 for ; Sun, 2 Mar 2014 23:19:22 -0800 (PST) Content-Disposition: inline In-Reply-To: <20140302235808.GA1791@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Ben Widawsky Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org 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 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