mm/vmscan: fix missing NR_ISOLATED counter update in MGLRU reclaim path

[PATCH] mm/vmscan: fix missing NR_ISOLATED counter update in MGLRU reclaim path

Posted by Hui Zhu 1 month, 2 weeks ago

From: Hui Zhu <zhuhui@kylinos.cn>

The MGLRU evict_folios() isolates folios from the LRU without updating
NR_ISOLATED_ANON/FILE counters, unlike the legacy shrink_inactive_list()
path.
This causes compaction's too_many_isolated() check and the OOMkiller to
under-count isolated pages when MGLRU reclaim is active, potentially
leading to unnecessary compaction attempts or incorrect OOM decisions
under memory pressure.

Add NR_ISOLATED counter updates in evict_folios(): increment after
isolate_folios() and decrement after all retry passes complete, using
the original isolated count saved before retry.

Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 mm/vmscan.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index bc324e37c5f1..723b513574b3 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4817,6 +4817,7 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	struct lru_gen_mm_walk *walk;
 	int scanned, reclaimed;
 	int isolated = 0, type, type_scanned;
+	int isolated_orig = 0;
 	bool skip_retry = false;
 	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
 	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
@@ -4829,6 +4830,11 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	scanned = isolate_folios(nr_to_scan, lruvec, sc, swappiness,
 				 &list, &isolated, &type, &type_scanned);
 
+	isolated_orig = isolated;
+	if (isolated)
+		__mod_node_page_state(pgdat, NR_ISOLATED_ANON + type,
+				      isolated);
+
 	/* Scanning may have emptied the oldest gen, flush it */
 	if (scanned)
 		try_to_inc_min_seq(lruvec, swappiness);
@@ -4891,6 +4897,10 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 		goto retry;
 	}
 
+	if (isolated_orig)
+		mod_node_page_state(pgdat, NR_ISOLATED_ANON + type,
+				    -isolated_orig);
+
 	return scanned;
 }
 
-- 
2.53.0

Re: [PATCH] mm/vmscan: fix missing NR_ISOLATED counter update in MGLRU reclaim path

Posted by Kairui Song 1 month, 2 weeks ago

On Tue, Aug 11, 2026 at 03:17:03PM +0800, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> The MGLRU evict_folios() isolates folios from the LRU without updating
> NR_ISOLATED_ANON/FILE counters, unlike the legacy shrink_inactive_list()
> path.
> This causes compaction's too_many_isolated() check and the OOMkiller to
> under-count isolated pages when MGLRU reclaim is active, potentially
> leading to unnecessary compaction attempts or incorrect OOM decisions
> under memory pressure.

This patch still doesn't cover the OOM problem, too_many_isolated is not
used for MGLRU. I prefer to keep unifying them like what we did previously
with writeback.

It might work for compaction though, did you test it?

> 
> Add NR_ISOLATED counter updates in evict_folios(): increment after
> isolate_folios() and decrement after all retry passes complete, using
> the original isolated count saved before retry.
> 
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> ---
>  mm/vmscan.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bc324e37c5f1..723b513574b3 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4817,6 +4817,7 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>  	struct lru_gen_mm_walk *walk;
>  	int scanned, reclaimed;
>  	int isolated = 0, type, type_scanned;
> +	int isolated_orig = 0;

The variable name looks a bit confusing, and what is your based commit?
In latest mm-unstable there is already a nr_isolated you can use.

Re: [PATCH] mm/vmscan: fix missing NR_ISOLATED counter update in MGLRU reclaim path

Posted by Hui Zhu 1 month, 2 weeks ago

> 
> On Tue, Aug 11, 2026 at 03:17:03PM +0800, Hui Zhu wrote:
> 
> > 
> > From: Hui Zhu <zhuhui@kylinos.cn>
> >  
> >  The MGLRU evict_folios() isolates folios from the LRU without updating
> >  NR_ISOLATED_ANON/FILE counters, unlike the legacy shrink_inactive_list()
> >  path.
> >  This causes compaction's too_many_isolated() check and the OOMkiller to
> >  under-count isolated pages when MGLRU reclaim is active, potentially
> >  leading to unnecessary compaction attempts or incorrect OOM decisions
> >  under memory pressure.
> > 
> This patch still doesn't cover the OOM problem, too_many_isolated is not
> used for MGLRU. I prefer to keep unifying them like what we did previously
> with writeback.

I will address this in the v2.

> 
> It might work for compaction though, did you test it?

I didn't test it, I just came to the conclusion through code analysis.
isolate_migratepages_block() calls too_many_isolated()
which reads NR_ISOLATED counters, and since MGLRU doesn't update them, the
check may under-count isolated pages and fail to throttle when it should.


> 
> > 
> > Add NR_ISOLATED counter updates in evict_folios(): increment after
> >  isolate_folios() and decrement after all retry passes complete, using
> >  the original isolated count saved before retry.


The current patch is based on mm-stable. I will rebase on mm-unstable
and use the existing nr_isolated variable.

Best,
Hui

> >  
> >  Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> >  ---
> >  mm/vmscan.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >  
> >  diff --git a/mm/vmscan.c b/mm/vmscan.c
> >  index bc324e37c5f1..723b513574b3 100644
> >  --- a/mm/vmscan.c
> >  +++ b/mm/vmscan.c
> >  @@ -4817,6 +4817,7 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> >  struct lru_gen_mm_walk *walk;
> >  int scanned, reclaimed;
> >  int isolated = 0, type, type_scanned;
> >  + int isolated_orig = 0;
> > 
> The variable name looks a bit confusing, and what is your based commit?
> In latest mm-unstable there is already a nr_isolated you can use.
>