Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Zimmermann <tzimmermann@suse.de>
To: maarten.lankhorst@linux.intel.com, mripard@kernel.org,
	airlied@gmail.com, daniel@ffwll.ch, lucas.demarchi@intel.com,
	rodrigo.vivi@intel.com, jani.nikula@linux.intel.com,
	ray.huang@amd.com, christian.koenig@amd.com, kraxel@redhat.com,
	airlied@redhat.com, suijingfeng@loongson.cn
Cc: dri-devel@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org,
	Thomas Zimmermann <tzimmermann@suse.de>
Subject: [PATCH 5/6] drm/xe: Remove vunmap calls object-freeing code
Date: Fri, 14 Jun 2024 15:21:59 +0200	[thread overview]
Message-ID: <20240614133556.11378-6-tzimmermann@suse.de> (raw)
In-Reply-To: <20240614133556.11378-1-tzimmermann@suse.de>

Move calls to unmap the buffer-object memory from the object-release
code in xe_gem_object_free() to the caller of the release.

Doing an unmap for a BO requires holding the reservation lock, which
is not allowed while releasing a GEM object. Without the reservation
lock, TTM can concurrently evict the buffer object that is to be
released; making the mapping invalid.

Pushing the unmap calls, namely xe_bo_vunmap(), releases the mapped
pages before the buffer object.

While at it, add a warning about buffer mappings to the GEM-object
release code. The warning message and unmap code can be removed after
the driver has been audited to not release mapped buffer objects.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/xe/display/intel_fb_bo.c | 12 +++++++-----
 drivers/gpu/drm/xe/xe_bo.c               | 14 +++++++++++++-
 drivers/gpu/drm/xe/xe_bo.h               | 23 ++++++++++++-----------
 drivers/gpu/drm/xe/xe_lrc.c              |  1 +
 4 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/xe/display/intel_fb_bo.c b/drivers/gpu/drm/xe/display/intel_fb_bo.c
index f835492f73fb4..cf2720dbdd51f 100644
--- a/drivers/gpu/drm/xe/display/intel_fb_bo.c
+++ b/drivers/gpu/drm/xe/display/intel_fb_bo.c
@@ -12,12 +12,14 @@
 
 void intel_fb_bo_framebuffer_fini(struct xe_bo *bo)
 {
-	if (bo->flags & XE_BO_FLAG_PINNED) {
-		/* Unpin our kernel fb first */
-		xe_bo_lock(bo, false);
+	xe_bo_lock(bo, false);
+	xe_bo_vunmap(bo);
+
+	/* Unpin our kernel fb first */
+	if (bo->flags & XE_BO_FLAG_PINNED)
 		xe_bo_unpin(bo);
-		xe_bo_unlock(bo);
-	}
+
+	xe_bo_unlock(bo);
 	xe_bo_put(bo);
 }
 
diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index 2bae01ce4e5b9..a98d857ff1e5a 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -1093,6 +1093,19 @@ static void xe_ttm_bo_destroy(struct ttm_buffer_object *ttm_bo)
 
 static void xe_gem_object_free(struct drm_gem_object *obj)
 {
+	struct xe_bo *bo = gem_to_xe_bo(obj);
+
+	/*
+	 * Trying to free the object with a mapping in place. Resolve
+	 * this warning by calling xe_bo_vunmap() in the code that leads
+	 * ot this object release.
+	 *
+	 * TODO: Audit the driver to not release mapped buffer objects and
+	 *       then remove this block.
+	 */
+	if (drm_WARN_ON(obj->dev, !iosys_map_is_null(&bo->vmap)))
+		__xe_bo_vunmap(bo);
+
 	/* Our BO reference counting scheme works as follows:
 	 *
 	 * The gem object kref is typically used throughout the driver,
@@ -1106,7 +1119,6 @@ static void xe_gem_object_free(struct drm_gem_object *obj)
 	 * driver ttm callbacks is allowed to use the ttm_buffer_object
 	 * refcount directly if needed.
 	 */
-	__xe_bo_vunmap(gem_to_xe_bo(obj));
 	ttm_bo_put(container_of(obj, struct ttm_buffer_object, base));
 }
 
diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
index 6de894c728f54..5d1f8f13fbf13 100644
--- a/drivers/gpu/drm/xe/xe_bo.h
+++ b/drivers/gpu/drm/xe/xe_bo.h
@@ -170,17 +170,6 @@ static inline bool xe_bo_is_pinned(struct xe_bo *bo)
 	return bo->ttm.pin_count;
 }
 
-static inline void xe_bo_unpin_map_no_vm(struct xe_bo *bo)
-{
-	if (likely(bo)) {
-		xe_bo_lock(bo, false);
-		xe_bo_unpin(bo);
-		xe_bo_unlock(bo);
-
-		xe_bo_put(bo);
-	}
-}
-
 bool xe_bo_is_xe_bo(struct ttm_buffer_object *bo);
 dma_addr_t __xe_bo_addr(struct xe_bo *bo, u64 offset, size_t page_size);
 dma_addr_t xe_bo_addr(struct xe_bo *bo, u64 offset, size_t page_size);
@@ -202,6 +191,18 @@ xe_bo_ggtt_addr(struct xe_bo *bo)
 int xe_bo_vmap(struct xe_bo *bo);
 void xe_bo_vunmap(struct xe_bo *bo);
 
+static inline void xe_bo_unpin_map_no_vm(struct xe_bo *bo)
+{
+	if (likely(bo)) {
+		xe_bo_lock(bo, false);
+		xe_bo_vunmap(bo);
+		xe_bo_unpin(bo);
+		xe_bo_unlock(bo);
+
+		xe_bo_put(bo);
+	}
+}
+
 bool mem_type_is_vram(u32 mem_type);
 bool xe_bo_is_vram(struct xe_bo *bo);
 bool xe_bo_is_stolen(struct xe_bo *bo);
diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
index 3a68fe6d592ed..8a52797fe79bb 100644
--- a/drivers/gpu/drm/xe/xe_lrc.c
+++ b/drivers/gpu/drm/xe/xe_lrc.c
@@ -812,6 +812,7 @@ static void xe_lrc_finish(struct xe_lrc *lrc)
 {
 	xe_hw_fence_ctx_finish(&lrc->fence_ctx);
 	xe_bo_lock(lrc->bo, false);
+	xe_bo_vunmap(lrc->bo);
 	xe_bo_unpin(lrc->bo);
 	xe_bo_unlock(lrc->bo);
 	xe_bo_put(lrc->bo);
-- 
2.45.2


  parent reply	other threads:[~2024-06-14 13:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-14 13:21 [PATCH 0/6] drm/{ttm,xe}: Improve ttm_bo_vmap() and update xe Thomas Zimmermann
2024-06-14 13:21 ` [PATCH 1/6] iosys-map: Add allocator flags Thomas Zimmermann
2024-06-14 13:21 ` [PATCH 2/6] drm/ttm: Store the bo_kmap_type in struct iosys_map Thomas Zimmermann
2024-06-14 14:31   ` Christian König
2024-06-17 12:32     ` Thomas Zimmermann
2024-06-17 12:56       ` Christian König
2024-06-14 13:21 ` [PATCH 3/6] drm/ttm: Support partial buffer mappings for ttm_bo_vmap() Thomas Zimmermann
2024-06-14 14:33   ` Christian König
2024-06-17 13:00     ` Thomas Zimmermann
2024-06-14 13:21 ` [PATCH 4/6] drm/ttm: Support kmap for single-page mappings in ttm_bo_vmap() Thomas Zimmermann
2024-06-14 13:21 ` Thomas Zimmermann [this message]
2024-06-14 13:22 ` [PATCH 6/6] drm/xe: Replace ttm_bo_kmap() with ttm_bo_vmap() Thomas Zimmermann
2024-06-14 13:41 ` ✓ CI.Patch_applied: success for drm/{ttm,xe}: Improve ttm_bo_vmap() and update xe Patchwork
2024-06-14 13:41 ` ✗ CI.checkpatch: warning " Patchwork
2024-06-14 13:42 ` ✓ CI.KUnit: success " Patchwork
2024-06-14 13:54 ` ✓ CI.Build: " Patchwork
2024-06-14 13:56 ` ✗ CI.Hooks: failure " Patchwork
2024-06-14 13:58 ` ✗ CI.checksparse: warning " Patchwork
2024-06-14 14:28 ` ✗ CI.BAT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240614133556.11378-6-tzimmermann@suse.de \
    --to=tzimmermann@suse.de \
    --cc=airlied@gmail.com \
    --cc=airlied@redhat.com \
    --cc=christian.koenig@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=kraxel@redhat.com \
    --cc=lucas.demarchi@intel.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=ray.huang@amd.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=suijingfeng@loongson.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox