[PATCH v3 0/3] perf annotate: Simple bugfixes

[PATCH v3 0/3] perf annotate: Simple bugfixes

[Taeung Song]

Hi,

perf-annotate has little bugs so I fix them.
I'd appreciate some feedback on this patchset. :)

Thanks,
Taeung

v3:
- rebased on current acme/perf/core

v2:
- more clearly commit log messages (Arnaldo)
- rebased on current acme/perf/core

Taeung Song (3):
  perf annotate: Fix a bug reading link name from a build-id file
  perf annotate: Fix a bug of division by zero when calculating percent
  perf annotate: Fix missing number of samples

 tools/perf/util/annotate.c | 24 +++++++++++++++++++-----
 tools/perf/util/annotate.h |  2 +-
 2 files changed, 20 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH v3 1/3] perf annotate: Fix a bug reading link name from a build-id file

[Taeung Song]

It is wrong way to read link name from a build-id file.
Because a build-id file is not symbolic link
but build-id directory of it is symbolic link, so fix it.

For example, if build-id file name gotten from
dso__build_id_filename() is as below,

  /root/.debug/.build-id/4f/75c7d197c951659d1c1b8b5fd49bcdf8f3f8b1/elf

To correctly read link name of build-id,
use the build-id dir path that is a symbolic link,
instead of the above build-id file name like below.

  /root/.debug/.build-id/4f/75c7d197c951659d1c1b8b5fd49bcdf8f3f8b1

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/util/annotate.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 3d0263e..6dc9148 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1307,6 +1307,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 {
 	char linkname[PATH_MAX];
 	char *build_id_filename;
+	char *build_id_path = NULL;
 
 	if (dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS &&
 	    !dso__is_kcore(dso))
@@ -1322,8 +1323,14 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 		goto fallback;
 	}
 
+	build_id_path = strdup(filename);
+	if (!build_id_path)
+		return -1;
+
+	dirname(build_id_path);
+
 	if (dso__is_kcore(dso) ||
-	    readlink(filename, linkname, sizeof(linkname)) < 0 ||
+	    readlink(build_id_path, linkname, sizeof(linkname)) < 0 ||
 	    strstr(linkname, DSO__NAME_KALLSYMS) ||
 	    access(filename, R_OK)) {
 fallback:
@@ -1335,6 +1342,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 		__symbol__join_symfs(filename, filename_size, dso->long_name);
 	}
 
+	free(build_id_path);
 	return 0;
 }
 
-- 
2.7.4

[PATCH v3 2/3] perf annotate: Fix a bug of division by zero when calculating percent

[Taeung Song]

Currently perf-annotate with --print-line can print
-nan(0x8000000000000) because of division by zero
when calculating percent. The division by zero happen
when a sum of samples is zero in symbol__get_source_line(),
so fix it.

For examples,
After running 'perf record' like below,

    $ perf record -e "{cycles,page-faults,branch-misses}" ./a.out

Before:

    $ perf annotate --stdio -l

Sorted summary for file /home/taeung/workspace/a.out
----------------------------------------------

   32.89    -nan    7.04 a.c:38
   25.14    -nan    0.00 a.c:34
   16.26    -nan   56.34 a.c:31
   15.88    -nan    1.41 a.c:37
    5.67    -nan    0.00 a.c:39
    1.13    -nan   35.21 a.c:26
    0.95    -nan    0.00 a.c:44
    0.57    -nan    0.00 a.c:32
 Percent                 |      Source code & Disassembly of a.out for cycles (529 samples)
-----------------------------------------------------------------------------------------
                         :
...

 a.c:26    0.57    -nan    4.23 :         40081a:       mov    %edi,-0x24(%rbp)
 a.c:26    0.00    -nan    9.86 :         40081d:       mov    %rsi,-0x30(%rbp)

...

However, if a sum of samples is zero (e.g. 'page-faults'),
skip calculating percent.

After:

    $ perf annotate --stdio -l

Sorted summary for file /home/taeung/workspace/a.out
----------------------------------------------

   32.89    0.00    7.04 a.c:38
   25.14    0.00    0.00 a.c:34
   16.26    0.00   56.34 a.c:31
   15.88    0.00    1.41 a.c:37
    5.67    0.00    0.00 a.c:39
    1.13    0.00   35.21 a.c:26
    0.95    0.00    0.00 a.c:44
    0.57    0.00    0.00 a.c:32
 Percent                 |      Source code & Disassembly of old for cycles (529 samples)
-----------------------------------------------------------------------------------------
                         :
...

a.c:26    0.57    0.00    4.23 :         40081a:       mov    %edi,-0x24(%rbp)
a.c:26    0.00    0.00    9.86 :         40081d:       mov    %rsi,-0x30(%rbp)

...

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/util/annotate.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 6dc9148..11af5f0 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1671,11 +1671,15 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
 		src_line->nr_pcnt = nr_pcnt;
 
 		for (k = 0; k < nr_pcnt; k++) {
+			double percent = 0.0;
+
 			h = annotation__histogram(notes, evidx + k);
-			src_line->samples[k].percent = 100.0 * h->addr[i] / h->sum;
+			if (h->sum)
+				percent = 100.0 * h->addr[i] / h->sum;
 
-			if (src_line->samples[k].percent > percent_max)
-				percent_max = src_line->samples[k].percent;
+			if (percent > percent_max)
+				percent_max = percent;
+			src_line->samples[k].percent = percent;
 		}
 
 		if (percent_max <= 0.5)
-- 
2.7.4

[PATCH v3 3/3] perf annotate: Fix missing number of samples

[Taeung Song]

If running 'perf annotate --stdio -l --show-total-period',
you can see a problem showing only zero '0' for number of samples.

Before:
    $ perf annotate --stdio -l --show-total-period
...
       0 :        400816:       push   %rbp
       0 :        400817:       mov    %rsp,%rbp
       0 :        40081a:       mov    %edi,-0x24(%rbp)
       0 :        40081d:       mov    %rsi,-0x30(%rbp)
       0 :        400821:       mov    -0x24(%rbp),%eax
       0 :        400824:       mov    -0x30(%rbp),%rdx
       0 :        400828:       mov    (%rdx),%esi
       0 :        40082a:       mov    $0x0,%edx
...

The reason is number of samples aren't set
in symbol__get_source_line(). so set it ordinarily.

After:
    $ perf annotate --stdio -l --show-total-period
...
       3 :        400816:       push   %rbp
       4 :        400817:       mov    %rsp,%rbp
       0 :        40081a:       mov    %edi,-0x24(%rbp)
       0 :        40081d:       mov    %rsi,-0x30(%rbp)
       1 :        400821:       mov    -0x24(%rbp),%eax
       2 :        400824:       mov    -0x30(%rbp),%rdx
       0 :        400828:       mov    (%rdx),%esi
       1 :        40082a:       mov    $0x0,%edx
...

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/util/annotate.c | 6 ++++--
 tools/perf/util/annotate.h | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 11af5f0..a37032b 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1665,7 +1665,7 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
 	start = map__rip_2objdump(map, sym->start);
 
 	for (i = 0; i < len; i++) {
-		u64 offset;
+		u64 offset, nr_samples;
 		double percent_max = 0.0;
 
 		src_line->nr_pcnt = nr_pcnt;
@@ -1674,12 +1674,14 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
 			double percent = 0.0;
 
 			h = annotation__histogram(notes, evidx + k);
+			nr_samples = h->addr[i];
 			if (h->sum)
-				percent = 100.0 * h->addr[i] / h->sum;
+				percent = 100.0 * nr_samples / h->sum;
 
 			if (percent > percent_max)
 				percent_max = percent;
 			src_line->samples[k].percent = percent;
+			src_line->samples[k].nr = nr_samples;
 		}
 
 		if (percent_max <= 0.5)
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 09776b5..948aa8e 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -98,7 +98,7 @@ struct cyc_hist {
 struct source_line_samples {
 	double		percent;
 	double		percent_sum;
-	double          nr;
+	u64		nr;
 };
 
 struct source_line {
-- 
2.7.4

Re: [PATCH v3 1/3] perf annotate: Fix a bug reading link name from a build-id file

[Arnaldo Carvalho de Melo]

Em Mon, Mar 27, 2017 at 04:10:36PM +0900, Taeung Song escreveu:
> It is wrong way to read link name from a build-id file.
> Because a build-id file is not symbolic link
> but build-id directory of it is symbolic link, so fix it.

Ok, applied, in the past it was always a symlink, but the following cset
changed that, so I added:

    Fixes: 01412261d994 ("perf buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid")

Thanks,

- Arnaldo
 
> For example, if build-id file name gotten from
> dso__build_id_filename() is as below,
> 
>   /root/.debug/.build-id/4f/75c7d197c951659d1c1b8b5fd49bcdf8f3f8b1/elf
> 
> To correctly read link name of build-id,
> use the build-id dir path that is a symbolic link,
> instead of the above build-id file name like below.
> 
>   /root/.debug/.build-id/4f/75c7d197c951659d1c1b8b5fd49bcdf8f3f8b1
> 
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/util/annotate.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 3d0263e..6dc9148 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1307,6 +1307,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>  {
>  	char linkname[PATH_MAX];
>  	char *build_id_filename;
> +	char *build_id_path = NULL;
>  
>  	if (dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS &&
>  	    !dso__is_kcore(dso))
> @@ -1322,8 +1323,14 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>  		goto fallback;
>  	}
>  
> +	build_id_path = strdup(filename);
> +	if (!build_id_path)
> +		return -1;
> +
> +	dirname(build_id_path);
> +
>  	if (dso__is_kcore(dso) ||
> -	    readlink(filename, linkname, sizeof(linkname)) < 0 ||
> +	    readlink(build_id_path, linkname, sizeof(linkname)) < 0 ||
>  	    strstr(linkname, DSO__NAME_KALLSYMS) ||
>  	    access(filename, R_OK)) {
>  fallback:
> @@ -1335,6 +1342,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>  		__symbol__join_symfs(filename, filename_size, dso->long_name);
>  	}
>  
> +	free(build_id_path);
>  	return 0;
>  }
>  
> -- 
> 2.7.4

Re: [PATCH v3 3/3] perf annotate: Fix missing number of samples

[Arnaldo Carvalho de Melo]

Em Mon, Mar 27, 2017 at 04:10:38PM +0900, Taeung Song escreveu:
> If running 'perf annotate --stdio -l --show-total-period',
> you can see a problem showing only zero '0' for number of samples.
> 
> Before:
>     $ perf annotate --stdio -l --show-total-period
> ...
>        0 :        400816:       push   %rbp
>        0 :        400817:       mov    %rsp,%rbp
>        0 :        40081a:       mov    %edi,-0x24(%rbp)
>        0 :        40081d:       mov    %rsi,-0x30(%rbp)
>        0 :        400821:       mov    -0x24(%rbp),%eax
>        0 :        400824:       mov    -0x30(%rbp),%rdx
>        0 :        400828:       mov    (%rdx),%esi
>        0 :        40082a:       mov    $0x0,%edx
> ...
> 
> The reason is number of samples aren't set
> in symbol__get_source_line(). so set it ordinarily.

Can you please take a look at:

  0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")

that introduced the --show-total-period code and take it into account in
this fix?

I.e. from a quick look it did the calculation setting that field in the
TUI code, where it should have done in the util/annotate.c file, so that
all UIs would be able to use it.

After your analysis, please add a Fixes: that cset, ok?

I applied the other two patches and added Martin to the CC list, as he
is the author of that patch and may have something to say here.

- Arnaldo

 
> After:
>     $ perf annotate --stdio -l --show-total-period
> ...
>        3 :        400816:       push   %rbp
>        4 :        400817:       mov    %rsp,%rbp
>        0 :        40081a:       mov    %edi,-0x24(%rbp)
>        0 :        40081d:       mov    %rsi,-0x30(%rbp)
>        1 :        400821:       mov    -0x24(%rbp),%eax
>        2 :        400824:       mov    -0x30(%rbp),%rdx
>        0 :        400828:       mov    (%rdx),%esi
>        1 :        40082a:       mov    $0x0,%edx
> ...
> 
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/util/annotate.c | 6 ++++--
>  tools/perf/util/annotate.h | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 11af5f0..a37032b 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1665,7 +1665,7 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
>  	start = map__rip_2objdump(map, sym->start);
>  
>  	for (i = 0; i < len; i++) {
> -		u64 offset;
> +		u64 offset, nr_samples;
>  		double percent_max = 0.0;
>  
>  		src_line->nr_pcnt = nr_pcnt;
> @@ -1674,12 +1674,14 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
>  			double percent = 0.0;
>  
>  			h = annotation__histogram(notes, evidx + k);
> +			nr_samples = h->addr[i];
>  			if (h->sum)
> -				percent = 100.0 * h->addr[i] / h->sum;
> +				percent = 100.0 * nr_samples / h->sum;
>  
>  			if (percent > percent_max)
>  				percent_max = percent;
>  			src_line->samples[k].percent = percent;
> +			src_line->samples[k].nr = nr_samples;
>  		}
>  
>  		if (percent_max <= 0.5)
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 09776b5..948aa8e 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -98,7 +98,7 @@ struct cyc_hist {
>  struct source_line_samples {
>  	double		percent;
>  	double		percent_sum;
> -	double          nr;
> +	u64		nr;
>  };
>  
>  struct source_line {
> -- 
> 2.7.4

[tip:perf/core] perf annotate: Fix a bug following symbolic link of a build-id file

[tip-bot for Taeung Song]

Commit-ID:  6ebd2547dd24daf95a21b2bc59931de8502afcc3
Gitweb:     http://git.kernel.org/tip/6ebd2547dd24daf95a21b2bc59931de8502afcc3
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Mon, 27 Mar 2017 16:10:36 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 27 Mar 2017 14:58:20 -0300

perf annotate: Fix a bug following symbolic link of a build-id file

It is wrong way to read link name from a build-id file.  Because a
build-id file is not anymore a symbolic link but build-id directory of
it is symbolic link, so fix it.

For example, if build-id file name gotten from
dso__build_id_filename() is as below,

  /root/.debug/.build-id/4f/75c7d197c951659d1c1b8b5fd49bcdf8f3f8b1/elf

To correctly read link name of build-id, use the build-id dir path that
is a symbolic link, instead of the above build-id file name like below.

  /root/.debug/.build-id/4f/75c7d197c951659d1c1b8b5fd49bcdf8f3f8b1

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1490598638-13947-2-git-send-email-treeze.taeung@gmail.com
Fixes: 01412261d994 ("perf buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 3d0263e..6dc9148 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1307,6 +1307,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 {
 	char linkname[PATH_MAX];
 	char *build_id_filename;
+	char *build_id_path = NULL;
 
 	if (dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS &&
 	    !dso__is_kcore(dso))
@@ -1322,8 +1323,14 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 		goto fallback;
 	}
 
+	build_id_path = strdup(filename);
+	if (!build_id_path)
+		return -1;
+
+	dirname(build_id_path);
+
 	if (dso__is_kcore(dso) ||
-	    readlink(filename, linkname, sizeof(linkname)) < 0 ||
+	    readlink(build_id_path, linkname, sizeof(linkname)) < 0 ||
 	    strstr(linkname, DSO__NAME_KALLSYMS) ||
 	    access(filename, R_OK)) {
 fallback:
@@ -1335,6 +1342,7 @@ fallback:
 		__symbol__join_symfs(filename, filename_size, dso->long_name);
 	}
 
+	free(build_id_path);
 	return 0;
 }
 

[tip:perf/core] perf annotate: Fix a bug of division by zero when calculating percent

[tip-bot for Taeung Song]

Commit-ID:  2e933b1274dc89cd1629f6c7fd9bf952248d84c2
Gitweb:     http://git.kernel.org/tip/2e933b1274dc89cd1629f6c7fd9bf952248d84c2
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Mon, 27 Mar 2017 16:10:37 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 27 Mar 2017 15:04:56 -0300

perf annotate: Fix a bug of division by zero when calculating percent

Currently perf-annotate with --print-line can print
-nan(0x8000000000000) because of division by zero when calculating
percent. The division by zero happens when a sum of samples is zero in
symbol__get_source_line(), so fix it.

For example:

After running 'perf record' like below,

    $ perf record -e "{cycles,page-faults,branch-misses}" ./a.out

Before:

    $ perf annotate --stdio -l

  Sorted summary for file /home/taeung/workspace/a.out
  ----------------------------------------------

   32.89    -nan    7.04 a.c:38
   25.14    -nan    0.00 a.c:34
   16.26    -nan   56.34 a.c:31
   15.88    -nan    1.41 a.c:37
    5.67    -nan    0.00 a.c:39
    1.13    -nan   35.21 a.c:26
    0.95    -nan    0.00 a.c:44
    0.57    -nan    0.00 a.c:32
   Percent                 |      Source code & Disassembly of a.out for cycles (529 samples)
  -----------------------------------------------------------------------------------------
                         :
  ...

   a.c:26    0.57    -nan    4.23 :         40081a:       mov    %edi,-0x24(%rbp)
   a.c:26    0.00    -nan    9.86 :         40081d:       mov    %rsi,-0x30(%rbp)

  ...

However, if a sum of samples is zero (e.g. 'page-faults'),
skip calculating percent.

After:

    $ perf annotate --stdio -l

  Sorted summary for file /home/taeung/workspace/a.out
  ----------------------------------------------

   32.89    0.00    7.04 a.c:38
   25.14    0.00    0.00 a.c:34
   16.26    0.00   56.34 a.c:31
   15.88    0.00    1.41 a.c:37
    5.67    0.00    0.00 a.c:39
    1.13    0.00   35.21 a.c:26
    0.95    0.00    0.00 a.c:44
    0.57    0.00    0.00 a.c:32
   Percent                 |      Source code & Disassembly of old for cycles (529 samples)
  -----------------------------------------------------------------------------------------
                         :
  ...

  a.c:26    0.57    0.00    4.23 :         40081a:       mov    %edi,-0x24(%rbp)
  a.c:26    0.00    0.00    9.86 :         40081d:       mov    %rsi,-0x30(%rbp)

  ...

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1490598638-13947-3-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 6dc9148..11af5f0 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1671,11 +1671,15 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
 		src_line->nr_pcnt = nr_pcnt;
 
 		for (k = 0; k < nr_pcnt; k++) {
+			double percent = 0.0;
+
 			h = annotation__histogram(notes, evidx + k);
-			src_line->samples[k].percent = 100.0 * h->addr[i] / h->sum;
+			if (h->sum)
+				percent = 100.0 * h->addr[i] / h->sum;
 
-			if (src_line->samples[k].percent > percent_max)
-				percent_max = src_line->samples[k].percent;
+			if (percent > percent_max)
+				percent_max = percent;
+			src_line->samples[k].percent = percent;
 		}
 
 		if (percent_max <= 0.5)

Re: [PATCH v3 1/3] perf annotate: Fix a bug reading link name from a build-id file

[Taeung Song]

On 03/28/2017 03:01 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Mar 27, 2017 at 04:10:36PM +0900, Taeung Song escreveu:
>> It is wrong way to read link name from a build-id file.
>> Because a build-id file is not symbolic link
>> but build-id directory of it is symbolic link, so fix it.
>
> Ok, applied, in the past it was always a symlink, but the following cset
> changed that, so I added:
>
>     Fixes: 01412261d994 ("perf buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid")
>
> Thanks,
>
> - Arnaldo

Thank you!

- Taeung

>
>> For example, if build-id file name gotten from
>> dso__build_id_filename() is as below,
>>
>>   /root/.debug/.build-id/4f/75c7d197c951659d1c1b8b5fd49bcdf8f3f8b1/elf
>>
>> To correctly read link name of build-id,
>> use the build-id dir path that is a symbolic link,
>> instead of the above build-id file name like below.
>>
>>   /root/.debug/.build-id/4f/75c7d197c951659d1c1b8b5fd49bcdf8f3f8b1
>>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>>  tools/perf/util/annotate.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
>> index 3d0263e..6dc9148 100644
>> --- a/tools/perf/util/annotate.c
>> +++ b/tools/perf/util/annotate.c
>> @@ -1307,6 +1307,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>>  {
>>  	char linkname[PATH_MAX];
>>  	char *build_id_filename;
>> +	char *build_id_path = NULL;
>>
>>  	if (dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS &&
>>  	    !dso__is_kcore(dso))
>> @@ -1322,8 +1323,14 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>>  		goto fallback;
>>  	}
>>
>> +	build_id_path = strdup(filename);
>> +	if (!build_id_path)
>> +		return -1;
>> +
>> +	dirname(build_id_path);
>> +
>>  	if (dso__is_kcore(dso) ||
>> -	    readlink(filename, linkname, sizeof(linkname)) < 0 ||
>> +	    readlink(build_id_path, linkname, sizeof(linkname)) < 0 ||
>>  	    strstr(linkname, DSO__NAME_KALLSYMS) ||
>>  	    access(filename, R_OK)) {
>>  fallback:
>> @@ -1335,6 +1342,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>>  		__symbol__join_symfs(filename, filename_size, dso->long_name);
>>  	}
>>
>> +	free(build_id_path);
>>  	return 0;
>>  }
>>
>> --
>> 2.7.4

Re: [PATCH v3 3/3] perf annotate: Fix missing number of samples

[Taeung Song]

Good morning, Arnaldo :)

On 03/28/2017 03:26 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Mar 27, 2017 at 04:10:38PM +0900, Taeung Song escreveu:
>> If running 'perf annotate --stdio -l --show-total-period',
>> you can see a problem showing only zero '0' for number of samples.
>>
>> Before:
>>     $ perf annotate --stdio -l --show-total-period
>> ...
>>        0 :        400816:       push   %rbp
>>        0 :        400817:       mov    %rsp,%rbp
>>        0 :        40081a:       mov    %edi,-0x24(%rbp)
>>        0 :        40081d:       mov    %rsi,-0x30(%rbp)
>>        0 :        400821:       mov    -0x24(%rbp),%eax
>>        0 :        400824:       mov    -0x30(%rbp),%rdx
>>        0 :        400828:       mov    (%rdx),%esi
>>        0 :        40082a:       mov    $0x0,%edx
>> ...
>>
>> The reason is number of samples aren't set
>> in symbol__get_source_line(). so set it ordinarily.
>
> Can you please take a look at:
>
>   0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
>
> that introduced the --show-total-period code and take it into account in
> this fix?
>
> I.e. from a quick look it did the calculation setting that field in the
> TUI code, where it should have done in the util/annotate.c file, so that
> all UIs would be able to use it.
>
> After your analysis, please add a Fixes: that cset, ok?
>
> I applied the other two patches and added Martin to the CC list, as he
> is the author of that patch and may have something to say here.
>
> - Arnaldo
>

Okey! I look into the cset 0c4a5bcea460.

It is fine but if running 'show-total-period' with '-l',
the problem happen. The reason is to miss setting number of samples
for source_line_samples, so will send v4 added Fixes: and Cc: Martin
(and a bit changed commit log message)

Thanks,
Taeung

Re: [PATCH v3 3/3] perf annotate: Fix missing number of samples

[Taeung Song]

On 03/28/2017 09:09 PM, Taeung Song wrote:
> Good morning, Arnaldo :)
>
> On 03/28/2017 03:26 AM, Arnaldo Carvalho de Melo wrote:
>> Em Mon, Mar 27, 2017 at 04:10:38PM +0900, Taeung Song escreveu:
>>> If running 'perf annotate --stdio -l --show-total-period',
>>> you can see a problem showing only zero '0' for number of samples.
>>>
>>> Before:
>>>     $ perf annotate --stdio -l --show-total-period
>>> ...
>>>        0 :        400816:       push   %rbp
>>>        0 :        400817:       mov    %rsp,%rbp
>>>        0 :        40081a:       mov    %edi,-0x24(%rbp)
>>>        0 :        40081d:       mov    %rsi,-0x30(%rbp)
>>>        0 :        400821:       mov    -0x24(%rbp),%eax
>>>        0 :        400824:       mov    -0x30(%rbp),%rdx
>>>        0 :        400828:       mov    (%rdx),%esi
>>>        0 :        40082a:       mov    $0x0,%edx
>>> ...
>>>
>>> The reason is number of samples aren't set
>>> in symbol__get_source_line(). so set it ordinarily.
>>
>> Can you please take a look at:
>>
>>   0c4a5bcea460 ("perf annotate: Display total number of samples with
>> --show-total-period")
>>
>> that introduced the --show-total-period code and take it into account in
>> this fix?
>>
>> I.e. from a quick look it did the calculation setting that field in the
>> TUI code, where it should have done in the util/annotate.c file, so that
>> all UIs would be able to use it.
>>

Sorry, I misunderstood what you said.

Already there is the calculation setting in util/annotate.c
Because of the cset e64aa75bf5 ("perf annotate browser: Use 
disasm__calc_percent()") by Namhyung.

So I think that we don't need to do as you guess.


Thanks,
Taeung

>> After your analysis, please add a Fixes: that cset, ok?
>>
>> I applied the other two patches and added Martin to the CC list, as he
>> is the author of that patch and may have something to say here.
>>
>> - Arnaldo
>>
>
> Okey! I look into the cset 0c4a5bcea460.
>
> It is fine but if running 'show-total-period' with '-l',
> the problem happen. The reason is to miss setting number of samples
> for source_line_samples, so will send v4 added Fixes: and Cc: Martin
> (and a bit changed commit log message)
>
> Thanks,
> Taeung