mm, vmscan: prevent useless kswapd loops
Subsystems (3)
PATCH 1/1 mm, vmscan: prevent useless kswapd loops
On production we have noticed hard lockups on large machines running large jobs due to kswaps hoarding lru lock within isolate_lru_pages when sc->reclaim_idx is 0 which is a small zone. The lru was couple hundred GiBs and the condition (page_zonenum(page) > sc->reclaim_idx) in isolate_lru_pages was basically skipping GiBs of pages while holding the LRU spinlock with interrupt disabled.
On further inspection, it seems like there are two issues:
1) If the kswapd on the return from balance_pgdat() could not sleep (maybe all zones are still unbalanced), the classzone_idx is set to 0, unintentionally, and the whole reclaim cycle of kswapd will try to reclaim only the lowest and smallest zone while traversing the whole memory.
2) Fundamentally isolate_lru_pages() is really bad when the allocation has woken kswapd for a smaller zone on a very large machine running very large jobs. It can hoard the LRU spinlock while skipping over 100s of GiBs of pages.
This patch only fixes the (1). The (2) needs a more fundamental solution.
due to mismatched classzone_idx")
--- mm/vmscan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/vmscan.c +1 -1
@@ -3908,7 +3908,7 @@ static int kswapd(void *p)/* Read the new order and classzone_idx */ alloc_order = reclaim_order = pgdat->kswapd_order; - classzone_idx =kswapd_classzone_idx (pgdat,0 ); + classzone_idx =kswapd_classzone_idx (pgdat, classzone_idx); pgdat->kswapd_order =0 ; pgdat->kswapd_classzone_idx =MAX_NR_ZONES ;
Signature · 1 line
2.22.0.410.gd8fdbe21b5-goog
Quoted 33 lines · Shakeel Butt
On production we have noticed hard lockups on large machines running large jobs due to kswaps hoarding lru lock within
isolate_lru_pageswhensc->reclaim_idxis 0 which is a small zone. The lru was couple hundred GiBs and the condition (page_zonenum(page)>sc->reclaim_idx) inisolate_lru_pageswas basically skipping GiBs of pages while holding the LRU spinlock with interrupt disabled.On further inspection, it seems like there are two issues:
1) If the kswapd on the return from
balance_pgdat()could not sleep (maybe all zones are still unbalanced), theclasszone_idxis set to 0, unintentionally, and the whole reclaim cycle of kswapd will try to reclaim only the lowest and smallest zone while traversing the whole memory.2) Fundamentally
isolate_lru_pages()is really bad when the allocation has woken kswapd for a smaller zone on a very large machine running very large jobs. It can hoard the LRU spinlock while skipping over 100s of GiBs of pages.This patch only fixes the (1). The (2) needs a more fundamental solution.
Fixes e716f2eb24de ("mm, vmscan: prevent kswapd sleeping prematurelydue to mismatched
classzone_idx")Signed-off-by Shakeel Butt <[email protected]>--- mm/vmscan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/vmscan.c +0
-0
mm/vmscan.c static int kswapd(void *p) · line 3911
Quoted 5 lines
mm/vmscan.c +1
-1@@ -3908,7 +3908,7 @@ static int kswapd(void *p)/* Read the new order and classzone_idx */ alloc_order = reclaim_order = pgdat->kswapd_order; - classzone_idx =kswapd_classzone_idx (pgdat,0 ); + classzone_idx =kswapd_classzone_idx (pgdat, classzone_idx);
I'm a little bit confused by the fix. What happen if kswapd is waken for a lower zone? It looks kswapd may just reclaim the higher zone instead of the lower zone?
For example, after bootup, classzone_idx should be (MAX_NR_ZONES - 1), if GFP_DMA is used for allocation and kswapd is waken up for ZONE_DMA, kswapd_classzone_idx would still return (MAX_NR_ZONES - 1) since kswapd_classzone_idx(pgdat, classzone_idx) returns the max classzone_idx.
Quoted 3 lines
mm/vmscan.c +0
-0pgdat->kswapd_order =0 ; pgdat->kswapd_classzone_idx =MAX_NR_ZONES ;
Quoted 47 lines · Yang Shi
Quoted 33 lines · Shakeel Butt
On production we have noticed hard lockups on large machines running large jobs due to kswaps hoarding lru lock within
isolate_lru_pageswhensc->reclaim_idxis 0 which is a small zone. The lru was couple hundred GiBs and the condition (page_zonenum(page)>sc->reclaim_idx) inisolate_lru_pageswas basically skipping GiBs of pages while holding the LRU spinlock with interrupt disabled.On further inspection, it seems like there are two issues:
1) If the kswapd on the return from
balance_pgdat()could not sleep (maybe all zones are still unbalanced), theclasszone_idxis set to 0, unintentionally, and the whole reclaim cycle of kswapd will try to reclaim only the lowest and smallest zone while traversing the whole memory.2) Fundamentally
isolate_lru_pages()is really bad when the allocation has woken kswapd for a smaller zone on a very large machine running very large jobs. It can hoard the LRU spinlock while skipping over 100s of GiBs of pages.This patch only fixes the (1). The (2) needs a more fundamental solution.
Fixes e716f2eb24de ("mm, vmscan: prevent kswapd sleeping prematurelydue to mismatched
classzone_idx")Signed-off-by Shakeel Butt <[email protected]>--- mm/vmscan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/vmscan.c +0
-0mm/vmscan.c static int kswapd(void *p) · line 3911
Quoted 5 lines
mm/vmscan.c +1
-1@@ -3908,7 +3908,7 @@ static int kswapd(void *p)/* Read the new order and classzone_idx */ alloc_order = reclaim_order = pgdat->kswapd_order; - classzone_idx =kswapd_classzone_idx (pgdat,0 ); + classzone_idx =kswapd_classzone_idx (pgdat, classzone_idx);I'm a little bit confused by the fix. What happen if kswapd is waken for a lower zone? It looks kswapd may just reclaim the higher zone instead of the lower zone?
Quoted 5 lines
For example, after bootup,
classzone_idxshould be (MAX_NR_ZONES- 1), ifGFP_DMAis used for allocation and kswapd is waken up forZONE_DMA,kswapd_classzone_idxwould still return (MAX_NR_ZONES- 1) sincekswapd_classzone_idx(pgdat, classzone_idx)returns the maxclasszone_idx.
Indeed you are right. I think kswapd_classzone_idx() is too much convoluted. It has different semantics when called from the wakers than when called from kswapd(). Let me see if we can decouple the logic in this function based on the context (or have two separate functions for both contexts).
thanks,
Shakeel