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 <[email protected]>
Signed-off-by: Simona Vetter <[email protected]>
Cc: Lucas De Marchi <[email protected]>
Cc: "Thomas Hellström" <[email protected]>
Cc: Rodrigo Vivi <[email protected]>
Cc: [email protected]
---
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
