From patchwork Fri Feb 28 20:06:50 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 3743931 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id A99009F35F for ; Fri, 28 Feb 2014 20:07:07 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C65E920240 for ; Fri, 28 Feb 2014 20:07:06 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 0475420237 for ; Fri, 28 Feb 2014 20:07:06 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A2F70FBD09; Fri, 28 Feb 2014 12:07:04 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from relay.fireflyinternet.com (hostedrelay.fireflyinternet.com [109.228.30.76]) by gabe.freedesktop.org (Postfix) with ESMTP id 74271FBD09 for ; Fri, 28 Feb 2014 12:07:03 -0800 (PST) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.73.22; Received: from haswell.alporthouse.com (unverified [78.156.73.22]) by relay.fireflyinternet.com (FireflyRelay1) with ESMTP id 10694405-1305619 for multiple; Fri, 28 Feb 2014 20:07:13 +0000 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Fri, 28 Feb 2014 20:06:50 +0000 Message-Id: <1393618010-24070-1-git-send-email-chris@chris-wilson.co.uk> X-Mailer: git-send-email 1.9.0 X-Authenticated-User: chris.alporthouse@surfanytime.net Subject: [Intel-gfx] [PATCH] drm/i915: Always use kref tracking for contexts. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham 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 If we always initialize kref for the context, even if we are using fake contexts for hangstats when there is no hw support, we can forgo the dance to dereference the ctx->obj and inspect whether we are permitted to use kref inside i915_gem_context_reference() and _unreference(). My ulterior motive here is to improve the debugging of a use-after-free of ctx->obj. This patch avoids the dereference here and instead forces the assertion checks associated with kref. Signed-off-by: Chris Wilson Reviewed-by: Ben Widawsky --- drivers/gpu/drm/i915/i915_drv.h | 6 ++---- drivers/gpu/drm/i915/i915_gem_context.c | 37 +++++++++++++++++++++------------ 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8af8e0dd3943..17a37578053c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2380,14 +2380,12 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id); void i915_gem_context_free(struct kref *ctx_ref); static inline void i915_gem_context_reference(struct i915_hw_context *ctx) { - if (ctx->obj && HAS_HW_CONTEXTS(ctx->obj->base.dev)) - kref_get(&ctx->ref); + kref_get(&ctx->ref); } static inline void i915_gem_context_unreference(struct i915_hw_context *ctx) { - if (ctx->obj && HAS_HW_CONTEXTS(ctx->obj->base.dev)) - kref_put(&ctx->ref, i915_gem_context_free); + kref_put(&ctx->ref, i915_gem_context_free); } static inline bool i915_gem_context_is_default(const struct i915_hw_context *c) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index e9f888ea67d6..48dc5f8f21bf 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -179,12 +179,9 @@ create_vm_for_ctx(struct drm_device *dev, struct i915_hw_context *ctx) } static struct i915_hw_context * -__create_hw_context(struct drm_device *dev, - struct drm_i915_file_private *file_priv) +__ctx_alloc(void) { - struct drm_i915_private *dev_priv = dev->dev_private; struct i915_hw_context *ctx; - int ret; ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (ctx == NULL) @@ -193,6 +190,21 @@ __create_hw_context(struct drm_device *dev, kref_init(&ctx->ref); INIT_LIST_HEAD(&ctx->link); + return ctx; +} + +static struct i915_hw_context * +__create_hw_context(struct drm_device *dev, + struct drm_i915_file_private *file_priv) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct i915_hw_context *ctx; + int ret; + + ctx = __ctx_alloc(); + if (IS_ERR(ctx)) + return ctx; + ctx->obj = i915_gem_object_create_stolen(dev, dev_priv->hw_context_size); if (ctx->obj == NULL) ctx->obj = i915_gem_alloc_object(dev, dev_priv->hw_context_size); @@ -492,11 +504,9 @@ int i915_gem_context_open(struct drm_device *dev, struct drm_file *file) if (!HAS_HW_CONTEXTS(dev)) { /* Cheat for hang stats */ - file_priv->private_default_ctx = - kzalloc(sizeof(struct i915_hw_context), GFP_KERNEL); - - if (file_priv->private_default_ctx == NULL) - return -ENOMEM; + file_priv->private_default_ctx = __ctx_alloc(); + if (IS_ERR(file_priv->private_default_ctx)) + return PTR_ERR(file_priv->private_default_ctx); file_priv->private_default_ctx->vm = &dev_priv->gtt.base; return 0; @@ -521,14 +531,15 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file) { struct drm_i915_file_private *file_priv = file->driver_priv; - if (!HAS_HW_CONTEXTS(dev)) { - kfree(file_priv->private_default_ctx); + if (IS_ERR_OR_NULL(file_priv->private_default_ctx)) return; + + 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); } struct i915_hw_context *