Re: [PATCH] perf kmem: Change a word from migratetype to migrate_type for unity

[Arnaldo Carvalho de Melo]

Em Thu, Nov 16, 2017 at 10:41:46PM +0900, Sihyeon Jang escreveu:
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> Signed-off-by: Sihyeon Jang <uneedsihyeon@xxxxxxxxx>

Please describe the problem and why such word change is warranted.

Its not enough to say what is being done, you need to say _why_.

And this seems bogus, look:

                { "kmem:mm_page_alloc",         perf_evsel__process_page_alloc_event, },

So that function handles:

[root@jouet ~]# grep field.*migrate /sys/kernel/debug/tracing/events/kmem/mm_page_alloc/format 
	field:int migratetype;	offset:24;	size:4;	signed:1;
[root@jouet ~]# 

The field is not named "migrate_type", so your change will _break_ the
'perf kmem' tool, not good :-)

- Arnaldo

> ---
>  tools/perf/builtin-kmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
> index 557d391..43858cd 100644
> --- a/tools/perf/builtin-kmem.c
> +++ b/tools/perf/builtin-kmem.c
> @@ -786,7 +786,7 @@ static int perf_evsel__process_page_alloc_event(struct perf_evsel *evsel,
>  	unsigned int order = perf_evsel__intval(evsel, sample, "order");
>  	unsigned int gfp_flags = perf_evsel__intval(evsel, sample, "gfp_flags");
>  	unsigned int migrate_type = perf_evsel__intval(evsel, sample,
> -						       "migratetype");
> +						       "migrate_type");
>  	u64 bytes = kmem_page_size << order;
>  	u64 callsite;
>  	struct page_stat *pstat;
> -- 
> 2.7.4