On Mon, 26 Feb 2024 10:00:15 +0000
Richard Chang <richard...@google.com> wrote:
> alloc_contig_migrate_range has every information to be able to
> understand big contiguous allocation latency. For example, how many
> pages are migrated, how many times they were needed to unmap from
> page tables.
>
> This patch adds the trace event to collect the allocation statistics.
> In the field, it was quite useful to understand CMA allocation
> latency.
>
> Signed-off-by: Richard Chang <richard...@google.com>
> ---
> include/trace/events/kmem.h | 39 +++++++++++++++++++++++++++++++++++++
> mm/internal.h | 3 ++-
> mm/page_alloc.c | 30 +++++++++++++++++++++++-----
> mm/page_isolation.c | 2 +-
> 4 files changed, 67 insertions(+), 7 deletions(-)
>
> diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h
> index 58688768ef0f..964704d76f9f 100644
> --- a/include/trace/events/kmem.h
> +++ b/include/trace/events/kmem.h
> @@ -304,6 +304,45 @@ TRACE_EVENT(mm_page_alloc_extfrag,
> __entry->change_ownership)
> );
>
> +TRACE_EVENT(mm_alloc_contig_migrate_range_info,
> +
> + TP_PROTO(unsigned long start,
> + unsigned long end,
> + int migratetype,
> + unsigned long nr_migrated,
> + unsigned long nr_reclaimed,
> + unsigned long nr_mapped),
> +
> + TP_ARGS(start, end, migratetype,
> + nr_migrated, nr_reclaimed, nr_mapped),
> +
> + TP_STRUCT__entry(
> + __field(unsigned long, start)
> + __field(unsigned long, end)
> + __field(int, migratetype)
Please move the int to the end of the longs, as it will cause a 4 byte hole
in 64 bit machines otherwise.
> + __field(unsigned long, nr_migrated)
> + __field(unsigned long, nr_reclaimed)
> + __field(unsigned long, nr_mapped)
> + ),
> +
> + TP_fast_assign(
> + __entry->start = start;
> + __entry->end = end;
> + __entry->migratetype = migratetype;
> + __entry->nr_migrated = nr_migrated;
> + __entry->nr_reclaimed = nr_reclaimed;
> + __entry->nr_mapped = nr_mapped;
> + ),
> +
> + TP_printk("start=0x%lx end=0x%lx migratetype=%d nr_migrated=%lu
> nr_reclaimed=%lu nr_mapped=%lu",
> + __entry->start,
> + __entry->end,
> + __entry->migratetype,
> + __entry->nr_migrated,
> + __entry->nr_reclaimed,
> + __entry->nr_mapped)
> +);
> +
> /*
> * Required for uniquely and securely identifying mm in rss_stat tracepoint.
> */
> diff --git a/mm/internal.h b/mm/internal.h
> index f309a010d50f..e114c647e278 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -537,7 +537,8 @@ isolate_migratepages_range(struct compact_control *cc,
> unsigned long low_pfn, unsigned long end_pfn);
>
> int __alloc_contig_migrate_range(struct compact_control *cc,
> - unsigned long start, unsigned long end);
> + unsigned long start, unsigned long end,
> + int migratetype);
>
> /* Free whole pageblock and set its migration type to MIGRATE_CMA. */
> void init_cma_reserved_pageblock(struct page *page);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 150d4f23b010..f840bc785afa 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6219,9 +6219,14 @@ static void alloc_contig_dump_pages(struct list_head
> *page_list)
> }
> }
>
> -/* [start, end) must belong to a single zone. */
> +/*
> + * [start, end) must belong to a single zone.
> + * @migratetype: using migratetype to filter the type of migration in
> + * trace_mm_alloc_contig_migrate_range_info.
> + */
> int __alloc_contig_migrate_range(struct compact_control *cc,
> - unsigned long start, unsigned long end)
> + unsigned long start, unsigned long end,
> + int migratetype)
> {
> /* This function is based on compact_zone() from compaction.c. */
> unsigned int nr_reclaimed;
> @@ -6232,6 +6237,10 @@ int __alloc_contig_migrate_range(struct
> compact_control *cc,
> .nid = zone_to_nid(cc->zone),
> .gfp_mask = GFP_USER | __GFP_MOVABLE | __GFP_RETRY_MAYFAIL,
> };
> + struct page *page;
> + unsigned long total_mapped = 0;
> + unsigned long total_migrated = 0;
> + unsigned long total_reclaimed = 0;
>
> lru_cache_disable();
>
> @@ -6257,9 +6266,16 @@ int __alloc_contig_migrate_range(struct
> compact_control *cc,
> &cc->migratepages);
> cc->nr_migratepages -= nr_reclaimed;
>
> + total_reclaimed += nr_reclaimed;
> + list_for_each_entry(page, &cc->migratepages, lru)
> + total_mapped += page_mapcount(page);
You're doing this calculation regardless of if tracing is enabled or not
and it's only used for tracing? Please add:
if (trace_mm_alloc_contig_migrate_range_info_enabled()) {
total_reclaimed += nr_reclaimed;
list_for_each_entry(page, &cc->migratepages, lru)
total_mapped += page_mapcount(page);
}
-- Steve
> +
> ret = migrate_pages(&cc->migratepages, alloc_migration_target,
> NULL, (unsigned long)&mtc, cc->mode, MR_CONTIG_RANGE,
> NULL);
>
> + if (!ret)
May want the above to be:
if (trace_mm_alloc_config_migrate_range_info_enabled() && !ret)
too.
-- Steve
> + total_migrated += cc->nr_migratepages;
> +
> /*
> * On -ENOMEM, migrate_pages() bails out right away. It is
> pointless
> * to retry again over this error, so do the same here.
> @@ -6273,9 +6289,13 @@ int __alloc_contig_migrate_range(struct
> compact_control *cc,
> if (!(cc->gfp_mask & __GFP_NOWARN) && ret == -EBUSY)
> alloc_contig_dump_pages(&cc->migratepages);
> putback_movable_pages(&cc->migratepages);
> - return ret;
> }
> - return 0;
> +
> + trace_mm_alloc_contig_migrate_range_info(start, end, migratetype,
> + total_migrated,
> + total_reclaimed,
> + total_mapped);
> + return (ret < 0) ? ret : 0;
> }
>
> /**
> @@ -6355,7 +6375,7 @@ int alloc_contig_range(unsigned long start, unsigned
> long end,
> * allocated. So, if we fall through be sure to clear ret so that
> * -EBUSY is not accidentally used or returned to caller.
> */
> - ret = __alloc_contig_migrate_range(&cc, start, end);
> + ret = __alloc_contig_migrate_range(&cc, start, end, migratetype);
> if (ret && ret != -EBUSY)
> goto done;
> ret = 0;
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index cd0ea3668253..a5c8fa4c2a75 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -434,7 +434,7 @@ static int isolate_single_pageblock(unsigned long
> boundary_pfn, int flags,
> }
>
> ret = __alloc_contig_migrate_range(&cc,
> head_pfn,
> - head_pfn + nr_pages);
> + head_pfn + nr_pages,
> page_mt);
>
> /*
> * restore the page's migratetype so that it can
