Re: [PATCH 3/3] drm/gem: Stop exposing the racy/unsafe drm_gem_lru_remove() helper

Steven Price
2026-05-06T15:40:39Z

On 06/05/2026 13:16, Boris Brezillon wrote:
> The only place where it's safe to call drm_gem_lru_remove() is when
> we know the drm_gem_object::lru field can't be concurrently updated,
> which we know is the case when the drm_gem_object is destroyed.
>
> Rather than trying to make that safe, let's kill the function and inline
> its content in drm_gem_object_release().
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> Reviewed-by: Chia-I Wu <olvaffe@gmail.com>

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
> drivers/gpu/drm/drm_gem.c | 49 +++++++++++++++++------------------------------
> include/drm/drm_gem.h | 1 -
> 2 files changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 97cf63de0112..b6df4f62f7b5 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1108,6 +1108,15 @@ drm_gem_release(struct drm_device *dev, struct drm_file *file_private)
> idr_destroy(&file_private->object_idr);
> }
>
> +static void
> +drm_gem_lru_remove_locked(struct drm_gem_object *obj)
> +{
> + obj->lru->count -= obj->size >> PAGE_SHIFT;
> + WARN_ON(obj->lru->count < 0);
> + list_del(&obj->lru_node);
> + obj->lru = NULL;
> +}
> +
> /**
> * drm_gem_object_release - release GEM buffer object resources
> * @obj: GEM buffer object
> @@ -1124,7 +1133,15 @@ drm_gem_object_release(struct drm_gem_object *obj)
> drm_gem_private_object_fini(obj);
>
> drm_gem_free_mmap_offset(obj);
> - drm_gem_lru_remove(obj);
> +
> + /* If the object refcount drops to zero, it means no one can change
> + * the LRU it's inserted into, so it's safe to dereference
> + * drm_gem_object::lru without the drm_gem_lru::lock held.
> + */
> + if (obj->lru) {
> + guard(mutex)(obj->lru->lock);
> + drm_gem_lru_remove_locked(obj);
> + }
> }
> EXPORT_SYMBOL(drm_gem_object_release);
>
> @@ -1552,36 +1569,6 @@ drm_gem_lru_init(struct drm_gem_lru *lru, struct mutex *lock)
> }
> EXPORT_SYMBOL(drm_gem_lru_init);
>
> -static void
> -drm_gem_lru_remove_locked(struct drm_gem_object *obj)
> -{
> - obj->lru->count -= obj->size >> PAGE_SHIFT;
> - WARN_ON(obj->lru->count < 0);
> - list_del(&obj->lru_node);
> - obj->lru = NULL;
> -}
> -
> -/**
> - * drm_gem_lru_remove - remove object from whatever LRU it is in
> - *
> - * If the object is currently in any LRU, remove it.
> - *
> - * @obj: The GEM object to remove from current LRU
> - */
> -void
> -drm_gem_lru_remove(struct drm_gem_object *obj)
> -{
> - struct drm_gem_lru *lru = obj->lru;
> -
> - if (!lru)
> - return;
> -
> - mutex_lock(lru->lock);
> - drm_gem_lru_remove_locked(obj);
> - mutex_unlock(lru->lock);
> -}
> -EXPORT_SYMBOL(drm_gem_lru_remove);
> -
> /**
> * drm_gem_lru_move_tail_locked - move the object to the tail of the LRU
> *
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 86f5846154f7..d527df98d142 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -611,7 +611,6 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
> u32 handle, u64 *offset);
>
> void drm_gem_lru_init(struct drm_gem_lru *lru, struct mutex *lock);
> -void drm_gem_lru_remove(struct drm_gem_object *obj);
> void drm_gem_lru_move_tail_locked(struct drm_gem_lru *lru, struct drm_gem_object *obj);
> void drm_gem_lru_move_tail(struct drm_gem_lru *lru, struct drm_gem_object *obj);
> unsigned long
>