On 2/20/24 07:16, Baolin Wang wrote: > Currently we will use 'cc->nr_freepages >= cc->nr_migratepages' comparison > to ensure that enough freepages are isolated in isolate_freepages(), however > it just decreases the cc->nr_freepages without updating cc->nr_migratepages > in compaction_alloc(), which will waste more CPU cycles and cause too many > freepages to be isolated. > > So we should also update the cc->nr_migratepages when allocating or freeing > the freepages to avoid isolating excess freepages. And I can see fewer free > pages are scanned and isolated when running thpcompact on my Arm64 server: > k6.7 k6.7_patched > Ops Compaction pages isolated 120692036.00 118160797.00 > Ops Compaction migrate scanned 131210329.00 154093268.00 > Ops Compaction free scanned 1090587971.00 1080632536.00 > Ops Compact scan efficiency 12.03 14.26 > > Moreover, I did not see an obvious latency improvements, this is likely because > isolating freepages is not the bottleneck in the thpcompact test case. > k6.7 k6.7_patched > Amean fault-both-1 1089.76 ( 0.00%) 1080.16 * 0.88%* > Amean fault-both-3 1616.48 ( 0.00%) 1636.65 * -1.25%* > Amean fault-both-5 2266.66 ( 0.00%) 2219.20 * 2.09%* > Amean fault-both-7 2909.84 ( 0.00%) 2801.90 * 3.71%* > Amean fault-both-12 4861.26 ( 0.00%) 4733.25 * 2.63%* > Amean fault-both-18 7351.11 ( 0.00%) 6950.51 * 5.45%* > Amean fault-both-24 9059.30 ( 0.00%) 9159.99 * -1.11%* > Amean fault-both-30 10685.68 ( 0.00%) 11399.02 * -6.68%* > > Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx> > Acked-by: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx> Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx> Thanks. > --- > Hi Andrew, please use this patch to replace below 2 old patches. Thanks. > https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?h=mm-everything-2024-02-20-04-19&id=cb30899d456d64fb83ee3e3d95cd9fbb18735d87 > https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?h=mm-everything-2024-02-20-04-19&id=65713d1c4fc498d35dc1f7c1234ef815aa128429 > > Changes from v2: > - Add acked tag from Mel. > - Fix the mm_compaction_migratepages trace event. > Changes from v1: > - Rebased on the latest mm-unstable branch. > --- > include/trace/events/compaction.h | 6 +++--- > mm/compaction.c | 12 ++++++++++-- > 2 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/include/trace/events/compaction.h b/include/trace/events/compaction.h > index 2b2a975efd20..d05759d18538 100644 > --- a/include/trace/events/compaction.h > +++ b/include/trace/events/compaction.h > @@ -78,10 +78,10 @@ DEFINE_EVENT(mm_compaction_isolate_template, mm_compaction_fast_isolate_freepage > #ifdef CONFIG_COMPACTION > TRACE_EVENT(mm_compaction_migratepages, > > - TP_PROTO(struct compact_control *cc, > + TP_PROTO(unsigned int nr_migratepages, > unsigned int nr_succeeded), > > - TP_ARGS(cc, nr_succeeded), > + TP_ARGS(nr_migratepages, nr_succeeded), > > TP_STRUCT__entry( > __field(unsigned long, nr_migrated) > @@ -90,7 +90,7 @@ TRACE_EVENT(mm_compaction_migratepages, > > TP_fast_assign( > __entry->nr_migrated = nr_succeeded; > - __entry->nr_failed = cc->nr_migratepages - nr_succeeded; > + __entry->nr_failed = nr_migratepages - nr_succeeded; > ), > > TP_printk("nr_migrated=%lu nr_failed=%lu", > diff --git a/mm/compaction.c b/mm/compaction.c > index 4494b2914386..218089b29f13 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -1798,6 +1798,7 @@ static struct folio *compaction_alloc(struct folio *src, unsigned long data) > dst = list_entry(cc->freepages.next, struct folio, lru); > list_del(&dst->lru); > cc->nr_freepages--; > + cc->nr_migratepages--; > > return dst; > } > @@ -1813,6 +1814,7 @@ static void compaction_free(struct folio *dst, unsigned long data) > > list_add(&dst->lru, &cc->freepages); > cc->nr_freepages++; > + cc->nr_migratepages++; > } > > /* possible outcome of isolate_migratepages */ > @@ -2435,7 +2437,7 @@ compact_zone(struct compact_control *cc, struct capture_control *capc) > unsigned long last_migrated_pfn; > const bool sync = cc->mode != MIGRATE_ASYNC; > bool update_cached; > - unsigned int nr_succeeded = 0; > + unsigned int nr_succeeded = 0, nr_migratepages; > > /* > * These counters track activities during zone compaction. Initialize > @@ -2553,11 +2555,17 @@ compact_zone(struct compact_control *cc, struct capture_control *capc) > pageblock_start_pfn(cc->migrate_pfn - 1)); > } > > + /* > + * Record the number of pages to migrate since the > + * compaction_alloc/free() will update cc->nr_migratepages > + * properly. > + */ > + nr_migratepages = cc->nr_migratepages; > err = migrate_pages(&cc->migratepages, compaction_alloc, > compaction_free, (unsigned long)cc, cc->mode, > MR_COMPACTION, &nr_succeeded); > > - trace_mm_compaction_migratepages(cc, nr_succeeded); > + trace_mm_compaction_migratepages(nr_migratepages, nr_succeeded); > > /* All pages were either migrated or will be released */ > cc->nr_migratepages = 0;
