Re: [PATCH v3 2/3] mm: workingset: use lruvec_page_state_local() to count lru pages

[Hui Zhu]

> On Fri,  4 Sep 2026 17:45:55 +0800 "Hui Zhu" <hui.zhu@xxxxxxxxx> wrote:
>
> > From: Hui Zhu <zhuhui@xxxxxxxxxx>
> >
> > Commit 7404bd37cfbe ("mm: workingset: use lruvec_lru_size() to get the
> > number of lru pages") switched count_shadow_nodes() to lruvec_lru_size().
> > With CONFIG_MEMCG enabled, lruvec_lru_size() reads mz->lru_zone_size,
> > which only the classic LRU paths maintain.  MGLRU accounts its pages
> > through __update_lru_size(), which skips that array, so with MGLRU on the
> > four evictable LRU lists are always seen as empty.  The shadow node budget
> > (pages >> 3) then collapses to slab plus unevictable pages, and the
> > workingset shadow shrinker reclaims eviction tokens almost as fast as they
> > are created, losing thrashing protection.
> >
> > lruvec_page_state_local() reads lruvec_stats->state_local instead, which
> > both classic LRU and MGLRU maintain.  Switch back to it.  The reparenting
> > race this re-exposes on cgroup v2 is closed by the follow-up patch that
> > redirects dying-memcg stat updates for all hierarchies.
> The follow-up patch is "mm: memcg: skip the RCU lock when the memcg is
> not dying"?  But that's an optimization so I'm confused.

Sorry for the confusion - the commit message still says "follow-up
patch" because that was the position in v2.  In v3 the series was
reordered as requested, so the patch that closes the race, "mm: memcg:
redirect stats updates of dying memcgs for all hierarchies", is now
patch 1 and comes before this one.  Patch 3 is indeed only an
optimization.  v4 will reword this sentence to:

The reparenting race this re-exposes on cgroup v2 is closed by the
preceding patch that redirects dying-memcg stat updates for all
hierarchies.

> If we're re-exposing a race, the fix for that race should have the same
> Fixes: and cc:stable as the commit which did the reexposure?

Yes, and it does.  Patch 1 carries exactly the same tags as this
patch:
Fixes: 7404bd37cfbe ("mm: workingset: use lruvec_lru_size() to get the 
number of lru pages")
Cc: stable@xxxxxxxxxxxxxxx
Patch 3 has no tags because it only recovers the fast-path overhead
that patch 1 adds; it fixes no bug by itself.

> It isn't clear why any of these patches is cc:stable.  The overall
> effect is a tiny performance improvement?  Very clear descriptions of
> end-user effects are always helpful.
The performance table in the cover letter measures the overhead that
the patches themselves add to the stat update fast path, not the
impact of the bug - sorry if that was misleading.

The end-user effect of the bug is the loss of thrashing protection.
Since 7404bd37cfbe, with MGLRU enabled count_shadow_nodes() sees the
four evictable LRU lists as always empty, so the workingset shadow
node budget collapses to slab plus unevictable pages: for a memcg
holding 1 GiB of page cache and 64 MiB of slab the budget drops from
~35k nodes to ~2k.  The shadow shrinker then reclaims eviction tokens
almost as fast as they are created, so a refault finds a live page
instead of a shadow entry and lru_gen_refault() cannot restore the
workingset state of the refaulting page.  Hot file pages that should
be protected are evicted again and re-read from disk.  In other words,
workloads that refault page cache under memory pressure thrash, with
visibly increased IO and latency.  This hits every memcg reclaim and,
since the root memcg is iterated as well, global reclaim too.
> So at this time I'll schedule the whole series for 7.4-rc1.  If there's
> some reason why some/all of these should be backported then please lmk.
I would suggest backporting all three patches together to the trees
that contain 7404bd37cfbe: it went into v7.1, so 7.1.y and 7.2.y
carry the regression.  Taking patch 3 along with patches 1 and 2
avoids leaving the ~0.6% fast-path overhead of patch 1 in the stable
trees.

So v4 will reword the reference in this patch's commit message ("the
preceding patch" instead of "the follow-up patch").  No code changes.

One question: should patch 3 ("mm: memcg: skip the RCU lock when the
memcg is not dying") also carry the same

  Fixes: 7404bd37cfbe ("mm: workingset: use lruvec_lru_size() to get 
the number of lru pages")
  Cc: stable@xxxxxxxxxxxxxxx

tags?  That would make the stable trees pick up the whole series
together, but since patch 3 only recovers the overhead of patch 1 and
fixes no bug by itself, a Fixes: tag on it may look odd.  Please let
me know your preference.

Best,
Hui