Increase success rates and reduce latency of compaction v1
Subsystems (5)
cover Increase success rates and reduce latency of compaction v1
This is a very preliminary RFC. I'm posting this early as the __GFP_THISNODE discussion continues and has started looking at the compaction implementation and it'd be worth looking at this series fdirst. The cc list is based on that dicussion just to make them aware it exists. A v2 will have a significantly trimmed cc.
This series reduces scan rates and success rates of compaction, primarily by using the free lists to shorten scans and better controlling of skip information and whether multiple scanners can target the same block. There still is much room for improvement but we probably should get these out of the way first as they are pre-requisites for anything smarter.
Test data is still incomplete but I'm not expecting major differences on 2-socket vs 1-socket given the type of series. That might be wrong.
Primarily I'm using thpscale to measure the impact of the series. The benchmark creates a large file, maps it, faults it, punches holes in the mapping so that the virtual address space is fragmented and then tries to allocate THP. It re-executes for different numbers of threads. From a fragmentation perspective, the workload is relatively benign but it does stress compaction.
The overall impact on latencies for a 1-socket machine is
4.20.0-rc6 4.20.0-rc6
mmotm-20181210 capture-v1r8
Amean fault-both-3 3842.11 ( 0.00%) 2898.64 * 24.56%*
Amean fault-both-5 5201.92 ( 0.00%) 4296.58 ( 17.40%)
Amean fault-both-7 7086.15 ( 0.00%) 6203.55 ( 12.46%)
Amean fault-both-12 11383.58 ( 0.00%) 9309.13 * 18.22%*
Amean fault-both-18 16616.53 ( 0.00%) 6245.27 * 62.42%*
Amean fault-both-24 18617.05 ( 0.00%) 15083.42 * 18.98%*
Amean fault-both-30 24372.88 ( 0.00%) 11498.60 * 52.82%*
Amean fault-both-32 22621.58 ( 0.00%) 9684.82 * 57.19%*24-62% reduction in fault latency (be it base or huge page)
The allocation success rates which are using MADV_HUGEPAGE are as follows;
4.20.0-rc6 4.20.0-rc6
mmotm-20181210 capture-v1r8
Percentage huge-3 85.74 ( 0.00%) 98.12 ( 14.44%)
Percentage huge-5 89.16 ( 0.00%) 98.83 ( 10.85%)
Percentage huge-7 85.98 ( 0.00%) 97.99 ( 13.97%)
Percentage huge-12 84.19 ( 0.00%) 99.00 ( 17.59%)
Percentage huge-18 81.20 ( 0.00%) 98.92 ( 21.83%)
Percentage huge-24 82.60 ( 0.00%) 99.08 ( 19.95%)
Percentage huge-30 82.87 ( 0.00%) 99.22 ( 19.74%)
Percentage huge-32 81.98 ( 0.00%) 98.97 ( 20.72%)So, it's showing that the series is not far short of allocating 100% of the requested pages as huge pages. Finally the overall scan rates are as follows
Compaction migrate scanned 677936161 4756927
Compaction free scanned 352045485 256525622
Kcompactd migrate scanned 751732 751080
Kcompactd free scanned 113784579 93099786
These are still pretty crazy scan rates but direct compaction migration scanning is reduced by 99% and the free scanner is reduced by 27% so it's a step in the right direction.
include/linux/compaction.h | 3 +- include/linux/gfp.h | 7 +- include/linux/sched.h | 4 + kernel/sched/core.c | 3 + mm/compaction.c | 638 ++++++++++++++++++++++++++++++++++++++------- mm/internal.h | 21 +- mm/migrate.c | 2 +- mm/page_alloc.c | 70 ++++- 8 files changed, 637 insertions(+), 111 deletions(-)
Signature · 1 line
2.16.4
PATCH 1/14 mm, compaction: Shrink compact_control
The isolate and migrate scanners should never isolate more than a pageblock of pages so unsigned int is sufficient saving 8 bytes on a 64-bit build.
--- mm/internal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
mm/internal.h +2 -2
@@ -185,8 +185,8 @@ struct compact_control {struct list_head freepages;/* List of free pages to migrate to */ struct list_head migratepages;/* List of pages being migrated */ struct zone *zone; -unsigned long nr_freepages;/* Number of isolated free pages */ -unsigned long nr_migratepages;/* Number of pages to migrate */ +unsigned int nr_freepages;/* Number of isolated free pages */ +unsigned int nr_migratepages;/* Number of pages to migrate */ unsigned long total_migrate_scanned;unsigned long total_free_scanned;unsigned long free_pfn;/* isolate_freepages search base */
Signature · 1 line
2.16.4
Re: [PATCH 01/14] mm, compaction: Shrink compact_control
Quoted 4 lines · Mel Gorman
The isolate and migrate scanners should never isolate more than a pageblock of pages so unsigned int is sufficient saving 8 bytes on a 64-bit build.
Signed-off-by Mel Gorman <[email protected]>
Quoted 20 lines
--- mm/internal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)mm/internal.h +2
-2@@ -185,8 +185,8 @@ struct compact_control {struct list_head freepages;/* List of free pages to migrate to */ struct list_head migratepages;/* List of pages being migrated */ struct zone *zone; -unsigned long nr_freepages;/* Number of isolated free pages */ -unsigned long nr_migratepages;/* Number of pages to migrate */ +unsigned int nr_freepages;/* Number of isolated free pages */ +unsigned int nr_migratepages;/* Number of pages to migrate */ unsigned long total_migrate_scanned;unsigned long total_free_scanned;unsigned long free_pfn;/* isolate_freepages search base */
PATCH 2/14 mm, compaction: Rearrange compact_control
In reply to Mel Gormancompact_control spans two cache lines with write-intensive lines on both. Rearrange so the most write-intensive fields are in the same cache line. This has a negligible impact on the overall performance of compaction and is more a tidying exercise than anything.
--- mm/internal.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
mm/internal.h +3 -3
@@ -184,14 +184,14 @@ extern int user_min_free_kbytes;struct compact_control {struct list_head freepages;/* List of free pages to migrate to */ struct list_head migratepages;/* List of pages being migrated */ -struct zone *zone;unsigned int nr_freepages;/* Number of isolated free pages */ unsigned int nr_migratepages;/* Number of pages to migrate */ -unsigned long total_migrate_scanned; -unsigned long total_free_scanned;unsigned long free_pfn;/* isolate_freepages search base */ unsigned long migrate_pfn;/* isolate_migratepages search base */ unsigned long last_migrated_pfn;/* Not yet flushed page being freed */ +struct zone *zone; +unsigned long total_migrate_scanned; +unsigned long total_free_scanned;const gfp_t gfp_mask;/* gfp mask of a direct compactor */ int order;/* order a direct compactor needs */ int migratetype;/* migratetype of direct compactor */
Signature · 1 line
2.16.4
Re: [PATCH 02/14] mm, compaction: Rearrange compact_control
Quoted 6 lines · Mel Gorman
compact_controlspans two cache lines with write-intensive lines on both. Rearrange so the most write-intensive fields are in the same cache line. This has a negligible impact on the overall performance of compaction and is more a tidying exercise than anything.Signed-off-by Mel Gorman <[email protected]>
PATCH 3/14 mm, compaction: Remove last_migrated_pfn from compact_control
In reply to Mel GormanThe last_migrated_pfn field is a bit dubious as to whether it really helps but either way, the information from it can be inferred without increasing the size of compact_control so remove the field.
--- mm/compaction.c | 25 +++++++++---------------- mm/internal.h | 1 - 2 files changed, 9 insertions(+), 17 deletions(-)
mm/compaction.c +9 -16
@@ -886,15 +886,6 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, cc->nr_migratepages++; nr_isolated++; -/* -* Record where we could have freed pages by migration and not -* yet flushed them to buddy allocator. -* - this is the lowest page that was isolated and likely be -* then freed by migration. -*/ -if (!cc->last_migrated_pfn) - cc->last_migrated_pfn = low_pfn; -/* Avoid isolating too much */ if (cc->nr_migratepages ==COMPACT_CLUSTER_MAX ) { ++low_pfn;
@@ -918,7 +909,6 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, }putback_movable_pages (&cc->migratepages); cc->nr_migratepages =0 ; - cc->last_migrated_pfn =0 ; nr_isolated =0 ; }
@@ -1539,6 +1529,7 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_controenum compact_result ret;unsigned long start_pfn = zone->zone_start_pfn;unsigned long end_pfn =zone_end_pfn (zone); +unsigned long last_migrated_pfn;const bool sync = cc->mode !=MIGRATE_ASYNC ; cc->migratetype =gfpflags_to_migratetype (cc->gfp_mask);
@@ -1584,7 +1575,7 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contro cc->whole_zone = true; } - cc->last_migrated_pfn =0 ; + last_migrated_pfn =0 ;trace_mm_compaction_begin (start_pfn, cc->migrate_pfn, cc->free_pfn, end_pfn, sync);
@@ -1593,12 +1584,14 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_controwhile ((ret =compact_finished (zone, cc)) ==COMPACT_CONTINUE ) {int err; +unsigned long start_pfn = cc->migrate_pfn;switch (isolate_migratepages (zone, cc)) {case ISOLATE_ABORT : ret =COMPACT_CONTENDED ;putback_movable_pages (&cc->migratepages); cc->nr_migratepages =0 ; + last_migrated_pfn =0 ; goto out;case ISOLATE_NONE :/*
@@ -1608,6 +1601,7 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contro*/ goto check_drain;case ISOLATE_SUCCESS : + last_migrated_pfn = start_pfn; ; }
@@ -1639,8 +1633,7 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contro cc->migrate_pfn =block_end_pfn ( cc->migrate_pfn -1 , cc->order);/* Draining pcplists is useless in this case */ - cc->last_migrated_pfn =0 ; - + last_migrated_pfn =0 ; } }
@@ -1652,18 +1645,18 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contro* compact_finished() can detect immediately if allocation * would succeed. */ -if (cc->order >0 && cc->last_migrated_pfn) { +if (cc->order >0 && last_migrated_pfn) {int cpu;unsigned long current_block_start =block_start_pfn (cc->migrate_pfn, cc->order); -if (cc->last_migrated_pfn < current_block_start) { +if (last_migrated_pfn < current_block_start) { cpu =get_cpu ();lru_add_drain_cpu (cpu);drain_local_pages (zone);put_cpu ();/* No more flushing until we migrate again */ - cc->last_migrated_pfn =0 ; + last_migrated_pfn =0 ; } }
mm/internal.h +0 -1
@@ -188,7 +188,6 @@ struct compact_control {unsigned int nr_migratepages;/* Number of pages to migrate */ unsigned long free_pfn;/* isolate_freepages search base */ unsigned long migrate_pfn;/* isolate_migratepages search base */ -unsigned long last_migrated_pfn;/* Not yet flushed page being freed */ struct zone *zone;unsigned long total_migrate_scanned;unsigned long total_free_scanned;
Signature · 1 line
2.16.4
Re: [PATCH 03/14] mm, compaction: Remove last_migrated_pfn from compact_control
Quoted 3 lines · Mel Gorman
The
last_migrated_pfnfield is a bit dubious as to whether it really helps but either way, the information from it can be inferred without increasing the size ofcompact_controlso remove the field.
Yeah looks like this won't cause more frequent drains.
Quoted 1 line
Signed-off-by Mel Gorman <[email protected]>
PATCH 4/14 mm, compaction: Rename map_pages to split_map_pages
In reply to Mel GormanIt's non-obvious that high-order free pages are split into order-0 pages from the function name. Fix it.
--- mm/compaction.c | 60 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 29 insertions(+), 31 deletions(-)
mm/compaction.c +29 -31
@@ -66,7 +66,7 @@ static unsigned long release_freepages(struct list_head *freelist)return high_pfn; } -static void map_pages (struct list_head *list) +static void split_map_pages (struct list_head *list) {unsigned int i, order, nr_pages;struct page *page, *next;
@@ -644,7 +644,7 @@ isolate_freepages_range(struct compact_control *cc, }/* __isolate_free_page() does not map the pages */ -map_pages (&freelist); +split_map_pages (&freelist);if (pfn < end_pfn) {/* Loop terminated early, cleanup. */
@@ -1141,7 +1141,7 @@ static void isolate_freepages(struct compact_control *cc) }/* __isolate_free_page() does not map the pages */ -map_pages (freelist); +split_map_pages (freelist);/* * Record where the free scanner will restart next time. Either we
@@ -1300,8 +1300,7 @@ static inline bool is_via_compact_memory(int order)return order ==-1 ; } -static enum compact_result __compact_finished (struct zone *zone, -struct compact_control *cc) +static enum compact_result __compact_finished (struct compact_control *cc) {unsigned int order;const int migratetype = cc->migratetype;
@@ -1312,7 +1311,7 @@ static enum compact_result __compact_finished(struct zone *zone,/* Compaction run completes if the migrate and free scanner meet */ if (compact_scanners_met (cc)) {/* Let the next compaction start anew. */ -reset_cached_positions (zone); +reset_cached_positions (cc->zone);/* * Mark that the PG_migrate_skip information should be cleared
@@ -1321,7 +1320,7 @@ static enum compact_result __compact_finished(struct zone *zone,* based on an allocation request. */ if (cc->direct_compaction) - zone->compact_blockskip_flush = true; + cc->zone->compact_blockskip_flush = true;if (cc->whole_zone)return COMPACT_COMPLETE ;
@@ -1345,7 +1344,7 @@ static enum compact_result __compact_finished(struct zone *zone,/* Direct compactor: Is a suitable page free? */ for (order = cc->order; order <MAX_ORDER ; order++) { -struct free_area *area = &zone->free_area[order]; +struct free_area *area = &cc->zone->free_area[order];bool can_steal;/* Job done if page is free of the right migratetype */
@@ -1391,13 +1390,12 @@ static enum compact_result __compact_finished(struct zone *zone,return COMPACT_NO_SUITABLE_PAGE ; } -static enum compact_result compact_finished (struct zone *zone, -struct compact_control *cc) +static enum compact_result compact_finished (struct compact_control *cc) {int ret; - ret =__compact_finished (zone, cc); -trace_mm_compaction_finished (zone, cc->order, ret); + ret =__compact_finished (cc); +trace_mm_compaction_finished (cc->zone, cc->order, ret);if (ret ==COMPACT_NO_SUITABLE_PAGE ) ret =COMPACT_CONTINUE ;
@@ -1524,16 +1522,16 @@ bool compaction_zonelist_suitable(struct alloc_context *ac, int order,return false; } -static enum compact_result compact_zone (struct zone *zone,struct compact_control *cc) +static enum compact_result compact_zone (struct compact_control *cc) {enum compact_result ret; -unsigned long start_pfn = zone->zone_start_pfn; -unsigned long end_pfn =zone_end_pfn (zone); +unsigned long start_pfn = cc->zone->zone_start_pfn; +unsigned long end_pfn =zone_end_pfn (cc->zone);unsigned long last_migrated_pfn;const bool sync = cc->mode !=MIGRATE_ASYNC ; cc->migratetype =gfpflags_to_migratetype (cc->gfp_mask); - ret =compaction_suitable (zone, cc->order, cc->alloc_flags, + ret =compaction_suitable (cc->zone, cc->order, cc->alloc_flags, cc->classzone_idx);/* Compaction is likely to fail */ if (ret ==COMPACT_SUCCESS || ret ==COMPACT_SKIPPED )
@@ -1546,8 +1544,8 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contro* Clear pageblock skip if there were failures recently and compaction * is about to be retried after being deferred. */ -if (compaction_restarting (zone, cc->order)) -__reset_isolation_suitable (zone); +if (compaction_restarting (cc->zone, cc->order)) +__reset_isolation_suitable (cc->zone);/* * Setup to move all movable pages to the end of the zone. Used cached
@@ -1559,16 +1557,16 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contro cc->migrate_pfn = start_pfn; cc->free_pfn =pageblock_start_pfn (end_pfn -1 ); }else { - cc->migrate_pfn = zone->compact_cached_migrate_pfn[sync]; - cc->free_pfn = zone->compact_cached_free_pfn; + cc->migrate_pfn = cc->zone->compact_cached_migrate_pfn[sync]; + cc->free_pfn = cc->zone->compact_cached_free_pfn;if (cc->free_pfn < start_pfn || cc->free_pfn >= end_pfn) { cc->free_pfn =pageblock_start_pfn (end_pfn -1 ); - zone->compact_cached_free_pfn = cc->free_pfn; + cc->zone->compact_cached_free_pfn = cc->free_pfn; }if (cc->migrate_pfn < start_pfn || cc->migrate_pfn >= end_pfn) { cc->migrate_pfn = start_pfn; - zone->compact_cached_migrate_pfn[0 ] = cc->migrate_pfn; - zone->compact_cached_migrate_pfn[1 ] = cc->migrate_pfn; + cc->zone->compact_cached_migrate_pfn[0 ] = cc->migrate_pfn; + cc->zone->compact_cached_migrate_pfn[1 ] = cc->migrate_pfn; }if (cc->migrate_pfn == start_pfn)
@@ -1582,11 +1580,11 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contromigrate_prep_local (); -while ((ret =compact_finished (zone, cc)) ==COMPACT_CONTINUE ) { +while ((ret =compact_finished (cc)) ==COMPACT_CONTINUE ) {int err;unsigned long start_pfn = cc->migrate_pfn; -switch (isolate_migratepages (zone, cc)) { +switch (isolate_migratepages (cc->zone, cc)) {case ISOLATE_ABORT : ret =COMPACT_CONTENDED ;putback_movable_pages (&cc->migratepages);
@@ -1653,7 +1651,7 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_controif (last_migrated_pfn < current_block_start) { cpu =get_cpu ();lru_add_drain_cpu (cpu); -drain_local_pages (zone); +drain_local_pages (cc->zone);put_cpu ();/* No more flushing until we migrate again */ last_migrated_pfn =0 ;
@@ -1678,8 +1676,8 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contro* Only go back, not forward. The cached pfn might have been * already reset to zone end in compact_finished() */ -if (free_pfn > zone->compact_cached_free_pfn) - zone->compact_cached_free_pfn = free_pfn; +if (free_pfn > cc->zone->compact_cached_free_pfn) + cc->zone->compact_cached_free_pfn = free_pfn; }count_compact_events (COMPACTMIGRATE_SCANNED , cc->total_migrate_scanned);
@@ -1716,7 +1714,7 @@ static enum compact_result compact_zone_order(struct zone *zone, int order,INIT_LIST_HEAD (&cc.freepages);INIT_LIST_HEAD (&cc.migratepages); - ret =compact_zone (zone, &cc); + ret =compact_zone (&cc);VM_BUG_ON (!list_empty (&cc.freepages));VM_BUG_ON (!list_empty (&cc.migratepages));
@@ -1834,7 +1832,7 @@ static void compact_node(int nid)INIT_LIST_HEAD (&cc.freepages);INIT_LIST_HEAD (&cc.migratepages); -compact_zone (zone, &cc); +compact_zone (&cc);VM_BUG_ON (!list_empty (&cc.freepages));VM_BUG_ON (!list_empty (&cc.migratepages));
@@ -1976,7 +1974,7 @@ static void kcompactd_do_work(pg_data_t *pgdat)if (kthread_should_stop ())return ; - status =compact_zone (zone, &cc); + status =compact_zone (&cc);if (status ==COMPACT_SUCCESS ) {compaction_defer_reset (zone, cc.order, false);
Signature · 1 line
2.16.4
Re: [PATCH 04/14] mm, compaction: Rename map_pages to split_map_pages
Quoted 2 lines · Mel Gorman
It's non-obvious that high-order free pages are split into order-0 pages from the function name. Fix it.
That's fine, but looks like the patch has another change squashed into it that removes zone parameter from several functions and uses cc->zone instead.
Quoted 206 lines
Signed-off-by Mel Gorman <[email protected]>--- mm/compaction.c | 60 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 29 insertions(+), 31 deletions(-)mm/compaction.c +29
-31@@ -66,7 +66,7 @@ static unsigned long release_freepages(struct list_head *freelist)return high_pfn; } -static void map_pages (struct list_head *list) +static void split_map_pages (struct list_head *list) {unsigned int i, order, nr_pages;struct page *page, *next;@@ -644,7 +644,7 @@ isolate_freepages_range(struct compact_control *cc, }/* __isolate_free_page() does not map the pages */ -map_pages (&freelist); +split_map_pages (&freelist);if (pfn < end_pfn) {/* Loop terminated early, cleanup. */ @@ -1141,7 +1141,7 @@ static void isolate_freepages(struct compact_control *cc) }/* __isolate_free_page() does not map the pages */ -map_pages (freelist); +split_map_pages (freelist);/* * Record where the free scanner will restart next time. Either we @@ -1300,8 +1300,7 @@ static inline bool is_via_compact_memory(int order)return order ==-1 ; } -static enum compact_result __compact_finished (struct zone *zone, -struct compact_control *cc) +static enum compact_result __compact_finished (struct compact_control *cc) {unsigned int order;const int migratetype = cc->migratetype;@@ -1312,7 +1311,7 @@ static enum compact_result __compact_finished(struct zone *zone,/* Compaction run completes if the migrate and free scanner meet */ if (compact_scanners_met (cc)) {/* Let the next compaction start anew. */ -reset_cached_positions (zone); +reset_cached_positions (cc->zone);/* * Mark that the PG_migrate_skip information should be cleared @@ -1321,7 +1320,7 @@ static enum compact_result __compact_finished(struct zone *zone,* based on an allocation request. */ if (cc->direct_compaction) - zone->compact_blockskip_flush = true; + cc->zone->compact_blockskip_flush = true;if (cc->whole_zone)return COMPACT_COMPLETE ;@@ -1345,7 +1344,7 @@ static enum compact_result __compact_finished(struct zone *zone,/* Direct compactor: Is a suitable page free? */ for (order = cc->order; order <MAX_ORDER ; order++) { -struct free_area *area = &zone->free_area[order]; +struct free_area *area = &cc->zone->free_area[order];bool can_steal;/* Job done if page is free of the right migratetype */ @@ -1391,13 +1390,12 @@ static enum compact_result __compact_finished(struct zone *zone,return COMPACT_NO_SUITABLE_PAGE ; } -static enum compact_result compact_finished (struct zone *zone, -struct compact_control *cc) +static enum compact_result compact_finished (struct compact_control *cc) {int ret; - ret =__compact_finished (zone, cc); -trace_mm_compaction_finished (zone, cc->order, ret); + ret =__compact_finished (cc); +trace_mm_compaction_finished (cc->zone, cc->order, ret);if (ret ==COMPACT_NO_SUITABLE_PAGE ) ret =COMPACT_CONTINUE ;@@ -1524,16 +1522,16 @@ bool compaction_zonelist_suitable(struct alloc_context *ac, int order,return false; } -static enum compact_result compact_zone (struct zone *zone,struct compact_control *cc) +static enum compact_result compact_zone (struct compact_control *cc) {enum compact_result ret; -unsigned long start_pfn = zone->zone_start_pfn; -unsigned long end_pfn =zone_end_pfn (zone); +unsigned long start_pfn = cc->zone->zone_start_pfn; +unsigned long end_pfn =zone_end_pfn (cc->zone);unsigned long last_migrated_pfn;const bool sync = cc->mode !=MIGRATE_ASYNC ; cc->migratetype =gfpflags_to_migratetype (cc->gfp_mask); - ret =compaction_suitable (zone, cc->order, cc->alloc_flags, + ret =compaction_suitable (cc->zone, cc->order, cc->alloc_flags, cc->classzone_idx);/* Compaction is likely to fail */ if (ret ==COMPACT_SUCCESS || ret ==COMPACT_SKIPPED )@@ -1546,8 +1544,8 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contro* Clear pageblock skip if there were failures recently and compaction * is about to be retried after being deferred. */ -if (compaction_restarting (zone, cc->order)) -__reset_isolation_suitable (zone); +if (compaction_restarting (cc->zone, cc->order)) +__reset_isolation_suitable (cc->zone);/* * Setup to move all movable pages to the end of the zone. Used cached @@ -1559,16 +1557,16 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contro cc->migrate_pfn = start_pfn; cc->free_pfn =pageblock_start_pfn (end_pfn -1 ); }else { - cc->migrate_pfn = zone->compact_cached_migrate_pfn[sync]; - cc->free_pfn = zone->compact_cached_free_pfn; + cc->migrate_pfn = cc->zone->compact_cached_migrate_pfn[sync]; + cc->free_pfn = cc->zone->compact_cached_free_pfn;if (cc->free_pfn < start_pfn || cc->free_pfn >= end_pfn) { cc->free_pfn =pageblock_start_pfn (end_pfn -1 ); - zone->compact_cached_free_pfn = cc->free_pfn; + cc->zone->compact_cached_free_pfn = cc->free_pfn; }if (cc->migrate_pfn < start_pfn || cc->migrate_pfn >= end_pfn) { cc->migrate_pfn = start_pfn; - zone->compact_cached_migrate_pfn[0 ] = cc->migrate_pfn; - zone->compact_cached_migrate_pfn[1 ] = cc->migrate_pfn; + cc->zone->compact_cached_migrate_pfn[0 ] = cc->migrate_pfn; + cc->zone->compact_cached_migrate_pfn[1 ] = cc->migrate_pfn; }if (cc->migrate_pfn == start_pfn)@@ -1582,11 +1580,11 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contromigrate_prep_local (); -while ((ret =compact_finished (zone, cc)) ==COMPACT_CONTINUE ) { +while ((ret =compact_finished (cc)) ==COMPACT_CONTINUE ) {int err;unsigned long start_pfn = cc->migrate_pfn; -switch (isolate_migratepages (zone, cc)) { +switch (isolate_migratepages (cc->zone, cc)) {case ISOLATE_ABORT : ret =COMPACT_CONTENDED ;putback_movable_pages (&cc->migratepages);@@ -1653,7 +1651,7 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_controif (last_migrated_pfn < current_block_start) { cpu =get_cpu ();lru_add_drain_cpu (cpu); -drain_local_pages (zone); +drain_local_pages (cc->zone);put_cpu ();/* No more flushing until we migrate again */ last_migrated_pfn =0 ;@@ -1678,8 +1676,8 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contro* Only go back, not forward. The cached pfn might have been * already reset to zone end in compact_finished() */ -if (free_pfn > zone->compact_cached_free_pfn) - zone->compact_cached_free_pfn = free_pfn; +if (free_pfn > cc->zone->compact_cached_free_pfn) + cc->zone->compact_cached_free_pfn = free_pfn; }count_compact_events (COMPACTMIGRATE_SCANNED , cc->total_migrate_scanned);@@ -1716,7 +1714,7 @@ static enum compact_result compact_zone_order(struct zone *zone, int order,INIT_LIST_HEAD (&cc.freepages);INIT_LIST_HEAD (&cc.migratepages); - ret =compact_zone (zone, &cc); + ret =compact_zone (&cc);VM_BUG_ON (!list_empty (&cc.freepages));VM_BUG_ON (!list_empty (&cc.migratepages));@@ -1834,7 +1832,7 @@ static void compact_node(int nid)INIT_LIST_HEAD (&cc.freepages);INIT_LIST_HEAD (&cc.migratepages); -compact_zone (zone, &cc); +compact_zone (&cc);VM_BUG_ON (!list_empty (&cc.freepages));VM_BUG_ON (!list_empty (&cc.migratepages));@@ -1976,7 +1974,7 @@ static void kcompactd_do_work(pg_data_t *pgdat)if (kthread_should_stop ())return ; - status =compact_zone (zone, &cc); + status =compact_zone (&cc);if (status ==COMPACT_SUCCESS ) {compaction_defer_reset (zone, cc.order, false);
Re: [PATCH 04/14] mm, compaction: Rename map_pages to split_map_pages
Quoted 8 lines · Vlastimil Babka
Quoted 2 lines · Mel Gorman
It's non-obvious that high-order free pages are split into order-0 pages from the function name. Fix it.
That's fine, but looks like the patch has another change squashed into it that removes zone parameter from several functions and uses
cc->zoneinstead.
Bah, it's a rebase mishap. It didn't flag when rereading the patches before sending because "yep, I did that on purpose". I'll split it out, the changelog will be uninspiring.
Signature · 2 lines
Mel Gorman SUSE Labs
PATCH 5/14 mm, compaction: Skip pageblocks with reserved pages
In reply to Mel GormanReserved pages are set at boot time, tend to be clustered and almost never become unreserved. When isolating pages for migrating, skip the entire pageblock is one PageReserved page is encountered on the grounds that it is highly probable the entire pageblock is reserved.
The impact depends on the machine and timing but both thpscale and thpfioscale when using MADV_HUGEPAGE show a reduction of scanning and fault latency on a 1-socket machine. The 2-socket results were too noisy to draw any meaningful conclusion but it's safe to assume less scanning is useful.
1-socket thpfioscale
4.20.0-rc6 4.20.0-rc6
mmotm-20181210 noreserved-v1r4
Amean fault-base-1 1481.32 ( 0.00%) 1443.63 ( 2.54%)
Amean fault-huge-1 1118.17 ( 0.00%) 981.30 * 12.24%*
Amean fault-both-1 1176.43 ( 0.00%) 1052.64 * 10.52%*Compaction migrate scanned 3860713 3294284
Compaction free scanned 613786341 433423502
Kcompactd migrate scanned 408711 291915
Kcompactd free scanned 242509759 217164988
--- mm/compaction.c | 7 +++++++ 1 file changed, 7 insertions(+)
mm/compaction.c +7 -0
@@ -827,6 +827,13 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, goto isolate_success; } +/* +* A reserved page is never freed and tend to be +* clustered in the same pageblocks. Skip the block. +*/ +if (PageReserved (page)) + low_pfn = end_pfn; + goto isolate_fail; }
Signature · 1 line
2.16.4
Re: [PATCH 05/14] mm, compaction: Skip pageblocks with reserved pages
Quoted 4 lines · Mel Gorman
Reserved pages are set at boot time, tend to be clustered and almost never become unreserved. When isolating pages for migrating, skip the entire pageblock is one PageReserved page is encountered on the grounds that it is highly probable the entire pageblock is reserved.
Agreed, but maybe since it's highly probable and not certain, this skipping should not be done on the highest compaction priority?
Quoted 42 lines
The impact depends on the machine and timing but both thpscale and thpfioscale when using
MADV_HUGEPAGEshow a reduction of scanning and fault latency on a 1-socket machine. The 2-socket results were too noisy to draw any meaningful conclusion but it's safe to assume less scanning is useful.1-socket thpfioscale
4.20.0-rc6 4.20.0-rc6 mmotm-20181210 noreserved-v1r4 Amean fault-base-1 1481.32 ( 0.00%) 1443.63 ( 2.54%) Amean fault-huge-1 1118.17 ( 0.00%) 981.30 * 12.24%* Amean fault-both-1 1176.43 ( 0.00%) 1052.64 * 10.52%*Compaction migrate scanned 3860713 3294284
Compaction free scanned 613786341 433423502
Kcompactd migrate scanned 408711 291915
Kcompactd free scanned 242509759 217164988Signed-off-by Mel Gorman <[email protected]>--- mm/compaction.c | 7 +++++++ 1 file changed, 7 insertions(+)mm/compaction.c +7
-0@@ -827,6 +827,13 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, goto isolate_success; } +/* +* A reserved page is never freed and tend to be +* clustered in the same pageblocks. Skip the block. +*/ +if (PageReserved (page)) + low_pfn = end_pfn; + goto isolate_fail; }
Re: [PATCH 05/14] mm, compaction: Skip pageblocks with reserved pages
Quoted 9 lines · Vlastimil Babka
Quoted 4 lines · Mel Gorman
Reserved pages are set at boot time, tend to be clustered and almost never become unreserved. When isolating pages for migrating, skip the entire pageblock is one PageReserved page is encountered on the grounds that it is highly probable the entire pageblock is reserved.
Agreed, but maybe since it's highly probable and not certain, this skipping should not be done on the highest compaction priority?
I don't think that's necessary at this time. For the most part, you are talking about one partial pageblock at best given how the early memory allocator works so it would only ever be useful for a high-order kernel allocation. Second, one of compactions primary problems is inefficient scanning where viable pageblocks are easily skipped over or only partially scanned which is something I'm still looking at. Lastly, maximum priority compaction is rarely hit in practice as far as I can tell.
Signature · 2 lines
Mel Gorman SUSE Labs
PATCH 6/14 mm, migrate: Immediately fail migration of a page with no migration handler
In reply to Mel GormanPages with no migration handler use a fallback hander which sometimes works and sometimes persistently fails such as blockdev pages. Migration will retry a number of times on these persistent pages which is wasteful during compaction. This patch will fail migration immediately unless the caller is in MIGRATE_SYNC mode which indicates the caller is willing to wait while being persistent.
This is not expected to help THP allocation success rates but it does reduce latencies slightly.
1-socket thpfioscale
4.20.0-rc6 4.20.0-rc6
noreserved-v1r4 failfast-v1r4
Amean fault-both-1 0.00 ( 0.00%) 0.00 * 0.00%*
Amean fault-both-3 2276.15 ( 0.00%) 3867.54 * -69.92%*
Amean fault-both-5 4992.20 ( 0.00%) 5313.20 ( -6.43%)
Amean fault-both-7 7373.30 ( 0.00%) 7039.11 ( 4.53%)
Amean fault-both-12 11911.52 ( 0.00%) 11328.29 ( 4.90%)
Amean fault-both-18 17209.42 ( 0.00%) 16455.34 ( 4.38%)
Amean fault-both-24 20943.71 ( 0.00%) 20448.94 ( 2.36%)
Amean fault-both-30 22703.00 ( 0.00%) 21655.07 ( 4.62%)
Amean fault-both-32 22461.41 ( 0.00%) 21415.35 ( 4.66%)The 2-socket results are not materially different. Scan rates are similar as expected.
--- mm/migrate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/migrate.c +1 -1
@@ -885,7 +885,7 @@ static int fallback_migrate_page(struct address_space *mapping,*/ if (page_has_private (page) && !try_to_release_page (page,GFP_KERNEL )) -return -EAGAIN ; +return mode ==MIGRATE_SYNC ? -EAGAIN : -EBUSY ;return migrate_page (mapping, newpage, page, mode); }
Signature · 1 line
2.16.4
Re: [PATCH 06/14] mm, migrate: Immediately fail migration of a page with no migration handler
Quoted 6 lines · Mel Gorman
Pages with no migration handler use a fallback hander which sometimes works and sometimes persistently fails such as blockdev pages. Migration will retry a number of times on these persistent pages which is wasteful during compaction. This patch will fail migration immediately unless the caller is in
MIGRATE_SYNCmode which indicates the caller is willing to wait while being persistent.
Right.
Quoted 8 lines
This is not expected to help THP allocation success rates but it does reduce latencies slightly.
1-socket thpfioscale
4.20.0-rc6 4.20.0-rc6 noreserved-v1r4 failfast-v1r4 Amean fault-both-1 0.00 ( 0.00%) 0.00 * 0.00%* Amean fault-both-3 2276.15 ( 0.00%) 3867.54 * -69.92%*
This is rather weird.
Quoted 8 lines
Amean fault-both-5 4992.20 ( 0.00%) 5313.20 ( -6.43%) Amean fault-both-7 7373.30 ( 0.00%) 7039.11 ( 4.53%) Amean fault-both-12 11911.52 ( 0.00%) 11328.29 ( 4.90%) Amean fault-both-18 17209.42 ( 0.00%) 16455.34 ( 4.38%) Amean fault-both-24 20943.71 ( 0.00%) 20448.94 ( 2.36%) Amean fault-both-30 22703.00 ( 0.00%) 21655.07 ( 4.62%) Amean fault-both-32 22461.41 ( 0.00%) 21415.35 ( 4.66%)
Quoted 4 lines
The 2-socket results are not materially different. Scan rates are similar as expected.
Signed-off-by Mel Gorman <[email protected]>
Quoted 18 lines
--- mm/migrate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/migrate.c +1
-1@@ -885,7 +885,7 @@ static int fallback_migrate_page(struct address_space *mapping,*/ if (page_has_private (page) && !try_to_release_page (page,GFP_KERNEL )) -return -EAGAIN ; +return mode ==MIGRATE_SYNC ? -EAGAIN : -EBUSY ;return migrate_page (mapping, newpage, page, mode); }
Re: [PATCH 06/14] mm, migrate: Immediately fail migration of a page with no migration handler
Quoted 13 lines · Vlastimil Babka
Quoted 6 lines · Mel Gorman
Pages with no migration handler use a fallback hander which sometimes works and sometimes persistently fails such as blockdev pages. Migration will retry a number of times on these persistent pages which is wasteful during compaction. This patch will fail migration immediately unless the caller is in
MIGRATE_SYNCmode which indicates the caller is willing to wait while being persistent.Right.
Quoted 3 lines
This is not expected to help THP allocation success rates but it does reduce latencies slightly.
Quoted 8 lines
Quoted 5 lines
1-socket thpfioscale
4.20.0-rc6 4.20.0-rc6 noreserved-v1r4 failfast-v1r4 Amean fault-both-1 0.00 ( 0.00%) 0.00 * 0.00%* Amean fault-both-3 2276.15 ( 0.00%) 3867.54 * -69.92%*This is rather weird.
Fault latency is extremely variable and there can be very large outliers that skew the mean (the full report includes quartiles but it makes for an excessive changelog). It can be down to luck about how often the migrate scanner advances and how often it gets reset. For this series, it'll not be unusual to see jitter in the latencies for individual patches that will not get nailed down reliably until later in the series. The alternative is massive patches that do multiple things which will look nice in changelogs and be horrible to review.
Quoted 8 lines
Quoted 8 lines
Amean fault-both-5 4992.20 ( 0.00%) 5313.20 ( -6.43%) Amean fault-both-7 7373.30 ( 0.00%) 7039.11 ( 4.53%) Amean fault-both-12 11911.52 ( 0.00%) 11328.29 ( 4.90%) Amean fault-both-18 17209.42 ( 0.00%) 16455.34 ( 4.38%) Amean fault-both-24 20943.71 ( 0.00%) 20448.94 ( 2.36%) Amean fault-both-30 22703.00 ( 0.00%) 21655.07 ( 4.62%) Amean fault-both-32 22461.41 ( 0.00%) 21415.35 ( 4.66%)
Quoted 7 lines
Quoted 4 lines
The 2-socket results are not materially different. Scan rates are similar as expected.
Signed-off-by Mel Gorman <[email protected]>Acked-by Vlastimil Babka <[email protected]>
Thanks.
Signature · 2 lines
Mel Gorman SUSE Labs
Re: [PATCH 06/14] mm, migrate: Immediately fail migration of a page with no migration handler
In reply to Mel GormanQuoted 3 lines · Mel Gorman
Pages with no migration handler use a fallback hander which sometimes works and sometimes persistently fails such as blockdev pages. Migration
A minor correction. The above statement sounds not accurate anymore since Jan Kara had patch series (blkdev: avoid migration stalls for blkdev pages) have blockdev use its own migration handler.
Thanks,
Yang
Quoted 45 lines
will retry a number of times on these persistent pages which is wasteful during compaction. This patch will fail migration immediately unless the caller is in
MIGRATE_SYNCmode which indicates the caller is willing to wait while being persistent.This is not expected to help THP allocation success rates but it does reduce latencies slightly.
1-socket thpfioscale
4.20.0-rc6 4.20.0-rc6 noreserved-v1r4 failfast-v1r4 Amean fault-both-1 0.00 ( 0.00%) 0.00 * 0.00%* Amean fault-both-3 2276.15 ( 0.00%) 3867.54 * -69.92%* Amean fault-both-5 4992.20 ( 0.00%) 5313.20 ( -6.43%) Amean fault-both-7 7373.30 ( 0.00%) 7039.11 ( 4.53%) Amean fault-both-12 11911.52 ( 0.00%) 11328.29 ( 4.90%) Amean fault-both-18 17209.42 ( 0.00%) 16455.34 ( 4.38%) Amean fault-both-24 20943.71 ( 0.00%) 20448.94 ( 2.36%) Amean fault-both-30 22703.00 ( 0.00%) 21655.07 ( 4.62%) Amean fault-both-32 22461.41 ( 0.00%) 21415.35 ( 4.66%)The 2-socket results are not materially different. Scan rates are similar as expected.
Signed-off-by Mel Gorman <[email protected]>--- mm/migrate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/migrate.c +1
-2@@ -885,7 +885,7 @@ static int fallback_migrate_page(struct address_space *mapping,*/ if (page_has_private (page) && !try_to_release_page (page,GFP_KERNEL )) -return -EAGAIN ; +return mode ==MIGRATE_SYNC ? -EAGAIN : -EBUSY ;return migrate_page (mapping, newpage, page, mode); } --2.16.4
Re: [PATCH 06/14] mm, migrate: Immediately fail migration of a page with no migration handler
Quoted 9 lines · Yang Shi
Quoted 3 lines · Mel Gorman
Pages with no migration handler use a fallback hander which sometimes works and sometimes persistently fails such as blockdev pages. Migration
A minor correction. The above statement sounds not accurate anymore since Jan Kara had patch series (blkdev: avoid migration stalls for blkdev pages) have blockdev use its own migration handler.
I'm aware given that I reviewed that series. The statement was correct at the time of writing. I'll alter the example when rebased on top of Jan's work.
Signature · 2 lines
Mel Gorman SUSE Labs
PATCH 7/14 mm, compaction: Always finish scanning of a full pageblock
In reply to Mel GormanWhen compaction is finishing, it uses a flag to ensure the pageblock is complete. However, in general it makes sense to always complete migration of a pageblock. Minimally, skip information is based on a pageblock and partially scanned pageblocks may incur more scanning in the future. The pageblock skip handling also becomes more strict later in the series and the hint is more useful if a complete pageblock was always scanned.
The impact here is potentially on latencies as more scanning is done but it's not a consistent win or loss as the scanning is not always a high percentage of the pageblock and sometimes it is offset by future reductions in scanning. Hence, the results are not presented this time as it's a mix of gains/losses without any clear pattern. However, completing scanning of the pageblock is important for later patches.
--- mm/compaction.c | 19 ++++++++----------- mm/internal.h | 1 - 2 files changed, 8 insertions(+), 12 deletions(-)
mm/compaction.c +8 -11
@@ -1338,16 +1338,14 @@ static enum compact_result __compact_finished(struct compact_control *cc)if (is_via_compact_memory (cc->order))return COMPACT_CONTINUE ; -if (cc->finishing_block) { -/* -* We have finished the pageblock, but better check again that -* we really succeeded. -*/ -if (IS_ALIGNED (cc->migrate_pfn, pageblock_nr_pages)) - cc->finishing_block = false; -else -return COMPACT_CONTINUE ; - } +/* +* Always finish scanning a pageblock to reduce the possibility of +* fallbacks in the future. This is particularly important when +* migration source is unmovable/reclaimable but it's not worth +* special casing. +*/ +if (!IS_ALIGNED (cc->migrate_pfn, pageblock_nr_pages)) +return COMPACT_CONTINUE ;/* Direct compactor: Is a suitable page free? */ for (order = cc->order; order <MAX_ORDER ; order++) {
@@ -1389,7 +1387,6 @@ static enum compact_result __compact_finished(struct compact_control *cc)return COMPACT_SUCCESS ; } - cc->finishing_block = true;return COMPACT_CONTINUE ; } }
mm/internal.h +0 -1
@@ -203,7 +203,6 @@ struct compact_control {bool direct_compaction;/* False from kcompactd or /proc/... */ bool whole_zone;/* Whole zone should/has been scanned */ bool contended;/* Signal lock or sched contention */ -bool finishing_block;/* Finishing current pageblock */ };unsigned long
Signature · 1 line
2.16.4
Re: [PATCH 07/14] mm, compaction: Always finish scanning of a full pageblock
Quoted 7 lines · Mel Gorman
When compaction is finishing, it uses a flag to ensure the pageblock is complete. However, in general it makes sense to always complete migration of a pageblock. Minimally, skip information is based on a pageblock and partially scanned pageblocks may incur more scanning in the future. The pageblock skip handling also becomes more strict later in the series and the hint is more useful if a complete pageblock was always scanned.
Quoted 8 lines
The impact here is potentially on latencies as more scanning is done but it's not a consistent win or loss as the scanning is not always a high percentage of the pageblock and sometimes it is offset by future reductions in scanning. Hence, the results are not presented this time as it's a mix of gains/losses without any clear pattern. However, completing scanning of the pageblock is important for later patches.
Signed-off-by Mel Gorman <[email protected]>
PATCH 8/14 mm, compaction: Use the page allocator bulk-free helper for lists of pages
In reply to Mel Gormanrelease_pages() is a simpler version of free_unref_page_list() but it tracks the highest PFN for caching the restart point of the compaction free scanner. This patch optionally tracks the highest PFN in the core helper and converts compaction to use it.
--- include/linux/gfp.h | 7 ++++++- mm/compaction.c | 12 +++--------- mm/page_alloc.c | 10 +++++++++- 3 files changed, 18 insertions(+), 11 deletions(-)
include/linux/gfp.h +6 -1
@@ -543,7 +543,12 @@ void * __meminit alloc_pages_exact_nid(int nid, size_t size, gfp_t gfp_mask);extern void __free_pages (struct page *page,unsigned int order);extern void free_pages (unsigned long addr,unsigned int order);extern void free_unref_page (struct page *page); -extern void free_unref_page_list (struct list_head *list); +extern void __free_page_list (struct list_head *list,bool dropref,unsigned long *highest_pfn); + +static inline void free_unref_page_list (struct list_head *list) +{ +return __free_page_list (list, false,NULL ); +}struct page_frag_cache ;extern void __page_frag_cache_drain (struct page *page,unsigned int count);
mm/compaction.c +3 -9
@@ -52,16 +52,10 @@ static inline void count_compact_events(enum vm_event_item item, long delta)static unsigned long release_freepages (struct list_head *freelist) { -struct page *page, *next; -unsigned long high_pfn =0 ; +unsigned long high_pfn; -list_for_each_entry_safe (page, next, freelist, lru) { -unsigned long pfn =page_to_pfn (page); -list_del (&page->lru); -__free_page (page); -if (pfn > high_pfn) - high_pfn = pfn; - } +__free_page_list (freelist, true, &high_pfn); +INIT_LIST_HEAD (freelist);return high_pfn; }
mm/page_alloc.c +9 -1
@@ -2961,18 +2961,26 @@ void free_unref_page(struct page *page)/* * Free a list of 0-order pages */ -void free_unref_page_list (struct list_head *list) +void __free_page_list (struct list_head *list,bool dropref, +unsigned long *highest_pfn) {struct page *page, *next;unsigned long flags, pfn;int batch_count =0 ; +if (highest_pfn) + *highest_pfn =0 ; +/* Prepare pages for freeing */ list_for_each_entry_safe (page, next, list, lru) { +if (dropref) +WARN_ON_ONCE (!put_page_testzero (page)); pfn =page_to_pfn (page);if (!free_unref_page_prepare (page, pfn))list_del (&page->lru);set_page_private (page, pfn); +if (highest_pfn && pfn > *highest_pfn) + *highest_pfn = pfn; }local_irq_save (flags);
Signature · 1 line
2.16.4
Re: [PATCH 08/14] mm, compaction: Use the page allocator bulk-free helper for lists of pages
Quoted 6 lines · Mel Gorman
release_pages()is a simpler version offree_unref_page_list()but it tracks the highest PFN for caching the restart point of the compaction free scanner. This patch optionally tracks the highest PFN in the core helper and converts compaction to use it.Signed-off-by Mel Gorman <[email protected]>
Nit below:
Quoted 11 lines
mm/page_alloc.c +2
-1@@ -2961,18 +2961,26 @@ void free_unref_page(struct page *page)/* * Free a list of 0-order pages */ -void free_unref_page_list (struct list_head *list) +void __free_page_list (struct list_head *list,bool dropref, +unsigned long *highest_pfn) {struct page *page, *next;
mm/page_alloc.c void free_unref_page(struct page *page) · line 2977
Quoted 10 lines
mm/page_alloc.c +5
-0unsigned long flags, pfn;int batch_count =0 ; +if (highest_pfn) + *highest_pfn =0 ; +/* Prepare pages for freeing */ list_for_each_entry_safe (page, next, list, lru) { +if (dropref) +WARN_ON_ONCE (!put_page_testzero (page));
That will warn just once, but then page will remain with elevated count and free_unref_page_prepare() will warn either immediately or later depending on DEBUG_VM, for each page.
Also IIRC it's legal for basically anyone to do get_page_unless_zero() and later put_page(), and this would now cause warning. Maybe just test for put_page_testzero() result without warning, and continue? Hm but then we should still do a list_del() and that becomes racy after dropping our ref...
Quoted 10 lines
mm/page_alloc.c +2
-0pfn =page_to_pfn (page);if (!free_unref_page_prepare (page, pfn))list_del (&page->lru);set_page_private (page, pfn); +if (highest_pfn && pfn > *highest_pfn) + *highest_pfn = pfn; }local_irq_save (flags);
Re: [PATCH 08/14] mm, compaction: Use the page allocator bulk-free helper for lists of pages
Quoted 26 lines · Vlastimil Babka
Quoted 6 lines · Mel Gorman
release_pages()is a simpler version offree_unref_page_list()but it tracks the highest PFN for caching the restart point of the compaction free scanner. This patch optionally tracks the highest PFN in the core helper and converts compaction to use it.Signed-off-by Mel Gorman <[email protected]>Acked-by Vlastimil Babka <[email protected]>Nit below:
Quoted 14 lines
mm/page_alloc.c +2
-1@@ -2961,18 +2961,26 @@ void free_unref_page(struct page *page)/* * Free a list of 0-order pages */ -void free_unref_page_list (struct list_head *list) +void __free_page_list (struct list_head *list,bool dropref, +unsigned long *highest_pfn) {struct page *page, *next;unsigned long flags, pfn;int batch_count =0 ;
Quoted 17 lines
mm/page_alloc.c void free_unref_page(struct page *page) · line 2977
Quoted 7 lines
mm/page_alloc.c +5
-0+if (highest_pfn) + *highest_pfn =0 ; +/* Prepare pages for freeing */ list_for_each_entry_safe (page, next, list, lru) { +if (dropref) +WARN_ON_ONCE (!put_page_testzero (page));That will warn just once, but then page will remain with elevated count and
free_unref_page_prepare()will warn either immediately or later depending onDEBUG_VM, for each page.
Also IIRC it's legal for basically anyone to doget_page_unless_zero()and laterput_page(), and this would now cause warning. Maybe just test forput_page_testzero()result without warning, and continue? Hm but then we should still do alist_del()and that becomes racy after dropping our ref...
While there are cases where such a pattern is legal, this function simply does not expect it and the callers do not violate the rule. If it ever gets a new user that makes mistakes, they'll get the warning. Sure, the page leaks but it'll be in a state where it's unsafe to do anything else with it.
Signature · 2 lines
Mel Gorman SUSE Labs
PATCH 9/14 mm, compaction: Ignore the fragmentation avoidance boost for isolation and compaction
In reply to Mel GormanWhen pageblocks get fragmented, watermarks are artifically boosted to pages are reclaimed to avoid further fragmentation events. However, compaction is often either fragmentation-neutral or moving movable pages away from unmovable/reclaimable pages. As the actual watermarks are preserved, allow compaction to ignore the boost factor.
1-socket thpscale
4.20.0-rc6 4.20.0-rc6
finishscan-v1r4 noboost-v1r4
Amean fault-both-1 0.00 ( 0.00%) 0.00 * 0.00%*
Amean fault-both-3 3849.90 ( 0.00%) 3753.53 ( 2.50%)
Amean fault-both-5 5054.13 ( 0.00%) 5396.32 ( -6.77%)
Amean fault-both-7 7061.77 ( 0.00%) 7393.46 ( -4.70%)
Amean fault-both-12 11560.59 ( 0.00%) 12155.50 ( -5.15%)
Amean fault-both-18 16120.15 ( 0.00%) 16445.96 ( -2.02%)
Amean fault-both-24 19804.31 ( 0.00%) 20465.03 ( -3.34%)
Amean fault-both-30 25018.73 ( 0.00%) 20813.54 * 16.81%*
Amean fault-both-32 24380.19 ( 0.00%) 22384.02 ( 8.19%)The impact on the scan rates is a mixed bag because this patch is very sensitive to timing and whether the boost was active or not. However, detailed tracing indicated that failure of migration due to a premature ENOMEM triggered by watermark checks were eliminated.
--- mm/page_alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/page_alloc.c +1 -1
@@ -3043,7 +3043,7 @@ int __isolate_free_page(struct page *page, unsigned int order)* watermark, because we already know our high-order page * exists. */ - watermark =min_wmark_pages (zone) + (1UL << order); + watermark = zone->_watermark[WMARK_MIN ] + (1UL << order);if (!zone_watermark_ok (zone,0 , watermark,0 ,ALLOC_CMA ))return 0 ;
Signature · 1 line
2.16.4
Re: [PATCH 09/14] mm, compaction: Ignore the fragmentation avoidance boost for isolation and compaction
Quoted 5 lines · Mel Gorman
When pageblocks get fragmented, watermarks are artifically boosted to pages are reclaimed to avoid further fragmentation events. However, compaction is often either fragmentation-neutral or moving movable pages away from unmovable/reclaimable pages. As the actual watermarks are preserved, allow compaction to ignore the boost factor.
Right, I should have realized that when reviewing the boost patch. I think it would be useful to do the same change in __compaction_suitable() as well. Compaction has its own "gap".
Quoted 37 lines
1-socket thpscale
4.20.0-rc6 4.20.0-rc6 finishscan-v1r4 noboost-v1r4 Amean fault-both-1 0.00 ( 0.00%) 0.00 * 0.00%* Amean fault-both-3 3849.90 ( 0.00%) 3753.53 ( 2.50%) Amean fault-both-5 5054.13 ( 0.00%) 5396.32 ( -6.77%) Amean fault-both-7 7061.77 ( 0.00%) 7393.46 ( -4.70%) Amean fault-both-12 11560.59 ( 0.00%) 12155.50 ( -5.15%) Amean fault-both-18 16120.15 ( 0.00%) 16445.96 ( -2.02%) Amean fault-both-24 19804.31 ( 0.00%) 20465.03 ( -3.34%) Amean fault-both-30 25018.73 ( 0.00%) 20813.54 * 16.81%* Amean fault-both-32 24380.19 ( 0.00%) 22384.02 ( 8.19%)The impact on the scan rates is a mixed bag because this patch is very sensitive to timing and whether the boost was active or not. However, detailed tracing indicated that failure of migration due to a premature ENOMEM triggered by watermark checks were eliminated.
Signed-off-by Mel Gorman <[email protected]>--- mm/page_alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/page_alloc.c +1
-1@@ -3043,7 +3043,7 @@ int __isolate_free_page(struct page *page, unsigned int order)* watermark, because we already know our high-order page * exists. */ - watermark =min_wmark_pages (zone) + (1UL << order); + watermark = zone->_watermark[WMARK_MIN ] + (1UL << order);if (!zone_watermark_ok (zone,0 , watermark,0 ,ALLOC_CMA ))return 0 ;
Re: [PATCH 09/14] mm, compaction: Ignore the fragmentation avoidance boost for isolation and compaction
Quoted 11 lines · Vlastimil Babka
Quoted 5 lines · Mel Gorman
When pageblocks get fragmented, watermarks are artifically boosted to pages are reclaimed to avoid further fragmentation events. However, compaction is often either fragmentation-neutral or moving movable pages away from unmovable/reclaimable pages. As the actual watermarks are preserved, allow compaction to ignore the boost factor.
Right, I should have realized that when reviewing the boost patch. I think it would be useful to do the same change in
__compaction_suitable()as well. Compaction has its own "gap".
That gap is somewhat static though so I'm a bit more wary of it. However, the check in __isolate_free_page looks too agressive. We isolate in units of COMPACT_CLUSTER_MAX yet the watermark check there is based on the allocation request. That means for THP that we check if 512 pages can be allocated when only somewhere between 1 and 32 is needed for that compaction cycle to complete. Adjusting that might be more appropriate?
Signature · 2 lines
Mel Gorman SUSE Labs
Re: [PATCH 09/14] mm, compaction: Ignore the fragmentation avoidance boost for isolation and compaction
Quoted 8 lines · Mel Gorman
Quoted 7 lines · Vlastimil Babka
Quoted 5 lines · Mel Gorman
When pageblocks get fragmented, watermarks are artifically boosted to pages are reclaimed to avoid further fragmentation events. However, compaction is often either fragmentation-neutral or moving movable pages away from unmovable/reclaimable pages. As the actual watermarks are preserved, allow compaction to ignore the boost factor.
Quoted 6 lines
Quoted 4 lines
Right, I should have realized that when reviewing the boost patch. I think it would be useful to do the same change in
__compaction_suitable()as well. Compaction has its own "gap".That gap is somewhat static though so I'm a bit more wary of it. However,
Well, watermark boost is dynamic, but based on allocations stealing from other migratetypes, not reflecting compaction chances of success.
Quoted 5 lines
the check in
__isolate_free_pagelooks too agressive. We isolate in units ofCOMPACT_CLUSTER_MAXyet the watermark check there is based on the allocation request. That means for THP that we check if 512 pages can be allocated when only somewhere between 1 and 32 is needed for that compaction cycle to complete. Adjusting that might be more appropriate?
AFAIU the code in __isolate_free_page() reflects that if there's less than 512 free pages gap, we might form a high-order page for THP but won't be able to allocate it afterwards due to watermark.
Re: [PATCH 09/14] mm, compaction: Ignore the fragmentation avoidance boost for isolation and compaction
Quoted 14 lines · Vlastimil Babka
Quoted 13 lines · Mel Gorman
Quoted 11 lines · Vlastimil Babka
Quoted 5 lines · Mel Gorman
When pageblocks get fragmented, watermarks are artifically boosted to pages are reclaimed to avoid further fragmentation events. However, compaction is often either fragmentation-neutral or moving movable pages away from unmovable/reclaimable pages. As the actual watermarks are preserved, allow compaction to ignore the boost factor.
Right, I should have realized that when reviewing the boost patch. I think it would be useful to do the same change in
__compaction_suitable()as well. Compaction has its own "gap".
Quoted 5 lines
Quoted 1 line
That gap is somewhat static though so I'm a bit more wary of it. However,
Well, watermark boost is dynamic, but based on allocations stealing from other migratetypes, not reflecting compaction chances of success.
True.
Quoted 9 lines
Quoted 5 lines
the check in
__isolate_free_pagelooks too agressive. We isolate in units ofCOMPACT_CLUSTER_MAXyet the watermark check there is based on the allocation request. That means for THP that we check if 512 pages can be allocated when only somewhere between 1 and 32 is needed for that compaction cycle to complete. Adjusting that might be more appropriate?AFAIU the code in
__isolate_free_page()reflects that if there's less than 512 free pages gap, we might form a high-order page for THP but won't be able to allocate it afterwards due to watermark.
Yeah but it used to be a lot more important when watermark checking for high-orders was very different. Now, if the watermark is met for order-0 and a large enough free page is allocated, the allocation succeeds so it's a lot less relevant than it used to be. kswapd will still run in the background for order-0 if necessary so a heavy watermark check there doesn't really help.
Signature · 2 lines
Mel Gorman SUSE Labs
PATCH 10/14 mm, compaction: Use free lists to quickly locate a migration source
In reply to Mel GormanThe migration scanner is a linear scan of a zone which is a potentially very large search space. Furthermore, many pageblocks are unusable such as those filled with reserved pages or partially filled with pages that cannot migrate. These still get scanned in the common case of allocating a THP and the cost accumulates.
The patch uses a partial search of the free lists to locate a migration source candidate that is marked as MOVABLE when allocating a THP. It prefers picking a block with a larger number of free pages already on the basis that there are fewer pages to migrate to free the entire block. The lowest PFN found during searches is tracked as the basis of the start for the linear search after the first search of the free list fails. After the search, the free list is shuffled so that the next search will not encounter the same page. If the search fails then the subsequent searches will be shorter and the linear scanner is used.
If this search fails, or if the request is for a small or unmovable/reclaimable allocation then the linear scanner is still used. It is somewhat pointless to use the list search in these cases. Small free pages must be used for the search and there is no guarantee that movable pages are located within that block that are contiguous.
4.20.0-rc6 4.20.0-rc6
noboost-v1r4 findmig-v1r4
Amean fault-both-3 3753.53 ( 0.00%) 3545.40 ( 5.54%)
Amean fault-both-5 5396.32 ( 0.00%) 5431.98 ( -0.66%)
Amean fault-both-7 7393.46 ( 0.00%) 7185.11 ( 2.82%)
Amean fault-both-12 12155.50 ( 0.00%) 11424.68 ( 6.01%)
Amean fault-both-18 16445.96 ( 0.00%) 14170.10 * 13.84%*
Amean fault-both-24 20465.03 ( 0.00%) 16143.57 * 21.12%*
Amean fault-both-30 20813.54 ( 0.00%) 19207.96 ( 7.71%)
Amean fault-both-32 22384.02 ( 0.00%) 20051.01 * 10.42%*Compaction migrate scanned 60836989 51005450
Compaction free scanned 890084421 780359464
This is showing a 16% reduction in migration scanning with some mild improvements on latency. A 2-socket machine showed similar reductions of scan rates in percentage terms.
--- mm/compaction.c | 179 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- mm/internal.h | 2 + 2 files changed, 179 insertions(+), 2 deletions(-)
mm/compaction.c +177 -2
@@ -1041,6 +1041,12 @@ static bool suitable_migration_target(struct compact_control *cc,return false; } +static inline unsigned int +freelist_scan_limit (struct compact_control *cc) +{ +return (COMPACT_CLUSTER_MAX >> cc->fast_search_fail) +1 ; +} +/* * Test whether the free scanner has reached the same or lower pageblock than * the migration scanner, and compaction should thus terminate.
@@ -1051,6 +1057,19 @@ static inline bool compact_scanners_met(struct compact_control *cc) <= (cc->migrate_pfn >> pageblock_order); } +/* Reorder the free list to reduce repeated future searches */ +static void +move_freelist_tail (struct list_head *freelist,struct page *freepage) +{ +LIST_HEAD (sublist); + +if (!list_is_last (freelist, &freepage->lru)) { +list_cut_position (&sublist, freelist, &freepage->lru); +if (!list_empty (&sublist)) +list_splice_tail (&sublist, freelist); + } +} +/* * Based on information in the current compact_control, find blocks * suitable for isolating free pages from and then isolate them.
@@ -1208,6 +1227,160 @@ typedef enum {*/ int sysctl_compact_unevictable_allowed__read_mostly =1 ; +static inline void +update_fast_start_pfn (struct compact_control *cc,unsigned long pfn) +{ +if (cc->fast_start_pfn ==ULONG_MAX ) +return ; + +if (!cc->fast_start_pfn) + cc->fast_start_pfn = pfn; + + cc->fast_start_pfn =min (cc->fast_start_pfn, pfn); +} + +static inline void +reinit_migrate_pfn (struct compact_control *cc) +{ +if (!cc->fast_start_pfn || cc->fast_start_pfn ==ULONG_MAX ) +return ; + + cc->migrate_pfn = cc->fast_start_pfn; + cc->fast_start_pfn =ULONG_MAX ; +} + +/* +* Briefly search the free lists for a migration source that already has +* some free pages to reduce the number of pages that need migration +* before a pageblock is free. +*/ +static unsigned long fast_find_migrateblock (struct compact_control *cc) +{ +unsigned int limit =freelist_scan_limit (cc); +unsigned int nr_scanned =0 ; +unsigned long distance; +unsigned long pfn = cc->migrate_pfn; +unsigned long high_pfn; +int order; + +/* Skip hints are relied on to avoid repeats on the fast search */ +if (cc->ignore_skip_hint) +return pfn; + +/* +* If the migrate_pfn is not at the start of a zone or the start +* of a pageblock then assume this is a continuation of a previous +* scan restarted due to COMPACT_CLUSTER_MAX. +*/ +if (pfn != cc->zone->zone_start_pfn && pfn !=pageblock_start_pfn (pfn)) +return pfn; + +/* +* For smaller orders, just linearly scan as the number of pages +* to migrate should be relatively small and does not necessarily +* justify freeing up a large block for a small allocation. +*/ +if (cc->order <=PAGE_ALLOC_COSTLY_ORDER ) +return pfn; + +/* +* Only allow kcompactd and direct requests for movable pages to +* quickly clear out a MOVABLE pageblock for allocation. This +* reduces the risk that a large movable pageblock is freed for +* an unmovable/reclaimable small allocation. +*/ +if (cc->direct_compaction && cc->migratetype !=MIGRATE_MOVABLE ) +return pfn; + +/* +* When starting the migration scanner, pick any pageblock within the +* first half of the search space. Otherwise try and pick a pageblock +* within the first eighth to reduce the chances that a migration +* target later becomes a source. +*/ + distance = (cc->free_pfn - cc->migrate_pfn) >>1 ; +if (cc->migrate_pfn != cc->zone->zone_start_pfn) + distance >>=2 ; + high_pfn =pageblock_start_pfn (cc->migrate_pfn + distance); + +for (order = cc->order -1 ; + order >=PAGE_ALLOC_COSTLY_ORDER && pfn == cc->migrate_pfn && nr_scanned < limit; + order--) { +struct free_area *area = &cc->zone->free_area[order]; +struct list_head *freelist; +unsigned long nr_skipped =0 ; +unsigned long flags; +struct page *freepage; + +if (!area->nr_free) +continue ; + +spin_lock_irqsave (&cc->zone->lock, flags); + freelist = &area->free_list[MIGRATE_MOVABLE ]; +list_for_each_entry (freepage, freelist, lru) { +unsigned long free_pfn; + + nr_scanned++; + free_pfn =page_to_pfn (freepage); +if (free_pfn < high_pfn) { +update_fast_start_pfn (cc, free_pfn); + +/* +* Avoid if skipped recently. Move to the tail +* of the list so it will not be found again +* soon +*/ +if (get_pageblock_skip (freepage)) { + +if (list_is_last (freelist, &freepage->lru)) +break ; + + nr_skipped++; +list_del (&freepage->lru); +list_add_tail (&freepage->lru, freelist); +if (nr_skipped >2 ) +break ; +continue ; + } + +/* Reorder to so a future search skips recent pages */ +move_freelist_tail (freelist, freepage); + + pfn =pageblock_start_pfn (free_pfn); + cc->fast_search_fail =0 ; +set_pageblock_skip (freepage); +break ; + } + +/* +* If low PFNs are being found and discarded then +* limit the scan as fast searching is finding +* poor candidates. +*/ +if (free_pfn < cc->migrate_pfn) + limit >>=1 ; + +if (nr_scanned >= limit) { + cc->fast_search_fail++; +move_freelist_tail (freelist, freepage); +break ; + } + } +spin_unlock_irqrestore (&cc->zone->lock, flags); + } + + cc->total_migrate_scanned += nr_scanned; + +/* +* If fast scanning failed then use a cached entry for a page block +* that had free pages as the basis for starting a linear scan. +*/ +if (pfn == cc->migrate_pfn) +reinit_migrate_pfn (cc); + +return pfn; +} +/* * Isolate all pages that can be migrated from the first suitable block, * starting at the block pointed to by the migrate scanner pfn within
@@ -1226,9 +1399,10 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,/* * Start at where we last stopped, or beginning of the zone as -* initialized by compact_zone() +* initialized by compact_zone(). The first failure will use +* the lowest PFN as the starting point for linear scanning. */ - low_pfn = cc->migrate_pfn; + low_pfn =fast_find_migrateblock (cc); block_start_pfn =pageblock_start_pfn (low_pfn);if (block_start_pfn < zone->zone_start_pfn) block_start_pfn = zone->zone_start_pfn;
@@ -1551,6 +1725,7 @@ static enum compact_result compact_zone(struct compact_control *cc)* want to compact the whole zone), but check that it is initialised * by ensuring the values are within zone boundaries. */ + cc->fast_start_pfn =0 ;if (cc->whole_zone) { cc->migrate_pfn = start_pfn; cc->free_pfn =pageblock_start_pfn (end_pfn -1 );
mm/internal.h +2 -0
@@ -188,9 +188,11 @@ struct compact_control {unsigned int nr_migratepages;/* Number of pages to migrate */ unsigned long free_pfn;/* isolate_freepages search base */ unsigned long migrate_pfn;/* isolate_migratepages search base */ +unsigned long fast_start_pfn;/* a pfn to start linear scan from */ struct zone *zone;unsigned long total_migrate_scanned;unsigned long total_free_scanned; +unsigned int fast_search_fail;/* failures to use free list searches */ const gfp_t gfp_mask;/* gfp mask of a direct compactor */ int order;/* order a direct compactor needs */ int migratetype;/* migratetype of direct compactor */
Signature · 1 line
2.16.4
PATCH 11/14 mm, compaction: Keep migration source private to a single compaction instance
In reply to Mel GormanDue to either a fast search of the free list or a linear scan, it's possible for multiple compaction instances to pick the same pageblock for migration. This is lucky for one scanner and increased scanning for all the others. It also opens a race to allocate the resulting free block.
This patch tests and updates the pageblock skip for the migration scanner more carefully. When isolating a block, it will check and skip if the block is already in use. Once the zone lock is acquired, it will be rechecked so that only one scanner can set the pageblock skip for exclusive use. Any scanner contending will continue with a linear scan. The skip bit is still set if no pages can be isolated in a range. While this may result in redundant scanning, it avoids unnecessarily acquiring the zone lock when there are no suitable migration sources.
1-socket thpscale
4.20.0-rc6 4.20.0-rc6
findmig-v1r4 isolmig-v1r4
Amean fault-both-3 3545.40 ( 0.00%) 2980.25 * 15.94%*
Amean fault-both-5 5431.98 ( 0.00%) 4393.04 * 19.13%*
Amean fault-both-7 7185.11 ( 0.00%) 5797.16 * 19.32%*
Amean fault-both-12 11424.68 ( 0.00%) 9849.61 ( 13.79%)
Amean fault-both-18 14170.10 ( 0.00%) 13816.96 ( 2.49%)
Amean fault-both-24 16143.57 ( 0.00%) 16255.20 ( -0.69%)
Amean fault-both-30 19207.96 ( 0.00%) 15741.25 * 18.05%*
Amean fault-both-32 20051.01 ( 0.00%) 16624.73 * 17.09%*This is the first patch that shows a significant reduction in latency as multiple compaction scanners do not operate on the same blocks. There is a small increase in the success rate
4.20.0-rc6 4.20.0-rc6
findmig-v1r4 isolmig-v1r4
Percentage huge-3 91.95 ( 0.00%) 95.97 ( 4.37%)
Percentage huge-5 91.40 ( 0.00%) 94.78 ( 3.70%)
Percentage huge-7 92.94 ( 0.00%) 93.94 ( 1.07%)
Percentage huge-12 92.13 ( 0.00%) 93.77 ( 1.78%)
Percentage huge-18 91.01 ( 0.00%) 94.57 ( 3.91%)
Percentage huge-24 89.56 ( 0.00%) 93.71 ( 4.63%)
Percentage huge-30 90.26 ( 0.00%) 94.14 ( 4.30%)
Percentage huge-32 90.70 ( 0.00%) 94.44 ( 4.12%)Compaction migrate scanned 51005450 25587453
Compaction free scanned 780359464 87735894
Migration scan rates are reduced by 49%. At the time of writing, the 2-socket results are not yet available.
--- mm/compaction.c | 112 +++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 87 insertions(+), 25 deletions(-)
mm/compaction.c +87 -25
@@ -279,13 +279,51 @@ void reset_isolation_suitable(pg_data_t *pgdat) } } +/* +* Sets the pageblock skip bit if it was clear. Note that this is a hint as +* locks are not required for read/writers. Returns true if it was already set. +*/ +static bool test_and_set_skip (struct compact_control *cc,struct page *page, +unsigned long pfn) +{ +bool skip; + +/* Do no update if skip hint is being ignored */ +if (cc->ignore_skip_hint) +return false; + +if (!IS_ALIGNED (pfn, pageblock_nr_pages)) +return false; + + skip =get_pageblock_skip (page); +if (!skip && !cc->no_set_skip_hint) +set_pageblock_skip (page); + +return skip; +} + +static void update_cached_migrate (struct compact_control *cc,unsigned long pfn) +{ +struct zone *zone = cc->zone; + pfn =pageblock_end_pfn (pfn); + +/* Set for isolation rather than compaction */ +if (cc->no_set_skip_hint) +return ; + +if (pfn > zone->compact_cached_migrate_pfn[0 ]) + zone->compact_cached_migrate_pfn[0 ] = pfn; +if (cc->mode !=MIGRATE_ASYNC && + pfn > zone->compact_cached_migrate_pfn[1 ]) + zone->compact_cached_migrate_pfn[1 ] = pfn; +} +/* * If no pages were isolated then mark this pageblock to be skipped in the * future. The information is later cleared by __reset_isolation_suitable(). */ static void update_pageblock_skip (struct compact_control *cc, -struct page *page,unsigned long nr_isolated, -bool migrate_scanner) +struct page *page,unsigned long nr_isolated) {struct zone *zone = cc->zone;unsigned long pfn;
@@ -304,16 +342,8 @@ static void update_pageblock_skip(struct compact_control *cc, pfn =page_to_pfn (page);/* Update where async and sync compaction should restart */ -if (migrate_scanner) { -if (pfn > zone->compact_cached_migrate_pfn[0 ]) - zone->compact_cached_migrate_pfn[0 ] = pfn; -if (cc->mode !=MIGRATE_ASYNC && - pfn > zone->compact_cached_migrate_pfn[1 ]) - zone->compact_cached_migrate_pfn[1 ] = pfn; - }else { -if (pfn < zone->compact_cached_free_pfn) - zone->compact_cached_free_pfn = pfn; - } +if (pfn < zone->compact_cached_free_pfn) + zone->compact_cached_free_pfn = pfn; }#else static inline bool isolation_suitable(struct compact_control *cc,
@@ -561,7 +591,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,/* Update the pageblock-skip if the whole pageblock was scanned */ if (blockpfn == end_pfn) -update_pageblock_skip (cc, valid_page, total_isolated, false); +update_pageblock_skip (cc, valid_page, total_isolated); cc->total_free_scanned += nr_scanned;if (total_isolated)
@@ -696,6 +726,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,unsigned long start_pfn = low_pfn;bool skip_on_failure = false;unsigned long next_skip_pfn =0 ; +bool skip_updated = false;/* * Ensure that there are not too many pages isolated from the LRU
@@ -762,8 +793,19 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, page =pfn_to_page (low_pfn); -if (!valid_page) +/* +* Check if the pageblock has already been marked skipped. +* Only the aligned PFN is checked as the caller isolates +* COMPACT_CLUSTER_MAX at a time so the second call must +* not falsely conclude that the block should be skipped. +*/ +if (!valid_page &&IS_ALIGNED (low_pfn, pageblock_nr_pages)) { +if (!cc->ignore_skip_hint &&get_pageblock_skip (page)) { + low_pfn = end_pfn; + goto isolate_abort; + } valid_page = page; + }/* * Skip if free. We read page order here without zone lock
@@ -851,8 +893,19 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,if (!locked) { locked =compact_trylock_irqsave (zone_lru_lock (zone), &flags, cc); -if (!locked) + +/* Allow future scanning if the lock is contended */ +if (!locked) { +clear_pageblock_skip (page);break ; + } + +/* Try get exclusive access under lock */ +if (!skip_updated) { + skip_updated = true; +if (test_and_set_skip (cc, page, low_pfn)) + goto isolate_abort; + }/* Recheck PageLRU and PageCompound under lock */ if (!PageLRU (page))
@@ -930,15 +983,20 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,if (unlikely (low_pfn > end_pfn)) low_pfn = end_pfn; +isolate_abort:if (locked)spin_unlock_irqrestore (zone_lru_lock (zone), flags);/* -* Update the pageblock-skip information and cached scanner pfn, -* if the whole pageblock was scanned without isolating any page. +* Updated the cached scanner pfn if the pageblock was scanned +* without isolating a page. The pageblock may not be marked +* skipped already if there were no LRU pages in the block. */ -if (low_pfn == end_pfn) -update_pageblock_skip (cc, valid_page, nr_isolated, true); +if (low_pfn == end_pfn && !nr_isolated) { +if (valid_page && !skip_updated) +set_pageblock_skip (valid_page); +update_cached_migrate (cc, low_pfn); + }trace_mm_compaction_isolate_migratepages (start_pfn, low_pfn, nr_scanned, nr_isolated);
@@ -1323,8 +1381,6 @@ static unsigned long fast_find_migrateblock(struct compact_control *cc) nr_scanned++; free_pfn =page_to_pfn (freepage);if (free_pfn < high_pfn) { -update_fast_start_pfn (cc, free_pfn); -/* * Avoid if skipped recently. Move to the tail * of the list so it will not be found again
@@ -1346,9 +1402,9 @@ static unsigned long fast_find_migrateblock(struct compact_control *cc)/* Reorder to so a future search skips recent pages */ move_freelist_tail (freelist, freepage); +update_fast_start_pfn (cc, free_pfn); pfn =pageblock_start_pfn (free_pfn); cc->fast_search_fail =0 ; -set_pageblock_skip (freepage);break ; }
@@ -1418,7 +1474,6 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone, low_pfn = block_end_pfn, block_start_pfn = block_end_pfn, block_end_pfn += pageblock_nr_pages) { -/* * This can potentially iterate a massively long zone with * many pageblocks unsuitable, so periodically check if we
@@ -1433,8 +1488,15 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,if (!page)continue ; -/* If isolation recently failed, do not retry */ -if (!isolation_suitable (cc, page)) +/* +* If isolation recently failed, do not retry. Only check the +* pageblock once. COMPACT_CLUSTER_MAX causes a pageblock +* to be visited multiple times. Assume skip was checked +* before making it "skip" so other compaction instances do +* not scan the same block. +*/ +if (IS_ALIGNED (low_pfn, pageblock_nr_pages) && + !isolation_suitable (cc, page))continue ;/*
Signature · 1 line
2.16.4
PATCH 12/14 mm, compaction: Use free lists to quickly locate a migration target
In reply to Mel GormanSimilar to the migration scanner, this uses the free lists to quickly locate a migration target. The search is different in that lower orders will be searched for a suitable high PFN if necessary but the search is still bound. This is justified on the grounds that the free scanner typically scans linearly much more than the migration scanner.
If a free page is found, it is isolated and the full pageblock is scanned for any remaining free pages. This is done so that it's possible to mark the pageblock for skipping in the near future.
1-socket thpfioscale
4.20.0-rc6 4.20.0-rc6
isolmig-v1r4 findfree-v1r8
Amean fault-both-3 2980.25 ( 0.00%) 2911.07 ( 2.32%)
Amean fault-both-5 4393.04 ( 0.00%) 4692.96 ( -6.83%)
Amean fault-both-7 5797.16 ( 0.00%) 6449.17 ( -11.25%)
Amean fault-both-12 9849.61 ( 0.00%) 9778.40 ( 0.72%)
Amean fault-both-18 13816.96 ( 0.00%) 11756.92 ( 14.91%)
Amean fault-both-24 16255.20 ( 0.00%) 13675.93 * 15.87%*
Amean fault-both-30 15741.25 ( 0.00%) 17195.41 ( -9.24%)
Amean fault-both-32 16624.73 ( 0.00%) 18150.08 ( -9.18%)The impact on latency is variable but the search is optimistic and sensitive to the exact system state. Success rates are similar but the major impact is to the rate of scanning
4.20.0-rc6 4.20.0-rc6
isolmig-v1r4findfree-v1r8
Compaction migrate scanned 25587453 27634284
Compaction free scanned 87735894 55279519The free scan rates are reduced by 37%.
--- mm/compaction.c | 201 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 197 insertions(+), 4 deletions(-)
mm/compaction.c +197 -4
@@ -1117,7 +1117,7 @@ static inline bool compact_scanners_met(struct compact_control *cc)/* Reorder the free list to reduce repeated future searches */ static void -move_freelist_tail (struct list_head *freelist,struct page *freepage) +move_freelist_head (struct list_head *freelist,struct page *freepage) {LIST_HEAD (sublist);
@@ -1128,6 +1128,193 @@ move_freelist_tail(struct list_head *freelist, struct page *freepage) } } +static void +move_freelist_tail (struct list_head *freelist,struct page *freepage) +{ +LIST_HEAD (sublist); + +if (!list_is_last (freelist, &freepage->lru)) { +list_cut_before (&sublist, freelist, &freepage->lru); +if (!list_empty (&sublist)) +list_splice_tail (&sublist, freelist); + } +} + +static void +fast_isolate_around (struct compact_control *cc,unsigned long pfn,unsigned long nr_isolated) +{ +unsigned long start_pfn, end_pfn; +struct page *page =pfn_to_page (pfn); + +/* Do not search around if there are enough pages already */ +if (cc->nr_freepages >= cc->nr_migratepages) +return ; + +/* Minimise scanning during async compaction */ +if (cc->direct_compaction && cc->mode ==MIGRATE_ASYNC ) +return ; + +/* Pageblock boundaries */ + start_pfn =pageblock_start_pfn (pfn); + end_pfn =min (start_pfn + pageblock_nr_pages,zone_end_pfn (cc->zone)); + +/* Scan before */ +if (start_pfn != pfn) { +isolate_freepages_block (cc, &start_pfn, pfn, &cc->freepages, false); +if (cc->nr_freepages >= cc->nr_migratepages) +return ; + } + +/* Scan after */ + start_pfn = pfn + nr_isolated; +if (start_pfn != end_pfn) +isolate_freepages_block (cc, &start_pfn, end_pfn, &cc->freepages, false); + +/* Skip this pageblock in the future as it's full or nearly full */ +if (cc->nr_freepages < cc->nr_migratepages) +set_pageblock_skip (page); +} + +static unsigned long +fast_isolate_freepages (struct compact_control *cc) +{ +unsigned int limit =min (1U ,freelist_scan_limit (cc) >>1 ); +unsigned int order_scanned =0 , nr_scanned =0 ; +unsigned long low_pfn, min_pfn, high_pfn =0 , highest =0 ; +unsigned long nr_isolated =0 ; +unsigned long distance; +struct page *page =NULL ; +bool scan_start = false; +int order; + +/* +* If starting the scan, use a deeper search and use the highest +* PFN found if a suitable one is not found. +*/ +if (cc->free_pfn ==pageblock_start_pfn (zone_end_pfn (cc->zone) -1 )) { + limit = pageblock_nr_pages >>1 ; + scan_start = true; + } + +/* +* Preferred point is in the top quarter of the scan space but take +* a pfn from the top half if the search is problematic. +*/ + distance = (cc->free_pfn - cc->migrate_pfn); + low_pfn =pageblock_start_pfn (cc->free_pfn - (distance >>2 )); + min_pfn =pageblock_start_pfn (cc->free_pfn - (distance >>1 )); + +if (WARN_ON_ONCE (min_pfn > low_pfn)) + low_pfn = min_pfn; + +for (order = cc->order -1 ; + order >=0 && !page; + order--) { +struct free_area *area = &cc->zone->free_area[order]; +struct list_head *freelist; +struct page *freepage; +unsigned long flags; + +if (!area->nr_free) +continue ; + +spin_lock_irqsave (&cc->zone->lock, flags); + freelist = &area->free_list[MIGRATE_MOVABLE ]; +list_for_each_entry_reverse (freepage, freelist, lru) { +unsigned long pfn; + + order_scanned++; + nr_scanned++; + pfn =page_to_pfn (freepage); + +if (pfn >= highest) + highest =pageblock_start_pfn (pfn); + +if (pfn >= low_pfn) { + cc->fast_search_fail =0 ; + page = freepage; +break ; + } + +if (pfn >= min_pfn && pfn > high_pfn) { + high_pfn = pfn; + +/* Shorten the scan if a candidate is found */ + limit >>=1 ; + } + +if (order_scanned >= limit) +break ; + } + +/* Use a minimum pfn if a preferred one was not found */ +if (!page && high_pfn) { + page =pfn_to_page (high_pfn); + +/* Update freepage for the list reorder below */ + freepage = page; + } + +/* Reorder to so a future search skips recent pages */ +move_freelist_head (freelist, freepage); + +/* Isolate the page if available */ +if (page) { +if (__isolate_free_page (page, order)) { +set_page_private (page, order); + nr_isolated =1 << order; + cc->nr_freepages += nr_isolated; +list_add_tail (&page->lru, &cc->freepages); +count_compact_events (COMPACTISOLATED , nr_isolated); + }else { +/* If isolation fails, abort the search */ + order =-1 ; + page =NULL ; + } + } + +spin_unlock_irqrestore (&cc->zone->lock, flags); + +/* +* Smaller scan on next order so the total scan ig related +* to freelist_scan_limit. +*/ +if (order_scanned >= limit) + limit =min (1U , limit >>1 ); + } + +if (!page) { + cc->fast_search_fail++; +if (scan_start) { +/* +* Use the highest PFN found above min. If one was +* not found, be pessemistic for direct compaction +* and use the min mark. +*/ +if (highest) { + page =pfn_to_page (highest); + cc->free_pfn = highest; + }else { +if (cc->direct_compaction) { + page =pfn_to_page (min_pfn); + cc->free_pfn = min_pfn; + } + } + } + } + +if (highest && highest > cc->zone->compact_cached_free_pfn) + cc->zone->compact_cached_free_pfn = highest; + + cc->total_free_scanned += nr_scanned; +if (!page) +return 0 ; + + low_pfn =page_to_pfn (page); +fast_isolate_around (cc, low_pfn, nr_isolated); +return low_pfn; +} +/* * Based on information in the current compact_control, find blocks * suitable for isolating free pages from and then isolate them.
@@ -1142,6 +1329,11 @@ static void isolate_freepages(struct compact_control *cc)unsigned long low_pfn;/* lowest pfn scanner is able to scan */ struct list_head *freelist = &cc->freepages; +/* Try a small search of the free lists for a candidate */ + isolate_start_pfn =fast_isolate_freepages (cc); +if (isolate_start_pfn) + goto splitmap; +/* * Initialise the free scanner. The starting point is where we last * successfully isolated from, zone-cached value, or the end of the
@@ -1218,9 +1410,6 @@ static void isolate_freepages(struct compact_control *cc) } } -/* __isolate_free_page() does not map the pages */ -split_map_pages (freelist); -/* * Record where the free scanner will restart next time. Either we * broke from the loop and set isolate_start_pfn based on the last
@@ -1228,6 +1417,10 @@ static void isolate_freepages(struct compact_control *cc)* and the loop terminated due to isolate_start_pfn < low_pfn */ cc->free_pfn = isolate_start_pfn; + +splitmap: +/* __isolate_free_page() does not map the pages */ +split_map_pages (freelist); }/*
Signature · 1 line
2.16.4
PATCH 13/14 mm, compaction: Capture a page under direct compaction
In reply to Mel GormanCompaction is inherently race-prone as a suitable page freed during compaction can be allocated by any parallel task. This patch uses a capture_control structure to isolate a page immediately when it is freed by a direct compactor in the slow path of the page allocator.
4.20.0-rc6 4.20.0-rc6
findfree-v1r8 capture-v1r8
Amean fault-both-3 2911.07 ( 0.00%) 2898.64 ( 0.43%)
Amean fault-both-5 4692.96 ( 0.00%) 4296.58 ( 8.45%)
Amean fault-both-7 6449.17 ( 0.00%) 6203.55 ( 3.81%)
Amean fault-both-12 9778.40 ( 0.00%) 9309.13 ( 4.80%)
Amean fault-both-18 11756.92 ( 0.00%) 6245.27 * 46.88%*
Amean fault-both-24 13675.93 ( 0.00%) 15083.42 ( -10.29%)
Amean fault-both-30 17195.41 ( 0.00%) 11498.60 * 33.13%*
Amean fault-both-32 18150.08 ( 0.00%) 9684.82 * 46.64%*As expected, the biggest reduction in latency is when there are multiple compaction instances that would previously compete for the same blocks. THP allocation rates are also slightly higher.
4.20.0-rc6 4.20.0-rc6
findfree-v1r8 capture-v1r8
Percentage huge-1 0.00 ( 0.00%) 0.00 ( 0.00%)
Percentage huge-3 97.63 ( 0.00%) 98.12 ( 0.49%)
Percentage huge-5 96.11 ( 0.00%) 98.83 ( 2.84%)
Percentage huge-7 95.44 ( 0.00%) 97.99 ( 2.68%)
Percentage huge-12 95.36 ( 0.00%) 99.00 ( 3.82%)
Percentage huge-18 95.32 ( 0.00%) 98.92 ( 3.78%)
Percentage huge-24 95.13 ( 0.00%) 99.08 ( 4.15%)
Percentage huge-30 95.53 ( 0.00%) 99.22 ( 3.86%)
Percentage huge-32 94.94 ( 0.00%) 98.97 ( 4.25%)And scan rates are reduced
Compaction migrate scanned 27634284 19002941 Compaction free scanned 55279519 46395714
--- include/linux/compaction.h | 3 ++- include/linux/sched.h | 4 ++++ kernel/sched/core.c | 3 +++ mm/compaction.c | 31 +++++++++++++++++++------ mm/internal.h | 9 +++++++ mm/page_alloc.c | 58 ++++++++++++++++++++++++++++++++++++++++++---- 6 files changed, 96 insertions(+), 12 deletions(-)
include/linux/compaction.h +2 -1
@@ -95,7 +95,8 @@ extern int sysctl_compact_unevictable_allowed;extern int fragmentation_index (struct zone *zone,unsigned int order);extern enum compact_result try_to_compact_pages (gfp_t gfp_mask,unsigned int order,unsigned int alloc_flags, -const struct alloc_context *ac,enum compact_priority prio); +const struct alloc_context *ac,enum compact_priority prio, +struct page **page);extern void reset_isolation_suitable (pg_data_t *pgdat);extern enum compact_result compaction_suitable (struct zone *zone,int order,unsigned int alloc_flags,int classzone_idx);
include/linux/sched.h +4 -0
@@ -47,6 +47,7 @@ struct pid_namespace;struct pipe_inode_info ;struct rcu_node ;struct reclaim_state ; +struct capture_control ;struct robust_list_head ;struct sched_attr ;struct sched_param ;
@@ -964,6 +965,9 @@ struct task_struct {struct io_context *io_context; +#ifdef CONFIG_COMPACTION +struct capture_control *capture_control; +#endif /* Ptrace state: */ unsigned long ptrace_message;kernel_siginfo_t *last_siginfo;
kernel/sched/core.c +3 -0
@@ -2177,6 +2177,9 @@ static void __sched_fork(unsigned long clone_flags, struct task_struct *p)INIT_HLIST_HEAD (&p->preempt_notifiers);#endif +#ifdef CONFIG_COMPACTION + p->capture_control =NULL ; +#endif init_numa_balancing (clone_flags, p); }
mm/compaction.c +24 -7
@@ -1949,7 +1949,8 @@ bool compaction_zonelist_suitable(struct alloc_context *ac, int order,return false; } -static enum compact_result compact_zone (struct compact_control *cc) +static enum compact_result +compact_zone (struct compact_control *cc,struct capture_control *capc) {enum compact_result ret;unsigned long start_pfn = cc->zone->zone_start_pfn;
@@ -2086,6 +2087,11 @@ static enum compact_result compact_zone(struct compact_control *cc) } } +/* Stop if a page has been captured */ +if (capc && capc->page) { + ret =COMPACT_SUCCESS ; +break ; + } } out:
@@ -2119,7 +2125,8 @@ static enum compact_result compact_zone(struct compact_control *cc)static enum compact_result compact_zone_order (struct zone *zone,int order,gfp_t gfp_mask,enum compact_priority prio, -unsigned int alloc_flags,int classzone_idx) +unsigned int alloc_flags,int classzone_idx, +struct page **capture) {enum compact_result ret;struct compact_control cc = {
@@ -2139,14 +2146,24 @@ static enum compact_result compact_zone_order(struct zone *zone, int order, .ignore_skip_hint = (prio == MIN_COMPACT_PRIORITY), .ignore_block_suitable = (prio == MIN_COMPACT_PRIORITY) }; +struct capture_control capc = { + .cc = &cc, + .page =NULL , + }; + +if (capture) + current->capture_control = &capc;INIT_LIST_HEAD (&cc.freepages);INIT_LIST_HEAD (&cc.migratepages); - ret =compact_zone (&cc); + ret =compact_zone (&cc, &capc);VM_BUG_ON (!list_empty (&cc.freepages));VM_BUG_ON (!list_empty (&cc.migratepages)); + *capture = capc.page; + current->capture_control =NULL ; +return ret; }
@@ -2164,7 +2181,7 @@ int sysctl_extfrag_threshold = 500;*/ enum compact_result try_to_compact_pages (gfp_t gfp_mask,unsigned int order,unsigned int alloc_flags,const struct alloc_context *ac, -enum compact_priority prio) +enum compact_priority prio,struct page **capture) {int may_perform_io = gfp_mask & __GFP_IO;struct zoneref *z;
@@ -2192,7 +2209,7 @@ enum compact_result try_to_compact_pages(gfp_t gfp_mask, unsigned int order, } status =compact_zone_order (zone, order, gfp_mask, prio, - alloc_flags,ac_classzone_idx (ac)); + alloc_flags,ac_classzone_idx (ac), capture); rc =max (status, rc);/* The allocation should succeed, stop compacting */
@@ -2260,7 +2277,7 @@ static void compact_node(int nid)INIT_LIST_HEAD (&cc.freepages);INIT_LIST_HEAD (&cc.migratepages); -compact_zone (&cc); +compact_zone (&cc,NULL );VM_BUG_ON (!list_empty (&cc.freepages));VM_BUG_ON (!list_empty (&cc.migratepages));
@@ -2402,7 +2419,7 @@ static void kcompactd_do_work(pg_data_t *pgdat)if (kthread_should_stop ())return ; - status =compact_zone (&cc); + status =compact_zone (&cc,NULL );if (status ==COMPACT_SUCCESS ) {compaction_defer_reset (zone, cc.order, false);
mm/internal.h +9 -0
@@ -207,6 +207,15 @@ struct compact_control {bool contended;/* Signal lock or sched contention */ }; +/* +* Used in direct compaction when a page should be taken from the freelists +* immediately when one is created during the free path. +*/ +struct capture_control { +struct compact_control *cc; +struct page *page; +}; +unsigned long isolate_freepages_range (struct compact_control *cc,unsigned long start_pfn,unsigned long end_pfn);
mm/page_alloc.c +54 -4
99 lines not on this page · raw message
Signature · 6 lines
2.16.4 -- Mel Gorman SUSE Labs
PATCH 14/14 mm, compaction: Do not direct compact remote memory
In reply to Mel GormanRemote compaction is expensive and possibly counter-productive. Locality is expected to often have better performance characteristics than remote high-order pages. For small allocations, it's expected that locality is generally required or fallbacks are possible. For larger allocations such as THP, they are forbidden at the time of writing but if __GFP_THISNODE is ever removed, then it would still be preferable to fallback to small local base pages over remote THP in the general case. kcompactd is still woken via kswapd so compaction happens eventually.
--- mm/compaction.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
mm/compaction.c +10 -0
@@ -2208,6 +2208,16 @@ enum compact_result try_to_compact_pages(gfp_t gfp_mask, unsigned int order,continue ; } +/* +* Do not compact remote memory. It's expensive and high-order +* small allocations are expected to prefer or require local +* memory. Similarly, larger requests such as THP can fallback +* to base pages in preference to remote huge pages if +* __GFP_THISNODE is not specified +*/ +if (zone_to_nid (zone) !=zone_to_nid (ac->preferred_zoneref->zone)) +continue ; + status =compact_zone_order (zone, order, gfp_mask, prio, alloc_flags,ac_classzone_idx (ac), capture); rc =max (status, rc);
Signature · 6 lines
2.16.4 -- Mel Gorman SUSE Labs