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

Groups > linux.kernel > #1338854 > unrolled thread

[PATCH 1/5] perf tools: Fix assertion failure on dynamic entry

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/5] perf tools: Fix assertion failure on dynamic entry Namhyung Kim <namhyung@kernel.org> - 2016-02-21 18:30 +0100
    Re: [PATCH 1/5] perf tools: Fix assertion failure on dynamic entry Jiri Olsa <jolsa@redhat.com> - 2016-02-21 18:40 +0100
      Re: [PATCH 1/5] perf tools: Fix assertion failure on dynamic entry Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-02-22 16:10 +0100
    Re: [PATCH 3/5] perf tools: Update srcline/file if needed Jiri Olsa <jolsa@redhat.com> - 2016-02-21 18:40 +0100
      [PATCH v2 3/5] perf tools: Update srcline/file if needed Namhyung Kim <namhyung@kernel.org> - 2016-02-22 01:40 +0100
        Re: [PATCH v2 3/5] perf tools: Update srcline/file if needed Jiri Olsa <jolsa@redhat.com> - 2016-02-22 07:50 +0100
    Re: [PATCH 2/5] perf tools: Fix segfault on dynamic entries Jiri Olsa <jolsa@redhat.com> - 2016-02-21 18:40 +0100
    Re: [PATCH 5/5] perf tools: Fix column width setting on 'trace' sort  key Jiri Olsa <jolsa@redhat.com> - 2016-02-21 18:50 +0100
    Re: [PATCH 4/5] perf tools: Fix alignment on some sort keys Jiri Olsa <jolsa@redhat.com> - 2016-02-21 18:50 +0100
      [PATCH v2 4/5] perf tools: Fix alignment on some sort keys Namhyung Kim <namhyung@kernel.org> - 2016-02-22 01:40 +0100
    [PATCH 2/5] perf tools: Fix segfault on dynamic entries Namhyung Kim <namhyung@kernel.org> - 2016-02-21 19:30 +0100
    [PATCH 4/5] perf tools: Fix alignment on some sort keys Namhyung Kim <namhyung@kernel.org> - 2016-02-21 19:30 +0100
    [PATCH 5/5] perf tools: Fix column width setting on 'trace' sort key Namhyung Kim <namhyung@kernel.org> - 2016-02-21 19:30 +0100
    [PATCH 3/5] perf tools: Update srcline/file if needed Namhyung Kim <namhyung@kernel.org> - 2016-02-21 19:40 +0100

#1338854 — [PATCH 1/5] perf tools: Fix assertion failure on dynamic entry

The dynamic entry is created for each field in a tracepoint event.
Since they have no fixed hpp format index, it should skip when
perf_hpp__reset_width() is called.

This caused following assertion failure..

  $ perf record -e sched:sched_switch -a sleep 1

  $ perf report -s comm,next_pid --stdio
  perf: ui/hist.c:651: perf_hpp__reset_width:
    Assertion `!(fmt->idx >= PERF_HPP__MAX_INDEX)' failed.

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

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 1ba4117d9c2d..12223d791e9f 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -645,6 +645,9 @@ void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists)
 	if (perf_hpp__is_sort_entry(fmt))
 		return perf_hpp__reset_sort_width(fmt, hists);
 
+	if (perf_hpp__is_dynamic_entry(fmt))
+		return;
+
 	BUG_ON(fmt->idx >= PERF_HPP__MAX_INDEX);
 
 	switch (fmt->idx) {
-- 
2.7.1

[toc] | [next] | [standalone]

#1338866

On Sun, Feb 21, 2016 at 11:22:34PM +0900, Namhyung Kim wrote:
> The dynamic entry is created for each field in a tracepoint event.
> Since they have no fixed hpp format index, it should skip when
> perf_hpp__reset_width() is called.
> 
> This caused following assertion failure..
> 
>   $ perf record -e sched:sched_switch -a sleep 1
> 
>   $ perf report -s comm,next_pid --stdio
>   perf: ui/hist.c:651: perf_hpp__reset_width:
>     Assertion `!(fmt->idx >= PERF_HPP__MAX_INDEX)' failed.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

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

thanks,
jirka

> ---
>  tools/perf/ui/hist.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> index 1ba4117d9c2d..12223d791e9f 100644
> --- a/tools/perf/ui/hist.c
> +++ b/tools/perf/ui/hist.c
> @@ -645,6 +645,9 @@ void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists)
>  	if (perf_hpp__is_sort_entry(fmt))
>  		return perf_hpp__reset_sort_width(fmt, hists);
>  
> +	if (perf_hpp__is_dynamic_entry(fmt))
> +		return;
> +
>  	BUG_ON(fmt->idx >= PERF_HPP__MAX_INDEX);
>  
>  	switch (fmt->idx) {
> -- 
> 2.7.1
> 

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

#1339616

Em Sun, Feb 21, 2016 at 06:37:32PM +0100, Jiri Olsa escreveu:
> On Sun, Feb 21, 2016 at 11:22:34PM +0900, Namhyung Kim wrote:
> > The dynamic entry is created for each field in a tracepoint event.
> > Since they have no fixed hpp format index, it should skip when
> > perf_hpp__reset_width() is called.
> > 
> > This caused following assertion failure..
> > 
> >   $ perf record -e sched:sched_switch -a sleep 1
> > 
> >   $ perf report -s comm,next_pid --stdio
> >   perf: ui/hist.c:651: perf_hpp__reset_width:
> >     Assertion `!(fmt->idx >= PERF_HPP__MAX_INDEX)' failed.
> > 
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Thanks, tested and applied the series,

- Arnaldo

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

#1338869 — Re: [PATCH 3/5] perf tools: Update srcline/file if needed

On Sun, Feb 21, 2016 at 11:22:36PM +0900, Namhyung Kim wrote:
> Normally the hist entry's srcline and/or srcfile is set during sorting.
> However sometime it's possible to a hist entry's srcline is not set yet
> after the sorting.  This is because the entry is so unique and other
> sort keys already make it distinct.  Then the srcline/file sort didn't
> have a chance to be called during the sorting.  In that case it has NULL
> srcline/srcfile field and shows nothing.
> 
> Before:
> 
>   $ perf report -s comm,sym,srcline
>   ...
>   Overhead  Command       Symbol
>   -----------------------------------------------------------------
>     34.42%  swapper       [k] intel_idle          intel_idle.c:0
>      2.44%  perf          [.] __poll_nocancel     (null)
>      1.70%  gnome-shell   [k] fw_domains_get      (null)
>      1.04%  Xorg          [k] sock_poll           (null)
> 
> After:
> 
>     34.42%  swapper       [k] intel_idle          intel_idle.c:0
>      2.44%  perf          [.] __poll_nocancel     .:0
>      1.70%  gnome-shell   [k] fw_domains_get      fw_domains_get+42
>      1.04%  Xorg          [k] sock_poll           socket.c:0
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/sort.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 7daea71691df..6808d73164b5 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -315,6 +315,16 @@ sort__srcline_cmp(struct hist_entry *left, struct hist_entry *right)
>  static int hist_entry__srcline_snprintf(struct hist_entry *he, char *bf,
>  					size_t size, unsigned int width)
>  {
> +	if (!he->srcline) {
> +		if (!he->ms.map)
> +			he->srcline = SRCLINE_UNKNOWN;
> +		else {
> +			struct map *map = he->ms.map;
> +			he->srcline = get_srcline(map->dso,
> +					   map__rip_2objdump(map, he->ip),
> +						  he->ms.sym, true);
> +		}
> +	}

could you put this into the function and use it
also within sort__srcline_cmp?

it's already 3 places duplicating the same code

thanks,
jirka

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

#1339035 — [PATCH v2 3/5] perf tools: Update srcline/file if needed

Normally the hist entry's srcline and/or srcfile is set during sorting.
However sometime it's possible to a hist entry's srcline is not set yet
after the sorting.  This is because the entry is so unique and other
sort keys already make it distinct.  Then the srcline/file sort didn't
have a chance to be called during the sorting.  In that case it has NULL
srcline/srcfile field and shows nothing.

Before:

  $ perf report -s comm,sym,srcline
  ...
  Overhead  Command       Symbol
  -----------------------------------------------------------------
    34.42%  swapper       [k] intel_idle          intel_idle.c:0
     2.44%  perf          [.] __poll_nocancel     (null)
     1.70%  gnome-shell   [k] fw_domains_get      (null)
     1.04%  Xorg          [k] sock_poll           (null)

After:

    34.42%  swapper       [k] intel_idle          intel_idle.c:0
     2.44%  perf          [.] __poll_nocancel     .:0
     1.70%  gnome-shell   [k] fw_domains_get      fw_domains_get+42
     1.04%  Xorg          [k] sock_poll           socket.c:0

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
factor out common code.

 tools/perf/util/sort.c | 64 ++++++++++++++++++++++++--------------------------
 1 file changed, 31 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 7daea71691df..6f4605b5beb5 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -286,35 +286,34 @@ struct sort_entry sort_sym = {
 
 /* --sort srcline */
 
+static char *hist_entry__get_srcline(struct hist_entry *he)
+{
+	struct map *map = he->ms.map;
+
+	if (!map)
+		return SRCLINE_UNKNOWN;
+
+	return get_srcline(map->dso, map__rip_2objdump(map, he->ip),
+			   he->ms.sym, true);
+}
+
 static int64_t
 sort__srcline_cmp(struct hist_entry *left, struct hist_entry *right)
 {
-	if (!left->srcline) {
-		if (!left->ms.map)
-			left->srcline = SRCLINE_UNKNOWN;
-		else {
-			struct map *map = left->ms.map;
-			left->srcline = get_srcline(map->dso,
-					   map__rip_2objdump(map, left->ip),
-						    left->ms.sym, true);
-		}
-	}
-	if (!right->srcline) {
-		if (!right->ms.map)
-			right->srcline = SRCLINE_UNKNOWN;
-		else {
-			struct map *map = right->ms.map;
-			right->srcline = get_srcline(map->dso,
-					     map__rip_2objdump(map, right->ip),
-						     right->ms.sym, true);
-		}
-	}
+	if (!left->srcline)
+		left->srcline = hist_entry__get_srcline(left);
+	if (!right->srcline)
+		right->srcline = hist_entry__get_srcline(right);
+
 	return strcmp(right->srcline, left->srcline);
 }
 
 static int hist_entry__srcline_snprintf(struct hist_entry *he, char *bf,
 					size_t size, unsigned int width)
 {
+	if (!he->srcline)
+		he->srcline = hist_entry__get_srcline(he);
+
 	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->srcline);
 }
 
@@ -329,11 +328,14 @@ struct sort_entry sort_srcline = {
 
 static char no_srcfile[1];
 
-static char *get_srcfile(struct hist_entry *e)
+static char *hist_entry__get_srcfile(struct hist_entry *e)
 {
 	char *sf, *p;
 	struct map *map = e->ms.map;
 
+	if (!map)
+		return no_srcfile;
+
 	sf = __get_srcline(map->dso, map__rip_2objdump(map, e->ip),
 			 e->ms.sym, false, true);
 	if (!strcmp(sf, SRCLINE_UNKNOWN))
@@ -350,24 +352,20 @@ static char *get_srcfile(struct hist_entry *e)
 static int64_t
 sort__srcfile_cmp(struct hist_entry *left, struct hist_entry *right)
 {
-	if (!left->srcfile) {
-		if (!left->ms.map)
-			left->srcfile = no_srcfile;
-		else
-			left->srcfile = get_srcfile(left);
-	}
-	if (!right->srcfile) {
-		if (!right->ms.map)
-			right->srcfile = no_srcfile;
-		else
-			right->srcfile = get_srcfile(right);
-	}
+	if (!left->srcfile)
+		left->srcfile = hist_entry__get_srcfile(left);
+	if (!right->srcfile)
+		right->srcfile = hist_entry__get_srcfile(right);
+
 	return strcmp(right->srcfile, left->srcfile);
 }
 
 static int hist_entry__srcfile_snprintf(struct hist_entry *he, char *bf,
 					size_t size, unsigned int width)
 {
+	if (!he->srcfile)
+		he->srcfile = hist_entry__get_srcfile(he);
+
 	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->srcfile);
 }
 
-- 
2.7.0

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

#1339155 — Re: [PATCH v2 3/5] perf tools: Update srcline/file if needed

On Mon, Feb 22, 2016 at 09:31:51AM +0900, Namhyung Kim wrote:
> Normally the hist entry's srcline and/or srcfile is set during sorting.
> However sometime it's possible to a hist entry's srcline is not set yet
> after the sorting.  This is because the entry is so unique and other
> sort keys already make it distinct.  Then the srcline/file sort didn't
> have a chance to be called during the sorting.  In that case it has NULL
> srcline/srcfile field and shows nothing.
> 
> Before:
> 
>   $ perf report -s comm,sym,srcline
>   ...
>   Overhead  Command       Symbol
>   -----------------------------------------------------------------
>     34.42%  swapper       [k] intel_idle          intel_idle.c:0
>      2.44%  perf          [.] __poll_nocancel     (null)
>      1.70%  gnome-shell   [k] fw_domains_get      (null)
>      1.04%  Xorg          [k] sock_poll           (null)
> 
> After:
> 
>     34.42%  swapper       [k] intel_idle          intel_idle.c:0
>      2.44%  perf          [.] __poll_nocancel     .:0
>      1.70%  gnome-shell   [k] fw_domains_get      fw_domains_get+42
>      1.04%  Xorg          [k] sock_poll           socket.c:0
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

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

thanks,
jirka

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

#1338870 — Re: [PATCH 2/5] perf tools: Fix segfault on dynamic entries

On Sun, Feb 21, 2016 at 11:22:35PM +0900, Namhyung Kim wrote:
> The dynamic entry is created for each tracepoint event.  When it sets up
> the sort key, it checks with existing keys using ->equal() callback.
> But it missed to set the ->equal for dynamic entries.  The following
> segfault was due to the missing ->equal() callback.
> 
>   (gdb) bt
>   #0  0x0000000000140003 in ?? ()
>   #1  0x0000000000537769 in fmt_equal (b=0x2106980, a=0x21067a0) at ui/hist.c:548
>   #2  perf_hpp__setup_output_field (list=0x8c6d80 <perf_hpp_list>) at ui/hist.c:560
>   #3  0x00000000004e927e in setup_sorting (evlist=<optimized out>) at util/sort.c:2642
>   #4  0x000000000043cf50 in cmd_report (argc=<optimized out>, argv=<optimized out>, prefix=<optimized out>)
>       at builtin-report.c:932
>   #5  0x00000000004865a1 in run_builtin (p=p@entry=0x8bbce0 <commands+192>, argc=argc@entry=7,
>       argv=argv@entry=0x7ffd24d56ce0) at perf.c:390
>   #6  0x000000000042dc1f in handle_internal_command (argv=0x7ffd24d56ce0, argc=7) at perf.c:451
>   #7  run_argv (argv=0x7ffd24d56a70, argcp=0x7ffd24d56a7c) at perf.c:495
>   #8  main (argc=7, argv=0x7ffd24d56ce0) at perf.c:620
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

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

thanks,
jirka

> ---
>  tools/perf/util/sort.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index de715756f281..7daea71691df 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1835,6 +1835,20 @@ bool perf_hpp__is_dynamic_entry(struct perf_hpp_fmt *fmt)
>  	return fmt->cmp == __sort__hde_cmp;
>  }
>  
> +static bool __sort__hde_equal(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b)
> +{
> +	struct hpp_dynamic_entry *hde_a;
> +	struct hpp_dynamic_entry *hde_b;
> +
> +	if (!perf_hpp__is_dynamic_entry(a) || !perf_hpp__is_dynamic_entry(b))
> +		return false;
> +
> +	hde_a = container_of(a, struct hpp_dynamic_entry, hpp);
> +	hde_b = container_of(b, struct hpp_dynamic_entry, hpp);
> +
> +	return hde_a->field == hde_b->field;
> +}
> +
>  static void hde_free(struct perf_hpp_fmt *fmt)
>  {
>  	struct hpp_dynamic_entry *hde;
> @@ -1867,6 +1881,7 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field)
>  	hde->hpp.cmp = __sort__hde_cmp;
>  	hde->hpp.collapse = __sort__hde_cmp;
>  	hde->hpp.sort = __sort__hde_cmp;
> +	hde->hpp.equal = __sort__hde_equal;
>  	hde->hpp.free = hde_free;
>  
>  	INIT_LIST_HEAD(&hde->hpp.list);
> -- 
> 2.7.1
> 

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

#1338885 — Re: [PATCH 5/5] perf tools: Fix column width setting on 'trace' sort key

On Sun, Feb 21, 2016 at 11:22:38PM +0900, Namhyung Kim wrote:
> It missed to update column length of the 'trace' sort key in the
> hists__calc_col_len() so it might truncate the output.  It calculated
> the column length in the ->cmp() callback originally but it doesn't
> guarantee it's called always.

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

thanks,
jirka

> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/hist.c | 3 +++
>  tools/perf/util/sort.c | 3 ---
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 827c6cbcd05d..017eb5c42c37 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -179,6 +179,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>  	if (h->transaction)
>  		hists__new_col_len(hists, HISTC_TRANSACTION,
>  				   hist_entry__transaction_len());
> +
> +	if (h->trace_output)
> +		hists__new_col_len(hists, HISTC_TRACE, strlen(h->trace_output));
>  }
>  
>  void hists__output_recalc_col_len(struct hists *hists, int max_rows)
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 1d2b85c808d0..ea05497cfee9 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -501,9 +501,6 @@ sort__trace_cmp(struct hist_entry *left, struct hist_entry *right)
>  	if (right->trace_output == NULL)
>  		right->trace_output = get_trace_output(right);
>  
> -	hists__new_col_len(left->hists, HISTC_TRACE, strlen(left->trace_output));
> -	hists__new_col_len(right->hists, HISTC_TRACE, strlen(right->trace_output));
> -
>  	return strcmp(right->trace_output, left->trace_output);
>  }
>  
> -- 
> 2.7.1
> 

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

#1338886 — Re: [PATCH 4/5] perf tools: Fix alignment on some sort keys

On Sun, Feb 21, 2016 at 11:22:37PM +0900, Namhyung Kim wrote:
> The srcline, srcfile and trace sort keys can have long entries.  With
> commit 89fee7094323 ("perf hists: Do column alignment on the format
> iterator"), it now aligns output with hist_entry__snprintf_alignment().
> So each (possibly long) sort entries don't need to do it themselves.

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

thanks,
jirka

> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/sort.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 6808d73164b5..1d2b85c808d0 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -325,7 +325,7 @@ static int hist_entry__srcline_snprintf(struct hist_entry *he, char *bf,
>  						  he->ms.sym, true);
>  		}
>  	}
> -	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->srcline);
> +	return repsep_snprintf(bf, size, "%-.*s", width, he->srcline);
>  }
>  
>  struct sort_entry sort_srcline = {
> @@ -384,7 +384,7 @@ static int hist_entry__srcfile_snprintf(struct hist_entry *he, char *bf,
>  		else
>  			he->srcfile = get_srcfile(he);
>  	}
> -	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->srcfile);
> +	return repsep_snprintf(bf, size, "%-.*s", width, he->srcfile);
>  }
>  
>  struct sort_entry sort_srcfile = {
> @@ -514,11 +514,11 @@ static int hist_entry__trace_snprintf(struct hist_entry *he, char *bf,
>  
>  	evsel = hists_to_evsel(he->hists);
>  	if (evsel->attr.type != PERF_TYPE_TRACEPOINT)
> -		return scnprintf(bf, size, "%-*.*s", width, width, "N/A");
> +		return scnprintf(bf, size, "%-.*s", width, "N/A");
>  
>  	if (he->trace_output == NULL)
>  		he->trace_output = get_trace_output(he);
> -	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->trace_output);
> +	return repsep_snprintf(bf, size, "%-.*s", width, he->trace_output);
>  }
>  
>  struct sort_entry sort_trace = {
> -- 
> 2.7.1
> 

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

#1339033 — [PATCH v2 4/5] perf tools: Fix alignment on some sort keys

The srcline, srcfile and trace sort keys can have long entries.  With
commit 89fee7094323 ("perf hists: Do column alignment on the format
iterator"), it now aligns output with hist_entry__snprintf_alignment().
So each (possibly long) sort entries don't need to do it themselves.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
rebased
 tools/perf/util/sort.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 6f4605b5beb5..a7d73e503b1b 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -314,7 +314,7 @@ static int hist_entry__srcline_snprintf(struct hist_entry *he, char *bf,
 	if (!he->srcline)
 		he->srcline = hist_entry__get_srcline(he);
 
-	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->srcline);
+	return repsep_snprintf(bf, size, "%-.*s", width, he->srcline);
 }
 
 struct sort_entry sort_srcline = {
@@ -366,7 +366,7 @@ static int hist_entry__srcfile_snprintf(struct hist_entry *he, char *bf,
 	if (!he->srcfile)
 		he->srcfile = hist_entry__get_srcfile(he);
 
-	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->srcfile);
+	return repsep_snprintf(bf, size, "%-.*s", width, he->srcfile);
 }
 
 struct sort_entry sort_srcfile = {
@@ -496,11 +496,11 @@ static int hist_entry__trace_snprintf(struct hist_entry *he, char *bf,
 
 	evsel = hists_to_evsel(he->hists);
 	if (evsel->attr.type != PERF_TYPE_TRACEPOINT)
-		return scnprintf(bf, size, "%-*.*s", width, width, "N/A");
+		return scnprintf(bf, size, "%-.*s", width, "N/A");
 
 	if (he->trace_output == NULL)
 		he->trace_output = get_trace_output(he);
-	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->trace_output);
+	return repsep_snprintf(bf, size, "%-.*s", width, he->trace_output);
 }
 
 struct sort_entry sort_trace = {
-- 
2.7.0

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

#1338933 — [PATCH 2/5] perf tools: Fix segfault on dynamic entries

The dynamic entry is created for each tracepoint event.  When it sets up
the sort key, it checks with existing keys using ->equal() callback.
But it missed to set the ->equal for dynamic entries.  The following
segfault was due to the missing ->equal() callback.

  (gdb) bt
  #0  0x0000000000140003 in ?? ()
  #1  0x0000000000537769 in fmt_equal (b=0x2106980, a=0x21067a0) at ui/hist.c:548
  #2  perf_hpp__setup_output_field (list=0x8c6d80 <perf_hpp_list>) at ui/hist.c:560
  #3  0x00000000004e927e in setup_sorting (evlist=<optimized out>) at util/sort.c:2642
  #4  0x000000000043cf50 in cmd_report (argc=<optimized out>, argv=<optimized out>, prefix=<optimized out>)
      at builtin-report.c:932
  #5  0x00000000004865a1 in run_builtin (p=p@entry=0x8bbce0 <commands+192>, argc=argc@entry=7,
      argv=argv@entry=0x7ffd24d56ce0) at perf.c:390
  #6  0x000000000042dc1f in handle_internal_command (argv=0x7ffd24d56ce0, argc=7) at perf.c:451
  #7  run_argv (argv=0x7ffd24d56a70, argcp=0x7ffd24d56a7c) at perf.c:495
  #8  main (argc=7, argv=0x7ffd24d56ce0) at perf.c:620

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

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index de715756f281..7daea71691df 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1835,6 +1835,20 @@ bool perf_hpp__is_dynamic_entry(struct perf_hpp_fmt *fmt)
 	return fmt->cmp == __sort__hde_cmp;
 }
 
+static bool __sort__hde_equal(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b)
+{
+	struct hpp_dynamic_entry *hde_a;
+	struct hpp_dynamic_entry *hde_b;
+
+	if (!perf_hpp__is_dynamic_entry(a) || !perf_hpp__is_dynamic_entry(b))
+		return false;
+
+	hde_a = container_of(a, struct hpp_dynamic_entry, hpp);
+	hde_b = container_of(b, struct hpp_dynamic_entry, hpp);
+
+	return hde_a->field == hde_b->field;
+}
+
 static void hde_free(struct perf_hpp_fmt *fmt)
 {
 	struct hpp_dynamic_entry *hde;
@@ -1867,6 +1881,7 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field)
 	hde->hpp.cmp = __sort__hde_cmp;
 	hde->hpp.collapse = __sort__hde_cmp;
 	hde->hpp.sort = __sort__hde_cmp;
+	hde->hpp.equal = __sort__hde_equal;
 	hde->hpp.free = hde_free;
 
 	INIT_LIST_HEAD(&hde->hpp.list);
-- 
2.7.1

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

#1338934 — [PATCH 4/5] perf tools: Fix alignment on some sort keys

The srcline, srcfile and trace sort keys can have long entries.  With
commit 89fee7094323 ("perf hists: Do column alignment on the format
iterator"), it now aligns output with hist_entry__snprintf_alignment().
So each (possibly long) sort entries don't need to do it themselves.

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

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 6808d73164b5..1d2b85c808d0 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -325,7 +325,7 @@ static int hist_entry__srcline_snprintf(struct hist_entry *he, char *bf,
 						  he->ms.sym, true);
 		}
 	}
-	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->srcline);
+	return repsep_snprintf(bf, size, "%-.*s", width, he->srcline);
 }
 
 struct sort_entry sort_srcline = {
@@ -384,7 +384,7 @@ static int hist_entry__srcfile_snprintf(struct hist_entry *he, char *bf,
 		else
 			he->srcfile = get_srcfile(he);
 	}
-	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->srcfile);
+	return repsep_snprintf(bf, size, "%-.*s", width, he->srcfile);
 }
 
 struct sort_entry sort_srcfile = {
@@ -514,11 +514,11 @@ static int hist_entry__trace_snprintf(struct hist_entry *he, char *bf,
 
 	evsel = hists_to_evsel(he->hists);
 	if (evsel->attr.type != PERF_TYPE_TRACEPOINT)
-		return scnprintf(bf, size, "%-*.*s", width, width, "N/A");
+		return scnprintf(bf, size, "%-.*s", width, "N/A");
 
 	if (he->trace_output == NULL)
 		he->trace_output = get_trace_output(he);
-	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->trace_output);
+	return repsep_snprintf(bf, size, "%-.*s", width, he->trace_output);
 }
 
 struct sort_entry sort_trace = {
-- 
2.7.1

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

#1338937 — [PATCH 5/5] perf tools: Fix column width setting on 'trace' sort key

It missed to update column length of the 'trace' sort key in the
hists__calc_col_len() so it might truncate the output.  It calculated
the column length in the ->cmp() callback originally but it doesn't
guarantee it's called always.

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

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 827c6cbcd05d..017eb5c42c37 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -179,6 +179,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	if (h->transaction)
 		hists__new_col_len(hists, HISTC_TRANSACTION,
 				   hist_entry__transaction_len());
+
+	if (h->trace_output)
+		hists__new_col_len(hists, HISTC_TRACE, strlen(h->trace_output));
 }
 
 void hists__output_recalc_col_len(struct hists *hists, int max_rows)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 1d2b85c808d0..ea05497cfee9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -501,9 +501,6 @@ sort__trace_cmp(struct hist_entry *left, struct hist_entry *right)
 	if (right->trace_output == NULL)
 		right->trace_output = get_trace_output(right);
 
-	hists__new_col_len(left->hists, HISTC_TRACE, strlen(left->trace_output));
-	hists__new_col_len(right->hists, HISTC_TRACE, strlen(right->trace_output));
-
 	return strcmp(right->trace_output, left->trace_output);
 }
 
-- 
2.7.1

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

#1338943 — [PATCH 3/5] perf tools: Update srcline/file if needed

Normally the hist entry's srcline and/or srcfile is set during sorting.
However sometime it's possible to a hist entry's srcline is not set yet
after the sorting.  This is because the entry is so unique and other
sort keys already make it distinct.  Then the srcline/file sort didn't
have a chance to be called during the sorting.  In that case it has NULL
srcline/srcfile field and shows nothing.

Before:

  $ perf report -s comm,sym,srcline
  ...
  Overhead  Command       Symbol
  -----------------------------------------------------------------
    34.42%  swapper       [k] intel_idle          intel_idle.c:0
     2.44%  perf          [.] __poll_nocancel     (null)
     1.70%  gnome-shell   [k] fw_domains_get      (null)
     1.04%  Xorg          [k] sock_poll           (null)

After:

    34.42%  swapper       [k] intel_idle          intel_idle.c:0
     2.44%  perf          [.] __poll_nocancel     .:0
     1.70%  gnome-shell   [k] fw_domains_get      fw_domains_get+42
     1.04%  Xorg          [k] sock_poll           socket.c:0

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

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 7daea71691df..6808d73164b5 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -315,6 +315,16 @@ sort__srcline_cmp(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__srcline_snprintf(struct hist_entry *he, char *bf,
 					size_t size, unsigned int width)
 {
+	if (!he->srcline) {
+		if (!he->ms.map)
+			he->srcline = SRCLINE_UNKNOWN;
+		else {
+			struct map *map = he->ms.map;
+			he->srcline = get_srcline(map->dso,
+					   map__rip_2objdump(map, he->ip),
+						  he->ms.sym, true);
+		}
+	}
 	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->srcline);
 }
 
@@ -368,6 +378,12 @@ sort__srcfile_cmp(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__srcfile_snprintf(struct hist_entry *he, char *bf,
 					size_t size, unsigned int width)
 {
+	if (!he->srcfile) {
+		if (!he->ms.map)
+			he->srcfile = no_srcfile;
+		else
+			he->srcfile = get_srcfile(he);
+	}
 	return repsep_snprintf(bf, size, "%-*.*s", width, width, he->srcfile);
 }
 
-- 
2.7.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web