Re: [PATCH v4 1/6] mm/memcontrol: make lru_zone_size atomic and simplify sanity check

[Kairui Song]

On Wed, Sep 2, 2026 at 1:37 AM Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote:
>
> On Tue, Sep 01, 2026 at 01:20:33PM +0800, Kairui Song wrote:
> > On Tue, Sep 1, 2026 at 12:38 PM Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote:
> > >
> > > On Mon, Aug 31, 2026 at 02:43:31AM +0800, Kairui Song via B4 Relay wrote:
> > > > From: Kairui Song <kasong@xxxxxxxxxxx>
> > > >
> > > > commit ca707239e8a7 ("mm: update_lru_size warn and reset bad lru_size")
> > > > introduced a sanity check to catch memcg counter underflow, which was
> > > > more of a workaround for another bug: lru_zone_size is unsigned, so
> > > > underflow wraps it around and returns an enormously large number, then
> > > > the memcg shrinker loops almost forever as the calculated number of
> > > > folios to shrink is huge. That commit also checked if a zero value
> > > > matches the empty LRU list, so we have to hold the LRU lock, and
> > > > handle the positive and negative deltas separately.
> > > >
> > > > But later commit b4536f0c829c ("mm, memcg: fix the active list aging
> > > > for lowmem requests when memcg is enabled") already removed the LRU
> > > > emptiness check, so handling the deltas separately is no longer
> > > > needed. And if we just turn it into an atomic long, underflow isn't a
> > > > big issue either,
> > >
> > > Why atomic long and not just long?
> >
> > Long is fine too; I was just trying to be defensive. The original
> > check was a defensive design for leaks or races, and atomic long is
> > more immune to race conditions. See below for the other conditions.
>
> Unless we really need it, let's stay with "long" for now. As you mentioned below
> this is busy/hot path, no need to add overhead of atomic here.

OK, no problem.

> >
> > So checking it in the reader side is much lighter and cleaner code-wise.
>
> So, one motivation is cleaner code?

Right that's one of the motivition.

>
> >
> > Checking in the reader side also tolerates deferred accounting false
> > positives: if a folio is added to / removed from LRU but have the
> > accounting happens later, that's fine, but might cause a temporary
> > underflow (there shouldn't be such usage right now, but cleanly the
> > oldest commit I mentioned is trying to catch something). It should
> > just proceed without performing extra operations on the counter or
> > raising a warning on the updater side.
> >
> > The underflow won't affect anything besides the two readers mentioned
> > above (reclaimer and reparenting).
> >
> > Besides, I don't think warning on the writer side will help catch the
> > real leak either, it just adds overhead to a busy path. The folio
> > triggering the underflow is very unlikely to be the one leaking the
> > count. And if that leak is actually caused by a race, the atomic long
> > change should cure it.
>
> I think you are going and looking too much into the history of this warning. I
> think you just need to explain that the current warning in the update side
> already can't always catch the actual leak and is costly. Moving to read side
> make the code cleaner and warnings will be at the location where the consumers
> we care about are going to consume it. Also add couple of sentences on behavior
> difference it will have i.e. warning on flipping into negative vs we will
> continue to operate even in negative but only warn on negative consumption.
>
> Another question: why is this patch part of this series? Just some orthogonal
> cleanups or are they related?
>

These were all part of the MGLRU-FG prerequisite, so I sent them
together. I can split it out.