Re: [PATCH] drm/gem: modernize locks to use scoped_guard()

From: Laurent Pinchart

Date: Mon Jun 22 2026 - 07:56:15 EST


On Tue, Jun 16, 2026 at 11:49:57PM +0530, Biren Pandya wrote:
> Several GEM core functions manually managed mutex_lock() and
> mutex_unlock() over single scopes or error paths. This adds boilerplate
> and carries the risk of lock leaks if error paths are refactored.
>
> Modernize these locks by deploying the <linux/cleanup.h> scoped_guard()
> macro. This ensures that the locks are reliably dropped when the block
> exits, cleanly removing goto out_unlock paths and tightening the
> lifecycle.

What's the reason for doing so in in drm_gem and not other areas in DRM
?

> Signed-off-by: Biren Pandya <birenpandya@xxxxxxxxx>
>
> Compiled locally, but requires IGT validation by the DRM CI.
> ---
> drivers/gpu/drm/drm_gem.c | 66 ++++++++++++++++++---------------------
> 1 file changed, 30 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 891c3bff5ae0..d3a061d42ba7 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -346,13 +346,13 @@ void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
> * we checked for a name.
> */
>
> - mutex_lock(&dev->object_name_lock);
> - if (--obj->handle_count == 0) {
> - drm_gem_object_handle_free(obj);
> - drm_gem_object_exported_dma_buf_free(obj);
> - final = true;
> + scoped_guard(mutex, &dev->object_name_lock) {
> + if (--obj->handle_count == 0) {
> + drm_gem_object_handle_free(obj);
> + drm_gem_object_exported_dma_buf_free(obj);
> + final = true;
> + }
> }
> - mutex_unlock(&dev->object_name_lock);
>
> if (final)
> drm_gem_object_put(obj);
> @@ -374,11 +374,8 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
> if (obj->funcs->close)
> obj->funcs->close(obj, file_priv);
>
> - mutex_lock(&file_priv->prime.lock);
> -
> - drm_prime_remove_buf_handle(&file_priv->prime, id);
> -
> - mutex_unlock(&file_priv->prime.lock);
> + scoped_guard(mutex, &file_priv->prime.lock)
> + drm_prime_remove_buf_handle(&file_priv->prime, id);
>
> drm_vma_node_revoke(&obj->vma_node, file_priv);
>
> @@ -1021,37 +1018,34 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data,
> goto out;
> }
>
> - mutex_lock(&file_priv->prime.lock);
> + scoped_guard(mutex, &file_priv->prime.lock) {
> + spin_lock(&file_priv->table_lock);
> + ret = idr_alloc(&file_priv->object_idr, obj, handle, handle + 1,
> + GFP_NOWAIT);
> + spin_unlock(&file_priv->table_lock);

And why don't you use guards for the spinlock as well ?

>
> - spin_lock(&file_priv->table_lock);
> - ret = idr_alloc(&file_priv->object_idr, obj, handle, handle + 1,
> - GFP_NOWAIT);
> - spin_unlock(&file_priv->table_lock);
> + if (ret < 0)
> + break;
>
> - if (ret < 0)
> - goto out_unlock;
> + if (obj->dma_buf) {
> + ret = drm_prime_add_buf_handle(&file_priv->prime, obj->dma_buf,
> + handle);
> + if (ret < 0) {
> + spin_lock(&file_priv->table_lock);
> + idr_remove(&file_priv->object_idr, handle);
> + spin_unlock(&file_priv->table_lock);
> + break;
> + }
>
> - if (obj->dma_buf) {
> - ret = drm_prime_add_buf_handle(&file_priv->prime, obj->dma_buf,
> - handle);
> - if (ret < 0) {
> - spin_lock(&file_priv->table_lock);
> - idr_remove(&file_priv->object_idr, handle);
> - spin_unlock(&file_priv->table_lock);
> - goto out_unlock;
> + drm_prime_remove_buf_handle(&file_priv->prime, args->handle);
> }
>
> - drm_prime_remove_buf_handle(&file_priv->prime, args->handle);
> - }
> -
> - ret = 0;
> -
> - spin_lock(&file_priv->table_lock);
> - idr_remove(&file_priv->object_idr, args->handle);
> - spin_unlock(&file_priv->table_lock);
> + ret = 0;
>
> -out_unlock:
> - mutex_unlock(&file_priv->prime.lock);
> + spin_lock(&file_priv->table_lock);
> + idr_remove(&file_priv->object_idr, args->handle);
> + spin_unlock(&file_priv->table_lock);
> + }
> out:
> drm_gem_object_put(obj);
>
>
> base-commit: 72c25183cac9bc584c9de21797a5883af44bcc7a

--
Regards,

Laurent Pinchart