mm/hotplug: fix a drain pcp bug when offline pages
Subsystems (2)
PATCH 1/1 mm/hotplug: fix a drain pcp bug when offline pages
__offline_pages()
start_isolate_page_range()
set_migratetype_isolate()
set_pageblock_migratetype() -> this pageblock will be marked as MIGRATE_ISOLATE
move_freepages_block() -> pages in PageBuddy will be moved into MIGRATE_ISOLATE list
drain_all_pages() -> drain PCP
free_pcppages_bulk()
mt = get_freepage_migratetype(page); -> PCP's migratetype is not MIGRATE_ISOLATE
__free_one_page(page, zone, 0, mt); -> so PCP will not be freed into into MIGRATE_ISOLATE listIn this case, the PCP may be allocated again, because they are not in PageBuddy's MIGRATE_ISOLATE list. This will cause offline_pages failed.
--- mm/page_alloc.c | 10 ++++++---- mm/page_isolation.c | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-)
mm/page_alloc.c +6 -4
@@ -965,11 +965,13 @@ int move_freepages(struct zone *zone, } order =page_order (page); -list_move (&page->lru, - &zone->free_area[order].free_list[migratetype]); -set_freepage_migratetype (page, migratetype); +if (get_freepage_migratetype (page) != migratetype) { +list_move (&page->lru, + &zone->free_area[order].free_list[migratetype]); +set_freepage_migratetype (page, migratetype); + pages_moved +=1 << order; + } page +=1 << order; - pages_moved +=1 << order; }return pages_moved;
mm/page_isolation.c +14 -1
@@ -65,8 +65,21 @@ out: }spin_unlock_irqrestore (&zone->lock, flags); -if (!ret) + +if (!ret) {drain_all_pages (); +/* +* When drain_all_pages() frees cached pages into the buddy +* system, it uses the stale migratetype cached in the +* page->index field, so try to move free pages to ISOLATE +* list again. +*/ +spin_lock_irqsave (&zone->lock, flags); + nr_pages =move_freepages_block (zone, page,MIGRATE_ISOLATE ); +__mod_zone_freepage_state (zone, -nr_pages, migratetype); +spin_unlock_irqrestore (&zone->lock, flags); + } +return ret; }
Signature · 7 lines
1.7.1 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 28 lines · Xishi Qiu
__offline_pages()start_isolate_page_range() set_migratetype_isolate() set_pageblock_migratetype() -> this pageblock will be marked as MIGRATE_ISOLATE move_freepages_block() -> pages in PageBuddy will be moved into MIGRATE_ISOLATE list drain_all_pages() -> drain PCP free_pcppages_bulk() mt = get_freepage_migratetype(page); -> PCP's migratetype is not MIGRATE_ISOLATE __free_one_page(page, zone, 0, mt); -> so PCP will not be freed into into MIGRATE_ISOLATE listIn this case, the PCP may be allocated again, because they are not in PageBuddy's
MIGRATE_ISOLATElist. This will causeoffline_pagesfailed.Signed-off-by Xishi Qiu <[email protected]>--- mm/page_alloc.c | 10 ++++++---- mm/page_isolation.c | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-)mm/page_alloc.c +0
-1@@ -965,11 +965,13 @@ int move_freepages(struct zone *zone, } order =page_order (page); - list_move(&page->lru,
mm/page_alloc.c int move_freepages(struct zone *zone, · line 972
Quoted 10 lines
mm/page_alloc.c +6
-3- &zone->free_area[order].free_list[migratetype]); -set_freepage_migratetype (page, migratetype); +if (get_freepage_migratetype (page) != migratetype) { +list_move (&page->lru, + &zone->free_area[order].free_list[migratetype]); +set_freepage_migratetype (page, migratetype); + pages_moved +=1 << order; + } page +=1 << order; - pages_moved +=1 << order;
So this looks like it changes the return from move_freepages() to be the "pages moved" from "the pages now belonging to the passed migrate type".
The user of move_freepages_block()'s return value (and thus the return value of move_freepages()) in mm/page_alloc.c expects that it is the original meaning. The users in page_isolation.c expect it is the new meaning. Those need to be reconciled.
Quoted 17 lines
mm/page_alloc.c +0
-0}return pages_moved;mm/page_isolation.c +4
-1@@ -65,8 +65,21 @@ out: }spin_unlock_irqrestore (&zone->lock, flags); -if (!ret) + +if (!ret) {drain_all_pages (); +/* +* When drain_all_pages() frees cached pages into the buddy
mm/page_isolation.c out: · line 82
Quoted 10 lines
mm/page_isolation.c +10
-0+* system, it uses the stale migratetype cached in the +* page->index field, so try to move free pages to ISOLATE +* list again. +*/ +spin_lock_irqsave (&zone->lock, flags); + nr_pages =move_freepages_block (zone, page,MIGRATE_ISOLATE ); +__mod_zone_freepage_state (zone, -nr_pages, migratetype); +spin_unlock_irqrestore (&zone->lock, flags); + } +
Could we teach drain_all_pages() to use the right migrate type instead (or add something similar that does)? (pages could be reallocated between the drain_all_pages() and move_freepages_block()).
Quoted 3 lines
mm/page_isolation.c +0
-0return ret; }
Quoted 66 lines · Cody P Schafer
Quoted 28 lines · Xishi Qiu
__offline_pages()start_isolate_page_range() set_migratetype_isolate() set_pageblock_migratetype() -> this pageblock will be marked as MIGRATE_ISOLATE move_freepages_block() -> pages in PageBuddy will be moved into MIGRATE_ISOLATE list drain_all_pages() -> drain PCP free_pcppages_bulk() mt = get_freepage_migratetype(page); -> PCP's migratetype is not MIGRATE_ISOLATE __free_one_page(page, zone, 0, mt); -> so PCP will not be freed into into MIGRATE_ISOLATE listIn this case, the PCP may be allocated again, because they are not in PageBuddy's
MIGRATE_ISOLATElist. This will causeoffline_pagesfailed.Signed-off-by Xishi Qiu <[email protected]>--- mm/page_alloc.c | 10 ++++++---- mm/page_isolation.c | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-)mm/page_alloc.c +0
-1@@ -965,11 +965,13 @@ int move_freepages(struct zone *zone, } order =page_order (page); - list_move(&page->lru,mm/page_alloc.c int move_freepages(struct zone *zone, · line 972
Quoted 10 lines
mm/page_alloc.c +6
-3- &zone->free_area[order].free_list[migratetype]); -set_freepage_migratetype (page, migratetype); +if (get_freepage_migratetype (page) != migratetype) { +list_move (&page->lru, + &zone->free_area[order].free_list[migratetype]); +set_freepage_migratetype (page, migratetype); + pages_moved +=1 << order; + } page +=1 << order; - pages_moved +=1 << order;So this looks like it changes the return from
move_freepages()to be the "pages moved" from "the pages now belonging to the passed migrate type".The user of move_freepages_block()'s return value (and thus the return value of
move_freepages()) inmm/page_alloc.cexpects that it is the original meaning. The users inpage_isolation.cexpect it is the new meaning. Those need to be reconciled.Quoted 21 lines
mm/page_alloc.c +0
-0}return pages_moved;mm/page_isolation.c +8
-1@@ -65,8 +65,21 @@ out: }spin_unlock_irqrestore (&zone->lock, flags); -if (!ret) + +if (!ret) {drain_all_pages (); +/* +* When drain_all_pages() frees cached pages into the buddy +* system, it uses the stale migratetype cached in the +* page->index field, so try to move free pages to ISOLATE +* list again. +*/
Quoted 9 lines
mm/page_isolation.c out: · line 82
Quoted 6 lines
mm/page_isolation.c +6
-0+spin_lock_irqsave (&zone->lock, flags); + nr_pages =move_freepages_block (zone, page,MIGRATE_ISOLATE ); +__mod_zone_freepage_state (zone, -nr_pages, migratetype); +spin_unlock_irqrestore (&zone->lock, flags); + } +Could we teach
drain_all_pages()to use the right migrate type instead (or add something similar that does)? (pages could be reallocated between thedrain_all_pages()andmove_freepages_block()).
Sorry, I made a mistake. __test_page_isolated_in_pageblock() will catch the pages and move them into MIGRATE_ISOLATE list. So the code is right.
Thanks,
Xishi Qiu
Quoted 6 lines
mm/page_isolation.c out: · line 85
Quoted 3 lines
mm/page_isolation.c +0
-0return ret; }.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>