From patchwork Tue Nov 17 16:27:12 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tvrtko Ursulin X-Patchwork-Id: 7638981 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.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 8808B9F392 for ; Tue, 17 Nov 2015 16:29:35 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AA6F92046F for ; Tue, 17 Nov 2015 16:29:34 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id ABAA820450 for ; Tue, 17 Nov 2015 16:29:33 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 028416E308; Tue, 17 Nov 2015 08:29:33 -0800 (PST) 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 2D33E6E308 for ; Tue, 17 Nov 2015 08:29:32 -0800 (PST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga101.jf.intel.com with ESMTP; 17 Nov 2015 08:29:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,308,1444719600"; d="scan'208";a="601912759" Received: from tursulin-linux.isw.intel.com ([10.102.226.89]) by FMSMGA003.fm.intel.com with ESMTP; 17 Nov 2015 08:29:18 -0800 From: Tvrtko Ursulin To: Intel-gfx@lists.freedesktop.org Date: Tue, 17 Nov 2015 16:27:12 +0000 Message-Id: <1447777632-32099-1-git-send-email-tvrtko.ursulin@linux.intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <20151117160440.GB14177@nuc-i3427.alporthouse.com> References: <20151117160440.GB14177@nuc-i3427.alporthouse.com> Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH v3] drm/i915: Ensure associated VMAs are inactive when contexts are destroyed 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.8 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 In the following commit: commit e9f24d5fb7cf3628b195b18ff3ac4e37937ceeae Author: Tvrtko Ursulin Date: Mon Oct 5 13:26:36 2015 +0100 drm/i915: Clean up associated VMAs on context destruction I added a WARN_ON assertion that VM's active list must be empty at the time of owning context is getting freed, but that turned out to be a wrong assumption. Due ordering of operations in i915_gem_object_retire__read, where contexts are unreferenced before VMAs are moved to the inactive list, the described situation can in fact happen. It feels wrong to do things in such order so this fix makes sure a reference to context is held until the move to inactive list is completed. v2: Rather than hold a temporary context reference move the request unreference to be the last operation. (Daniel Vetter) v3: Fix use after free. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92638 Cc: Michel Thierry Cc: Chris Wilson Cc: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 98c83286ab68..094ac17a712d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2404,29 +2404,32 @@ i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring) RQ_BUG_ON(!(obj->active & (1 << ring))); list_del_init(&obj->ring_list[ring]); - i915_gem_request_assign(&obj->last_read_req[ring], NULL); if (obj->last_write_req && obj->last_write_req->ring->id == ring) i915_gem_object_retire__write(obj); obj->active &= ~(1 << ring); - if (obj->active) - return; - /* Bump our place on the bound list to keep it roughly in LRU order - * so that we don't steal from recently used but inactive objects - * (unless we are forced to ofc!) - */ - list_move_tail(&obj->global_list, - &to_i915(obj->base.dev)->mm.bound_list); + if (!obj->active) { + /* Bump our place on the bound list to keep it roughly in LRU order + * so that we don't steal from recently used but inactive objects + * (unless we are forced to ofc!) + */ + list_move_tail(&obj->global_list, + &to_i915(obj->base.dev)->mm.bound_list); - list_for_each_entry(vma, &obj->vma_list, vma_link) { - if (!list_empty(&vma->mm_list)) - list_move_tail(&vma->mm_list, &vma->vm->inactive_list); - } + list_for_each_entry(vma, &obj->vma_list, vma_link) { + if (!list_empty(&vma->mm_list)) + list_move_tail(&vma->mm_list, + &vma->vm->inactive_list); + } - i915_gem_request_assign(&obj->last_fenced_req, NULL); - drm_gem_object_unreference(&obj->base); + i915_gem_request_assign(&obj->last_fenced_req, NULL); + i915_gem_request_assign(&obj->last_read_req[ring], NULL); + drm_gem_object_unreference(&obj->base); + } else { + i915_gem_request_assign(&obj->last_read_req[ring], NULL); + } } static int