[PATCH v1] mm/shrinker: simplify shrinker_memcg_alloc() using guard()

wangxuewen posted 1 patch 1 month ago
There is a newer version of this series
mm/shrinker.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
[PATCH v1] mm/shrinker: simplify shrinker_memcg_alloc() using guard()
Posted by wangxuewen 1 month ago
Use guard(mutex) to automatically handle shrinker_mutex locking and
unlocking in shrinker_memcg_alloc(). This removes the explicit
mutex_unlock() call, the goto-based error path, and the redundant
ret variable, resulting in cleaner and more concise code.

Signed-off-by: wangxuewen <wangxuewen@kylinos.cn>
---
 mm/shrinker.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/mm/shrinker.c b/mm/shrinker.c
index 76b3f750cf65..1274130323bf 100644
--- a/mm/shrinker.c
+++ b/mm/shrinker.c
@@ -222,22 +222,19 @@ static int shrinker_memcg_alloc(struct shrinker *shrinker)
 	if (mem_cgroup_kmem_disabled() && !(shrinker->flags & SHRINKER_NONSLAB))
 		return -ENOSYS;
 
-	mutex_lock(&shrinker_mutex);
+	guard(mutex)(&shrinker_mutex);
 	id = idr_alloc(&shrinker_idr, shrinker, 0, 0, GFP_KERNEL);
 	if (id < 0)
-		goto unlock;
+		return id;
 
 	if (id >= shrinker_nr_max) {
 		if (expand_shrinker_info(id)) {
 			idr_remove(&shrinker_idr, id);
-			goto unlock;
+			return -ENOMEM;
 		}
 	}
 	shrinker->id = id;
-	ret = 0;
-unlock:
-	mutex_unlock(&shrinker_mutex);
-	return ret;
+	return 0;
 }
 
 static void shrinker_memcg_remove(struct shrinker *shrinker)
-- 
2.25.1
Re: [PATCH v1] mm/shrinker: simplify shrinker_memcg_alloc() using guard()
Posted by kernel test robot 4 weeks ago
Hi wangxuewen,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v7.1-rc3]
[also build test WARNING on linus/master next-20260508]
[cannot apply to akpm-mm/mm-everything]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/wangxuewen/mm-shrinker-simplify-shrinker_memcg_alloc-using-guard/20260515-014803
base:   v7.1-rc3
patch link:    https://lore.kernel.org/r/20260512085546.368911-1-18810879172%40163.com
patch subject: [PATCH v1] mm/shrinker: simplify shrinker_memcg_alloc() using guard()
config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20260515/202605151100.lH0pRo2I-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260515/202605151100.lH0pRo2I-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605151100.lH0pRo2I-lkp@intel.com/

All warnings (new ones prefixed by >>):

   mm/shrinker.c: In function 'shrinker_memcg_alloc':
>> mm/shrinker.c:218:17: warning: unused variable 'ret' [-Wunused-variable]
     218 |         int id, ret = -ENOMEM;
         |                 ^~~


vim +/ret +218 mm/shrinker.c

96f7b2b9bbb1d68 Qi Zheng      2023-09-11  215  
48a7a0996a0089f Qi Zheng      2023-09-11  216  static int shrinker_memcg_alloc(struct shrinker *shrinker)
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  217  {
96f7b2b9bbb1d68 Qi Zheng      2023-09-11 @218  	int id, ret = -ENOMEM;
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  219  
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  220  	if (mem_cgroup_disabled())
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  221  		return -ENOSYS;
03375203e1da8f4 Michal Koutný 2026-02-25  222  	if (mem_cgroup_kmem_disabled() && !(shrinker->flags & SHRINKER_NONSLAB))
03375203e1da8f4 Michal Koutný 2026-02-25  223  		return -ENOSYS;
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  224  
5ce02a6f6282d95 wangxuewen    2026-05-12  225  	guard(mutex)(&shrinker_mutex);
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  226  	id = idr_alloc(&shrinker_idr, shrinker, 0, 0, GFP_KERNEL);
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  227  	if (id < 0)
5ce02a6f6282d95 wangxuewen    2026-05-12  228  		return id;
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  229  
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  230  	if (id >= shrinker_nr_max) {
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  231  		if (expand_shrinker_info(id)) {
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  232  			idr_remove(&shrinker_idr, id);
5ce02a6f6282d95 wangxuewen    2026-05-12  233  			return -ENOMEM;
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  234  		}
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  235  	}
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  236  	shrinker->id = id;
5ce02a6f6282d95 wangxuewen    2026-05-12  237  	return 0;
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  238  }
96f7b2b9bbb1d68 Qi Zheng      2023-09-11  239  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v1] mm/shrinker: simplify shrinker_memcg_alloc() using guard()
Posted by Andrew Morton 3 weeks, 3 days ago
On Fri, 15 May 2026 11:15:40 +0800 kernel test robot <lkp@intel.com> wrote:

> Hi wangxuewen,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on v7.1-rc3]
> [also build test WARNING on linus/master next-20260508]
> [cannot apply to akpm-mm/mm-everything]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/wangxuewen/mm-shrinker-simplify-shrinker_memcg_alloc-using-guard/20260515-014803
> base:   v7.1-rc3
> patch link:    https://lore.kernel.org/r/20260512085546.368911-1-18810879172%40163.com
> patch subject: [PATCH v1] mm/shrinker: simplify shrinker_memcg_alloc() using guard()
> config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20260515/202605151100.lH0pRo2I-lkp@intel.com/config)
> compiler: sh4-linux-gcc (GCC) 15.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260515/202605151100.lH0pRo2I-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202605151100.lH0pRo2I-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    mm/shrinker.c: In function 'shrinker_memcg_alloc':
> >> mm/shrinker.c:218:17: warning: unused variable 'ret' [-Wunused-variable]
>      218 |         int id, ret = -ENOMEM;
>          |                 ^~~

Something went wrong here - that patch *removes* this definition of
`ret'.

https://lore.kernel.org/20260513075214.2655710-1-18810879172@163.com
Re: [PATCH v1] mm/shrinker: simplify shrinker_memcg_alloc() using guard()
Posted by wangxuewen 3 weeks, 3 days ago
Hi all,

Thanks for the notice.
I have already removed the unused ret variable in my v2 patch to fix 
this compiler warning.This problem only exists in the old v1 version, 
and the v2 patch has fully resolved it.

v2 patch link:
https://lore.kernel.org/all/20260513075214.2655710-1-18810879172@163.com/

And this revised v2 has been merged into mm-new branch already.

Thanks,
Xuewen Wang

在 2026/5/19 8:36, Andrew Morton 写道:
> On Fri, 15 May 2026 11:15:40 +0800 kernel test robot <lkp@intel.com> wrote:
> 
>> Hi wangxuewen,
>>
>> kernel test robot noticed the following build warnings:
>>
>> [auto build test WARNING on v7.1-rc3]
>> [also build test WARNING on linus/master next-20260508]
>> [cannot apply to akpm-mm/mm-everything]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>
>> url:    https://github.com/intel-lab-lkp/linux/commits/wangxuewen/mm-shrinker-simplify-shrinker_memcg_alloc-using-guard/20260515-014803
>> base:   v7.1-rc3
>> patch link:    https://lore.kernel.org/r/20260512085546.368911-1-18810879172%40163.com
>> patch subject: [PATCH v1] mm/shrinker: simplify shrinker_memcg_alloc() using guard()
>> config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20260515/202605151100.lH0pRo2I-lkp@intel.com/config)
>> compiler: sh4-linux-gcc (GCC) 15.2.0
>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260515/202605151100.lH0pRo2I-lkp@intel.com/reproduce)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202605151100.lH0pRo2I-lkp@intel.com/
>>
>> All warnings (new ones prefixed by >>):
>>
>>     mm/shrinker.c: In function 'shrinker_memcg_alloc':
>>>> mm/shrinker.c:218:17: warning: unused variable 'ret' [-Wunused-variable]
>>       218 |         int id, ret = -ENOMEM;
>>           |                 ^~~
> 
> Something went wrong here - that patch *removes* this definition of
> `ret'.
> 
> https://lore.kernel.org/20260513075214.2655710-1-18810879172@163.com

Re: [PATCH v1] mm/shrinker: simplify shrinker_memcg_alloc() using guard()
Posted by Qi Zheng 1 month ago

On 5/12/26 4:55 PM, wangxuewen wrote:
> Use guard(mutex) to automatically handle shrinker_mutex locking and
> unlocking in shrinker_memcg_alloc(). This removes the explicit
> mutex_unlock() call, the goto-based error path, and the redundant
> ret variable, resulting in cleaner and more concise code.

I'm inclined to use guard() only for newly added code. If we were to
convert existing code, there would be countless places across the
kernel that need updating.
Re: [PATCH v1] mm/shrinker: simplify shrinker_memcg_alloc() using guard()
Posted by Muchun Song 1 month ago

> On May 12, 2026, at 16:55, wangxuewen <18810879172@163.com> wrote:
> 
> Use guard(mutex) to automatically handle shrinker_mutex locking and
> unlocking in shrinker_memcg_alloc(). This removes the explicit
> mutex_unlock() call, the goto-based error path, and the redundant
> ret variable, resulting in cleaner and more concise code.
> 
> Signed-off-by: wangxuewen <wangxuewen@kylinos.cn>
> ---
> mm/shrinker.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/shrinker.c b/mm/shrinker.c
> index 76b3f750cf65..1274130323bf 100644
> --- a/mm/shrinker.c
> +++ b/mm/shrinker.c
> @@ -222,22 +222,19 @@ static int shrinker_memcg_alloc(struct shrinker *shrinker)
> if (mem_cgroup_kmem_disabled() && !(shrinker->flags & SHRINKER_NONSLAB))
> return -ENOSYS;
> 
> - 	mutex_lock(&shrinker_mutex);
> + 	guard(mutex)(&shrinker_mutex);
> 	id = idr_alloc(&shrinker_idr, shrinker, 0, 0, GFP_KERNEL);
> 	if (id < 0)
> - 		goto unlock;
> + 	return id;
> 
> 	if (id >= shrinker_nr_max) {
> 		if (expand_shrinker_info(id)) {
> 			idr_remove(&shrinker_idr, id);
> - 			goto unlock;
> + 			return -ENOMEM;
> 		}
> 	}
> 	shrinker->id = id;
> - 	ret = 0;
> -unlock:
> - 	mutex_unlock(&shrinker_mutex);
> - 	return ret;

One small thing: since ret is no longer used after this change,
it should be dropped from the declaration to avoid an unused-variable
warning:

    -       int id, ret = -ENOMEM;
    +       int id;

Otherwise looks good to me.

Thanks,
Muchun

> + 	return 0;
> }
> 
> static void shrinker_memcg_remove(struct shrinker *shrinker)
> -- 
> 2.25.1
> 
Re: [PATCH v1] mm/shrinker: simplify shrinker_memcg_alloc() using guard()
Posted by wangxuewen 1 month ago
Hi,Muchun Song,

Thanks for your review.
I've updated the patch to v2 following your suggestions.

This change is only a local cleanup for this specific function, not part 
of any large-scale conversion to use the guard() mechanism.

Please review v2, thank you.

在 2026/5/12 17:10, Muchun Song 写道:
> 
> 
>> On May 12, 2026, at 16:55, wangxuewen <18810879172@163.com> wrote:
>>
>> Use guard(mutex) to automatically handle shrinker_mutex locking and
>> unlocking in shrinker_memcg_alloc(). This removes the explicit
>> mutex_unlock() call, the goto-based error path, and the redundant
>> ret variable, resulting in cleaner and more concise code.
>>
>> Signed-off-by: wangxuewen <wangxuewen@kylinos.cn>
>> ---
>> mm/shrinker.c | 11 ++++-------
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/mm/shrinker.c b/mm/shrinker.c
>> index 76b3f750cf65..1274130323bf 100644
>> --- a/mm/shrinker.c
>> +++ b/mm/shrinker.c
>> @@ -222,22 +222,19 @@ static int shrinker_memcg_alloc(struct shrinker *shrinker)
>> if (mem_cgroup_kmem_disabled() && !(shrinker->flags & SHRINKER_NONSLAB))
>> return -ENOSYS;
>>
>> - 	mutex_lock(&shrinker_mutex);
>> + 	guard(mutex)(&shrinker_mutex);
>> 	id = idr_alloc(&shrinker_idr, shrinker, 0, 0, GFP_KERNEL);
>> 	if (id < 0)
>> - 		goto unlock;
>> + 	return id;
>>
>> 	if (id >= shrinker_nr_max) {
>> 		if (expand_shrinker_info(id)) {
>> 			idr_remove(&shrinker_idr, id);
>> - 			goto unlock;
>> + 			return -ENOMEM;
>> 		}
>> 	}
>> 	shrinker->id = id;
>> - 	ret = 0;
>> -unlock:
>> - 	mutex_unlock(&shrinker_mutex);
>> - 	return ret;
> 
> One small thing: since ret is no longer used after this change,
> it should be dropped from the declaration to avoid an unused-variable
> warning:
> 
>      -       int id, ret = -ENOMEM;
>      +       int id;
> 
> Otherwise looks good to me.
> 
> Thanks,
> Muchun
> 
>> + 	return 0;
>> }
>>
>> static void shrinker_memcg_remove(struct shrinker *shrinker)
>> -- 
>> 2.25.1
>>