* [PATCH v4] mm: mglru: fix stale batch updates after memcg reparenting
@ 2026-07-01 7:52 Qi Zheng
2026-07-01 14:57 ` Usama Arif
0 siblings, 1 reply; 4+ messages in thread
From: Qi Zheng @ 2026-07-01 7:52 UTC (permalink / raw)
To: akpm, david, kasong, shakeel.butt, baohua, axelrasmussen, yuanchu,
weixugc, hannes, harry, muchun.song, peiyang_he, mhocko,
roman.gushchin, ljs
Cc: linux-mm, linux-kernel, Qi Zheng, stable
From: Qi Zheng <zhengqi.arch@bytedance.com>
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@smail.nju.edu.cn>
Closes: https://lore.kernel.org/all/5A9E929D82717101+12fcf643-efb8-4b9a-a53a-1e28cc894f0b@smail.nju.edu.cn
Fixes: f304652609ea ("mm: vmscan: prepare for reparenting MGLRU folios")
Cc: <stable@vger.kernel.org>
Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
Reviewed-by: Harry Yoo (Oracle) <harry@kernel.org>
---
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);
+
+ return lruvec;
+}
+#else
+static struct lruvec *lock_batch_lruvec(struct lruvec *lruvec)
+{
+ lruvec_lock_irq(lruvec);
+
+ return lruvec;
+}
+#endif
+
static void reset_batch_size(struct lru_gen_mm_walk *walk)
{
int gen, type, zone;
- struct lruvec *lruvec = walk->lruvec;
+ struct lruvec *lruvec = lock_batch_lruvec(walk->lruvec);
struct lru_gen_folio *lrugen = &lruvec->lrugen;
walk->batched = 0;
@@ -3285,6 +3315,8 @@ static void reset_batch_size(struct lru_gen_mm_walk *walk)
lru += LRU_ACTIVE;
__update_lru_size(lruvec, lru, zone, delta);
}
+
+ lruvec_unlock_irq(lruvec);
}
static int should_skip_vma(unsigned long start, unsigned long end, struct mm_walk *args)
@@ -3779,11 +3811,8 @@ static void walk_mm(struct mm_struct *mm, struct lru_gen_mm_walk *walk)
mmap_read_unlock(mm);
}
- if (walk->batched) {
- lruvec_lock_irq(lruvec);
+ if (walk->batched)
reset_batch_size(walk);
- lruvec_unlock_irq(lruvec);
- }
cond_resched();
} while (err == -EAGAIN);
@@ -4867,9 +4896,7 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
walk = current->reclaim_state->mm_walk;
if (walk && walk->batched) {
walk->lruvec = lruvec;
- lruvec_lock_irq(lruvec);
reset_batch_size(walk);
- lruvec_unlock_irq(lruvec);
}
mod_lruvec_state(lruvec, PGDEMOTE_KSWAPD + reclaimer_offset(sc),
--
2.54.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4] mm: mglru: fix stale batch updates after memcg reparenting
2026-07-01 7:52 [PATCH v4] mm: mglru: fix stale batch updates after memcg reparenting Qi Zheng
@ 2026-07-01 14:57 ` Usama Arif
2026-07-01 15:36 ` Harry Yoo
0 siblings, 1 reply; 4+ messages in thread
From: Usama Arif @ 2026-07-01 14:57 UTC (permalink / raw)
To: Qi Zheng
Cc: Usama Arif, akpm, david, kasong, shakeel.butt, baohua,
axelrasmussen, yuanchu, weixugc, hannes, harry, muchun.song,
peiyang_he, mhocko, roman.gushchin, ljs, linux-mm, linux-kernel,
Qi Zheng, stable
On Wed, 1 Jul 2026 15:52:51 +0800 Qi Zheng <qi.zheng@linux.dev> wrote:
> From: Qi Zheng <zhengqi.arch@bytedance.com>
>
> 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@smail.nju.edu.cn>
> Closes: https://lore.kernel.org/all/5A9E929D82717101+12fcf643-efb8-4b9a-a53a-1e28cc894f0b@smail.nju.edu.cn
> Fixes: f304652609ea ("mm: vmscan: prepare for reparenting MGLRU folios")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> Reviewed-by: Harry Yoo (Oracle) <harry@kernel.org>
> ---
> 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?
> +
> + return lruvec;
> +}
> +#else
> +static struct lruvec *lock_batch_lruvec(struct lruvec *lruvec)
> +{
> + lruvec_lock_irq(lruvec);
> +
> + return lruvec;
> +}
> +#endif
> +
> static void reset_batch_size(struct lru_gen_mm_walk *walk)
> {
> int gen, type, zone;
> - struct lruvec *lruvec = walk->lruvec;
> + struct lruvec *lruvec = lock_batch_lruvec(walk->lruvec);
> struct lru_gen_folio *lrugen = &lruvec->lrugen;
>
> walk->batched = 0;
> @@ -3285,6 +3315,8 @@ static void reset_batch_size(struct lru_gen_mm_walk *walk)
> lru += LRU_ACTIVE;
> __update_lru_size(lruvec, lru, zone, delta);
> }
> +
> + lruvec_unlock_irq(lruvec);
> }
>
> static int should_skip_vma(unsigned long start, unsigned long end, struct mm_walk *args)
> @@ -3779,11 +3811,8 @@ static void walk_mm(struct mm_struct *mm, struct lru_gen_mm_walk *walk)
> mmap_read_unlock(mm);
> }
>
> - if (walk->batched) {
> - lruvec_lock_irq(lruvec);
> + if (walk->batched)
> reset_batch_size(walk);
> - lruvec_unlock_irq(lruvec);
> - }
>
> cond_resched();
> } while (err == -EAGAIN);
> @@ -4867,9 +4896,7 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> walk = current->reclaim_state->mm_walk;
> if (walk && walk->batched) {
> walk->lruvec = lruvec;
> - lruvec_lock_irq(lruvec);
> reset_batch_size(walk);
> - lruvec_unlock_irq(lruvec);
> }
>
> mod_lruvec_state(lruvec, PGDEMOTE_KSWAPD + reclaimer_offset(sc),
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] mm: mglru: fix stale batch updates after memcg reparenting
2026-07-01 14:57 ` Usama Arif
@ 2026-07-01 15:36 ` Harry Yoo
2026-07-01 15:59 ` Johannes Weiner
0 siblings, 1 reply; 4+ messages in thread
From: Harry Yoo @ 2026-07-01 15:36 UTC (permalink / raw)
To: Usama Arif, Qi Zheng
Cc: akpm, david, kasong, shakeel.butt, baohua, axelrasmussen, yuanchu,
weixugc, hannes, muchun.song, peiyang_he, mhocko, roman.gushchin,
ljs, linux-mm, linux-kernel, Qi Zheng, stable
On 7/1/26 11:57 PM, Usama Arif wrote:
> On Wed, 1 Jul 2026 15:52:51 +0800 Qi Zheng <qi.zheng@linux.dev> wrote:
>
>> From: Qi Zheng <zhengqi.arch@bytedance.com>
>>
>> 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@smail.nju.edu.cn>
>> Closes: https://lore.kernel.org/all/5A9E929D82717101+12fcf643-efb8-4b9a-a53a-1e28cc894f0b@smail.nju.edu.cn
>> Fixes: f304652609ea ("mm: vmscan: prepare for reparenting MGLRU folios")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
>> Reviewed-by: Harry Yoo (Oracle) <harry@kernel.org>
>> ---
>> 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.
--
Cheers,
Harry / Hyeonggon
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] mm: mglru: fix stale batch updates after memcg reparenting
2026-07-01 15:36 ` Harry Yoo
@ 2026-07-01 15:59 ` Johannes Weiner
0 siblings, 0 replies; 4+ messages in thread
From: Johannes Weiner @ 2026-07-01 15:59 UTC (permalink / raw)
To: Harry Yoo
Cc: Usama Arif, Qi Zheng, akpm, david, kasong, shakeel.butt, baohua,
axelrasmussen, yuanchu, weixugc, muchun.song, peiyang_he, mhocko,
roman.gushchin, ljs, linux-mm, linux-kernel, Qi Zheng, stable
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@linux.dev> wrote:
> >
> >> From: Qi Zheng <zhengqi.arch@bytedance.com>
> >>
> >> 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@smail.nju.edu.cn>
> >> Closes: https://lore.kernel.org/all/5A9E929D82717101+12fcf643-efb8-4b9a-a53a-1e28cc894f0b@smail.nju.edu.cn
> >> Fixes: f304652609ea ("mm: vmscan: prepare for reparenting MGLRU folios")
> >> Cc: <stable@vger.kernel.org>
> >> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> >> Reviewed-by: Harry Yoo (Oracle) <harry@kernel.org>
> >> ---
> >> 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()?
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-01 15:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-01 7:52 [PATCH v4] mm: mglru: fix stale batch updates after memcg reparenting Qi Zheng
2026-07-01 14:57 ` Usama Arif
2026-07-01 15:36 ` Harry Yoo
2026-07-01 15:59 ` Johannes Weiner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox