[PATCH 1/5] perf tools: Fix segfault when using -s trace_fields

[PATCH 1/5] perf tools: Fix segfault when using -s trace_fields

[Namhyung Kim]

When the 'trace_fields' sort key is used explicitly for non-tracepoint
events, it'll get segfault since it assumed evsel->tp_format was set.
Skip those events in add_all_dynamic_fields().

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index e558e87cafaf..59c4c8586d79 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1955,6 +1955,9 @@ static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
 	struct perf_evsel *evsel;
 
 	evlist__for_each(evlist, evsel) {
+		if (evsel->attr.type != PERF_TYPE_TRACEPOINT)
+			continue;
+
 		ret = add_evsel_fields(evsel, raw_trace);
 		if (ret < 0)
 			return ret;
-- 
2.6.4

[PATCH 2/5] perf tools: Add all matching dynamic sort keys for field name

[Namhyung Kim]

When a perf.data file has multiple events, it's likely to be similar
(tracepoint) events.  In that case, they might have same field name so
add all of them to sort keys instead of bailing out.

In addition, it contains a trivial whitespace fix at callsite of
add_all_dynamic_fields().

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 59c4c8586d79..9618a64875c0 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1965,6 +1965,28 @@ static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
 	return 0;
 }
 
+static int add_all_matching_fields(struct perf_evlist *evlist,
+				   char *field_name, bool raw_trace)
+{
+	int ret = -ESRCH;
+	struct perf_evsel *evsel;
+	struct format_field *field;
+
+	evlist__for_each(evlist, evsel) {
+		if (evsel->attr.type != PERF_TYPE_TRACEPOINT)
+			continue;
+
+		field = pevent_find_any_field(evsel->tp_format, field_name);
+		if (field == NULL)
+			continue;
+
+		ret = __dynamic_dimension__add(evsel, field, raw_trace);
+		if (ret < 0)
+			break;
+	}
+	return ret;
+}
+
 static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
 {
 	char *str, *event_name, *field_name, *opt_name;
@@ -1995,7 +2017,12 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
 	}
 
 	if (!strcmp(field_name, "trace_fields")) {
-		ret = add_all_dynamic_fields(evlist ,raw_trace);
+		ret = add_all_dynamic_fields(evlist, raw_trace);
+		goto out;
+	}
+
+	if (event_name == NULL) {
+		ret = add_all_matching_fields(evlist, field_name, raw_trace);
 		goto out;
 	}
 
-- 
2.6.4

[PATCH 3/5] perf tools: Add document for dynamic sort keys

[Namhyung Kim]

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-report.txt | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index ae7cd91727f6..8a301f6afb37 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -117,6 +117,30 @@ OPTIONS
 	And default sort keys are changed to comm, dso_from, symbol_from, dso_to
 	and symbol_to, see '--branch-stack'.
 
+	If the data file has tracepoint event(s), following (dynamic) sort keys
+	are also available:
+	trace, trace_fields, [<event>.]<field>[/raw]
+
+	- trace: pretty printed trace output in a single column
+	- trace_fields: fields in tracepoints in separate columns
+	- <field name>: optional event and field name for a specific field
+
+	The last form consists of event and field names.  If event name is
+	omitted, it searches all events for matching field name.  The matched
+	field will be shown only for the event has the field.  The event name
+	supports substring match so user doesn't need to specify full subsystem
+	and event name everytime.  For example, 'sched:sched_switch' event can
+	be shortened to 'switch' as long as it's not ambiguous.  Also event can
+	be specified by its index (starting from 1) preceded by the '%'.
+	So '%1' is the first event, '%2' is the second, and so on.
+
+	The field name can have '/raw' suffix which disables pretty printing
+	and shows raw field value like hex numbers.  The --raw-trace option
+	has the same effect for all dynamic sort keys.
+
+	The default sort keys are changed to 'trace' if all events in the data
+	file are tracepoint.
+
 -F::
 --fields=::
 	Specify output field - multiple keys can be specified in CSV format.
-- 
2.6.4

[PATCH 4/5] perf tools: Support dynamic sort keys for -F/--fields

[Namhyung Kim]

Now dynamic sort keys are supported for tracepoint events, add it to
output fields too.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.c | 51 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 9618a64875c0..65ae0487da5a 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1919,7 +1919,7 @@ static struct perf_evsel *find_evsel(struct perf_evlist *evlist, char *event_nam
 
 static int __dynamic_dimension__add(struct perf_evsel *evsel,
 				    struct format_field *field,
-				    bool raw_trace)
+				    bool raw_trace, bool is_sort_key)
 {
 	struct hpp_dynamic_entry *hde;
 
@@ -1929,18 +1929,24 @@ static int __dynamic_dimension__add(struct perf_evsel *evsel,
 
 	hde->raw_trace = raw_trace;
 
-	perf_hpp__register_sort_field(&hde->hpp);
+	if (is_sort_key)
+		perf_hpp__register_sort_field(&hde->hpp);
+	else
+		perf_hpp__column_register(&hde->hpp);
+
 	return 0;
 }
 
-static int add_evsel_fields(struct perf_evsel *evsel, bool raw_trace)
+static int add_evsel_fields(struct perf_evsel *evsel, bool raw_trace,
+			    bool is_sort_key)
 {
 	int ret;
 	struct format_field *field;
 
 	field = evsel->tp_format->format.fields;
 	while (field) {
-		ret = __dynamic_dimension__add(evsel, field, raw_trace);
+		ret = __dynamic_dimension__add(evsel, field, raw_trace,
+					       is_sort_key);
 		if (ret < 0)
 			return ret;
 
@@ -1949,7 +1955,8 @@ static int add_evsel_fields(struct perf_evsel *evsel, bool raw_trace)
 	return 0;
 }
 
-static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
+static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace,
+				  bool is_sort_key)
 {
 	int ret;
 	struct perf_evsel *evsel;
@@ -1958,7 +1965,7 @@ static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
 		if (evsel->attr.type != PERF_TYPE_TRACEPOINT)
 			continue;
 
-		ret = add_evsel_fields(evsel, raw_trace);
+		ret = add_evsel_fields(evsel, raw_trace, is_sort_key);
 		if (ret < 0)
 			return ret;
 	}
@@ -1966,7 +1973,8 @@ static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
 }
 
 static int add_all_matching_fields(struct perf_evlist *evlist,
-				   char *field_name, bool raw_trace)
+				   char *field_name, bool raw_trace,
+				   bool is_sort_key)
 {
 	int ret = -ESRCH;
 	struct perf_evsel *evsel;
@@ -1980,14 +1988,16 @@ static int add_all_matching_fields(struct perf_evlist *evlist,
 		if (field == NULL)
 			continue;
 
-		ret = __dynamic_dimension__add(evsel, field, raw_trace);
+		ret = __dynamic_dimension__add(evsel, field, raw_trace,
+					       is_sort_key);
 		if (ret < 0)
 			break;
 	}
 	return ret;
 }
 
-static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
+static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok,
+			     bool is_sort_key)
 {
 	char *str, *event_name, *field_name, *opt_name;
 	struct perf_evsel *evsel;
@@ -2017,12 +2027,13 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
 	}
 
 	if (!strcmp(field_name, "trace_fields")) {
-		ret = add_all_dynamic_fields(evlist, raw_trace);
+		ret = add_all_dynamic_fields(evlist, raw_trace, is_sort_key);
 		goto out;
 	}
 
 	if (event_name == NULL) {
-		ret = add_all_matching_fields(evlist, field_name, raw_trace);
+		ret = add_all_matching_fields(evlist, field_name, raw_trace,
+					      is_sort_key);
 		goto out;
 	}
 
@@ -2040,7 +2051,7 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
 	}
 
 	if (!strcmp(field_name, "*")) {
-		ret = add_evsel_fields(evsel, raw_trace);
+		ret = add_evsel_fields(evsel, raw_trace, is_sort_key);
 	} else {
 		field = pevent_find_any_field(evsel->tp_format, field_name);
 		if (field == NULL) {
@@ -2049,7 +2060,8 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
 			return -ENOENT;
 		}
 
-		ret = __dynamic_dimension__add(evsel, field, raw_trace);
+		ret = __dynamic_dimension__add(evsel, field, raw_trace,
+					       is_sort_key);
 	}
 
 out:
@@ -2193,7 +2205,7 @@ static int sort_dimension__add(const char *tok,
 		return 0;
 	}
 
-	if (!add_dynamic_entry(evlist, tok))
+	if (!add_dynamic_entry(evlist, tok, true))
 		return 0;
 
 	return -ESRCH;
@@ -2398,7 +2410,7 @@ void sort__setup_elide(FILE *output)
 	}
 }
 
-static int output_field_add(char *tok)
+static int output_field_add(struct perf_evlist *evlist, char *tok)
 {
 	unsigned int i;
 
@@ -2438,6 +2450,9 @@ static int output_field_add(char *tok)
 		return __sort_dimension__add_output(sd);
 	}
 
+	if (!add_dynamic_entry(evlist, tok, false))
+		return 0;
+
 	return -ESRCH;
 }
 
@@ -2463,7 +2478,7 @@ bool is_strict_order(const char *order)
 	return order && (*order != '+');
 }
 
-static int __setup_output_field(void)
+static int __setup_output_field(struct perf_evlist *evlist)
 {
 	char *tmp, *tok, *str, *strp;
 	int ret = -EINVAL;
@@ -2487,7 +2502,7 @@ static int __setup_output_field(void)
 
 	for (tok = strtok_r(strp, ", ", &tmp);
 			tok; tok = strtok_r(NULL, ", ", &tmp)) {
-		ret = output_field_add(tok);
+		ret = output_field_add(evlist, tok);
 		if (ret == -EINVAL) {
 			error("Invalid --fields key: `%s'", tok);
 			break;
@@ -2524,7 +2539,7 @@ int setup_sorting(struct perf_evlist *evlist)
 	if (sort__mode != SORT_MODE__DIFF)
 		perf_hpp__init();
 
-	err = __setup_output_field();
+	err = __setup_output_field(evlist);
 	if (err < 0)
 		return err;
 
-- 
2.6.4

[PATCH 5/5] perf evlist: Add -T/--trace option to show trace fields

[Namhyung Kim]

To use dynamic sort keys, it might be good to add an option to see the
list of field names.

  $ perf evlist -T -i perf.data.sched
  sched:sched_switch: trace_fields=prev_comm,prev_pid,prev_prio,prev_state,next_comm,next_pid,next_prio
  sched:sched_stat_wait: trace_fields=comm,pid,delay
  sched:sched_stat_sleep: trace_fields=comm,pid,delay
  sched:sched_stat_iowait: trace_fields=comm,pid,delay
  sched:sched_stat_runtime: trace_fields=comm,pid,runtime,vruntime
  sched:sched_process_fork: trace_fields=parent_comm,parent_pid,child_comm,child_pid
  sched:sched_wakeup: trace_fields=comm,pid,prio,success,target_cpu
  sched:sched_wakeup_new: trace_fields=comm,pid,prio,success,target_cpu
  sched:sched_migrate_task: trace_fields=comm,pid,prio,orig_cpu,dest_cpu

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-evlist.txt |  4 ++++
 tools/perf/builtin-evlist.c              |  1 +
 tools/perf/util/evsel.c                  | 23 +++++++++++++++++++++++
 tools/perf/util/evsel.h                  |  1 +
 4 files changed, 29 insertions(+)

diff --git a/tools/perf/Documentation/perf-evlist.txt b/tools/perf/Documentation/perf-evlist.txt
index 1ceb3700ffbb..b0d99c276cf5 100644
--- a/tools/perf/Documentation/perf-evlist.txt
+++ b/tools/perf/Documentation/perf-evlist.txt
@@ -32,6 +32,10 @@ OPTIONS
 --group::
 	Show event group information.
 
+-T::
+--trace::
+	Show tracepoint field names.
+
 SEE ALSO
 --------
 linkperf:perf-record[1], linkperf:perf-list[1],
diff --git a/tools/perf/builtin-evlist.c b/tools/perf/builtin-evlist.c
index 08a7d36a2cf8..2820bb2d8cf8 100644
--- a/tools/perf/builtin-evlist.c
+++ b/tools/perf/builtin-evlist.c
@@ -49,6 +49,7 @@ int cmd_evlist(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_BOOLEAN('g', "group", &details.event_group,
 		    "Show event group information"),
 	OPT_BOOLEAN('f', "force", &details.force, "don't complain, do it"),
+	OPT_BOOLEAN('T', "trace", &details.trace, "Show tracepoint fields"),
 	OPT_END()
 	};
 	const char * const evlist_usage[] = {
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 544e4400de13..174d0029e038 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2298,6 +2298,29 @@ int perf_evsel__fprintf(struct perf_evsel *evsel,
 		printed += comma_fprintf(fp, &first, " %s=%" PRIu64,
 					 term, (u64)evsel->attr.sample_freq);
 	}
+
+	if (details->trace) {
+		struct format_field *field;
+
+		if (evsel->attr.type != PERF_TYPE_TRACEPOINT) {
+			printed += comma_fprintf(fp, &first, " (not a tracepoint)");
+			goto out;
+		}
+
+		field = evsel->tp_format->format.fields;
+		if (field == NULL) {
+			printed += comma_fprintf(fp, &first, " (no trace field)");
+			goto out;
+		}
+
+		printed += comma_fprintf(fp, &first, " trace_fields=%s", field->name);
+
+		field = field->next;
+		while (field) {
+			printed += comma_fprintf(fp, &first, "%s", field->name);
+			field = field->next;
+		}
+	}
 out:
 	fputc('\n', fp);
 	return ++printed;
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 5ded1fc0341e..9b36dd584d4a 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -369,6 +369,7 @@ struct perf_attr_details {
 	bool verbose;
 	bool event_group;
 	bool force;
+	bool trace;
 };
 
 int perf_evsel__fprintf(struct perf_evsel *evsel,
-- 
2.6.4

Re: [PATCH 1/5] perf tools: Fix segfault when using -s trace_fields

[Jiri Olsa]

On Tue, Jan 05, 2016 at 12:03:43PM +0900, Namhyung Kim wrote:
> When the 'trace_fields' sort key is used explicitly for non-tracepoint
> events, it'll get segfault since it assumed evsel->tp_format was set.
> Skip those events in add_all_dynamic_fields().

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/sort.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index e558e87cafaf..59c4c8586d79 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1955,6 +1955,9 @@ static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
>  	struct perf_evsel *evsel;
>  
>  	evlist__for_each(evlist, evsel) {
> +		if (evsel->attr.type != PERF_TYPE_TRACEPOINT)
> +			continue;
> +
>  		ret = add_evsel_fields(evsel, raw_trace);
>  		if (ret < 0)
>  			return ret;
> -- 
> 2.6.4
> 

Re: [PATCH 2/5] perf tools: Add all matching dynamic sort keys for field name

[Jiri Olsa]

On Tue, Jan 05, 2016 at 12:03:44PM +0900, Namhyung Kim wrote:

SNIP

>  static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
>  {
>  	char *str, *event_name, *field_name, *opt_name;
> @@ -1995,7 +2017,12 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
>  	}
>  
>  	if (!strcmp(field_name, "trace_fields")) {
> -		ret = add_all_dynamic_fields(evlist ,raw_trace);
> +		ret = add_all_dynamic_fields(evlist, raw_trace);
> +		goto out;
> +	}
> +
> +	if (event_name == NULL) {
> +		ret = add_all_matching_fields(evlist, field_name, raw_trace);
>  		goto out;

should this be handled within find_evsel function:

        /* case 1 */
        if (event_name == NULL) {
                if (evlist->nr_entries != 1) {


looks like it'd be dead code otherwise

thanks,
jirka

Re: [PATCH 3/5] perf tools: Add document for dynamic sort keys

[Jiri Olsa]

On Tue, Jan 05, 2016 at 12:03:45PM +0900, Namhyung Kim wrote:
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> ---
>  tools/perf/Documentation/perf-report.txt | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index ae7cd91727f6..8a301f6afb37 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -117,6 +117,30 @@ OPTIONS
>  	And default sort keys are changed to comm, dso_from, symbol_from, dso_to
>  	and symbol_to, see '--branch-stack'.
>  
> +	If the data file has tracepoint event(s), following (dynamic) sort keys
> +	are also available:
> +	trace, trace_fields, [<event>.]<field>[/raw]
> +
> +	- trace: pretty printed trace output in a single column
> +	- trace_fields: fields in tracepoints in separate columns
> +	- <field name>: optional event and field name for a specific field
> +
> +	The last form consists of event and field names.  If event name is
> +	omitted, it searches all events for matching field name.  The matched
> +	field will be shown only for the event has the field.  The event name
> +	supports substring match so user doesn't need to specify full subsystem
> +	and event name everytime.  For example, 'sched:sched_switch' event can
> +	be shortened to 'switch' as long as it's not ambiguous.  Also event can
> +	be specified by its index (starting from 1) preceded by the '%'.
> +	So '%1' is the first event, '%2' is the second, and so on.
> +
> +	The field name can have '/raw' suffix which disables pretty printing
> +	and shows raw field value like hex numbers.  The --raw-trace option
> +	has the same effect for all dynamic sort keys.
> +
> +	The default sort keys are changed to 'trace' if all events in the data
> +	file are tracepoint.
> +
>  -F::
>  --fields=::
>  	Specify output field - multiple keys can be specified in CSV format.
> -- 
> 2.6.4
> 

Re: [PATCH 4/5] perf tools: Support dynamic sort keys for -F/--fields

[Jiri Olsa]

On Tue, Jan 05, 2016 at 12:03:46PM +0900, Namhyung Kim wrote:
> Now dynamic sort keys are supported for tracepoint events, add it to
> output fields too.

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

Re: [PATCH 5/5] perf evlist: Add -T/--trace option to show trace fields

[Jiri Olsa]

On Tue, Jan 05, 2016 at 12:03:47PM +0900, Namhyung Kim wrote:
> To use dynamic sort keys, it might be good to add an option to see the
> list of field names.
> 
>   $ perf evlist -T -i perf.data.sched
>   sched:sched_switch: trace_fields=prev_comm,prev_pid,prev_prio,prev_state,next_comm,next_pid,next_prio
>   sched:sched_stat_wait: trace_fields=comm,pid,delay
>   sched:sched_stat_sleep: trace_fields=comm,pid,delay
>   sched:sched_stat_iowait: trace_fields=comm,pid,delay
>   sched:sched_stat_runtime: trace_fields=comm,pid,runtime,vruntime
>   sched:sched_process_fork: trace_fields=parent_comm,parent_pid,child_comm,child_pid
>   sched:sched_wakeup: trace_fields=comm,pid,prio,success,target_cpu
>   sched:sched_wakeup_new: trace_fields=comm,pid,prio,success,target_cpu
>   sched:sched_migrate_task: trace_fields=comm,pid,prio,orig_cpu,dest_cpu
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

Re: [PATCH 2/5] perf tools: Add all matching dynamic sort keys for field name

[Namhyung Kim]

Hi Jiri,

Thanks for your review!

On Tue, Jan 05, 2016 at 10:24:27AM +0100, Jiri Olsa wrote:
> On Tue, Jan 05, 2016 at 12:03:44PM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> >  static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
> >  {
> >  	char *str, *event_name, *field_name, *opt_name;
> > @@ -1995,7 +2017,12 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
> >  	}
> >  
> >  	if (!strcmp(field_name, "trace_fields")) {
> > -		ret = add_all_dynamic_fields(evlist ,raw_trace);
> > +		ret = add_all_dynamic_fields(evlist, raw_trace);
> > +		goto out;
> > +	}
> > +
> > +	if (event_name == NULL) {
> > +		ret = add_all_matching_fields(evlist, field_name, raw_trace);
> >  		goto out;
> 
> should this be handled within find_evsel function:
> 
>         /* case 1 */
>         if (event_name == NULL) {
>                 if (evlist->nr_entries != 1) {
> 
> 
> looks like it'd be dead code otherwise

Hmm.. OK.  But the find_evsel() is to get a evsel so it's not good
place to add the code IMHO.  I'll remove the case 1 from it.

Thanks,
Namhyung