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

[wangxuewen <18810879172@163.com>]

Hi SeongJae,

Thank you for pointing out the patch changelog convention.
I appreciate the feedback and will make sure to follow the rule of 
placing version changelogs after the --- separator in all my future 
submissions, as well as adding links to previous versions.

Thanks,
Wang Xuewen

在 2026/5/14 9:12, SeongJae Park 写道:
> On Wed, 13 May 2026 15:52:14 +0800 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.
>>
>> v2:
>> - Drop unused 'ret' variable to avoid compiler warning (Muchun Song).
> 
> Patch changelog should go to commentary area [1].  Also adding links to
> previous versions would be helpful.
> 
>>
>> Signed-off-by: wangxuewen <wangxuewen@kylinos.cn>
>> ---
>>   mm/shrinker.c | 13 +++++--------
>>   1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/mm/shrinker.c b/mm/shrinker.c
>> index 76b3f750cf65..cb03fbecc75d 100644
>> --- a/mm/shrinker.c
>> +++ b/mm/shrinker.c
>> @@ -215,29 +215,26 @@ static DEFINE_IDR(shrinker_idr);
>>   
>>   static int shrinker_memcg_alloc(struct shrinker *shrinker)
>>   {
>> -	int id, ret = -ENOMEM;
>> +	int id;
>>   
>>   	if (mem_cgroup_disabled())
>>   		return -ENOSYS;
>>   	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;
> 
> Could this change the return value?  This code was always returning -ENOMEM
> before, but seems it can now return -ENOSPC?  Seems that doesn't matter,
> though.
> 
>>   
>>   	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;
>>   }
> 
> [1] https://docs.kernel.org/process/submitting-patches.html#commentary
> 
> 
> Thanks,
> SJ
> 
> [...]