From patchwork Tue Jul 15 16:20:48 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michel Thierry X-Patchwork-Id: 4555451 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B523FC0514 for ; Tue, 15 Jul 2014 16:23:10 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DA60E20120 for ; Tue, 15 Jul 2014 16:23:09 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 0B2F420145 for ; Tue, 15 Jul 2014 16:23:09 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 67C076E364; Tue, 15 Jul 2014 09:23:08 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 906EB6E188 for ; Tue, 15 Jul 2014 09:23:06 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 15 Jul 2014 09:21:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.01,666,1400050800"; d="scan'208";a="573515420" Received: from michelth-linux.iwi.intel.com ([172.28.253.148]) by orsmga002.jf.intel.com with ESMTP; 15 Jul 2014 09:21:01 -0700 From: Michel Thierry To: intel-gfx@lists.freedesktop.org Date: Tue, 15 Jul 2014 17:20:48 +0100 Message-Id: <1405441251-28744-12-git-send-email-michel.thierry@intel.com> X-Mailer: git-send-email 1.9.0 In-Reply-To: <1405441251-28744-1-git-send-email-michel.thierry@intel.com> References: <1405441251-28744-1-git-send-email-michel.thierry@intel.com> Subject: [Intel-gfx] [PATCH 11/14] drm/i915: Reorder ctx unref on ppgtt cleanup X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Ben Widawsky 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 --- 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 0410bd9..70b3776 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->legacy_hw_ctx.rcs_state->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->legacy_hw_ctx.rcs_state->base); } if (ppgtt) kref_put(&ppgtt->ref, ppgtt_release); + if (ctx->legacy_hw_ctx.rcs_state) + drm_gem_object_unreference(&ctx->legacy_hw_ctx.rcs_state->base); list_del(&ctx->link); kfree(ctx); }