Re: [PATCH] fs/resctrl: using guard to simplify lock/unlock code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2025/6/23 23:14, Dave Martin wrote:
Hi,

On Mon, Jun 23, 2025 at 07:25:41PM +0800, Su Hui wrote:
Using guard() to replace *unlock* label. guard() is better than goto
unlock patterns and is more concise. No functional changes.

Signed-off-by: Su Hui <suhui@xxxxxxxxxxxx>
How were these cases chosen?
I chosen the code  that match with "*unlock*:" label.

I notice that this patch only handles some straightforward mutex_unlock()
cases.  There are other opportunities in some places -- particularly
alloc/free patterns.
Yes, as Dan mentioned[1], there are too many these patterns and I'm not sure how much value we can get to do this things. This patch is a try that using guard() to
remove some  lock/unlock pattern and simplify the lock code.
Overall, I'm not totally convinced that backporting the guard()
infrastructure into code that wasn't originally written to use it is
always worthwhile.

If the code is simple, there is not much benefit.  Otherwise, there is
a significant chance of unintentionally changing the behaviour of the
code (though the exercise may be useful if it identifies actual bugs).

Either way, such changes will get in the way of people who are rebasing
on top of this code.
Got it, it's ok to omit this patch. It seems this patch has not enough value.
FWIW, this patch looks OK though, and the diffstat looks reasonable.
Since this code was recently moved into fs/, diff context noise may be
less of a concern.
Maybe only for some complex lock/unlock code, guard() can bring some value.
Thanks for your advice!

[1] https://lore.kernel.org/all/d07fe2d9-3548-43fc-b430-2947eee3145b@suswa.mountain/

Regards,
Su Hui






[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux