public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Ben Widawsky <benjamin.widawsky@intel.com>
To: Intel GFX <intel-gfx@lists.freedesktop.org>
Cc: Ben Widawsky <ben@bwidawsk.net>,
	Ben Widawsky <benjamin.widawsky@intel.com>
Subject: [PATCH 12/16] drm/i915: Reorder ctx unref on ppgtt cleanup
Date: Tue,  1 Jul 2014 11:17:47 -0700	[thread overview]
Message-ID: <1404238671-18760-13-git-send-email-benjamin.widawsky@intel.com> (raw)
In-Reply-To: <1404238671-18760-1-git-send-email-benjamin.widawsky@intel.com>

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>
---
 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

  parent reply	other threads:[~2014-07-01 18:18 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-01 18:17 [PATCH 00/16] Enabling GEN8 full PPGTT + fixes Ben Widawsky
2014-07-01 18:17 ` [PATCH 01/16] drm/i915: Split up do_switch Ben Widawsky
2014-07-01 18:17 ` [PATCH 02/16] drm/i915: Extract l3 remapping out of ctx switch Ben Widawsky
2014-07-01 18:17 ` [PATCH 03/16] drm/i915/ppgtt: Load address space after mi_set_context Ben Widawsky
2014-07-01 18:17 ` [PATCH 04/16] drm/i915: Fix another another use-after-free in do_switch Ben Widawsky
2014-08-09 20:15   ` [PATCH] [v2] " Ben Widawsky
2014-08-10  8:04     ` Chris Wilson
2014-08-11  9:26       ` Daniel Vetter
2014-07-01 18:17 ` [PATCH 05/16] drm/i915/ctx: Return earlier on failure Ben Widawsky
2014-07-04  8:14   ` Chris Wilson
2014-07-01 18:17 ` [PATCH 06/16] drm/i915/error: Check the potential ctx obj's vm Ben Widawsky
2014-07-17  8:47   ` Daniel Vetter
2014-07-01 18:17 ` [PATCH 07/16] drm/i915/error: vma error capture prettyify Ben Widawsky
2014-07-01 18:17 ` [PATCH 08/16] drm/i915/error: Do a better job of disambiguating VMAs Ben Widawsky
2014-07-04  7:57   ` Chris Wilson
2014-07-04 16:56     ` Ben Widawsky
2014-07-17  8:51       ` Daniel Vetter
2014-07-20 23:49         ` Ben Widawsky
2014-07-01 18:17 ` [PATCH 09/16] drm/i915/error: Capture vmas instead of BOs Ben Widawsky
2014-07-01 18:17 ` [PATCH 10/16] drm/i915: Add some extra guards in evict_vm Ben Widawsky
2014-07-01 18:17 ` [PATCH 11/16] drm/i915: Make an uninterruptible evict Ben Widawsky
2014-07-01 18:17 ` Ben Widawsky [this message]
2014-07-17  9:56   ` [PATCH 12/16] drm/i915: Reorder ctx unref on ppgtt cleanup Daniel Vetter
2014-07-01 18:17 ` [PATCH 13/16] drm/i915: More correct (slower) " Ben Widawsky
2014-07-17  9:49   ` Daniel Vetter
2014-07-01 18:17 ` [PATCH 14/16] drm/i915: Defer PPGTT cleanup Ben Widawsky
2014-07-01 18:17 ` [PATCH 15/16] drm/i915/bdw: Enable full PPGTT Ben Widawsky
2014-07-01 18:17 ` [PATCH 16/16] drm/i915: Get the error state over the wire (HACKish) Ben Widawsky
2014-07-04  8:02   ` Chris Wilson
2014-07-03 22:01 ` [PATCH 1/2] drm/i915/gen8: Invalidate TLBs before PDP reload Ben Widawsky
2014-07-03 22:01   ` [PATCH 2/2] drm/i915: Remove false assertion in ppgtt_release Ben Widawsky
2014-07-04  7:51   ` [PATCH 1/2] drm/i915/gen8: Invalidate TLBs before PDP reload Chris Wilson
2014-07-04 16:55     ` Ben Widawsky
2014-07-17 12:04 ` [PATCH 00/16] Enabling GEN8 full PPGTT + fixes Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1404238671-18760-13-git-send-email-benjamin.widawsky@intel.com \
    --to=benjamin.widawsky@intel.com \
    --cc=ben@bwidawsk.net \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox