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

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

[Biren Pandya]

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.

Signed-off-by: Biren Pandya <birenpandya@gmail.com>

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);
 
-	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
-- 
2.50.1 (Apple Git-155)

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

[Laurent Pinchart]

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@gmail.com>
> 
> 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

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

[biren pandya]

On Mon, Jun 22, 2026 at 5:25 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> 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
> ?

Hi Laurent,

Thanks for taking a look.
No deeper reason than it being where I happened to start — I didn't
mean to single it out, and I'd rather the treatment be consistent than
piecemeal.

> > @@ -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 ?

Fair point — the spinlocks here are equally good candidates; I only
kept v1 to mutexes to keep it small.

That said, this is a pure cleanup with no functional change, so it's
entirely your call whether it's worth carrying.
If you'd like it, I'll send a v2 that converts both the mutexes and
the spinlocks in drm_gem.c consistently. If you'd prefer not to take
cleanup-only churn, I'm happy to drop it — no problem either way.

Thanks,
Biren

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

[Laurent Pinchart]

On Mon, Jun 22, 2026 at 06:22:27PM +0530, biren pandya wrote:
> On Mon, Jun 22, 2026 at 5:25 PM Laurent Pinchart wrote:
> > 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
> > ?
> 
> Hi Laurent,
> 
> Thanks for taking a look.
> No deeper reason than it being where I happened to start — I didn't
> mean to single it out, and I'd rather the treatment be consistent than
> piecemeal.
> 
> > > @@ -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 ?
> 
> Fair point — the spinlocks here are equally good candidates; I only
> kept v1 to mutexes to keep it small.
> 
> That said, this is a pure cleanup with no functional change, so it's
> entirely your call whether it's worth carrying.
> If you'd like it, I'll send a v2 that converts both the mutexes and
> the spinlocks in drm_gem.c consistently. If you'd prefer not to take
> cleanup-only churn, I'm happy to drop it — no problem either way.

I'll let the maintainers of that code decide (I'm not one of them).

-- 
Regards,

Laurent Pinchart

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

[Thomas Zimmermann]

Hi

Am 16.06.26 um 20:19 schrieb Biren Pandya:
> 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.
>
> Signed-off-by: Biren Pandya <birenpandya@gmail.com>
>
> 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,

There have been a number of revisions to this function recently and you 
want to make sure that you have the latest.  At least the code in v7.1 
looks different from yours.

>   		goto out;
>   	}
>   
> -	mutex_lock(&file_priv->prime.lock);
> +	scoped_guard(mutex, &file_priv->prime.lock) {
> +		spin_lock(&file_priv->table_lock);

Could this spin lock also use a guard?

Best regards
Thomas

> +		ret = idr_alloc(&file_priv->object_idr, obj, handle, handle + 1,
> +				GFP_NOWAIT);
> +		spin_unlock(&file_priv->table_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);
> +		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

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)