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: 23+ 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:18 ` ✗ Fi.CI.CHECKPATCH: " Patchwork
2024-06-14 14:18 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-06-14 14:28 ` ✗ CI.BAT: failure " Patchwork
2024-06-14 14:32 ` ✓ Fi.CI.BAT: success " Patchwork
2024-06-16 21:28 ` ✗ Fi.CI.IGT: 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.