Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1302304 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 1/5] perf tools: Add all matching dynamic sort keys for field name Namhyung Kim <namhyung@kernel.org> - 2016-01-06 02:00 +0100
    Re: [PATCH v3 1/5] perf tools: Add all matching dynamic sort keys  for field name Jiri Olsa <jolsa@redhat.com> - 2016-01-06 12:20 +0100
      Re: [PATCH v3 1/5] perf tools: Add all matching dynamic sort keys  for field name Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-06 17:40 +0100
        Re: [PATCH v3 1/5] perf tools: Add all matching dynamic sort keys  for field name Namhyung Kim <namhyung@kernel.org> - 2016-01-07 00:10 +0100

#1302304 — [PATCH v3 1/5] perf tools: Add all matching dynamic sort keys for field name

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().

Acked-by: Jiri Olsa <jolsa@kernel.org>
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

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

[toc] | [next] | [standalone]

#1302668 — Re: [PATCH v3 1/5] perf tools: Add all matching dynamic sort keys for field name

On Wed, Jan 06, 2016 at 09:54:57AM +0900, Namhyung Kim wrote:
> 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().
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

hum, I haven't tried with this last version but I get all
hist tests failing on acme's perf/core and it seems to be
related to sorting changes:

[jolsa@krava perf]$ ./perf test hist 
15: Test matching and linking multiple hists                 : FAILED!
25: Test filtering hist entries                              : FAILED!
28: Test output sorting of hist entries                      : FAILED!
29: Test cumulation of child hist entries                    : FAILED!


[jolsa@krava perf]$ ./perf test 15 -v
15: Test matching and linking multiple hists                 :
--- start ---
test child forked, pid 10676
perf: Segmentation fault
Obtained 16 stack frames.
./perf(dump_stack+0x2d) [0x50f1d7]
./perf(sighandler_dump_stack+0x2d) [0x50f2b7]
/lib64/libc.so.6(+0x34a4f) [0x7fb0b1178a4f]
./perf() [0x508c40]
./perf() [0x508e23]
./perf(setup_sorting+0x26) [0x5097b0]
./perf(test__hists_link+0xb6) [0x487f43]
./perf() [0x4725c0]
./perf() [0x4726ff]
./perf() [0x472986]
./perf(cmd_test+0x1fe) [0x472ddc]
./perf() [0x49b2ab]
./perf() [0x49b513]
./perf() [0x49b661]
./perf(main+0x258) [0x49b9e2]
/lib64/libc.so.6(__libc_start_main+0xef) [0x7fb0b11646ff]
test child interrupted
---- end ----
Test matching and linking multiple hists: FAILED!


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

[toc] | [prev] | [next] | [standalone]

#1302886 — Re: [PATCH v3 1/5] perf tools: Add all matching dynamic sort keys for field name

Em Wed, Jan 06, 2016 at 12:19:39PM +0100, Jiri Olsa escreveu:
> On Wed, Jan 06, 2016 at 09:54:57AM +0900, Namhyung Kim wrote:
> > 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().
> > 
> > Acked-by: Jiri Olsa <jolsa@kernel.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> hum, I haven't tried with this last version but I get all
> hist tests failing on acme's perf/core and it seems to be
> related to sorting changes:

Bisected it down to

0337e6473845 ("perf tools: Make 'trace' or 'trace_fields' sort key default for tracepoint events")

Fixed with the following patch, which I'm folding into the above commit,
thanks for the report!

- Arnaldo

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index acd222907bd6..4b4b1c5cccef 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -2187,6 +2187,9 @@ static const char *get_default_sort_order(struct perf_evlist *evlist)
 
 	BUG_ON(sort__mode >= ARRAY_SIZE(default_sort_orders));
 
+	if (evlist == NULL)
+		goto out_no_evlist;
+
 	evlist__for_each(evlist, evsel) {
 		if (evsel->attr.type != PERF_TYPE_TRACEPOINT) {
 			use_trace = false;
@@ -2199,7 +2202,7 @@ static const char *get_default_sort_order(struct perf_evlist *evlist)
 		if (symbol_conf.raw_trace)
 			return "trace_fields";
 	}
-
+out_no_evlist:
 	return default_sort_orders[sort__mode];
 }
 
 
> [jolsa@krava perf]$ ./perf test hist 
> 15: Test matching and linking multiple hists                 : FAILED!
> 25: Test filtering hist entries                              : FAILED!
> 28: Test output sorting of hist entries                      : FAILED!
> 29: Test cumulation of child hist entries                    : FAILED!
> 
> 
> [jolsa@krava perf]$ ./perf test 15 -v
> 15: Test matching and linking multiple hists                 :
> --- start ---
> test child forked, pid 10676
> perf: Segmentation fault
> Obtained 16 stack frames.
> ./perf(dump_stack+0x2d) [0x50f1d7]
> ./perf(sighandler_dump_stack+0x2d) [0x50f2b7]
> /lib64/libc.so.6(+0x34a4f) [0x7fb0b1178a4f]
> ./perf() [0x508c40]
> ./perf() [0x508e23]
> ./perf(setup_sorting+0x26) [0x5097b0]
> ./perf(test__hists_link+0xb6) [0x487f43]
> ./perf() [0x4725c0]
> ./perf() [0x4726ff]
> ./perf() [0x472986]
> ./perf(cmd_test+0x1fe) [0x472ddc]
> ./perf() [0x49b2ab]
> ./perf() [0x49b513]
> ./perf() [0x49b661]
> ./perf(main+0x258) [0x49b9e2]
> /lib64/libc.so.6(__libc_start_main+0xef) [0x7fb0b11646ff]
> test child interrupted
> ---- end ----
> Test matching and linking multiple hists: FAILED!
> 
> 
> thanks,
> jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303128 — Re: [PATCH v3 1/5] perf tools: Add all matching dynamic sort keys for field name

Hi Arnaldo and Jiri,

On Wed, Jan 06, 2016 at 01:29:48PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jan 06, 2016 at 12:19:39PM +0100, Jiri Olsa escreveu:
> > On Wed, Jan 06, 2016 at 09:54:57AM +0900, Namhyung Kim wrote:
> > > 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().
> > > 
> > > Acked-by: Jiri Olsa <jolsa@kernel.org>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > 
> > hum, I haven't tried with this last version but I get all
> > hist tests failing on acme's perf/core and it seems to be
> > related to sorting changes:
> 
> Bisected it down to
> 
> 0337e6473845 ("perf tools: Make 'trace' or 'trace_fields' sort key default for tracepoint events")
> 
> Fixed with the following patch, which I'm folding into the above commit,
> thanks for the report!

Ouch, thank you for the fix! :)

Thanks,
Namhyung


> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index acd222907bd6..4b4b1c5cccef 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -2187,6 +2187,9 @@ static const char *get_default_sort_order(struct perf_evlist *evlist)
>  
>  	BUG_ON(sort__mode >= ARRAY_SIZE(default_sort_orders));
>  
> +	if (evlist == NULL)
> +		goto out_no_evlist;
> +
>  	evlist__for_each(evlist, evsel) {
>  		if (evsel->attr.type != PERF_TYPE_TRACEPOINT) {
>  			use_trace = false;
> @@ -2199,7 +2202,7 @@ static const char *get_default_sort_order(struct perf_evlist *evlist)
>  		if (symbol_conf.raw_trace)
>  			return "trace_fields";
>  	}
> -
> +out_no_evlist:
>  	return default_sort_orders[sort__mode];
>  }
>  
>  
> > [jolsa@krava perf]$ ./perf test hist 
> > 15: Test matching and linking multiple hists                 : FAILED!
> > 25: Test filtering hist entries                              : FAILED!
> > 28: Test output sorting of hist entries                      : FAILED!
> > 29: Test cumulation of child hist entries                    : FAILED!
> > 
> > 
> > [jolsa@krava perf]$ ./perf test 15 -v
> > 15: Test matching and linking multiple hists                 :
> > --- start ---
> > test child forked, pid 10676
> > perf: Segmentation fault
> > Obtained 16 stack frames.
> > ./perf(dump_stack+0x2d) [0x50f1d7]
> > ./perf(sighandler_dump_stack+0x2d) [0x50f2b7]
> > /lib64/libc.so.6(+0x34a4f) [0x7fb0b1178a4f]
> > ./perf() [0x508c40]
> > ./perf() [0x508e23]
> > ./perf(setup_sorting+0x26) [0x5097b0]
> > ./perf(test__hists_link+0xb6) [0x487f43]
> > ./perf() [0x4725c0]
> > ./perf() [0x4726ff]
> > ./perf() [0x472986]
> > ./perf(cmd_test+0x1fe) [0x472ddc]
> > ./perf() [0x49b2ab]
> > ./perf() [0x49b513]
> > ./perf() [0x49b661]
> > ./perf(main+0x258) [0x49b9e2]
> > /lib64/libc.so.6(__libc_start_main+0xef) [0x7fb0b11646ff]
> > test child interrupted
> > ---- end ----
> > Test matching and linking multiple hists: FAILED!
> > 
> > 
> > thanks,
> > jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web