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

[Nick Huang <sef1548@gmail.com>]

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@linux.dev>
> >> Closes: https://lore.kernel.org/all/c052a064-cddb-494f-a0d8-f8a10b4b1c4d@linux.dev/
> >> Suggested-by: Nhat Pham <nphamcs@gmail.com>
> >> Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
> >> ---
> >>  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@gmail.com>
> 
> 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@gmail.com/
> 
> 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.

Regards,
Nick Huang