From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 12/16] drm/i915: Reorder ctx unref on ppgtt cleanup Date: Thu, 17 Jul 2014 11:56:22 +0200 Message-ID: <20140717095622.GK15237@phenom.ffwll.local> References: <1404238671-18760-1-git-send-email-benjamin.widawsky@intel.com> <1404238671-18760-13-git-send-email-benjamin.widawsky@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f170.google.com (mail-wi0-f170.google.com [209.85.212.170]) by gabe.freedesktop.org (Postfix) with ESMTP id D07B989CD4 for ; Thu, 17 Jul 2014 02:56:20 -0700 (PDT) Received: by mail-wi0-f170.google.com with SMTP id f8so6214530wiw.3 for ; Thu, 17 Jul 2014 02:56:16 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1404238671-18760-13-git-send-email-benjamin.widawsky@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ben Widawsky Cc: Intel GFX , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org 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 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