From 69fb09f6ccdb2f070557fd1f4c56c4d646694c8e Mon Sep 17 00:00:00 2001 From: Jin Yao Date: Fri, 7 Jul 2017 13:06:34 +0800 Subject: perf annotate: Check for fused instructions Macro fusion merges two instructions to a single micro-op. Intel core platform performs this hardware optimization under limited circumstances. For example, CMP + JCC can be "fused" and executed /retired together. While with sampling this can result in the sample sometimes being on the JCC and sometimes on the CMP. So for the fused instruction pair, they could be considered together. On Nehalem, fused instruction pairs: cmp/test + jcc. On other new CPU: cmp/test/add/sub/and/inc/dec + jcc. This patch adds an x86-specific function which checks if 2 instructions are in a "fused" pair. For non-x86 arch, the function is just NULL. Changelog: v4: Move the CPU model checking to symbol__disassemble and save the CPU family/model in arch structure. It avoids checking every time when jump arrow printed. v3: Add checking for Nehalem (CMP, TEST). For other newer Intel CPUs just check it by default (CMP, TEST, ADD, SUB, AND, INC, DEC). v2: Remove the original weak function. Arnaldo points out that doing it as a weak function that will be overridden by the host arch doesn't work. So now it's implemented as an arch-specific function. Committer fix: Do not access evsel->evlist->env->cpuid, ->env can be null, introduce perf_evsel__env_cpuid(), just like perf_evsel__env_arch(), also used in this function call. The original patch was segfaulting 'perf top' + annotation. But this essentially disables this fused instructions augmentation in 'perf top', the right thing is to get the cpuid from the running kernel, left for a later patch tho. Signed-off-by: Yao Jin Tested-by: Arnaldo Carvalho de Melo Cc: Alexander Shishkin Cc: Andi Kleen Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1499403995-19857-2-git-send-email-yao.jin@linux.intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index be1caabb9290..8748ebb3f932 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -47,7 +47,12 @@ struct arch { bool sorted_instructions; bool initialized; void *priv; + unsigned int model; + unsigned int family; int (*init)(struct arch *arch); + bool (*ins_is_fused)(struct arch *arch, const char *ins1, + const char *ins2); + int (*cpuid_parse)(struct arch *arch, char *cpuid); struct { char comment_char; char skip_functions_char; @@ -129,6 +134,8 @@ static struct arch architectures[] = { .name = "x86", .instructions = x86__instructions, .nr_instructions = ARRAY_SIZE(x86__instructions), + .ins_is_fused = x86__ins_is_fused, + .cpuid_parse = x86__cpuid_parse, .objdump = { .comment_char = '#', }, @@ -171,6 +178,14 @@ int ins__scnprintf(struct ins *ins, char *bf, size_t size, return ins__raw_scnprintf(ins, bf, size, ops); } +bool ins__is_fused(struct arch *arch, const char *ins1, const char *ins2) +{ + if (!arch || !arch->ins_is_fused) + return false; + + return arch->ins_is_fused(arch, ins1, ins2); +} + static int call__parse(struct arch *arch, struct ins_operands *ops, struct map *map) { char *endptr, *tok, *name; @@ -1381,7 +1396,7 @@ static const char *annotate__norm_arch(const char *arch_name) int symbol__disassemble(struct symbol *sym, struct map *map, const char *arch_name, size_t privsize, - struct arch **parch) + struct arch **parch, char *cpuid) { struct dso *dso = map->dso; char command[PATH_MAX * 2]; @@ -1418,6 +1433,9 @@ int symbol__disassemble(struct symbol *sym, struct map *map, } } + if (arch->cpuid_parse && cpuid) + arch->cpuid_parse(arch, cpuid); + pr_debug("%s: filename=%s, sym=%s, start=%#" PRIx64 ", end=%#" PRIx64 "\n", __func__, symfs_filename, sym->name, map->unmap_ip(map, sym->start), map->unmap_ip(map, sym->end)); @@ -1907,7 +1925,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map, u64 len; if (symbol__disassemble(sym, map, perf_evsel__env_arch(evsel), - 0, NULL) < 0) + 0, NULL, NULL) < 0) return -1; len = symbol__size(sym); -- cgit From 7e63a13a266da652f82731b845b5c35dd866ec7e Mon Sep 17 00:00:00 2001 From: Jin Yao Date: Fri, 7 Jul 2017 13:06:35 +0800 Subject: perf annotate: Implement visual marker for macro fusion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For marking fused instructions clearly this patch adds a line before the first instruction of pair and joins it with the arrow of the jump to its target. For example, when "je" is selected in annotate view, the line before cmpl is displayed and joins the arrow of "je". │ ┌──cmpl $0x0,argp_program_version_hook 81.93 │ ├──je 20 │ │ lock cmpxchg %esi,0x38a9a4(%rip) │ │↓ jne 29 │ │↓ jmp 43 11.47 │20:└─→cmpxch %esi,0x38a999(%rip) That means the cmpl+je is a fused instruction pair and they should be considered together. Changelog: v3: Use Arnaldo's fix to improve the arrow origin rendering. To get the evsel->evlist->env->cpuid, save the evsel in annotate_browser. v2: new function "ins__is_fused" to check if the instructions are fused. Signed-off-by: Yao Jin Tested-by: Arnaldo Carvalho de Melo Cc: Alexander Shishkin Cc: Andi Kleen Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1499403995-19857-3-git-send-email-yao.jin@linux.intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/ui/browser.c | 29 +++++++++++++++++++++++++++++ tools/perf/ui/browser.h | 2 ++ tools/perf/ui/browsers/annotate.c | 26 ++++++++++++++++++++++++++ tools/perf/util/annotate.c | 5 +++++ tools/perf/util/annotate.h | 1 + 5 files changed, 63 insertions(+) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c index 83874b0e266c..f73f3f13e01d 100644 --- a/tools/perf/ui/browser.c +++ b/tools/perf/ui/browser.c @@ -738,6 +738,35 @@ void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column, __ui_browser__line_arrow_down(browser, column, start, end); } +void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column, + unsigned int row, bool arrow_down) +{ + unsigned int end_row; + + if (row >= browser->top_idx) + end_row = row - browser->top_idx; + else + return; + + SLsmg_set_char_set(1); + + if (arrow_down) { + ui_browser__gotorc(browser, end_row, column - 1); + SLsmg_write_char(SLSMG_ULCORN_CHAR); + ui_browser__gotorc(browser, end_row, column); + SLsmg_draw_hline(2); + ui_browser__gotorc(browser, end_row + 1, column - 1); + SLsmg_write_char(SLSMG_LTEE_CHAR); + } else { + ui_browser__gotorc(browser, end_row, column - 1); + SLsmg_write_char(SLSMG_LTEE_CHAR); + ui_browser__gotorc(browser, end_row, column); + SLsmg_draw_hline(2); + } + + SLsmg_set_char_set(0); +} + void ui_browser__init(void) { int i = 0; diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h index be3b70eb5fca..a12eff75638b 100644 --- a/tools/perf/ui/browser.h +++ b/tools/perf/ui/browser.h @@ -43,6 +43,8 @@ void ui_browser__printf(struct ui_browser *browser, const char *fmt, ...); void ui_browser__write_graph(struct ui_browser *browser, int graph); void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column, u64 start, u64 end); +void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column, + unsigned int row, bool arrow_down); void __ui_browser__show_title(struct ui_browser *browser, const char *title); void ui_browser__show_title(struct ui_browser *browser, const char *title); int ui_browser__show(struct ui_browser *browser, const char *title, diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index c4336138b673..8d3f6f53c122 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -273,6 +273,25 @@ static bool disasm_line__is_valid_jump(struct disasm_line *dl, struct symbol *sy return true; } +static bool is_fused(struct annotate_browser *ab, struct disasm_line *cursor) +{ + struct disasm_line *pos = list_prev_entry(cursor, node); + const char *name; + + if (!pos) + return false; + + if (ins__is_lock(&pos->ins)) + name = pos->ops.locked.ins.name; + else + name = pos->ins.name; + + if (!name || !cursor->ins.name) + return false; + + return ins__is_fused(ab->arch, name, cursor->ins.name); +} + static void annotate_browser__draw_current_jump(struct ui_browser *browser) { struct annotate_browser *ab = container_of(browser, struct annotate_browser, b); @@ -308,6 +327,13 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser) ui_browser__set_color(browser, HE_COLORSET_JUMP_ARROWS); __ui_browser__line_arrow(browser, pcnt_width + 2 + ab->addr_width, from, to); + + if (is_fused(ab, cursor)) { + ui_browser__mark_fused(browser, + pcnt_width + 3 + ab->addr_width, + from - 1, + to > from ? true : false); + } } static unsigned int annotate_browser__refresh(struct ui_browser *browser) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 8748ebb3f932..ef434b53d849 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -517,6 +517,11 @@ bool ins__is_ret(const struct ins *ins) return ins->ops == &ret_ops; } +bool ins__is_lock(const struct ins *ins) +{ + return ins->ops == &lock_ops; +} + static int ins__key_cmp(const void *name, const void *insp) { const struct ins *ins = insp; diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index 72d72728a0fc..bac698d7cc6a 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -52,6 +52,7 @@ struct ins_ops { bool ins__is_jump(const struct ins *ins); bool ins__is_call(const struct ins *ins); bool ins__is_ret(const struct ins *ins); +bool ins__is_lock(const struct ins *ins); int ins__scnprintf(struct ins *ins, char *bf, size_t size, struct ins_operands *ops); bool ins__is_fused(struct arch *arch, const char *ins1, const char *ins2); -- cgit From d2396999c998b4e0006aef247e154eff0ed3d8f9 Mon Sep 17 00:00:00 2001 From: Krister Johansen Date: Wed, 5 Jul 2017 18:48:13 -0700 Subject: perf buildid-cache: Cache debuginfo If a stripped binary is placed in the cache, the user is in a situation where there's a cached elf file present, but it doesn't have any symtab to use for name resolution. Grab the debuginfo for binaries that don't end in .ko. This yields a better chance of resolving symbols from older traces. Signed-off-by: Krister Johansen Cc: Alexander Shishkin Cc: Brendan Gregg Cc: Peter Zijlstra Cc: Thomas-Mich Richter Link: http://lkml.kernel.org/r/1499305693-1599-7-git-send-email-kjlx@templeofstupid.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-buildid-cache.c | 2 +- tools/perf/util/annotate.c | 2 +- tools/perf/util/build-id.c | 72 +++++++++++++++++++++++++++++++++++--- tools/perf/util/build-id.h | 3 +- tools/perf/util/dso.c | 8 ++++- tools/perf/util/dso.h | 1 + tools/perf/util/machine.c | 3 +- tools/perf/util/symbol.c | 12 +++++-- 8 files changed, 90 insertions(+), 13 deletions(-) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c index d65bd86bee99..e3eb6240ced0 100644 --- a/tools/perf/builtin-buildid-cache.c +++ b/tools/perf/builtin-buildid-cache.c @@ -243,7 +243,7 @@ static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused) char filename[PATH_MAX]; u8 build_id[BUILD_ID_SIZE]; - if (dso__build_id_filename(dso, filename, sizeof(filename)) && + if (dso__build_id_filename(dso, filename, sizeof(filename), false) && filename__read_build_id(filename, build_id, sizeof(build_id)) != sizeof(build_id)) { if (errno == ENOENT) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index ef434b53d849..1742510f0120 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -1347,7 +1347,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil !dso__is_kcore(dso)) return SYMBOL_ANNOTATE_ERRNO__NO_VMLINUX; - build_id_filename = dso__build_id_filename(dso, NULL, 0); + build_id_filename = dso__build_id_filename(dso, NULL, 0, false); if (build_id_filename) { __symbol__join_symfs(filename, filename_size, build_id_filename); free(build_id_filename); diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c index f7bfd90a7388..e9665150e9b1 100644 --- a/tools/perf/util/build-id.c +++ b/tools/perf/util/build-id.c @@ -243,12 +243,15 @@ static bool build_id_cache__valid_id(char *sbuild_id) return result; } -static const char *build_id_cache__basename(bool is_kallsyms, bool is_vdso) +static const char *build_id_cache__basename(bool is_kallsyms, bool is_vdso, + bool is_debug) { - return is_kallsyms ? "kallsyms" : (is_vdso ? "vdso" : "elf"); + return is_kallsyms ? "kallsyms" : (is_vdso ? "vdso" : (is_debug ? + "debug" : "elf")); } -char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size) +char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size, + bool is_debug) { bool is_kallsyms = dso__is_kallsyms((struct dso *)dso); bool is_vdso = dso__is_vdso((struct dso *)dso); @@ -270,7 +273,8 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size) ret = asnprintf(&bf, size, "%s", linkname); else ret = asnprintf(&bf, size, "%s/%s", linkname, - build_id_cache__basename(is_kallsyms, is_vdso)); + build_id_cache__basename(is_kallsyms, is_vdso, + is_debug)); if (ret < 0 || (!alloc && size < (unsigned int)ret)) bf = NULL; free(linkname); @@ -603,12 +607,40 @@ static int build_id_cache__add_sdt_cache(const char *sbuild_id, #define build_id_cache__add_sdt_cache(sbuild_id, realname, nsi) (0) #endif +static char *build_id_cache__find_debug(const char *sbuild_id, + struct nsinfo *nsi) +{ + char *realname = NULL; + char *debugfile; + struct nscookie nsc; + size_t len = 0; + + debugfile = calloc(1, PATH_MAX); + if (!debugfile) + goto out; + + len = __symbol__join_symfs(debugfile, PATH_MAX, + "/usr/lib/debug/.build-id/"); + snprintf(debugfile + len, PATH_MAX - len, "%.2s/%s.debug", sbuild_id, + sbuild_id + 2); + + nsinfo__mountns_enter(nsi, &nsc); + realname = realpath(debugfile, NULL); + if (realname && access(realname, R_OK)) + zfree(&realname); + nsinfo__mountns_exit(&nsc); +out: + free(debugfile); + return realname; +} + int build_id_cache__add_s(const char *sbuild_id, const char *name, struct nsinfo *nsi, bool is_kallsyms, bool is_vdso) { const size_t size = PATH_MAX; char *realname = NULL, *filename = NULL, *dir_name = NULL, *linkname = zalloc(size), *tmp; + char *debugfile = NULL; int err = -1; if (!is_kallsyms) { @@ -635,7 +667,8 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name, /* Save the allocated buildid dirname */ if (asprintf(&filename, "%s/%s", dir_name, - build_id_cache__basename(is_kallsyms, is_vdso)) < 0) { + build_id_cache__basename(is_kallsyms, is_vdso, + false)) < 0) { filename = NULL; goto out_free; } @@ -652,6 +685,34 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name, goto out_free; } + /* Some binaries are stripped, but have .debug files with their symbol + * table. Check to see if we can locate one of those, since the elf + * file itself may not be very useful to users of our tools without a + * symtab. + */ + if (!is_kallsyms && !is_vdso && + strncmp(".ko", name + strlen(name) - 3, 3)) { + debugfile = build_id_cache__find_debug(sbuild_id, nsi); + if (debugfile) { + zfree(&filename); + if (asprintf(&filename, "%s/%s", dir_name, + build_id_cache__basename(false, false, true)) < 0) { + filename = NULL; + goto out_free; + } + if (access(filename, F_OK)) { + if (nsi && nsi->need_setns) { + if (copyfile_ns(debugfile, filename, + nsi)) + goto out_free; + } else if (link(debugfile, filename) && + errno != EEXIST && + copyfile(debugfile, filename)) + goto out_free; + } + } + } + if (!build_id_cache__linkname(sbuild_id, linkname, size)) goto out_free; tmp = strrchr(linkname, '/'); @@ -676,6 +737,7 @@ out_free: if (!is_kallsyms) free(realname); free(filename); + free(debugfile); free(dir_name); free(linkname); return err; diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h index 23970847d4c4..113dc0615c57 100644 --- a/tools/perf/util/build-id.h +++ b/tools/perf/util/build-id.h @@ -17,7 +17,8 @@ int filename__sprintf_build_id(const char *pathname, char *sbuild_id); char *build_id_cache__kallsyms_path(const char *sbuild_id, char *bf, size_t size); -char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size); +char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size, + bool is_debug); int build_id__mark_dso_hit(struct perf_tool *tool, union perf_event *event, struct perf_sample *sample, struct perf_evsel *evsel, diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index dc9b49533a8f..b9e087fb8247 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -32,6 +32,7 @@ char dso__symtab_origin(const struct dso *dso) [DSO_BINARY_TYPE__JAVA_JIT] = 'j', [DSO_BINARY_TYPE__DEBUGLINK] = 'l', [DSO_BINARY_TYPE__BUILD_ID_CACHE] = 'B', + [DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO] = 'D', [DSO_BINARY_TYPE__FEDORA_DEBUGINFO] = 'f', [DSO_BINARY_TYPE__UBUNTU_DEBUGINFO] = 'u', [DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO] = 'o', @@ -97,7 +98,12 @@ int dso__read_binary_type_filename(const struct dso *dso, break; } case DSO_BINARY_TYPE__BUILD_ID_CACHE: - if (dso__build_id_filename(dso, filename, size) == NULL) + if (dso__build_id_filename(dso, filename, size, false) == NULL) + ret = -1; + break; + + case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO: + if (dso__build_id_filename(dso, filename, size, true) == NULL) ret = -1; break; diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index 78ec637fc68b..f886141678eb 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -21,6 +21,7 @@ enum dso_binary_type { DSO_BINARY_TYPE__JAVA_JIT, DSO_BINARY_TYPE__DEBUGLINK, DSO_BINARY_TYPE__BUILD_ID_CACHE, + DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO, DSO_BINARY_TYPE__FEDORA_DEBUGINFO, DSO_BINARY_TYPE__UBUNTU_DEBUGINFO, DSO_BINARY_TYPE__BUILDID_DEBUGINFO, diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 246b441110a1..a54a2be5eda4 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -705,7 +705,8 @@ size_t machine__fprintf_vmlinux_path(struct machine *machine, FILE *fp) if (kdso->has_build_id) { char filename[PATH_MAX]; - if (dso__build_id_filename(kdso, filename, sizeof(filename))) + if (dso__build_id_filename(kdso, filename, sizeof(filename), + false)) printed += fprintf(fp, "[0] %s\n", filename); } diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 8c7bae545617..971b990557b4 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -53,6 +53,7 @@ static enum dso_binary_type binary_type_symtab[] = { DSO_BINARY_TYPE__JAVA_JIT, DSO_BINARY_TYPE__DEBUGLINK, DSO_BINARY_TYPE__BUILD_ID_CACHE, + DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO, DSO_BINARY_TYPE__FEDORA_DEBUGINFO, DSO_BINARY_TYPE__UBUNTU_DEBUGINFO, DSO_BINARY_TYPE__BUILDID_DEBUGINFO, @@ -1418,6 +1419,7 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod, return kmod && dso->symtab_type == type; case DSO_BINARY_TYPE__BUILD_ID_CACHE: + case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO: return true; case DSO_BINARY_TYPE__NOT_FOUND: @@ -1565,10 +1567,14 @@ int dso__load(struct dso *dso, struct map *map) struct symsrc *ss = &ss_[ss_pos]; bool next_slot = false; bool is_reg; + bool nsexit; int sirc; enum dso_binary_type symtab_type = binary_type_symtab[i]; + nsexit = (symtab_type == DSO_BINARY_TYPE__BUILD_ID_CACHE || + symtab_type == DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO); + if (!dso__is_compatible_symtab_type(dso, kmod, symtab_type)) continue; @@ -1576,13 +1582,13 @@ int dso__load(struct dso *dso, struct map *map) root_dir, name, PATH_MAX)) continue; - if (symtab_type == DSO_BINARY_TYPE__BUILD_ID_CACHE) + if (nsexit) nsinfo__mountns_exit(&nsc); is_reg = is_regular_file(name); sirc = symsrc__init(ss, dso, name, symtab_type); - if (symtab_type == DSO_BINARY_TYPE__BUILD_ID_CACHE) + if (nsexit) nsinfo__mountns_enter(dso->nsinfo, &nsc); if (!is_reg || sirc < 0) { @@ -1724,7 +1730,7 @@ int dso__load_vmlinux_path(struct dso *dso, struct map *map) } if (!symbol_conf.ignore_vmlinux_buildid) - filename = dso__build_id_filename(dso, NULL, 0); + filename = dso__build_id_filename(dso, NULL, 0, false); if (filename != NULL) { err = dso__load_vmlinux(dso, map, filename, true); if (err > 0) -- cgit From 896bccd3cb8d95cbc565687715516009c5169e71 Mon Sep 17 00:00:00 2001 From: Taeung Song Date: Thu, 20 Jul 2017 06:36:45 +0900 Subject: perf annotate: Introduce struct sym_hist_entry struct sym_hist has addr[] but it should have not only number of samples but also the sample period. So use new struct symhist_entry to pave the way to have that. Committer notes: This initial patch will only introduce the struct sym_hist_entry and use only the nr_samples member, which makes the code clearer and paves the way to save the period as well. Signed-off-by: Taeung Song Suggested-by: Arnaldo Carvalho de Melo Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/r/1500500205-16553-1-git-send-email-treeze.taeung@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/ui/browsers/annotate.c | 6 +++--- tools/perf/ui/gtk/annotate.c | 4 ++-- tools/perf/util/annotate.c | 45 ++++++++++++++++++++------------------- tools/perf/util/annotate.h | 9 ++++++-- 4 files changed, 35 insertions(+), 29 deletions(-) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 6794a8bec404..dbe4e630b90f 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -450,14 +450,14 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser, next = disasm__get_next_ip_line(¬es->src->source, pos); for (i = 0; i < browser->nr_events; i++) { - u64 nr_samples; + struct sym_hist_entry sample; bpos->samples[i].percent = disasm__calc_percent(notes, evsel->idx + i, pos->offset, next ? next->offset : len, - &path, &nr_samples); - bpos->samples[i].nr = nr_samples; + &path, &sample); + bpos->samples[i].nr = sample.nr_samples; if (max_percent < bpos->samples[i].percent) max_percent = bpos->samples[i].percent; diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c index 87e3760624f2..d736fd57ab9b 100644 --- a/tools/perf/ui/gtk/annotate.c +++ b/tools/perf/ui/gtk/annotate.c @@ -34,10 +34,10 @@ static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym, return 0; symhist = annotation__histogram(symbol__annotation(sym), evidx); - if (!symbol_conf.event_group && !symhist->addr[dl->offset]) + if (!symbol_conf.event_group && !symhist->addr[dl->offset].nr_samples) return 0; - percent = 100.0 * symhist->addr[dl->offset] / symhist->sum; + percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->sum; markup = perf_gtk__get_percent_color(percent); if (markup) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 1742510f0120..c3829555ce1c 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -610,10 +610,10 @@ int symbol__alloc_hist(struct symbol *sym) size_t sizeof_sym_hist; /* Check for overflow when calculating sizeof_sym_hist */ - if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(u64)) + if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(struct sym_hist_entry)) return -1; - sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(u64)); + sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(struct sym_hist_entry)); /* Check for overflow in zalloc argument */ if (sizeof_sym_hist > (SIZE_MAX - sizeof(*notes->src)) @@ -714,11 +714,11 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map, offset = addr - sym->start; h = annotation__histogram(notes, evidx); h->sum++; - h->addr[offset]++; + h->addr[offset].nr_samples++; pr_debug3("%#" PRIx64 " %s: period++ [addr: %#" PRIx64 ", %#" PRIx64 ", evidx=%d] => %" PRIu64 "\n", sym->start, sym->name, - addr, addr - sym->start, evidx, h->addr[offset]); + addr, addr - sym->start, evidx, h->addr[offset].nr_samples); return 0; } @@ -928,11 +928,12 @@ struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa } double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, - s64 end, const char **path, u64 *nr_samples) + s64 end, const char **path, struct sym_hist_entry *sample) { struct source_line *src_line = notes->src->lines; double percent = 0.0; - *nr_samples = 0; + + sample->nr_samples = 0; if (src_line) { size_t sizeof_src_line = sizeof(*src_line) + @@ -946,7 +947,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, *path = src_line->path; percent += src_line->samples[evidx].percent; - *nr_samples += src_line->samples[evidx].nr; + sample->nr_samples += src_line->samples[evidx].nr; offset++; } } else { @@ -954,10 +955,10 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, unsigned int hits = 0; while (offset < end) - hits += h->addr[offset++]; + hits += h->addr[offset++].nr_samples; if (h->sum) { - *nr_samples = hits; + sample->nr_samples = hits; percent = 100.0 * hits / h->sum; } } @@ -1057,10 +1058,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st if (dl->offset != -1) { const char *path = NULL; - u64 nr_samples; double percent, max_percent = 0.0; double *ppercents = &percent; - u64 *psamples = &nr_samples; + struct sym_hist_entry sample; + struct sym_hist_entry *psamples = &sample; int i, nr_percent = 1; const char *color; struct annotation *notes = symbol__annotation(sym); @@ -1074,7 +1075,7 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st if (perf_evsel__is_group_event(evsel)) { nr_percent = evsel->nr_members; ppercents = calloc(nr_percent, sizeof(double)); - psamples = calloc(nr_percent, sizeof(u64)); + psamples = calloc(nr_percent, sizeof(struct sym_hist_entry)); if (ppercents == NULL || psamples == NULL) { return -1; } @@ -1085,10 +1086,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st notes->src->lines ? i : evsel->idx + i, offset, next ? next->offset : (s64) len, - &path, &nr_samples); + &path, &sample); ppercents[i] = percent; - psamples[i] = nr_samples; + psamples[i] = sample; if (percent > max_percent) max_percent = percent; } @@ -1126,12 +1127,12 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st for (i = 0; i < nr_percent; i++) { percent = ppercents[i]; - nr_samples = psamples[i]; + sample = psamples[i]; color = get_percent_color(percent); if (symbol_conf.show_total_period) color_fprintf(stdout, color, " %7" PRIu64, - nr_samples); + sample.nr_samples); else color_fprintf(stdout, color, " %7.2f", percent); } @@ -1147,7 +1148,7 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st if (ppercents != &percent) free(ppercents); - if (psamples != &nr_samples) + if (psamples != &sample) free(psamples); } else if (max_lines && printed >= max_lines) @@ -1702,7 +1703,7 @@ 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]; + nr_samples = h->addr[i].nr_samples; if (h->sum) percent = 100.0 * nr_samples / h->sum; @@ -1773,9 +1774,9 @@ static void symbol__annotate_hits(struct symbol *sym, struct perf_evsel *evsel) u64 len = symbol__size(sym), offset; for (offset = 0; offset < len; ++offset) - if (h->addr[offset] != 0) + if (h->addr[offset].nr_samples != 0) printf("%*" PRIx64 ": %" PRIu64 "\n", BITS_PER_LONG / 2, - sym->start + offset, h->addr[offset]); + sym->start + offset, h->addr[offset].nr_samples); printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->sum", h->sum); } @@ -1878,8 +1879,8 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx) h->sum = 0; for (offset = 0; offset < len; ++offset) { - h->addr[offset] = h->addr[offset] * 7 / 8; - h->sum += h->addr[offset]; + h->addr[offset].nr_samples = h->addr[offset].nr_samples * 7 / 8; + h->sum += h->addr[offset].nr_samples; } } diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index bac698d7cc6a..3a176633b324 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -74,16 +74,21 @@ static inline bool disasm_line__has_offset(const struct disasm_line *dl) return dl->ops.target.offset_avail; } +struct sym_hist_entry { + u64 nr_samples; + u64 period; +}; + void disasm_line__free(struct disasm_line *dl); struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disasm_line *pos); int disasm_line__scnprintf(struct disasm_line *dl, char *bf, size_t size, bool raw); size_t disasm__fprintf(struct list_head *head, FILE *fp); double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, - s64 end, const char **path, u64 *nr_samples); + s64 end, const char **path, struct sym_hist_entry *sample); struct sym_hist { u64 sum; - u64 addr[0]; + struct sym_hist_entry addr[0]; }; struct cyc_hist { -- cgit From 8158683da3d30e0346275702a8e08f2b22726c45 Mon Sep 17 00:00:00 2001 From: Taeung Song Date: Thu, 20 Jul 2017 06:36:51 +0900 Subject: perf annotate: Rename 'sum' to 'nr_samples' in struct sym_hist To make it more clear that it is the sum of all the nr_samples fields in the addr[] entries, i.e.: sym_hist->nr_samples = sum(sym_hist->addr[0 .. symbol__size(sym)]->nr_samples) Committer notes: Taeung had renamed it to total_samples, but using nr_samples, as in the added explanation above, looks clearer and establishes the direct connection, making clear it is about the _number_ of samples. Signed-off-by: Taeung Song Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/r/1500500211-16599-1-git-send-email-treeze.taeung@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/ui/gtk/annotate.c | 2 +- tools/perf/util/annotate.c | 26 +++++++++++++------------- tools/perf/util/annotate.h | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c index d736fd57ab9b..02176193f427 100644 --- a/tools/perf/ui/gtk/annotate.c +++ b/tools/perf/ui/gtk/annotate.c @@ -37,7 +37,7 @@ static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym, if (!symbol_conf.event_group && !symhist->addr[dl->offset].nr_samples) return 0; - percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->sum; + percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->nr_samples; markup = perf_gtk__get_percent_color(percent); if (markup) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index c3829555ce1c..58c6b63ff049 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -713,7 +713,7 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map, offset = addr - sym->start; h = annotation__histogram(notes, evidx); - h->sum++; + h->nr_samples++; h->addr[offset].nr_samples++; pr_debug3("%#" PRIx64 " %s: period++ [addr: %#" PRIx64 ", %#" PRIx64 @@ -957,9 +957,9 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, while (offset < end) hits += h->addr[offset++].nr_samples; - if (h->sum) { + if (h->nr_samples) { sample->nr_samples = hits; - percent = 100.0 * hits / h->sum; + percent = 100.0 * hits / h->nr_samples; } } @@ -1672,19 +1672,19 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map, struct sym_hist *h = annotation__histogram(notes, evidx); struct rb_root tmp_root = RB_ROOT; int nr_pcnt = 1; - u64 h_sum = h->sum; + u64 nr_samples = h->nr_samples; size_t sizeof_src_line = sizeof(struct source_line); if (perf_evsel__is_group_event(evsel)) { for (i = 1; i < evsel->nr_members; i++) { h = annotation__histogram(notes, evidx + i); - h_sum += h->sum; + nr_samples += h->nr_samples; } nr_pcnt = evsel->nr_members; sizeof_src_line += (nr_pcnt - 1) * sizeof(src_line->samples); } - if (!h_sum) + if (!nr_samples) return 0; src_line = notes->src->lines = calloc(len, sizeof_src_line); @@ -1694,7 +1694,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, nr_samples; + u64 offset; double percent_max = 0.0; src_line->nr_pcnt = nr_pcnt; @@ -1704,8 +1704,8 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map, h = annotation__histogram(notes, evidx + k); nr_samples = h->addr[i].nr_samples; - if (h->sum) - percent = 100.0 * nr_samples / h->sum; + if (h->nr_samples) + percent = 100.0 * nr_samples / h->nr_samples; if (percent > percent_max) percent_max = percent; @@ -1777,7 +1777,7 @@ static void symbol__annotate_hits(struct symbol *sym, struct perf_evsel *evsel) if (h->addr[offset].nr_samples != 0) printf("%*" PRIx64 ": %" PRIu64 "\n", BITS_PER_LONG / 2, sym->start + offset, h->addr[offset].nr_samples); - printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->sum", h->sum); + printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->nr_samples", h->nr_samples); } int symbol__annotate_printf(struct symbol *sym, struct map *map, @@ -1813,7 +1813,7 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, 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->sum); + width, width, "Percent", d_filename, evsel_name, h->nr_samples); printf("%-*.*s----\n", graph_dotted_len, graph_dotted_len, graph_dotted_line); @@ -1877,10 +1877,10 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx) struct sym_hist *h = annotation__histogram(notes, evidx); int len = symbol__size(sym), offset; - h->sum = 0; + h->nr_samples = 0; for (offset = 0; offset < len; ++offset) { h->addr[offset].nr_samples = h->addr[offset].nr_samples * 7 / 8; - h->sum += h->addr[offset].nr_samples; + h->nr_samples += h->addr[offset].nr_samples; } } diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index 3a176633b324..e8c246ec53df 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -87,7 +87,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, s64 end, const char **path, struct sym_hist_entry *sample); struct sym_hist { - u64 sum; + u64 nr_samples; struct sym_hist_entry addr[0]; }; -- cgit From bab89f6aed7e745893e009014354d0caaf62acf7 Mon Sep 17 00:00:00 2001 From: Taeung Song Date: Thu, 20 Jul 2017 16:28:53 -0300 Subject: perf hists: Pass perf_sample to __symbol__inc_addr_samples() To pave the way to use perf_sample fields in the annotate code, storing sample->period in sym_hist->addr->period and its sum in sym_hist->period. Signed-off-by: Taeung Song Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/r/1500500215-16646-1-git-send-email-treeze.taeung@gmail.com [ split and adjusted from a larger patch ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-annotate.c | 2 +- tools/perf/builtin-report.c | 15 ++++++++------- tools/perf/builtin-top.c | 5 +++-- tools/perf/util/annotate.c | 18 +++++++++++------- tools/perf/util/annotate.h | 6 ++++-- 5 files changed, 27 insertions(+), 19 deletions(-) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 5205408e795b..96fe1a88c1e5 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -184,7 +184,7 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel, 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 cea25d03f4dd..983b238d5eea 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -115,37 +115,38 @@ 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) { bi = he->branch_info; - err = addr_map_symbol__inc_samples(&bi->from, evsel->idx); + err = addr_map_symbol__inc_samples(&bi->from, sample, evsel->idx); if (err) goto out; - err = addr_map_symbol__inc_samples(&bi->to, evsel->idx); + err = addr_map_symbol__inc_samples(&bi->to, sample, evsel->idx); } else if (rep->mem_mode) { mi = he->mem_info; - err = addr_map_symbol__inc_samples(&mi->daddr, evsel->idx); + err = addr_map_symbol__inc_samples(&mi->daddr, sample, evsel->idx); 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, + 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 022486dc67f5..e5a8f249077f 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(¬es->lock)) return; - err = hist_entry__inc_addr_samples(he, counter, ip); + err = hist_entry__inc_addr_samples(he, sample, counter, ip); pthread_mutex_unlock(¬es->lock); @@ -671,7 +672,7 @@ 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 58c6b63ff049..c2fe16d5e473 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -697,7 +697,8 @@ static int __symbol__account_cycles(struct annotation *notes, } static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map, - struct annotation *notes, int evidx, u64 addr) + struct annotation *notes, int evidx, u64 addr, + struct perf_sample *sample __maybe_unused) { unsigned offset; struct sym_hist *h; @@ -738,7 +739,8 @@ static struct annotation *symbol__get_annotation(struct symbol *sym, bool cycles } static int symbol__inc_addr_samples(struct symbol *sym, struct map *map, - int evidx, u64 addr) + int evidx, u64 addr, + struct perf_sample *sample) { struct annotation *notes; @@ -747,7 +749,7 @@ static int symbol__inc_addr_samples(struct symbol *sym, struct map *map, notes = symbol__get_annotation(sym, false); if (notes == NULL) return -ENOMEM; - return __symbol__inc_addr_samples(sym, map, notes, evidx, addr); + return __symbol__inc_addr_samples(sym, map, notes, evidx, addr, sample); } static int symbol__account_cycles(u64 addr, u64 start, @@ -811,14 +813,16 @@ int addr_map_symbol__account_cycles(struct addr_map_symbol *ams, return err; } -int addr_map_symbol__inc_samples(struct addr_map_symbol *ams, int evidx) +int addr_map_symbol__inc_samples(struct addr_map_symbol *ams, struct perf_sample *sample, + int evidx) { - return symbol__inc_addr_samples(ams->sym, ams->map, evidx, ams->al_addr); + return symbol__inc_addr_samples(ams->sym, ams->map, evidx, ams->al_addr, sample); } -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 ip) { - return symbol__inc_addr_samples(he->ms.sym, he->ms.map, evidx, ip); + return symbol__inc_addr_samples(he->ms.sym, he->ms.map, evidx, ip, sample); } static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch, struct map *map) diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index e8c246ec53df..720f18195046 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -154,13 +154,15 @@ static inline struct annotation *symbol__annotation(struct symbol *sym) return (void *)sym - symbol_conf.priv_size; } -int addr_map_symbol__inc_samples(struct addr_map_symbol *ams, int evidx); +int addr_map_symbol__inc_samples(struct addr_map_symbol *ams, struct perf_sample *sample, + int evidx); 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); -- cgit From 461c17f00f400f95116880d91d20a7fcd84263a9 Mon Sep 17 00:00:00 2001 From: Taeung Song Date: Thu, 20 Jul 2017 17:18:05 -0300 Subject: perf annotate: Store the sample period in each histogram bucket We'll use it soon, when fixing --show-total-period. Signed-off-by: Taeung Song Cc: Adrian Hunter Cc: David Ahern Cc: Jiri Olsa Cc: Namhyung Kim Cc: Wang Nan Link: http://lkml.kernel.org/r/1500500215-16646-1-git-send-email-treeze.taeung@gmail.com [ split from a larger patch, do the math in __symbol__inc_addr_samples() ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate.c | 17 ++++++++++++----- tools/perf/util/annotate.h | 1 + 2 files changed, 13 insertions(+), 5 deletions(-) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index c2fe16d5e473..00e085fc945b 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -698,7 +698,7 @@ static int __symbol__account_cycles(struct annotation *notes, static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map, struct annotation *notes, int evidx, u64 addr, - struct perf_sample *sample __maybe_unused) + struct perf_sample *sample) { unsigned offset; struct sym_hist *h; @@ -716,10 +716,13 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map, h = annotation__histogram(notes, evidx); h->nr_samples++; h->addr[offset].nr_samples++; + h->period += sample->period; + h->addr[offset].period += sample->period; pr_debug3("%#" PRIx64 " %s: period++ [addr: %#" PRIx64 ", %#" PRIx64 - ", evidx=%d] => %" PRIu64 "\n", sym->start, sym->name, - addr, addr - sym->start, evidx, h->addr[offset].nr_samples); + ", evidx=%d] => nr_samples: %" PRIu64 ", period: %" PRIu64 "\n", + sym->start, sym->name, addr, addr - sym->start, evidx, + h->addr[offset].nr_samples, h->addr[offset].period); return 0; } @@ -937,7 +940,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, struct source_line *src_line = notes->src->lines; double percent = 0.0; - sample->nr_samples = 0; + sample->nr_samples = sample->period = 0; if (src_line) { size_t sizeof_src_line = sizeof(*src_line) + @@ -957,11 +960,15 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, } else { struct sym_hist *h = annotation__histogram(notes, evidx); unsigned int hits = 0; + u64 period = 0; - while (offset < end) + while (offset < end) { hits += h->addr[offset++].nr_samples; + period += h->addr[offset++].period; + } if (h->nr_samples) { + sample->period = period; sample->nr_samples = hits; percent = 100.0 * hits / h->nr_samples; } diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index 720f18195046..9ce575c25fd9 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 nr_samples; + u64 period; struct sym_hist_entry addr[0]; }; -- cgit From 585d93c5ffccced26689e34095c0d74ef20a07d6 Mon Sep 17 00:00:00 2001 From: Taeung Song Date: Fri, 21 Jul 2017 11:58:20 -0300 Subject: perf annotate stdio: Fix --show-total-period MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We were showing the total number of samples, not the total period as asked by the user, fix it. Reported-by: Namhyung Kim Cc: Jiri Olsa Cc: Martin Liška Cc: Milian Wolff Link: http://lkml.kernel.org/n/tip-lh2nh89rtqn5x5vbfthw6qml@git.kernel.org Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period") [ split from a larger patch ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 00e085fc945b..004072f82511 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -1143,7 +1143,7 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st if (symbol_conf.show_total_period) color_fprintf(stdout, color, " %7" PRIu64, - sample.nr_samples); + sample.period); else color_fprintf(stdout, color, " %7.2f", percent); } -- cgit From 38d2dcd0ccf85b55d783edbfc14fd8dea4d55b73 Mon Sep 17 00:00:00 2001 From: Taeung Song Date: Tue, 25 Jul 2017 06:28:42 +0900 Subject: perf annotate stdio: Fix column header when using --show-total-period Currently the first column header is always "Percent", fix it to show correct column name based on given options, i.e. if using --show-total-period, show "Event count" as a first column. Reported-by: Milian Wolff Signed-off-by: Taeung Song Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/r/c3c902e7-95bc-16d4-366f-12eb034c5c8d@gmail.com [ Extracted from a larger patch ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 004072f82511..c2b4b00166ed 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -1824,7 +1824,8 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, 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->nr_samples); + width, width, symbol_conf.show_total_period ? "Event count" : "Percent", + d_filename, evsel_name, h->nr_samples); printf("%-*.*s----\n", graph_dotted_len, graph_dotted_len, graph_dotted_line); -- cgit From ce9ee4a2de20062a97ad50ecc11ebda7e7618fd1 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 26 Jul 2017 17:16:46 -0300 Subject: perf annotate stdio: Set enough columns for --show-total-period Now that we set the first column header according to wether --show-total-period is being used, we need to size it accordingly. Based-on-a-patch-by: Taeung Song Cc: Jiri Olsa Cc: Milian Wolff Cc: Namhyung Kim Link: http://lkml.kernel.org/n/tip-pu504ffnit4m334k09hxcbs3@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index c2b4b00166ed..5125c2bbacaa 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -1142,7 +1142,7 @@ 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, + color_fprintf(stdout, color, " %11" PRIu64, sample.period); else color_fprintf(stdout, color, " %7.2f", percent); @@ -1165,7 +1165,7 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st } else if (max_lines && printed >= max_lines) return 1; else { - int width = 8; + int width = symbol_conf.show_total_period ? 12 : 8; if (queue) return -1; @@ -1806,7 +1806,7 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, int printed = 2, queue_len = 0; int more = 0; u64 len; - int width = 8; + int width = symbol_conf.show_total_period ? 12 : 8; int graph_dotted_len; filename = strdup(dso->long_name); -- cgit From 48cc33085253d607706e68a67ac98fe2a6abdd3d Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 28 Jul 2017 12:49:02 -0300 Subject: perf annotate: Fix storing per line sym_hist_entry The existing loop incremented the offset while using it as the array index, when we went to an array of sym_hist_entry instances, we should've moved the increment to outside of the array element reference, oops, fix it. Cc: Adrian Hunter Cc: David Ahern Cc: Jiri Olsa Cc: Namhyung Kim Cc: Taeung Song Cc: Wang Nan Fixes: 461c17f00f40 ("perf annotate: Store the sample period in each histogram bucket") Link: http://lkml.kernel.org/n/tip-s3dm6uyrazlpag3f0psfia07@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 5125c2bbacaa..2dab0e5a7f2f 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -963,8 +963,9 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, u64 period = 0; while (offset < end) { - hits += h->addr[offset++].nr_samples; - period += h->addr[offset++].period; + hits += h->addr[offset].nr_samples; + period += h->addr[offset].period; + ++offset; } if (h->nr_samples) { -- cgit From 1ac39372e06f5009982aaaf890fc5bbd044bb047 Mon Sep 17 00:00:00 2001 From: Taeung Song Date: Fri, 18 Aug 2017 17:46:48 +0900 Subject: perf annotate stdio: Support --show-nr-samples option Add --show-nr-samples option to "perf annotate" so that it matches "perf report". Committer note: Note that it can't be used together with --show-total-period, which seems like a silly limitation, that can be lifted at some point. Made it bail out if not on --stdio. Signed-off-by: Taeung Song Tested-by: Arnaldo Carvalho de Melo Cc: Jiri Olsa Cc: Milian Wolff Cc: Namhyung Kim Link: http://lkml.kernel.org/r/1503046008-5511-1-git-send-email-treeze.taeung@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf-annotate.txt | 4 ++++ tools/perf/builtin-annotate.c | 16 ++++++++++++++-- tools/perf/util/annotate.c | 6 +++++- 3 files changed, 23 insertions(+), 3 deletions(-) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt index a89273d8e744..2a5975c91cbd 100644 --- a/tools/perf/Documentation/perf-annotate.txt +++ b/tools/perf/Documentation/perf-annotate.txt @@ -43,6 +43,10 @@ OPTIONS --quiet:: Do not show any message. (Suppress -v) +-n:: +--show-nr-samples:: + Show the number of samples for each symbol + -D:: --dump-raw-trace:: Dump raw trace in ASCII. diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 658c920d74b9..89fc0389dc76 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -403,7 +403,7 @@ int cmd_annotate(int argc, const char **argv) struct perf_data_file file = { .mode = PERF_DATA_MODE_READ, }; - const struct option options[] = { + struct option options[] = { OPT_STRING('i', "input", &input_name, "file", "input file name"), OPT_STRING('d', "dsos", &symbol_conf.dso_list_str, "dso[,dso...]", @@ -445,13 +445,20 @@ int cmd_annotate(int argc, const char **argv) "Show event group information together"), OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period, "Show a column with the sum of periods"), + OPT_BOOLEAN('n', "show-nr-samples", &symbol_conf.show_nr_samples, + "Show a column with the number of samples"), OPT_CALLBACK_DEFAULT(0, "stdio-color", NULL, "mode", "'always' (default), 'never' or 'auto' only applicable to --stdio mode", stdio__config_color, "always"), OPT_END() }; - int ret = hists__init(); + int ret; + + set_option_flag(options, 0, "show-total-period", PARSE_OPT_EXCLUSIVE); + set_option_flag(options, 0, "show-nr-samples", PARSE_OPT_EXCLUSIVE); + + ret = hists__init(); if (ret < 0) return ret; @@ -467,6 +474,11 @@ int cmd_annotate(int argc, const char **argv) annotate.sym_hist_filter = argv[0]; } + if (symbol_conf.show_nr_samples && !annotate.use_stdio) { + pr_err("--show-nr-samples is only available in --stdio mode at this time\n"); + return ret; + } + if (quiet) perf_quiet_option(); diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 2dab0e5a7f2f..4397a8b6e6cd 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -1145,6 +1145,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st if (symbol_conf.show_total_period) color_fprintf(stdout, color, " %11" PRIu64, sample.period); + else if (symbol_conf.show_nr_samples) + color_fprintf(stdout, color, " %7" PRIu64, + sample.nr_samples); else color_fprintf(stdout, color, " %7.2f", percent); } @@ -1825,7 +1828,8 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, width *= evsel->nr_members; graph_dotted_len = printf(" %-*.*s| Source code & Disassembly of %s for %s (%" PRIu64 " samples)\n", - width, width, symbol_conf.show_total_period ? "Event count" : "Percent", + width, width, symbol_conf.show_total_period ? "Period" : + symbol_conf.show_nr_samples ? "Samples" : "Percent", d_filename, evsel_name, h->nr_samples); printf("%-*.*s----\n", -- cgit