mm/vmscan: fix missing NR_ISOLATED counter update in MGLRU reclaim path
Subsystems (3)
PATCH 1/1 mm/vmscan: fix missing NR_ISOLATED counter update in MGLRU reclaim path
From: Hui Zhu <[email protected]>
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.
--- mm/vmscan.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
mm/vmscan.c +10 -0
@@ -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; }
Signature · 1 line
2.53.0
Quoted 9 lines · Hui Zhu
From: Hui Zhu <[email protected]>
The MGLRU
evict_folios()isolates folios from the LRU without updating NR_ISOLATED_ANON/FILE counters, unlike the legacyshrink_inactive_list()path.
This causes compaction'stoo_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?
Quoted 15 lines
Add
NR_ISOLATEDcounter updates inevict_folios(): increment afterisolate_folios()and decrement after all retry passes complete, using the original isolated count saved before retry.Signed-off-by Hui Zhu <[email protected]>--- mm/vmscan.c | 10 ++++++++++ 1 file changed, 10 insertions(+)mm/vmscan.c +0
-0
mm/vmscan.c static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec, · line 4820
Quoted 4 lines
mm/vmscan.c +1
-0@@ -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.
Quoted 14 lines · Hui Zhu
Quoted 4 lines
From: Hui Zhu <[email protected]>
Quoted 8 lines
The MGLRU
evict_folios()isolates folios from the LRU without updating NR_ISOLATED_ANON/FILE counters, unlike the legacyshrink_inactive_list()path.
This causes compaction'stoo_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.
Quoted 3 lines
This patch still doesn't cover the OOM problem,
too_many_isolatedis not used for MGLRU. I prefer to keep unifying them like what we did previously with writeback.
I will address this in the v2.
Quoted 2 lines
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.
Quoted 5 lines
Quoted 5 lines
Add
NR_ISOLATEDcounter updates inevict_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
Quoted 19 lines
Quoted 6 lines
Signed-off-by Hui Zhu <[email protected]>---
mm/vmscan.c | 10 ++++++++++ 1 file changed, 10 insertions(+)Quoted 10 lines
diff --git
a/mm/vmscan.cb/mm/vmscan.c
indexbc324e37c5f1..723b513574b3100644
---a/mm/vmscan.c
+++b/mm/vmscan.c
@@ -4817,6 +4817,7 @@ static int evict_folios(unsigned longnr_to_scan, struct lruvec *lruvec, structlru_gen_mm_walk*walk;
int scanned, reclaimed;
int isolated = 0, type,type_scanned;
+ intisolated_orig= 0;The variable name looks a bit confusing, and what is your based commit? In latest mm-unstable there is already a
nr_isolatedyou can use.