Re: [PATCH -next 1/5] mm/mglru: use mem_cgroup_iter for global reclaim

Shakeel Butt
2025-12-22T21:18:30Z

On Mon, Dec 22, 2025 at 03:27:26PM +0800, Chen Ridong wrote:
>
[...]
>
> >> - if (should_abort_scan(lruvec, sc))
> >> + if (should_abort_scan(lruvec, sc)) {
> >> + if (cookie)
> >> + mem_cgroup_iter_break(target, memcg);
> >> break;
> >
> > This seems buggy as we may break the loop without calling
> > mem_cgroup_iter_break(). I think for kswapd the cookie will be NULL and
> > if should_abort_scan() returns true, we will break the loop without
> > calling mem_cgroup_iter_break() and will leak a reference to memcg.
> >
>
> Thank you for catching that—my mistake.
>
> This also brings up another point: In kswapd, the traditional LRU iterates through all memcgs, but
> stops for the generational LRU (GENLRU) when should_abort_scan is met (i.e., enough pages are
> reclaimed or the watermark is satisfied). Shouldn't both behave consistently?
>
> Perhaps we should add should_abort_scan(lruvec, sc) in shrink_node_memcgs for the traditional LRU as
> well?

We definitely should discuss about should_abort_scan() for traditional
reclaim but to keep things simple, let's do that after this series. For
now, follow Johannes' suggestion of lru_gen_should_abort_scan().