From: Kairui Song <ryncsn@gmail.com>
To: Chen Ridong <chenridong@huaweicloud.com>
Cc: linux-mm@kvack.org, Johannes Weiner <hannes@cmpxchg.org>,
Hugh Dickins <hughd@google.com>,
Michal Hocko <mhocko@kernel.org>,
Roman Gushchin <roman.gushchin@linux.dev>,
Shakeel Butt <shakeel.butt@linux.dev>,
Muchun Song <muchun.song@linux.dev>,
Andrew Morton <akpm@linux-foundation.org>,
cgroups@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC] mm/memcontrol: make lru_zone_size atomic and simplify sanity check
Date: Mon, 15 Dec 2025 16:29:04 +0800 [thread overview]
Message-ID: <CAMgjq7DVcpdBskYTRMz1U_k9qt4b0Vgrz3Qt5V7kzdj=GJ7CQg@mail.gmail.com> (raw)
In-Reply-To: <3edf7d6a-5e32-45f1-a6fc-ca5ca786551b@huaweicloud.com>
On Mon, Dec 15, 2025 at 3:38 PM Chen Ridong <chenridong@huaweicloud.com> wrote:
>
> On 2025/12/15 14:45, Kairui Song wrote:
> > From: Kairui Song <kasong@tencent.com>
> >
> > commit ca707239e8a7 ("mm: update_lru_size warn and reset bad lru_size")
> > introduced a sanity check to catch memcg counter underflow, which is
> > more like a workaround for another bug: lru_zone_size is unsigned, so
> > underflow will wrap it around and return an enormously large number,
> > then the memcg shrinker will loop almost forever as the calculated
> > number of folios to shrink is huge. That commit also checks if a zero
> > value matches the empty LRU list, so we have to hold the LRU lock, and
> > do the counter adding differently depending on whether the nr_pages is
> > negative.
> >
> > But later commit b4536f0c829c ("mm, memcg: fix the active list aging for
> > lowmem requests when memcg is enabled") already removed the LRU
> > emptiness check, doing the adding differently is meaningless now. And if
>
> Agree.
>
> I did submit a patch to address that, but it was not accepted.
>
> For reference, here is the link to the discussion:
> https://lore.kernel.org/lkml/CAOUHufbCCkOBGcSPZqNY+FXcrH8+U7_nRvftzOzKUBS4hn+kuQ@mail.gmail.com/
>
Thanks for letting me know, I wasn't aware that you noticed this too.
From my side I'm noticing that, lru_zone_size has only one reader:
shrink_lruvec -> get_scan_count -> lruvec_lru_size, while the updater
is every folio on LRU.
The oldest commit introduced this was trying to catch an underflow,
with extra sanity check to catch LRU emptiness mis-match. A later
commit removed the LRU emptiness mis-match, and the only thing left
here is the underflow check.
LRU counter leak (if there are) may happen anytime due to different
reasons, and I think the time an updater sees an underflowed value is
not unlikely to be the time the actual leak happens. (e.g. A folio was
removed without updating the counter minutes ago while there are other
folios still on the LRU, then an updater will trigger the WARN much
later). So the major issue here is the underflow mitigation.
Turning it into an atomic long should help mitigate both underflow,
and race (e.g. forgot to put the counter update inside LRU lock).
Overflow is really unlikely to happen IMO, and if that's corruption,
corruption could happen anywhere.
The reason I sent this patch is trying to move
mem_cgroup_update_lru_size out of lru lock scope in another series for
some other feature, just to gather some comments for this particular
sanity check, it seems a valid clean up and micro optimization on its
own.
next prev parent reply other threads:[~2025-12-15 8:29 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-15 6:45 Kairui Song
2025-12-15 7:38 ` Chen Ridong
2025-12-15 8:29 ` Kairui Song [this message]
2025-12-15 8:34 ` Kairui Song
2025-12-16 1:38 ` Chen Ridong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMgjq7DVcpdBskYTRMz1U_k9qt4b0Vgrz3Qt5V7kzdj=GJ7CQg@mail.gmail.com' \
--to=ryncsn@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=cgroups@vger.kernel.org \
--cc=chenridong@huaweicloud.com \
--cc=hannes@cmpxchg.org \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=muchun.song@linux.dev \
--cc=roman.gushchin@linux.dev \
--cc=shakeel.butt@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox