From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 137C5C433FE for ; Thu, 13 Jan 2022 11:45:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2000210F761; Thu, 13 Jan 2022 11:45:10 +0000 (UTC) Received: from mblankhorst.nl (mblankhorst.nl [IPv6:2a02:2308:0:7ec:e79c:4e97:b6c4:f0ae]) by gabe.freedesktop.org (Postfix) with ESMTPS id 418CE10E444; Thu, 13 Jan 2022 11:45:08 +0000 (UTC) From: Maarten Lankhorst To: intel-gfx@lists.freedesktop.org Subject: [PATCH v5 5/6] drm/i915: Remove support for unlocked i915_vma unbind Date: Thu, 13 Jan 2022 12:44:59 +0100 Message-Id: <20220113114500.2039439-6-maarten.lankhorst@linux.intel.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220113114500.2039439-1-maarten.lankhorst@linux.intel.com> References: <20220113114500.2039439-1-maarten.lankhorst@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Niranjana Vishwanathapura , dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Now that we require the object lock for all ops, some code handling race conditions can be removed. This is required to not take short-term pins inside execbuf. Signed-off-by: Maarten Lankhorst Acked-by: Niranjana Vishwanathapura --- drivers/gpu/drm/i915/i915_vma.c | 55 +++++---------------------------- 1 file changed, 8 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 1213888b82d1..7b30a4ae11d1 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -839,7 +839,6 @@ i915_vma_detach(struct i915_vma *vma) static bool try_qad_pin(struct i915_vma *vma, unsigned int flags) { unsigned int bound; - bool pinned = true; bound = atomic_read(&vma->flags); do { @@ -849,34 +848,10 @@ static bool try_qad_pin(struct i915_vma *vma, unsigned int flags) if (unlikely(bound & (I915_VMA_OVERFLOW | I915_VMA_ERROR))) return false; - if (!(bound & I915_VMA_PIN_MASK)) - goto unpinned; - GEM_BUG_ON(((bound + 1) & I915_VMA_PIN_MASK) == 0); } while (!atomic_try_cmpxchg(&vma->flags, &bound, bound + 1)); return true; - -unpinned: - /* - * If pin_count==0, but we are bound, check under the lock to avoid - * racing with a concurrent i915_vma_unbind(). - */ - mutex_lock(&vma->vm->mutex); - do { - if (unlikely(bound & (I915_VMA_OVERFLOW | I915_VMA_ERROR))) { - pinned = false; - break; - } - - if (unlikely(flags & ~bound)) { - pinned = false; - break; - } - } while (!atomic_try_cmpxchg(&vma->flags, &bound, bound + 1)); - mutex_unlock(&vma->vm->mutex); - - return pinned; } static struct scatterlist * @@ -1215,7 +1190,6 @@ static int __i915_vma_get_pages(struct i915_vma *vma) { struct sg_table *pages; - int ret; /* * The vma->pages are only valid within the lifespan of the borrowed @@ -1248,18 +1222,16 @@ __i915_vma_get_pages(struct i915_vma *vma) break; } - ret = 0; if (IS_ERR(pages)) { - ret = PTR_ERR(pages); - pages = NULL; drm_err(&vma->vm->i915->drm, - "Failed to get pages for VMA view type %u (%d)!\n", - vma->ggtt_view.type, ret); + "Failed to get pages for VMA view type %u (%ld)!\n", + vma->ggtt_view.type, PTR_ERR(pages)); + return PTR_ERR(pages); } vma->pages = pages; - return ret; + return 0; } I915_SELFTEST_EXPORT int i915_vma_get_pages(struct i915_vma *vma) @@ -1291,25 +1263,14 @@ I915_SELFTEST_EXPORT int i915_vma_get_pages(struct i915_vma *vma) static void __vma_put_pages(struct i915_vma *vma, unsigned int count) { /* We allocate under vma_get_pages, so beware the shrinker */ - struct sg_table *pages = READ_ONCE(vma->pages); - GEM_BUG_ON(atomic_read(&vma->pages_count) < count); if (atomic_sub_return(count, &vma->pages_count) == 0) { - /* - * The atomic_sub_return is a read barrier for the READ_ONCE of - * vma->pages above. - * - * READ_ONCE is safe because this is either called from the same - * function (i915_vma_pin_ww), or guarded by vma->vm->mutex. - * - * TODO: We're leaving vma->pages dangling, until vma->obj->resv - * lock is required. - */ - if (pages != vma->obj->mm.pages) { - sg_free_table(pages); - kfree(pages); + if (vma->pages != vma->obj->mm.pages) { + sg_free_table(vma->pages); + kfree(vma->pages); } + vma->pages = NULL; i915_gem_object_unpin_pages(vma->obj); } -- 2.34.1 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4ECE4C433EF for ; Thu, 13 Jan 2022 11:45:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3872810F895; Thu, 13 Jan 2022 11:45:10 +0000 (UTC) Received: from mblankhorst.nl (mblankhorst.nl [IPv6:2a02:2308:0:7ec:e79c:4e97:b6c4:f0ae]) by gabe.freedesktop.org (Postfix) with ESMTPS id 418CE10E444; Thu, 13 Jan 2022 11:45:08 +0000 (UTC) From: Maarten Lankhorst To: intel-gfx@lists.freedesktop.org Date: Thu, 13 Jan 2022 12:44:59 +0100 Message-Id: <20220113114500.2039439-6-maarten.lankhorst@linux.intel.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220113114500.2039439-1-maarten.lankhorst@linux.intel.com> References: <20220113114500.2039439-1-maarten.lankhorst@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Intel-gfx] [PATCH v5 5/6] drm/i915: Remove support for unlocked i915_vma unbind X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dri-devel@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Now that we require the object lock for all ops, some code handling race conditions can be removed. This is required to not take short-term pins inside execbuf. Signed-off-by: Maarten Lankhorst Acked-by: Niranjana Vishwanathapura --- drivers/gpu/drm/i915/i915_vma.c | 55 +++++---------------------------- 1 file changed, 8 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 1213888b82d1..7b30a4ae11d1 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -839,7 +839,6 @@ i915_vma_detach(struct i915_vma *vma) static bool try_qad_pin(struct i915_vma *vma, unsigned int flags) { unsigned int bound; - bool pinned = true; bound = atomic_read(&vma->flags); do { @@ -849,34 +848,10 @@ static bool try_qad_pin(struct i915_vma *vma, unsigned int flags) if (unlikely(bound & (I915_VMA_OVERFLOW | I915_VMA_ERROR))) return false; - if (!(bound & I915_VMA_PIN_MASK)) - goto unpinned; - GEM_BUG_ON(((bound + 1) & I915_VMA_PIN_MASK) == 0); } while (!atomic_try_cmpxchg(&vma->flags, &bound, bound + 1)); return true; - -unpinned: - /* - * If pin_count==0, but we are bound, check under the lock to avoid - * racing with a concurrent i915_vma_unbind(). - */ - mutex_lock(&vma->vm->mutex); - do { - if (unlikely(bound & (I915_VMA_OVERFLOW | I915_VMA_ERROR))) { - pinned = false; - break; - } - - if (unlikely(flags & ~bound)) { - pinned = false; - break; - } - } while (!atomic_try_cmpxchg(&vma->flags, &bound, bound + 1)); - mutex_unlock(&vma->vm->mutex); - - return pinned; } static struct scatterlist * @@ -1215,7 +1190,6 @@ static int __i915_vma_get_pages(struct i915_vma *vma) { struct sg_table *pages; - int ret; /* * The vma->pages are only valid within the lifespan of the borrowed @@ -1248,18 +1222,16 @@ __i915_vma_get_pages(struct i915_vma *vma) break; } - ret = 0; if (IS_ERR(pages)) { - ret = PTR_ERR(pages); - pages = NULL; drm_err(&vma->vm->i915->drm, - "Failed to get pages for VMA view type %u (%d)!\n", - vma->ggtt_view.type, ret); + "Failed to get pages for VMA view type %u (%ld)!\n", + vma->ggtt_view.type, PTR_ERR(pages)); + return PTR_ERR(pages); } vma->pages = pages; - return ret; + return 0; } I915_SELFTEST_EXPORT int i915_vma_get_pages(struct i915_vma *vma) @@ -1291,25 +1263,14 @@ I915_SELFTEST_EXPORT int i915_vma_get_pages(struct i915_vma *vma) static void __vma_put_pages(struct i915_vma *vma, unsigned int count) { /* We allocate under vma_get_pages, so beware the shrinker */ - struct sg_table *pages = READ_ONCE(vma->pages); - GEM_BUG_ON(atomic_read(&vma->pages_count) < count); if (atomic_sub_return(count, &vma->pages_count) == 0) { - /* - * The atomic_sub_return is a read barrier for the READ_ONCE of - * vma->pages above. - * - * READ_ONCE is safe because this is either called from the same - * function (i915_vma_pin_ww), or guarded by vma->vm->mutex. - * - * TODO: We're leaving vma->pages dangling, until vma->obj->resv - * lock is required. - */ - if (pages != vma->obj->mm.pages) { - sg_free_table(pages); - kfree(pages); + if (vma->pages != vma->obj->mm.pages) { + sg_free_table(vma->pages); + kfree(vma->pages); } + vma->pages = NULL; i915_gem_object_unpin_pages(vma->obj); } -- 2.34.1