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

[Nick Huang]

Krzysztof Kozlowski <krzk@xxxxxxxxxx> 於 2026年8月25日週二 下午9:56寫道：
>
> 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
Hi Krzysztof
Sorry, I misread it and didn't realize that the maintainers had
already reviewed it.
Thanks for pointing that out, and thanks for taking the time to respond.
-- 
Regards,
Nick Huang