Re: [PATCH v4] mm: mglru: fix stale batch updates after memcg reparenting
From: Qi Zheng
Date: Wed Jul 01 2026 - 21:44:52 EST
On 7/1/26 11:59 PM, Johannes Weiner wrote:
On Thu, Jul 02, 2026 at 12:36:42AM +0900, Harry Yoo wrote:
On 7/1/26 11:57 PM, Usama Arif wrote:
On Wed, 1 Jul 2026 15:52:51 +0800 Qi Zheng <qi.zheng@xxxxxxxxx> wrote:
From: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
The mglru page table walker batches per-generation size deltas in
walk->nr_pages while walking page tables without holding the lruvec lock.
The reset_batch_size() later folds those deltas into walk->lruvec under
the lruvec lock.
The page table walker can run concurrently with the memcg reparenting path
as follows:
CPU0 CPU1
==== ====
walk_mm
--> walk_page_range
--> update_batch_size
--> walk->nr_pages += delta
mem_cgroup_css_offline
--> memcg_reparent_objcgs
--> lock lruvec
lru_gen_reparent_memcg
--> reparent child folios to parent
unlock lruvec
lock lruvec
reset_batch_size
--> child lrugen->nr_pages += delta
This will trigger the following warning in lru_gen_exit_memcg():
VM_WARN_ON_ONCE(memchr_inv(lruvec->lrugen.nr_pages, 0,
sizeof(lruvec->lrugen.nr_pages)));
And the user-visible impact of underestimated nr_pages in MGLRU was
premature OOMs because MGLRU does not try to reclaim memory when nr_pages
reaches zero, but there are still more pages.
To fix it, make reset_batch_size() check CSS_DYING under RCU before
flushing the pending batch. A non-dying memcg keeps the original lruvec
stable against RCU-delayed offlining; a dying memcg redirects the deltas
to the first non-dying ancestor.
Reported-by: Peiyang He <peiyang_he@xxxxxxxxxxxxxxxx>
Closes: https://lore.kernel.org/all/5A9E929D82717101+12fcf643-efb8-4b9a-a53a-1e28cc894f0b@xxxxxxxxxxxxxxxx
Fixes: f304652609ea ("mm: vmscan: prepare for reparenting MGLRU folios")
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
Reviewed-by: Harry Yoo (Oracle) <harry@xxxxxxxxxx>
---
Changes in v4:
- re-implement lock_batch_lruvec() in a simpler way
(suggested by Johannes and Harry)
- collect Reviewed-by
- rebase onto the next-20260630
Changes in v3:
- re-implement lock_batch_lruvec() by checking CSS_DYING under the RCU lock
(suggested by Harry)
- update the commit message (suggested by Harry)
- temporarily drop the previous Reviewed-by tags
(since the sync method has changed)
- rebase onto the next-20260624
Changes in v2:
- update the commit message (pointed by Barry)
- collect Reviewed-by
mm/vmscan.c | 41 ++++++++++++++++++++++++++++++++++-------
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 35c3bb15ae96..ca1e2a870d51 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3262,10 +3262,40 @@ static void update_batch_size(struct lru_gen_mm_walk *walk, struct folio *folio,
walk->nr_pages[new_gen][type][zone] += delta;
}
+#ifdef CONFIG_MEMCG
+static struct lruvec *lock_batch_lruvec(struct lruvec *lruvec)
+{
+ struct pglist_data *pgdat = lruvec_pgdat(lruvec);
+ struct mem_cgroup *memcg = lruvec_memcg(lruvec);
+
+ rcu_read_lock();
+
+ /*
+ * The memcg can be NULL when the memory controller is disabled.
+ * Otherwise, the caller keeps the memcg owning @lruvec alive.
+ */
+ while (unlikely(memcg && css_is_dying(&memcg->css))) {
+ memcg = parent_mem_cgroup(memcg);
+ lruvec = mem_cgroup_lruvec(memcg, pgdat);
+ }
+
+ spin_lock_irq(&lruvec->lru_lock);
Do we need an rcu_read_unlock() here?
lruvec_unlock_irq() does that.
Yeah, that tripped me up too. And it makes me think Shakeel was right
after all: this should live next to the other lruvec_lock() primitives.
Sure, MGLRU is the only user, but it's still much easier to understand
this if the code sits next to the rest of the API (and the unlock!).
lruvec_live_lock_irq()?
But lruvec_lock_irq() grabs the rcu lock too. :(