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

Simona Vetter simona.vetter at ffwll.ch
Wed May 28 09:13:04 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.

Aside: amdgpu_gem_force_release() looks questionable and should
probably be revisited in the light of the revised hotunplug design
we're aiming for. But that's an entirely separate can of worms.

Cc: Alex Deucher <alexander.deucher at amd.com>
Cc: "Christian König" <christian.koenig at amd.com>
Cc: Arvind Yadav <Arvind.Yadav at amd.com>
Cc: Shashank Sharma <shashank.sharma at amd.com>
Cc: Simona Vetter <simona.vetter at ffwll.ch>
Cc: Yunxiang Li <Yunxiang.Li at amd.com>
Cc: Frank Min <Frank.Min at amd.com>
Cc: Kent Russell <kent.russell at amd.com>
Signed-off-by: Simona Vetter <simona.vetter at intel.com>
Signed-off-by: Simona Vetter <simona.vetter at ffwll.ch>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 2c68118fe9fd..90723b13fa7d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -249,6 +249,7 @@ void amdgpu_gem_force_release(struct amdgpu_device *adev)
 
 		WARN_ONCE(1, "Still active user space clients!\n");
 		spin_lock(&file->table_lock);
+		/* FIXME: Use idr_for_each to handle transient NULL pointers */
 		idr_for_each_entry(&file->object_idr, gobj, handle) {
 			WARN_ONCE(1, "And also active allocations!\n");
 			drm_gem_object_put(gobj);
@@ -1167,6 +1168,7 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m, void *unused)
 		rcu_read_unlock();
 
 		spin_lock(&file->table_lock);
+		/* FIXME: Use idr_for_each to handle transient NULL pointers */
 		idr_for_each_entry(&file->object_idr, gobj, id) {
 			struct amdgpu_bo *bo = gem_to_amdgpu_bo(gobj);
 
-- 
2.49.0



More information about the dri-devel mailing list