On Fri, Aug 23, 2024 at 02:18:21PM +0800, Li Zetao wrote: > Hi all, > > This patch set is dedicated to using scope-based resource management > functions to replace the direct use of lock/unlock methods, so that > developers can focus more on using resources in a certain scope and > avoid overly focusing on resource leakage issues. > > At the same time, some functions can remove the controversial goto > label(eg: patch 3), which usually only releases resources and then > exits the function. After replacement, these functions can exit > directly without worrying about resources not being released. > > This patch set has been tested by fsstress for a long time and no > problems were found. Hmm, I'm torn. I do like the simplification that these patches can offer. The potential downsides/problems that are worrying me: 1) The zero day test bot has flagged a number of warnings[1] [1] https://lore.kernel.org/r/202408290407.XQuWf1oH-lkp@xxxxxxxxx 2) The documentation for guard() and scoped_guard() is pretty sparse, and the comments in include/linux/cleanup.h are positively confusing. There is a real need for a tutorial which explains how they should be used in the Documentation directory, or maybe a LWN.net article. Still, after staring that the implementation, I was able to figure it out, but I'm bit worried that people who aren't familiar with this construt which appears to have laned in August 2023, might find the code less readable. 3) Once this this lands, I could see potential problems when bug fixes are backported to stable kernels older than 6.6, since this changes how lock and unlock calls in the ext4 code. So unless include/linux/cleanup.h is backported to all of the LTS kernels, as well as these ext4 patches, there is a ris that a future (possibly security) bug fix will result in a missing unlock leading to hilarity and/or sadness. I'm reminded of the story of XFS changing the error return semantics from errno to -errno, and resulting bugs when patches were automatically backported to the stable kernels leading to real problems, which is why XFS opted out of LTS backports. This patch series could have the same problem.... and I haven't been able to recruit someone to be the ext4 stable kernel maintainers who could monitor xfstests resullts with lockdep enabled to catch potential problems. That being said, I do see the value of the change What do other ext4 developers think? - Ted
