Re: [PATCH V2 3/4] mm: shrinker: optimize the allocation of shrinker_info when setting cgroup_memory_nokmem

[Haifeng Xu <haifeng.xu@shopee.com>]

On 2026/3/10 19:05, Usama Arif wrote:
> On Tue, 10 Mar 2026 11:12:49 +0800 Haifeng Xu <haifeng.xu@shopee.com> wrote:
> 
>> When kmem is disabled, memcg slab shrink only call non-slab shrinkers,
>> so just allocates shrinker info for non-slab shrinkers to non-root memcgs.
>>
>> Therefore, if memcg_kmem_online is true, all things keep same as before.
>> Otherwise, root memcg allocates id from shrinker_idr to identify each
>> shrinker and non-root memcgs use nonslab_id to identify non-slab shrinkers.
>> The size of shrinkers_info in non-root memcgs can be very low because the
>> number of shrinkers marked as SHRINKER_NONSLAB | SHRINKER_MEMCG_AWARE is
>> few. Also, the time spending in expand_shrinker_info() can reduce a lot.
>>
>> When setting shrinker bit or updating nr_deferred, use nonslab_id for
>> non-root memcgs if the shrinker is marked as SHRINKER_NONSLAB.
>>
>> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
>> ---
>>  include/linux/memcontrol.h |   8 ++-
>>  include/linux/shrinker.h   |   3 +
>>  mm/shrinker.c              | 116 +++++++++++++++++++++++++++++++++----
>>  3 files changed, 114 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
>> index ce7b5101bc02..3edd6211aed2 100644
>> --- a/include/linux/memcontrol.h
>> +++ b/include/linux/memcontrol.h
>> @@ -1804,7 +1804,13 @@ void reparent_shrinker_deferred(struct mem_cgroup *memcg);
>>  
>>  static inline int shrinker_id(struct mem_cgroup *memcg, struct shrinker *shrinker)
>>  {
>> -	return shrinker->id;
>> +	int id = shrinker->id;
>> +
>> +	if (!memcg_kmem_online() && (shrinker->flags & SHRINKER_NONSLAB) &&
>> +	    memcg != root_mem_cgroup)
>> +		id = shrinker->nonslab_id;
>> +
>> +	return id;
>>  }
>>  #else
>>  #define mem_cgroup_sockets_enabled 0
>> diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
>> index 1a00be90d93a..df53008ed8b5 100644
>> --- a/include/linux/shrinker.h
>> +++ b/include/linux/shrinker.h
>> @@ -107,6 +107,9 @@ struct shrinker {
>>  #ifdef CONFIG_MEMCG
>>  	/* ID in shrinker_idr */
>>  	int id;
>> +
>> +	/* ID in shrinker_nonslab_idr */
>> +	int nonslab_id;
>>  #endif
>>  #ifdef CONFIG_SHRINKER_DEBUG
>>  	int debugfs_id;
>> diff --git a/mm/shrinker.c b/mm/shrinker.c
>> index 61dbb6afae52..68ea2d49495c 100644
>> --- a/mm/shrinker.c
>> +++ b/mm/shrinker.c
>> @@ -12,6 +12,7 @@ DEFINE_MUTEX(shrinker_mutex);
>>  
>>  #ifdef CONFIG_MEMCG
>>  static int shrinker_nr_max;
>> +static int shrinker_nonslab_nr_max;
>>  
>>  static inline int shrinker_unit_size(int nr_items)
>>  {
>> @@ -78,15 +79,25 @@ int alloc_shrinker_info(struct mem_cgroup *memcg)
>>  {
>>  	int nid, ret = 0;
>>  	int array_size = 0;
>> +	int alloc_nr_max;
>> +
>> +	if (memcg_kmem_online()) {
>> +		alloc_nr_max = shrinker_nr_max;
>> +	} else {
>> +		if (memcg == root_mem_cgroup)
>> +			alloc_nr_max = shrinker_nr_max;
>> +		else
>> +			alloc_nr_max = shrinker_nonslab_nr_max;
>> +	}
>>  
>>  	mutex_lock(&shrinker_mutex);
> 
> Hello!
> 
> The patch reads shrinker_nonslab_nr_max and shrinker_nr_max before
> acquiring shrinker_mutex. The original code read shrinker_nr_max
> UNDER the lock. Both variables are modified under shrinker_mutex by
> concurrent shrinker registrations. A stale read could cause
> alloc_shrinker_info() to allocate an undersized shrinker_info,
> leading to out-of-bounds access in set_shrinker_bit() or the
> nr_deferred functions.
> 

Yes，thanks for pointing out the problems.