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 <[email protected]> > > 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
