Re: [PATCH 03/52] perf tools: Add thread_map event

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Sun, Oct 25, 2015 at 03:51:19PM +0100, Jiri Olsa escreveu:
> Adding thread_map event to pass/store thread maps
> as data in pipe/perf.data.
> 
> Storing thread ID along with the standard comm[16]
> thread name string.
> 
> Link: http://lkml.kernel.org/n/tip-2l07qyf3buhnt83q4ezqz5sj@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/event.c   |  1 +
>  tools/perf/util/event.h   | 13 +++++++++++++
>  tools/perf/util/session.c | 26 ++++++++++++++++++++++++++
>  tools/perf/util/tool.h    |  3 ++-
>  4 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 8b10621b415c..771545a27b9b 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -37,6 +37,7 @@ static const char *perf_event__names[] = {
>  	[PERF_RECORD_AUXTRACE_INFO]		= "AUXTRACE_INFO",
>  	[PERF_RECORD_AUXTRACE]			= "AUXTRACE",
>  	[PERF_RECORD_AUXTRACE_ERROR]		= "AUXTRACE_ERROR",
> +	[PERF_RECORD_THREAD_MAP]		= "THREAD_MAP",
>  };
>  
>  const char *perf_event__name(unsigned int id)
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index a0dbcbd4f6d8..f075f9ed0051 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -226,6 +226,7 @@ enum perf_user_event_type { /* above any possible kernel type */
>  	PERF_RECORD_AUXTRACE_INFO		= 70,
>  	PERF_RECORD_AUXTRACE			= 71,
>  	PERF_RECORD_AUXTRACE_ERROR		= 72,
> +	PERF_RECORD_THREAD_MAP			= 73,
>  	PERF_RECORD_HEADER_MAX
>  };
>  
> @@ -356,6 +357,17 @@ struct context_switch_event {
>  	u32 next_prev_tid;
>  };
>  
> +struct thread_map_data_event {

Humm, I think "data" here is way too vague here, how about
"thread_map_event_entry"?

Moving of "entry" to the end also helps in understanding that this is
not an "event" per se, it doesn't have the perv_event_header, etc, its
just an entry in a 'struct thread_map_event'.

I'm tentatively doing this change in my local branch, please let me know
if you have any reason to object to such a change,

- Arnaldo

> +	u64	pid;
> +	char	comm[16];
> +};
> +
> +struct thread_map_event {
> +	struct perf_event_header	header;
> +	u64				nr;
> +	struct thread_map_data_event	data[];
> +};
> +
>  union perf_event {
>  	struct perf_event_header	header;
>  	struct mmap_event		mmap;
> @@ -378,6 +390,7 @@ union perf_event {
>  	struct aux_event		aux;
>  	struct itrace_start_event	itrace_start;
>  	struct context_switch_event	context_switch;
> +	struct thread_map_event		thread_map;
>  };
>  
>  void perf_event__print_totals(void);
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 428149bc64d2..ec1ecd31003b 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -296,6 +296,16 @@ int process_event_auxtrace_error_stub(struct perf_tool *tool __maybe_unused,
>  	return 0;
>  }
>  
> +
> +static
> +int process_event_thread_map_stub(struct perf_tool *tool __maybe_unused,
> +				  union perf_event *event __maybe_unused,
> +				  struct perf_session *session __maybe_unused)
> +{
> +	dump_printf(": unhandled!\n");
> +	return 0;
> +}
> +
>  void perf_tool__fill_defaults(struct perf_tool *tool)
>  {
>  	if (tool->sample == NULL)
> @@ -346,6 +356,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
>  		tool->auxtrace = process_event_auxtrace_stub;
>  	if (tool->auxtrace_error == NULL)
>  		tool->auxtrace_error = process_event_auxtrace_error_stub;
> +	if (tool->thread_map == NULL)
> +		tool->thread_map = process_event_thread_map_stub;
>  }
>  
>  static void swap_sample_id_all(union perf_event *event, void *data)
> @@ -616,6 +628,17 @@ static void perf_event__auxtrace_error_swap(union perf_event *event,
>  	event->auxtrace_error.ip   = bswap_64(event->auxtrace_error.ip);
>  }
>  
> +static void perf_event__thread_map_swap(union perf_event *event,
> +					bool sample_id_all __maybe_unused)
> +{
> +	unsigned i;
> +
> +	event->thread_map.nr = bswap_64(event->thread_map.nr);
> +
> +	for (i = 0; i < event->thread_map.nr; i++)
> +		event->thread_map.data[i].pid = bswap_64(event->thread_map.data[i].pid);
> +}
> +
>  typedef void (*perf_event__swap_op)(union perf_event *event,
>  				    bool sample_id_all);
>  
> @@ -643,6 +666,7 @@ static perf_event__swap_op perf_event__swap_ops[] = {
>  	[PERF_RECORD_AUXTRACE_INFO]	  = perf_event__auxtrace_info_swap,
>  	[PERF_RECORD_AUXTRACE]		  = perf_event__auxtrace_swap,
>  	[PERF_RECORD_AUXTRACE_ERROR]	  = perf_event__auxtrace_error_swap,
> +	[PERF_RECORD_THREAD_MAP]	  = perf_event__thread_map_swap,
>  	[PERF_RECORD_HEADER_MAX]	  = NULL,
>  };
>  
> @@ -1179,6 +1203,8 @@ static s64 perf_session__process_user_event(struct perf_session *session,
>  	case PERF_RECORD_AUXTRACE_ERROR:
>  		perf_session__auxtrace_error_inc(session, event);
>  		return tool->auxtrace_error(tool, event, session);
> +	case PERF_RECORD_THREAD_MAP:
> +		return tool->thread_map(tool, event, session);
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
> index cab8cc24831b..1af4774960c3 100644
> --- a/tools/perf/util/tool.h
> +++ b/tools/perf/util/tool.h
> @@ -55,7 +55,8 @@ struct perf_tool {
>  	event_op2	build_id,
>  			id_index,
>  			auxtrace_info,
> -			auxtrace_error;
> +			auxtrace_error,
> +			thread_map;
>  	event_op3	auxtrace;
>  	bool		ordered_events;
>  	bool		ordering_requires_timestamps;
> -- 
> 2.4.3