Re: [PATCH] mm: compaction: Trace compaction begin and end

[Mel Gorman]

On Wed, Dec 04, 2013 at 03:51:57PM +0100, Vlastimil Babka wrote:
> On 12/04/2013 03:30 PM, Mel Gorman wrote:
> >This patch adds two tracepoints for compaction begin and end of a zone. Using
> >this it is possible to calculate how much time a workload is spending
> >within compaction and potentially debug problems related to cached pfns
> >for scanning.
> 
> I guess for debugging pfns it would be also useful to print their
> values also in mm_compaction_end.
> 

What additional information would we get from it and what new
conclusions could we draw? We could guess how much work the
scanners did but the trace_mm_compaction_isolate_freepages and
trace_mm_compaction_isolate_migratepages tracepoints already accurately
tell us that. The scanner PFNs alone do not tell us if the cached pfns
were updated and even if it did, the information can be changed by
parallel resets so it would be hard to draw reasonable conclusions from
the information. We could guess where compaction hotspots might be but
without the skip information, we could not detect it accurately.  If we
wanted to detect that accurately, the mm_compaction_isolate* tracepoints
would be the one to update.

I was primarily concerned about compaction time so I might be looking
at this the wrong way but it feels like having the PFNs at the end of a
compaction cycle would be of marginal benefit.

> >In combination with the direct reclaim and slab trace points
> >it should be possible to estimate most allocation-related overhead for
> >a workload.
> >
> >Signed-off-by: Mel Gorman <mgorman@xxxxxxx>
> >---
> >  include/trace/events/compaction.h | 42 +++++++++++++++++++++++++++++++++++++++
> >  mm/compaction.c                   |  4 ++++
> >  2 files changed, 46 insertions(+)
> >
> >diff --git a/include/trace/events/compaction.h b/include/trace/events/compaction.h
> >index fde1b3e..f4e115a 100644
> >--- a/include/trace/events/compaction.h
> >+++ b/include/trace/events/compaction.h
> >@@ -67,6 +67,48 @@ TRACE_EVENT(mm_compaction_migratepages,
> >  		__entry->nr_failed)
> >  );
> >
> >+TRACE_EVENT(mm_compaction_begin,
> >+	TP_PROTO(unsigned long zone_start, unsigned long migrate_start,
> >+		unsigned long zone_end, unsigned long free_start),
> >+
> >+	TP_ARGS(zone_start, migrate_start, zone_end, free_start),
> 
> IMHO a better order would be:
>  zone_start, migrate_start, free_start, zone_end
> (well especially in the TP_printk part anyway).
> 

Ok, that would put them in PFN order which may be easier to visualise.
I'll post a V2 with that change at least.

-- 
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/