| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 26 Feb 2024 12:07:28 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Richard Chang <richardycc@...gle.com>
Cc: mhiramat@...nel.org, mathieu.desnoyers@...icios.com,
akpm@...ux-foundation.org, liumartin@...gle.com, surenb@...gle.com,
minchan@...nel.org, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH] mm: add alloc_contig_migrate_range allocation
statistics
On Mon, 26 Feb 2024 12:06:29 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:
> On Mon, 26 Feb 2024 10:00:15 +0000
> Richard Chang <richardycc@...gle.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 <richardycc@...gle.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
Oops, I forgot to remove this sig when I came back and added the below
reply too.
-- 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
>
Powered by blists - more mailing lists