From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: biren pandya <birenpandya@gmail.com>
Cc: maarten.lankhorst@linux.intel.com, mripard@kernel.org,
tzimmermann@suse.de, airlied@gmail.com, simona@ffwll.ch,
sumit.semwal@linaro.org, christian.koenig@amd.com,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org
Subject: Re: [PATCH] drm/gem: modernize locks to use scoped_guard()
Date: Mon, 22 Jun 2026 16:01:50 +0300 [thread overview]
Message-ID: <20260622130150.GA3948118@killaraus.ideasonboard.com> (raw)
In-Reply-To: <CAAeMi9LMootkxmRE0tjo0mwijAuA0u+49HhxWxRGU-pSz+Cggw@mail.gmail.com>
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
next prev parent reply other threads:[~2026-06-22 13:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-16 18:19 [PATCH] drm/gem: modernize locks to use scoped_guard() Biren Pandya
2026-06-22 11:55 ` Laurent Pinchart
2026-06-22 12:52 ` biren pandya
2026-06-22 13:01 ` Laurent Pinchart [this message]
2026-06-22 13:39 ` Thomas Zimmermann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260622130150.GA3948118@killaraus.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=airlied@gmail.com \
--cc=birenpandya@gmail.com \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linaro-mm-sig@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=simona@ffwll.ch \
--cc=sumit.semwal@linaro.org \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox