From patchwork Mon Sep 21 16:00:46 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tvrtko Ursulin X-Patchwork-Id: 7232761 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.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B15D5BEEC1 for ; Mon, 21 Sep 2015 16:05:07 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BDD9B207AF for ; Mon, 21 Sep 2015 16:05:06 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 549FA20710 for ; Mon, 21 Sep 2015 16:05:05 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C68346E98E; Mon, 21 Sep 2015 09:05:04 -0700 (PDT) X-Original-To: Intel-gfx@lists.freedesktop.org Delivered-To: Intel-gfx@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id C35856E98E for ; Mon, 21 Sep 2015 09:05:03 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 21 Sep 2015 09:01:50 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,568,1437462000"; d="scan'208";a="809505171" Received: from tursulin-linux.isw.intel.com ([10.102.226.64]) by orsmga002.jf.intel.com with ESMTP; 21 Sep 2015 09:00:49 -0700 From: Tvrtko Ursulin To: Intel-gfx@lists.freedesktop.org Date: Mon, 21 Sep 2015 17:00:46 +0100 Message-Id: <1442851246-9035-1-git-send-email-tvrtko.ursulin@linux.intel.com> X-Mailer: git-send-email 2.5.1 Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH v2] drm/i915: Clean up associated VMAs on context destruction X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 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: Tvrtko Ursulin Prevent leaking VMAs and PPGTT VMs when objects are imported via flink. Scenario is that any VMAs created by the importer will be left dangling after the importer exits, or destroys the PPGTT context with which they are associated. This is caused by object destruction not running when the importer closes the buffer object handle due the reference held by the exporter. This also leaks the VM since the VMA has a reference on it. In practice these leaks can be observed by stopping and starting the X server on a kernel with fbcon compiled in. Every time X server exits another VMA will be leaked against the fbcon's frame buffer object. Also on systems where flink buffer sharing is used extensively, like Android, this leak has even more serious consequences. This version is takes a general approach from the earlier work by Rafael Barabalho (drm/i915: Clean-up PPGTT on context destruction) and tries to incorporate the subsequent discussion between Chris Wilson and Daniel Vetter. On context destruction a worker thread is scheduled to unbind all inactive VMAs for this VM. v2: Removed immediate cleanup on object retire - it was causing a recursive VMA unbind via i915_gem_object_wait_rendering. And it is in fact not even needed since by definition context cleanup worker runs only after the last context reference has been dropped, hence all VMAs against the VM belonging to the context are already on the inactive list. Signed-off-by: Tvrtko Ursulin Testcase: igt/gem_ppgtt.c/flink-and-exit-vma-leak Cc: Daniel Vetter Cc: Chris Wilson Cc: Rafael Barbalho Cc: Michel Thierry --- drivers/gpu/drm/i915/i915_gem_context.c | 50 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++ 2 files changed, 52 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 74aa0c9929ba..714d71bae572 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -133,6 +133,49 @@ static int get_context_size(struct drm_device *dev) return ret; } +static void +i915_gem_context_cleanup_worker(struct work_struct *work) +{ + struct i915_hw_ppgtt *ppgtt = container_of(work, typeof(*ppgtt), + cleanup_work); + struct drm_device *dev = ppgtt->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct i915_vma *vma, *next; + bool was_interruptible; + + mutex_lock(&dev->struct_mutex); + was_interruptible = dev_priv->mm.interruptible; + + WARN_ON(!list_empty(&ppgtt->base.active_list)); + + list_for_each_entry_safe(vma, next, &ppgtt->base.inactive_list, + mm_list) { + dev_priv->mm.interruptible = false; + if (WARN_ON(i915_vma_unbind(vma))) + break; + } + + i915_ppgtt_put(ppgtt); + + dev_priv->mm.interruptible = was_interruptible; + mutex_unlock(&dev->struct_mutex); +} + +static void i915_gem_context_clean(struct intel_context *ctx) +{ + struct i915_hw_ppgtt *ppgtt = ctx->ppgtt; + + if (!ppgtt) + return; + + /* + * Unbind all inactive VMAs for this VM, but do it asynchronously. + */ + INIT_WORK(&ppgtt->cleanup_work, i915_gem_context_cleanup_worker); + i915_ppgtt_get(ppgtt); + schedule_work(&ppgtt->cleanup_work); +} + void i915_gem_context_free(struct kref *ctx_ref) { struct intel_context *ctx = container_of(ctx_ref, typeof(*ctx), ref); @@ -142,6 +185,13 @@ void i915_gem_context_free(struct kref *ctx_ref) if (i915.enable_execlists) intel_lr_context_free(ctx); + /* + * This context is going away and we need to remove all VMAs still + * around. This is to handle imported shared objects for which + * destructor did not run when their handles were closed. + */ + i915_gem_context_clean(ctx); + i915_ppgtt_put(ctx->ppgtt); if (ctx->legacy_hw_ctx.rcs_state) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 430cc283d3c9..c7426b76cd1d 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -375,6 +375,8 @@ struct i915_hw_ppgtt { struct drm_i915_file_private *file_priv; + struct work_struct cleanup_work; + gen6_pte_t __iomem *pd_addr; int (*enable)(struct i915_hw_ppgtt *ppgtt);