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
next prev 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