Re: [PATCH] mm/shrinker: do not hold RCU lock in shrinker_debugfs_count_show()

[Krzysztof Kozlowski]

On 25/08/2026 15:54, Nick Huang wrote:
> On Tue, Aug 25, 2026 at 08:12:09AM +0200, Krzysztof Kozlowski wrote:
>> On 24/08/2026 15:42, Nick Huang wrote:
>>> On Wed, Jun 10, 2026 at 04:20:48PM -0700, Shakeel Butt wrote:
>>>> Reading the debugfs "count" file of a memcg-aware shrinker can sleep
>>>> inside an RCU read-side critical section:
>>>>
>>>>   BUG: sleeping function called from invalid context at kernel/cgroup/rstat.c:421
>>>>   RCU nest depth: 1, expected: 0
>>>>    css_rstat_flush
>>>>    mem_cgroup_flush_stats
>>>>    zswap_shrinker_count
>>>>    shrinker_debugfs_count_show
>>>>
>>>> shrinker_debugfs_count_show() invokes the ->count_objects() callback
>>>> under rcu_read_lock(). The zswap callback flushes memcg stats via
>>>> css_rstat_flush(), which may sleep, so it must not run under RCU.
>>>>
>>>> The RCU lock is not needed here. mem_cgroup_iter() takes RCU internally
>>>> and returns a memcg holding a css reference (dropped on the next
>>>> iteration or by mem_cgroup_iter_break()), so the memcg stays alive
>>>> without it. The shrinker is kept alive by the open debugfs file:
>>>> shrinker_free() removes the debugfs entries via
>>>> debugfs_remove_recursive(), which waits for in-flight readers to drain,
>>>> before call_rcu(..., shrinker_free_rcu_cb). The sibling "scan" handler
>>>> already invokes the sleeping ->scan_objects() callback with no RCU
>>>> section.
>>>>
>>>> Drop the rcu_read_lock()/rcu_read_unlock().
>>>>
>>>> Fixes: 5035ebc644ae ("mm: shrinkers: introduce debugfs interface for memory shrinkers")
>>>> Reported-by: Zenghui Yu <zenghui.yu@xxxxxxxxx>
>>>> Closes: https://lore.kernel.org/all/c052a064-cddb-494f-a0d8-f8a10b4b1c4d@xxxxxxxxx/
>>>> Suggested-by: Nhat Pham <nphamcs@xxxxxxxxx>
>>>> Signed-off-by: Shakeel Butt <shakeel.butt@xxxxxxxxx>
>>>> ---
>>>>  mm/shrinker_debug.c | 4 ----
>>>>  1 file changed, 4 deletions(-)
>>>>
>>>> diff --git a/mm/shrinker_debug.c b/mm/shrinker_debug.c
>>>> index affa64437302..cda4e86428c8 100644
>>>> --- a/mm/shrinker_debug.c
>>>> +++ b/mm/shrinker_debug.c
>>>> @@ -57,8 +57,6 @@ static int shrinker_debugfs_count_show(struct seq_file *m, void *v)
>>>>  	if (!count_per_node)
>>>>  		return -ENOMEM;
>>>>  
>>>> -	rcu_read_lock();
>>>> -
>>>>  	memcg_aware = shrinker->flags & SHRINKER_MEMCG_AWARE;
>>>>  
>>>>  	memcg = mem_cgroup_iter(NULL, NULL, NULL);
>>>> @@ -88,8 +86,6 @@ static int shrinker_debugfs_count_show(struct seq_file *m, void *v)
>>>>  		}
>>>>  	} while ((memcg = mem_cgroup_iter(NULL, memcg, NULL)) != NULL);
>>>>  
>>>> -	rcu_read_unlock();
>>>> -
>>>>  	kfree(count_per_node);
>>>>  	return ret;
>>>>  }
>>>> -- 
>>>> 2.53.0-Meta
>>>>
>>> Reviewed-by: Nick Huang <sef1548@xxxxxxxxx>
>>
>> Sorry, but what exactly did you review here? In the past you were
>> sending AI slop:
>> https://lore.kernel.org/all/aeNGbNyPxJssnkbO@lucifer/
>>
>> Not even properly following the process.
>> https://lore.kernel.org/all/20260724143408.175870-1-sef1548@xxxxxxxxx/
>>
>> https://lore.kernel.org/all/?q=f%3Asef1548%40gmail.com
>>
>> Why suddenly reviewing MM patches?
>>
>> Best regards,
>> Krzysztof
> 
> Hi Krzysztof
> 
> I saw this patch on the mailing list and was interested in the RCU issue, so I looked at the related code.
> I checked that removing the outer RCU read-side critical section doesn't break the memcg lifetime, 
> since mem_cgroup_iter() returns the memcg with a CSS reference held.
> I should have explained what I checked instead of just adding a Reviewed-by.
> Thanks for pointing that out.

After all the maintainers already provided a review...

Best regards,
Krzysztof