Re: [PATCH 1/6] drm/panthor: Driver-wide xxx_[un]lock -> [scoped_]guard replacement

[Boris Brezillon <boris.brezillon@collabora.com>]

On Thu, 14 May 2026 10:09:10 -0700
Chia-I Wu <olvaffe@gmail.com> wrote:

> On Thu, May 14, 2026 at 6:24 AM Steven Price <steven.price@arm.com> wrote:
> >
> > On 13/05/2026 17:58, Boris Brezillon wrote:  
> > > Right now panthor is mixed bag of manual locks and guards. Let's
> > > make that more consitent and thus encourage new submissions to go
> > > for guards.  
> >
> > I'm fine with encouraging guards for future code - but I'm a little wary
> > of a big change like this - it's hard to review it and check that
> > everything works the same. And it's a little dubious that the mechanical
> > refactoring produces more readable code in some cases.  
> I agree with Steven in general, although I am in favor of landing now
> that you've gone through the trouble.

Honestly, I agree with you. The only reason I went for it is
because the mix we have right now is pretty confusing. This has to do
with the fact the scopes are often loosely defined unless you used
scoped_guard(), so it's pretty easy to mess up the lock/unlock
ordering. For instance,

	mutex_lock(locka);
	guard(lockb);
	mutex_unlock(locka);

	...

once expanded, turns into inconsistent locked sections, where the inner
lock (lockb) is released after the outer one (locka).

> 
> I also have mixed feelings about some of the non-scoped guards. Their
> scopes are extended slightly than before, supposedly to avoid adding
> another level of indentation. But other than slightly slower,

I tried to used scoped_guard()s every where the extra non-guarded
section could be CPU heavy (the only bits left are some very simple
bit/arithmetic ops, and a couple queue_work() IIRC).

> it also
> becomes less clear what exactly do the guards protect.

I know, and I have pretty much the same feeling, but we've crossed that
bridge when we started accepting non-scoped guard()s, unfortunately.