lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 20 Jul 2017 06:36:55 +0900
From:   Taeung Song <treeze.taeung@...il.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     linux-kernel@...r.kernel.org, Namhyung Kim <namhyung@...nel.org>,
        Milian Wolff <milian.wolff@...b.com>,
        Jiri Olsa <jolsa@...hat.com>
Subject: [PATCH v3 3/9] perf annotate: Fix wrong --show-total-period option showing number of samples

Currently the --show-total-period option of perf-annotate
is different from perf-report's.

For example, perf-report ordinarily shows period and number of samples.

 # Overhead       Samples        Period  Command  Shared Object   Symbol
 # ........  ............  ............  .......  ..............  ............
 #
      9.83%           102      84813704  test     test            [.] knapsack

But --show-total-period of perf-annotate has the problem that show
number of samples, not period.
So fix this option to show period instead of number of samples.

Before:

  Percent |      Source code & Disassembly of old for cycles:ppp (102 samples)
 -----------------------------------------------------------------------------
          :
          :
          :
          :      Disassembly of section .text:
          :
          :      0000000000400816 <knapsack>:
          :      knapsack():
        1 :        400816:       push   %rbp

After:

  Percent |  Source code & Disassembly of old for cycles:ppp (84813704 event count)
 ----------------------------------------------------------------------------------
          :
          :
          :
          :  Disassembly of section .text:
          :
          :  0000000000400816 <knapsack>:
          :  knapsack():
   743737 :    400816:       push   %rbp

Reported-by: Namhyung Kim <namhyung@...nel.org>
Cc: Milian Wolff <milian.wolff@...b.com>
Cc: Jiri Olsa <jolsa@...hat.com>
Signed-off-by: Taeung Song <treeze.taeung@...il.com>
---
 tools/perf/builtin-annotate.c |  4 +---
 tools/perf/builtin-report.c   | 13 ++++++----
 tools/perf/builtin-top.c      |  6 +++--
 tools/perf/util/annotate.c    | 55 ++++++++++++++++++++++++++++++++++++-------
 tools/perf/util/annotate.h    |  4 +++-
 5 files changed, 63 insertions(+), 19 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 5205408..0381408 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -177,14 +177,12 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
 	 */
 	process_branch_stack(sample->branch_stack, al, sample);
 
-	sample->period = 1;
 	sample->weight = 1;
-
 	he = hists__add_entry(hists, al, NULL, NULL, NULL, sample, true);
 	if (he == NULL)
 		return -ENOMEM;
 
-	ret = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+	ret = hist_entry__inc_addr_samples(he, sample, evsel->idx, al->addr);
 	hists__inc_nr_samples(hists, true);
 	return ret;
 }
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index cea25d0..a9bd011 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -115,13 +115,14 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter,
 	struct report *rep = arg;
 	struct hist_entry *he = iter->he;
 	struct perf_evsel *evsel = iter->evsel;
+	struct perf_sample *sample = iter->sample;
 	struct mem_info *mi;
 	struct branch_info *bi;
 
 	if (!ui__has_annotation())
 		return 0;
 
-	hist__account_cycles(iter->sample->branch_stack, al, iter->sample,
+	hist__account_cycles(sample->branch_stack, al, sample,
 			     rep->nonany_branch_mode);
 
 	if (sort__mode == SORT_MODE__BRANCH) {
@@ -138,14 +139,16 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter,
 		if (err)
 			goto out;
 
-		err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+		err = hist_entry__inc_addr_samples(he, sample,
+						   evsel->idx, al->addr);
 
 	} else if (symbol_conf.cumulate_callchain) {
 		if (single)
-			err = hist_entry__inc_addr_samples(he, evsel->idx,
-							   al->addr);
+			err = hist_entry__inc_addr_samples(he, sample,
+							   evsel->idx, al->addr);
 	} else {
-		err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+		err = hist_entry__inc_addr_samples(he, sample,
+						   evsel->idx, al->addr);
 	}
 
 out:
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 022486d..09885a4 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -183,6 +183,7 @@ static void ui__warn_map_erange(struct map *map, struct symbol *sym, u64 ip)
 
 static void perf_top__record_precise_ip(struct perf_top *top,
 					struct hist_entry *he,
+					struct perf_sample *sample,
 					int counter, u64 ip)
 {
 	struct annotation *notes;
@@ -199,7 +200,7 @@ static void perf_top__record_precise_ip(struct perf_top *top,
 	if (pthread_mutex_trylock(&notes->lock))
 		return;
 
-	err = hist_entry__inc_addr_samples(he, counter, ip);
+	err = hist_entry__inc_addr_samples(he, sample, counter, ip);
 
 	pthread_mutex_unlock(&notes->lock);
 
@@ -671,7 +672,8 @@ static int hist_iter__top_callback(struct hist_entry_iter *iter,
 	struct perf_evsel *evsel = iter->evsel;
 
 	if (perf_hpp_list.sym && single)
-		perf_top__record_precise_ip(top, he, evsel->idx, al->addr);
+		perf_top__record_precise_ip(top, he, iter->sample,
+					    evsel->idx, al->addr);
 
 	hist__account_cycles(iter->sample->branch_stack, al, iter->sample,
 		     !(top->record_opts.branch_stack & PERF_SAMPLE_BRANCH_ANY));
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index a62067a..d4f1a0a 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -816,9 +816,33 @@ int addr_map_symbol__inc_samples(struct addr_map_symbol *ams, int evidx)
 	return symbol__inc_addr_samples(ams->sym, ams->map, evidx, ams->al_addr);
 }
 
-int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 ip)
+int hist_entry__inc_addr_samples(struct hist_entry *he, struct perf_sample *sample,
+				 int evidx, u64 addr)
 {
-	return symbol__inc_addr_samples(he->ms.sym, he->ms.map, evidx, ip);
+	struct symbol *sym = he->ms.sym;
+	struct annotation *notes;
+	struct sym_hist *h;
+	s64 offset;
+
+	if (sym == NULL)
+		return 0;
+
+	notes = symbol__get_annotation(sym, false);
+	if (notes == NULL)
+		return -ENOMEM;
+
+	if ((addr < sym->start || addr >= sym->end) &&
+	    (addr != sym->end || sym->start != sym->end))
+		return -ERANGE;
+
+	offset = addr - sym->start;
+	h = annotation__histogram(notes, evidx);
+	h->total_samples++;
+	h->addr[offset].nr_samples++;
+	h->total_period += sample->period;
+	h->addr[offset].period += sample->period;
+
+	return 0;
 }
 
 static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch, struct map *map)
@@ -934,6 +958,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
 	double percent = 0.0;
 
 	sample->nr_samples = 0;
+	sample->period = 0;
 
 	if (src_line) {
 		size_t sizeof_src_line = sizeof(*src_line) +
@@ -953,12 +978,16 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
 	} else {
 		struct sym_hist *h = annotation__histogram(notes, evidx);
 		unsigned int hits = 0;
+		unsigned int p = 0;
 
-		while (offset < end)
-			hits += h->addr[offset++].nr_samples;
+		while (offset < end) {
+			hits += h->addr[offset].nr_samples;
+			p += h->addr[offset++].period;
+		}
 
 		if (h->total_samples) {
 			sample->nr_samples = hits;
+			sample->period = p;
 			percent = 100.0 * hits / h->total_samples;
 		}
 	}
@@ -1131,8 +1160,8 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 			color = get_percent_color(percent);
 
 			if (symbol_conf.show_total_period)
-				color_fprintf(stdout, color, " %7" PRIu64,
-					      sample.nr_samples);
+				color_fprintf(stdout, color, " %11" PRIu64,
+					      sample.period);
 			else
 				color_fprintf(stdout, color, " %7.2f", percent);
 		}
@@ -1162,6 +1191,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 		if (perf_evsel__is_group_event(evsel))
 			width *= evsel->nr_members;
 
+		if (symbol_conf.show_total_period)
+			width += perf_evsel__is_group_event(evsel) ?
+				4 * evsel->nr_members : 4;
+
 		if (!*dl->line)
 			printf(" %*s:\n", width, " ");
 		else
@@ -1812,8 +1845,14 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map,
 	if (perf_evsel__is_group_event(evsel))
 		width *= evsel->nr_members;
 
-	graph_dotted_len = printf(" %-*.*s|	Source code & Disassembly of %s for %s (%" PRIu64 " samples)\n",
-	       width, width, "Percent", d_filename, evsel_name, h->total_samples);
+	if (symbol_conf.show_total_period)
+		width += perf_evsel__is_group_event(evsel) ?
+			4 * evsel->nr_members : 4;
+
+	graph_dotted_len = printf(" %-*.*s|	Source code & Disassembly of %s for %s (%" PRIu64 " %s)\n",
+				  width, width, "Percent", d_filename, evsel_name,
+				  symbol_conf.show_total_period ? h->total_period : h->total_samples,
+				  symbol_conf.show_total_period ? "event count" : "samples");
 
 	printf("%-*.*s----\n",
 	       graph_dotted_len, graph_dotted_len, graph_dotted_line);
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index bef1d02..4406352 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -88,6 +88,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
 
 struct sym_hist {
 	u64		total_samples;
+	u64		total_period;
 	struct sym_hist_entry addr[0];
 };
 
@@ -160,7 +161,8 @@ int addr_map_symbol__account_cycles(struct addr_map_symbol *ams,
 				    struct addr_map_symbol *start,
 				    unsigned cycles);
 
-int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 addr);
+int hist_entry__inc_addr_samples(struct hist_entry *he, struct perf_sample *sample,
+				 int evidx, u64 addr);
 
 int symbol__alloc_hist(struct symbol *sym);
 void symbol__annotate_zero_histograms(struct symbol *sym);
-- 
2.7.4

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ