linux-kernel - Re: [PATCH 2/2] mm: compaction.c: Add/Modify direct compaction tracepoints

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7ab4a23a-1311-9579-2d58-263bbcdcd725@suse.cz>
Date:	Mon, 1 Aug 2016 15:25:09 +0200
From:	Vlastimil Babka <vbabka@...e.cz>
To:	Janani Ravichandran <janani.rvchndrn@...il.com>,
	linux-mm@...ck.org, linux-kernel@...r.kernel.org
Cc:	riel@...riel.com, akpm@...ux-foundation.org, hannes@...pxchg.org,
	vdavydov@...tuozzo.com, mhocko@...e.com,
	mgorman@...hsingularity.net, kirill.shutemov@...ux.intel.com,
	bywxiaobai@....com, rostedt@...dmis.org
Subject: Re: [PATCH 2/2] mm: compaction.c: Add/Modify direct compaction
 tracepoints

On 07/27/2016 04:51 PM, Janani Ravichandran wrote:
> Add zone information to an existing tracepoint in compact_zone(). Also,
> add a new tracepoint at the end of the compaction code so that latency
> information can be derived.
>
> Signed-off-by: Janani Ravichandran <janani.rvchndrn@...il.com>
> ---
>  include/trace/events/compaction.h | 38 +++++++++++++++++++++++++++++++++-----
>  mm/compaction.c                   |  6 ++++--
>  2 files changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/include/trace/events/compaction.h b/include/trace/events/compaction.h
> index 36e2d6f..4d86769 100644
> --- a/include/trace/events/compaction.h
> +++ b/include/trace/events/compaction.h
> @@ -158,12 +158,15 @@ TRACE_EVENT(mm_compaction_migratepages,
>  );
>
>  TRACE_EVENT(mm_compaction_begin,
> -	TP_PROTO(unsigned long zone_start, unsigned long migrate_pfn,
> -		unsigned long free_pfn, unsigned long zone_end, bool sync),
> +	TP_PROTO(struct zone *zone, unsigned long zone_start,
> +		unsigned long migrate_pfn, unsigned long free_pfn,
> +		unsigned long zone_end, bool sync),
>
> -	TP_ARGS(zone_start, migrate_pfn, free_pfn, zone_end, sync),
> +	TP_ARGS(zone, zone_start, migrate_pfn, free_pfn, zone_end, sync),
>
>  	TP_STRUCT__entry(
> +		__field(int, nid)
> +		__field(int, zid)
>  		__field(unsigned long, zone_start)
>  		__field(unsigned long, migrate_pfn)
>  		__field(unsigned long, free_pfn)
> @@ -172,6 +175,8 @@ TRACE_EVENT(mm_compaction_begin,
>  	),
>
>  	TP_fast_assign(
> +		__entry->nid = zone_to_nid(zone);
> +		__entry->zid = zone_idx(zone);
>  		__entry->zone_start = zone_start;
>  		__entry->migrate_pfn = migrate_pfn;
>  		__entry->free_pfn = free_pfn;
> @@ -179,7 +184,9 @@ TRACE_EVENT(mm_compaction_begin,
>  		__entry->sync = sync;
>  	),
>
> -	TP_printk("zone_start=0x%lx migrate_pfn=0x%lx free_pfn=0x%lx zone_end=0x%lx, mode=%s",
> +	TP_printk("nid=%d zid=%d zone_start=0x%lx migrate_pfn=0x%lx free_pfn=0x%lx zone_end=0x%lx, mode=%s",

Yea, this tracepoint has been odd in not printing node/zone in a 
friendly way (it's possible to determine it from zone_start/zone_end 
though, so this is good in general. But instead of printing nid and zid 
like this, it would be nice to unify the output with the other 
tracepoints, e.g.:

DECLARE_EVENT_CLASS(mm_compaction_suitable_template,
[...]
         TP_printk("node=%d zone=%-8s order=%d ret=%s",
                 __entry->nid,
                 __print_symbolic(__entry->idx, ZONE_TYPE),

Thanks,
Vlastimil