On Wed, May 07 2014, Vlastimil Babka wrote: > During compaction, update_nr_listpages() has been used to count remaining > non-migrated and free pages after a call to migrage_pages(). The freepages > counting has become unneccessary, and it turns out that migratepages counting > is also unnecessary in most cases. > > The only situation when it's needed to count cc->migratepages is when > migrate_pages() returns with a negative error code. Otherwise, the non-negative > return value is the number of pages that were not migrated, which is exactly > the count of remaining pages in the cc->migratepages list. > > Furthermore, any non-zero count is only interesting for the tracepoint of > mm_compaction_migratepages events, because after that all remaining unmigrated > pages are put back and their count is set to 0. > > This patch therefore removes update_nr_listpages() completely, and changes the > tracepoint definition so that the manual counting is done only when the > tracepoint is enabled, and only when migrate_pages() returns a negative error > code. > > Furthermore, migrate_pages() and the tracepoints won't be called when there's > nothing to migrate. This potentially avoids some wasted cycles and reduces the > volume of uninteresting mm_compaction_migratepages events where "nr_migrated=0 > nr_failed=0". In the stress-highalloc mmtest, this was about 75% of the events. > The mm_compaction_isolate_migratepages event is better for determining that > nothing was isolated for migration, and this one was just duplicating the info. > > Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx> > Reviewed-by: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx> > Cc: Minchan Kim <minchan@xxxxxxxxxx> > Cc: Mel Gorman <mgorman@xxxxxxx> > Cc: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> > Cc: Michal Nazarewicz <mina86@xxxxxxxxxx> Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx> One tiny comment below: > Cc: Christoph Lameter <cl@xxxxxxxxx> > Cc: Rik van Riel <riel@xxxxxxxxxx> > --- > v2: checkpack and other non-functional fixes suggested by Naoya Horiguchi > > include/trace/events/compaction.h | 26 ++++++++++++++++++++++---- > mm/compaction.c | 31 +++++++------------------------ > 2 files changed, 29 insertions(+), 28 deletions(-) > > diff --git a/include/trace/events/compaction.h b/include/trace/events/compaction.h > index 06f544e..aacaf0f 100644 > --- a/include/trace/events/compaction.h > +++ b/include/trace/events/compaction.h > @@ -58,7 +61,22 @@ TRACE_EVENT(mm_compaction_migratepages, > ), > > TP_fast_assign( > - __entry->nr_migrated = nr_migrated; > + unsigned long nr_failed = 0; > + struct page *page; > + > + /* > + * migrate_pages() returns either a non-negative number > + * with the number of pages that failed migration, or an > + * error code, in which case we need to count the remaining > + * pages manually > + */ > + if (migrate_rc >= 0) > + nr_failed = migrate_rc; > + else > + list_for_each_entry(page, migratepages, lru) > + nr_failed++; list_for_each would suffice here. > + > + __entry->nr_migrated = nr_all - nr_failed; > __entry->nr_failed = nr_failed; > ), > -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz (o o) ooo +--<mpn@xxxxxxxxxx>--<xmpp:mina86@xxxxxxxxxx>--ooO--(_)--Ooo--
Attachment:
signature.asc
Description: PGP signature