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

[wangxuewen]

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@xxxxxxx> 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@xxxxxxxxxx>
> > ---
> >   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
>
> [...]