From patchwork Tue Aug 20 06:14:13 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Widawsky X-Patchwork-Id: 2846887 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 B3348BF546 for ; Tue, 20 Aug 2013 07:25:49 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AEFB320437 for ; Tue, 20 Aug 2013 07:25:48 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 8A44320435 for ; Tue, 20 Aug 2013 07:25:47 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 85B4CE69E4 for ; Tue, 20 Aug 2013 00:25:47 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail.bwidawsk.net (bwidawsk.net [166.78.191.112]) by gabe.freedesktop.org (Postfix) with ESMTP id EBE72E5EF9 for ; Mon, 19 Aug 2013 23:14:42 -0700 (PDT) Received: by mail.bwidawsk.net (Postfix, from userid 5001) id 23425599B4; Mon, 19 Aug 2013 23:14:24 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from lundgren.intel.com (c-24-21-100-90.hsd1.or.comcast.net [24.21.100.90]) by mail.bwidawsk.net (Postfix) with ESMTPSA id A31CF58152; Mon, 19 Aug 2013 23:14:21 -0700 (PDT) From: Ben Widawsky To: intel-gfx@lists.freedesktop.org Date: Mon, 19 Aug 2013 23:14:13 -0700 Message-Id: <1376979255-17887-1-git-send-email-benjamin.widawsky@intel.com> X-Mailer: git-send-email 1.8.3.4 Cc: Ben Widawsky , Ben Widawsky Subject: [Intel-gfx] [PATCH 1/3] drm/i915/vma: Add a busy flag; means don't destroy 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+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org X-Virus-Scanned: ClamAV using ClamSMTP The code has at least one place currently, and maybe more in the future where we want to only unbind a VMA, but not destroy it. There are two reasons for this, correctness and optimization. Using the one actual case as an example: 1. Correctness: When doing the execbuf sequence we end up traversing the "exec_list" looking for misaligned VMAs. At this point we'd like to unbind the VMA, which could lead to it's destruction. In the current code, this is "fine" because our vma destroy doesn't remove the VMA from the exec_list. The latter fact was an oversight which was of trivial consequence in the original PPGTT series - but now actually matters since we have a multistage execbuffer. 2. Optimization: Unbinding a VMA is a relatively expensive operation because it has to go and modify the PTEs. At present, therefore, it isn't terribly interesting to try to optimize this case. However, one could potentially do the PTE writes lazily (with full PPGTT there is no security risk, only a correctness one) - where we don't even update PTEs until later. Destroying and recreating the VMA in this case can be spared with this busy flag. "Hey, but isn't that a leak?" As you might recall from previous patches, VMAs will be destroyed on object freeing, and can be cheaply looked up later. Therefore the dangling VMA isn't a problem. In the future if we end up with an overabundance of dangling VMAs, we can potentially add some sort of garbage collection for them. NOTE: This patch doesn't actually fix anything. The explanation above is the problem, this is the harness, and the fix comes next. Cc: Chris Wilson Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1eb4d98..28c0e0a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -574,6 +574,7 @@ struct i915_vma { unsigned long exec_handle; struct drm_i915_gem_exec_object2 *exec_entry; + bool busy; }; struct i915_ctx_hang_stats { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c7efb60..6d2d7df 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4156,6 +4156,9 @@ i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, void i915_gem_vma_destroy(struct i915_vma *vma) { + if (vma->busy) + return; + WARN_ON(vma->node.allocated); list_del(&vma->vma_link); kfree(vma);