[PATCH 8/8] drm/xe: Add comments about drm_file.object_idr issues

Simona Vetter simona.vetter at ffwll.ch
Wed May 28 09:13:06 UTC 2025


idr_for_each_entry() is fine, but will prematurely terminate on
transient NULL entries. It should be switched over to idr_for_each,
which allows you to handle this explicitly.

Note that transient NULL pointers in drm_file.object_idr have been a
thing since f6cd7daecff5 ("drm: Release driver references to handle
before making it available again"), this is a really old issue.

Since it's just a premature loop terminate the impact should be fairly
benign, at least for any debugfs or fdinfo code.

On top of that this code also drops the drm_file.table_lock lock while
iterating, which can mess up the iterator state. And that's actually
bad.

Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
Signed-off-by: Simona Vetter <simona.vetter at ffwll.ch>
Cc: Lucas De Marchi <lucas.demarchi at intel.com>
Cc: "Thomas Hellström" <thomas.hellstrom at linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
Cc: intel-xe at lists.freedesktop.org
---
 drivers/gpu/drm/xe/xe_drm_client.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c
index 31f688e953d7..2542f265a221 100644
--- a/drivers/gpu/drm/xe/xe_drm_client.c
+++ b/drivers/gpu/drm/xe/xe_drm_client.c
@@ -205,6 +205,7 @@ static void show_meminfo(struct drm_printer *p, struct drm_file *file)
 
 	/* Public objects. */
 	spin_lock(&file->table_lock);
+	/* FIXME: Use idr_for_each to handle transient NULL pointers */
 	idr_for_each_entry(&file->object_idr, obj, id) {
 		struct xe_bo *bo = gem_to_xe_bo(obj);
 
@@ -213,6 +214,8 @@ static void show_meminfo(struct drm_printer *p, struct drm_file *file)
 			xe_bo_unlock(bo);
 		} else {
 			xe_bo_get(bo);
+			/* FIXME: dropping the lock can mess the idr iterator
+			 * state up */
 			spin_unlock(&file->table_lock);
 
 			xe_bo_lock(bo, false);
-- 
2.49.0



More information about the dri-devel mailing list