From c7ac24178c50a01f14eebcddf5c7b7a2e54676cc Mon Sep 17 00:00:00 2001 From: Taeung Song Date: Thu, 11 Feb 2016 02:51:17 +0900 Subject: [PATCH 01/13] perf config: Add '--system' and '--user' options to select which config file is used The '--system' option means $(sysconfdir)/perfconfig and '--user' means $HOME/.perfconfig. If none is used, both system and user config file are read. E.g.: # perf config [] [options] With an specific config file: # perf config --user | --system or both user and system config file: # perf config Signed-off-by: Taeung Song Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/r/1455126685-32367-2-git-send-email-treeze.taeung@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf-config.txt | 14 +++++++++++- tools/perf/builtin-config.c | 27 +++++++++++++++++++++--- tools/perf/util/cache.h | 3 +++ tools/perf/util/config.c | 4 ++-- 4 files changed, 42 insertions(+), 6 deletions(-) diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt index c7158bfb1649..15949e2a7805 100644 --- a/tools/perf/Documentation/perf-config.txt +++ b/tools/perf/Documentation/perf-config.txt @@ -8,7 +8,7 @@ perf-config - Get and set variables in a configuration file. SYNOPSIS -------- [verse] -'perf config' -l | --list +'perf config' [] -l | --list DESCRIPTION ----------- @@ -21,6 +21,14 @@ OPTIONS --list:: Show current config variables, name and value, for all sections. +--user:: + For writing and reading options: write to user + '$HOME/.perfconfig' file or read it. + +--system:: + For writing and reading options: write to system-wide + '$(sysconfdir)/perfconfig' or read it. + CONFIGURATION FILE ------------------ @@ -30,6 +38,10 @@ The '$HOME/.perfconfig' file is used to store a per-user configuration. The file '$(sysconfdir)/perfconfig' can be used to store a system-wide default configuration. +When reading or writing, the values are read from the system and user +configuration files by default, and options '--system' and '--user' +can be used to tell the command to read from or write to only that location. + Syntax ~~~~~~ diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c index f04e804a9fad..c42448ed5dfe 100644 --- a/tools/perf/builtin-config.c +++ b/tools/perf/builtin-config.c @@ -13,8 +13,10 @@ #include "util/util.h" #include "util/debug.h" +static bool use_system_config, use_user_config; + static const char * const config_usage[] = { - "perf config [options]", + "perf config [] [options]", NULL }; @@ -25,6 +27,8 @@ enum actions { static struct option config_options[] = { OPT_SET_UINT('l', "list", &actions, "show current config variables", ACTION_LIST), + OPT_BOOLEAN(0, "system", &use_system_config, "use system config file"), + OPT_BOOLEAN(0, "user", &use_user_config, "use user config file"), OPT_END() }; @@ -42,10 +46,23 @@ static int show_config(const char *key, const char *value, int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) { int ret = 0; + char *user_config = mkpath("%s/.perfconfig", getenv("HOME")); argc = parse_options(argc, argv, config_options, config_usage, PARSE_OPT_STOP_AT_NON_OPTION); + if (use_system_config && use_user_config) { + pr_err("Error: only one config file at a time\n"); + parse_options_usage(config_usage, config_options, "user", 0); + parse_options_usage(NULL, config_options, "system", 0); + return -1; + } + + if (use_system_config) + config_exclusive_filename = perf_etc_perfconfig(); + else if (use_user_config) + config_exclusive_filename = user_config; + switch (actions) { case ACTION_LIST: if (argc) { @@ -53,9 +70,13 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) parse_options_usage(config_usage, config_options, "l", 1); } else { ret = perf_config(show_config, NULL); - if (ret < 0) + if (ret < 0) { + const char * config_filename = config_exclusive_filename; + if (!config_exclusive_filename) + config_filename = user_config; pr_err("Nothing configured, " - "please check your ~/.perfconfig file\n"); + "please check your %s \n", config_filename); + } } break; default: diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h index 07b5d63947b1..3ca453f0c51f 100644 --- a/tools/perf/util/cache.h +++ b/tools/perf/util/cache.h @@ -23,6 +23,8 @@ #define PERF_TRACEFS_ENVIRONMENT "PERF_TRACEFS_DIR" #define PERF_PAGER_ENVIRONMENT "PERF_PAGER" +extern const char *config_exclusive_filename; + typedef int (*config_fn_t)(const char *, const char *, void *); extern int perf_default_config(const char *, const char *, void *); extern int perf_config(config_fn_t fn, void *); @@ -31,6 +33,7 @@ extern u64 perf_config_u64(const char *, const char *); extern int perf_config_bool(const char *, const char *); extern int config_error_nonbool(const char *); extern const char *perf_config_dirname(const char *, const char *); +extern const char *perf_etc_perfconfig(void); char *alias_lookup(const char *alias); int split_cmdline(char *cmdline, const char ***argv); diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c index d3e12e30e1d5..4e727635476e 100644 --- a/tools/perf/util/config.c +++ b/tools/perf/util/config.c @@ -26,7 +26,7 @@ static const char *config_file_name; static int config_linenr; static int config_file_eof; -static const char *config_exclusive_filename; +const char *config_exclusive_filename; static int get_next_char(void) { @@ -434,7 +434,7 @@ static int perf_config_from_file(config_fn_t fn, const char *filename, void *dat return ret; } -static const char *perf_etc_perfconfig(void) +const char *perf_etc_perfconfig(void) { static const char *system_wide; if (!system_wide) From e7ee404757609067c8f261d90251f1e96459c535 Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Fri, 5 Feb 2016 14:01:27 +0000 Subject: [PATCH 02/13] perf symbols: Fix symbols searching for module in buildid-cache Before this patch, if a sample is triggered inside a module not in /lib/modules/`uname -r`/, even if the module is in buildid-cache, 'perf report' will still be unable to find the correct symbol. For example: # rm -rf ~/.debug/ # perf buildid-cache -a ./mymodule.ko # perf probe -m ./mymodule.ko -a get_mymodule_val Added new event: probe:get_mymodule_val (on get_mymodule_val in mymodule) You can now use it in all perf tools, such as: perf record -e probe:get_mymodule_val -aR sleep 1 # perf record -e probe:get_mymodule_val cat /proc/mymodule mymodule:3 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ] # perf report --stdio [SNIP] # # Overhead Command Shared Object Symbol # ........ ....... ................ ...................... # 100.00% cat [mymodule] [k] 0x0000000000000001 # perf report -vvvv --stdio dso__load_sym: adjusting symbol: st_value: 0 sh_addr: 0 sh_offset: 0x70 symbol__new: get_mymodule_val 0x70-0x8a [SNIP] This is caused by dso__load() -> dso__load_sym(). In dso__load(), kmod is true only when its file is found in some well know directories. All files loaded from buildid-cache are treated as user programs. Following dso__load_sym() set map->pgoff incorrectly. This patch gives kernel modules in buildid-cache a chance to adjust value of kmod. After dso__load() get the type of symbols, if it is buildid, check the last 3 chars of original filename against '.ko', and adjust the value of kmod if the file is a kernel module. Signed-off-by: Wang Nan Cc: Adrian Hunter Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Cody P Schafer Cc: He Kuang Cc: Jeremie Galarneau Cc: Jiri Olsa Cc: Kirill Smelkov Cc: Li Zefan Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1454680939-24963-3-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/build-id.c | 44 ++++++++++++++++++++++++++++++++++++++ tools/perf/util/build-id.h | 1 + tools/perf/util/symbol.c | 4 ++++ 3 files changed, 49 insertions(+) diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c index b28100ee1732..f1479eeef7da 100644 --- a/tools/perf/util/build-id.c +++ b/tools/perf/util/build-id.c @@ -166,6 +166,50 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size) return build_id__filename(build_id_hex, bf, size); } +bool dso__build_id_is_kmod(const struct dso *dso, char *bf, size_t size) +{ + char *id_name, *ch; + struct stat sb; + + id_name = dso__build_id_filename(dso, bf, size); + if (!id_name) + goto err; + if (access(id_name, F_OK)) + goto err; + if (lstat(id_name, &sb) == -1) + goto err; + if ((size_t)sb.st_size > size - 1) + goto err; + if (readlink(id_name, bf, size - 1) < 0) + goto err; + + bf[sb.st_size] = '\0'; + + /* + * link should be: + * ../../lib/modules/4.4.0-rc4/kernel/net/ipv4/netfilter/nf_nat_ipv4.ko/a09fe3eb3147dafa4e3b31dbd6257e4d696bdc92 + */ + ch = strrchr(bf, '/'); + if (!ch) + goto err; + if (ch - 3 < bf) + goto err; + + return strncmp(".ko", ch - 3, 3) == 0; +err: + /* + * If dso__build_id_filename work, get id_name again, + * because id_name points to bf and is broken. + */ + if (id_name) + id_name = dso__build_id_filename(dso, bf, size); + pr_err("Invalid build id: %s\n", id_name ? : + dso->long_name ? : + dso->short_name ? : + "[unknown]"); + return false; +} + #define dsos__for_each_with_build_id(pos, head) \ list_for_each_entry(pos, head, node) \ if (!pos->has_build_id) \ diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h index 27a14a8a945b..64af3e20610d 100644 --- a/tools/perf/util/build-id.h +++ b/tools/perf/util/build-id.h @@ -16,6 +16,7 @@ int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id); int filename__sprintf_build_id(const char *pathname, char *sbuild_id); char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size); +bool dso__build_id_is_kmod(const struct dso *dso, char *bf, size_t size); 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/symbol.c b/tools/perf/util/symbol.c index 90cedfa30e43..e7588dc91518 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -1529,6 +1529,10 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter) if (!runtime_ss && syms_ss) runtime_ss = syms_ss; + if (syms_ss && syms_ss->type == DSO_BINARY_TYPE__BUILD_ID_CACHE) + if (dso__build_id_is_kmod(dso, name, PATH_MAX)) + kmod = true; + if (syms_ss) ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, kmod); else From 37b4e2020a5f4dbecf22ee3efe92de6dbea1c5f0 Mon Sep 17 00:00:00 2001 From: Zubair Lutfullah Kakakhel Date: Tue, 9 Feb 2016 13:33:38 +0000 Subject: [PATCH 03/13] perf build: Add EXTRA_LDFLAGS option to makefile To compile for little-endian systems, you need to pass -EL to CC and LD. EXTRA_CFLAGS works to pass -EL to CC. Add EXTRA_LDFLAGS to pass -EL to LD. Signed-off-by: Zubair Lutfullah Kakakhel Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1455024818-15842-1-git-send-email-Zubair.Kakakhel@imgtec.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Makefile.perf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf index d404117810a7..4a4fad4182f5 100644 --- a/tools/perf/Makefile.perf +++ b/tools/perf/Makefile.perf @@ -139,6 +139,8 @@ $(call allow-override,CC,$(CROSS_COMPILE)gcc) $(call allow-override,AR,$(CROSS_COMPILE)ar) $(call allow-override,LD,$(CROSS_COMPILE)ld) +LD += $(EXTRA_LDFLAGS) + PKG_CONFIG = $(CROSS_COMPILE)pkg-config RM = rm -f From b416e204f88dd91d9e99f6deee3d57fbc90aee40 Mon Sep 17 00:00:00 2001 From: Taeung Song Date: Tue, 9 Feb 2016 20:53:10 +0900 Subject: [PATCH 04/13] perf python scripting: Append examples to err msg about audit-libs-python To print syscall names, the audit-libs-python package is required.. If not installed, it prints this error string: # perf script syscall-counts Install the audit-libs-python package to get syscall names. But the package name is different in Ubuntu, mention that in the error message, similar to a error message of util/trace-event-scripting.c: # perf script syscall-counts Install the audit-libs-python package to get syscall names. For example: # apt-get install python-audit (Ubuntu) # yum install audit-libs-python (Fedora) etc. Signed-off-by: Taeung Song Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/r/1455018790-13425-1-git-send-email-treeze.taeung@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- .../scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py b/tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py index 15c8400240fd..1d95009592eb 100644 --- a/tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py +++ b/tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py @@ -71,7 +71,10 @@ try: except: if not audit_package_warned: audit_package_warned = True - print "Install the audit-libs-python package to get syscall names" + print "Install the audit-libs-python package to get syscall names.\n" \ + "For example:\n # apt-get install python-audit (Ubuntu)" \ + "\n # yum install audit-libs-python (Fedora)" \ + "\n etc.\n" def syscall_name(id): try: From 37d9bb580aa73c171c51fb93edf67a902bcb186f Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 12 Feb 2016 11:27:51 -0300 Subject: [PATCH 05/13] perf tools: Add comment explaining the repsep_snprintf function Cc: Adrian Hunter Cc: Jiri Olsa Cc: Namhyung Kim Cc: Wang Nan Link: http://lkml.kernel.org/n/tip-4j67nvlfwbnkg85b969ewnkr@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/sort.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index de620f7f40f4..8b54ede7ec1f 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -28,7 +28,15 @@ int sort__has_socket = 0; int sort__has_thread = 0; enum sort_mode sort__mode = SORT_MODE__NORMAL; - +/* + * Replaces all occurrences of a char used with the: + * + * -t, --field-separator + * + * option, that uses a special separator character and don't pad with spaces, + * replacing all occurances of this separator in symbol names (and other + * output) with a '.' character, that thus it's the only non valid separator. +*/ static int repsep_snprintf(char *bf, size_t size, const char *fmt, ...) { int n; From 89fee70943232d73e3cc328634e0da253b6de9b5 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 11 Feb 2016 17:14:13 -0300 Subject: [PATCH 06/13] perf hists: Do column alignment on the format iterator We were doing column alignment in the format function for each cell, returning a string padded with spaces so that when the next column is printed the cursor is at its column alignment. This ends up needlessly printing trailing spaces, do it at the format iterator, that is where we know if it is needed, i.e. if there is more columns to be printed. This eliminates the need for triming lines when doing a dump using 'P' in the TUI browser and also produces far saner results with things like piping 'perf report' to 'less'. Right now only the formatters for sym->name and the 'locked' column (perf mem report), that are the ones that end up at the end of lines in the default 'perf report', 'perf top' and 'perf mem report' tools, the others will be done in a subsequent patch. In the end the 'width' parameter for the formatters now mean, in 'printf' terms, the 'precision', where before it was the field 'width'. Reported-by: Dave Jones Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/n/tip-s7iwl2gj23w92l6tibnrcqzr@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/ui/browsers/hists.c | 27 ++++++++++++++++++--------- tools/perf/ui/stdio/hist.c | 1 + tools/perf/util/hist.c | 21 +++++++++++++++++++++ tools/perf/util/hist.h | 5 +++++ tools/perf/util/sort.c | 13 +++---------- 5 files changed, 48 insertions(+), 19 deletions(-) diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c index a5a5390476ac..1819771243f9 100644 --- a/tools/perf/ui/browsers/hists.c +++ b/tools/perf/ui/browsers/hists.c @@ -1061,7 +1061,6 @@ static int hist_browser__show_entry(struct hist_browser *browser, struct hist_entry *entry, unsigned short row) { - char s[256]; int printed = 0; int width = browser->b.width; char folded_sign = ' '; @@ -1086,16 +1085,18 @@ static int hist_browser__show_entry(struct hist_browser *browser, .folded_sign = folded_sign, .current_entry = current_entry, }; - struct perf_hpp hpp = { - .buf = s, - .size = sizeof(s), - .ptr = &arg, - }; int column = 0; hist_browser__gotorc(browser, row, 0); hists__for_each_format(browser->hists, fmt) { + char s[2048]; + struct perf_hpp hpp = { + .buf = s, + .size = sizeof(s), + .ptr = &arg, + }; + if (perf_hpp__should_skip(fmt, entry->hists) || column++ < browser->b.horiz_scroll) continue; @@ -1120,11 +1121,18 @@ static int hist_browser__show_entry(struct hist_browser *browser, } if (fmt->color) { - width -= fmt->color(fmt, &hpp, entry); + int ret = fmt->color(fmt, &hpp, entry); + hist_entry__snprintf_alignment(entry, &hpp, fmt, ret); + /* + * fmt->color() already used ui_browser to + * print the non alignment bits, skip it (+ret): + */ + ui_browser__printf(&browser->b, "%s", s + ret); } else { - width -= fmt->entry(fmt, &hpp, entry); + hist_entry__snprintf_alignment(entry, &hpp, fmt, fmt->entry(fmt, &hpp, entry)); ui_browser__printf(&browser->b, "%s", s); } + width -= hpp.buf - s; } /* The scroll bar isn't being used */ @@ -1452,9 +1460,10 @@ static int hist_browser__fprintf_entry(struct hist_browser *browser, first = false; ret = fmt->entry(fmt, &hpp, he); + ret = hist_entry__snprintf_alignment(he, &hpp, fmt, ret); advance_hpp(&hpp, ret); } - printed += fprintf(fp, "%s\n", rtrim(s)); + printed += fprintf(fp, "%s\n", s); if (folded_sign == '-') printed += hist_browser__fprintf_callchain(browser, he, fp); diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c index 1a6e8f7f38c4..87b022ff03d8 100644 --- a/tools/perf/ui/stdio/hist.c +++ b/tools/perf/ui/stdio/hist.c @@ -403,6 +403,7 @@ static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp) else ret = fmt->entry(fmt, hpp, he); + ret = hist_entry__snprintf_alignment(he, hpp, fmt, ret); advance_hpp(hpp, ret); } diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 12f2d794dc28..561e9473a915 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -1014,6 +1014,27 @@ void hist_entry__delete(struct hist_entry *he) free(he); } +/* + * If this is not the last column, then we need to pad it according to the + * pre-calculated max lenght for this column, otherwise don't bother adding + * spaces because that would break viewing this with, for instance, 'less', + * that would show tons of trailing spaces when a long C++ demangled method + * names is sampled. +*/ +int hist_entry__snprintf_alignment(struct hist_entry *he, struct perf_hpp *hpp, + struct perf_hpp_fmt *fmt, int printed) +{ + if (!list_is_last(&fmt->list, &he->hists->hpp_list->fields)) { + const int width = fmt->width(fmt, hpp, hists_to_evsel(he->hists)); + if (printed < width) { + advance_hpp(hpp, printed); + printed = scnprintf(hpp->buf, hpp->size, "%-*s", width - printed, " "); + } + } + + return printed; +} + /* * collapse the histogram */ diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 1c7544a8fe1a..840b6d6aa44f 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -122,11 +122,16 @@ struct hist_entry *__hists__add_entry(struct hists *hists, int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al, int max_stack_depth, void *arg); +struct perf_hpp; +struct perf_hpp_fmt; + int64_t hist_entry__cmp(struct hist_entry *left, struct hist_entry *right); int64_t hist_entry__collapse(struct hist_entry *left, struct hist_entry *right); int hist_entry__transaction_len(void); int hist_entry__sort_snprintf(struct hist_entry *he, char *bf, size_t size, struct hists *hists); +int hist_entry__snprintf_alignment(struct hist_entry *he, struct perf_hpp *hpp, + struct perf_hpp_fmt *fmt, int printed); void hist_entry__delete(struct hist_entry *he); void perf_evsel__output_resort(struct perf_evsel *evsel, struct ui_progress *prog); diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index 8b54ede7ec1f..de715756f281 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -255,10 +255,8 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym, ret += repsep_snprintf(bf + ret, size - ret, "%s", sym->name); ret += repsep_snprintf(bf + ret, size - ret, "+0x%llx", ip - map->unmap_ip(map, sym->start)); - ret += repsep_snprintf(bf + ret, size - ret, "%-*s", - width - ret, ""); } else { - ret += repsep_snprintf(bf + ret, size - ret, "%-*s", + ret += repsep_snprintf(bf + ret, size - ret, "%.*s", width - ret, sym->name); } @@ -266,14 +264,9 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym, size_t len = BITS_PER_LONG / 4; ret += repsep_snprintf(bf + ret, size - ret, "%-#.*llx", len, ip); - ret += repsep_snprintf(bf + ret, size - ret, "%-*s", - width - ret, ""); } - if (ret > width) - bf[width] = '\0'; - - return width; + return ret; } static int hist_entry__sym_snprintf(struct hist_entry *he, char *bf, @@ -819,7 +812,7 @@ static int hist_entry__locked_snprintf(struct hist_entry *he, char *bf, else out = "No"; - return repsep_snprintf(bf, size, "%-*s", width, out); + return repsep_snprintf(bf, size, "%.*s", width, out); } static int64_t From a8adfceb389a0045e06af22517fa3326797b160a Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Fri, 12 Feb 2016 16:31:23 -0300 Subject: [PATCH 07/13] perf tools: Unlink entries from terms list We were just freeing them, better unlink and init its nodes to catch bugs faster if we keep dangling references to them. Signed-off-by: Wang Nan Acked-by: Jiri Olsa Cc: Alexei Starovoitov Cc: He Kuang Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Zefan Li Cc: pi3orama@163.com [ Spun off from another patch, use list_del_init() instead of list_del() ] Link: http://lkml.kernel.org/r/1454680939-24963-2-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 813d9b272c81..133c8d28f36c 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -2072,8 +2072,10 @@ void parse_events__free_terms(struct list_head *terms) { struct parse_events_term *term, *h; - list_for_each_entry_safe(term, h, terms, list) + list_for_each_entry_safe(term, h, terms, list) { + list_del_init(&term->list); free(term); + } } void parse_events_evlist_error(struct parse_events_evlist *data, From fc0a2c1d59beac70b8738f4ce14431b798837374 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 12 Feb 2016 16:43:02 -0300 Subject: [PATCH 08/13] perf tools: Introduce parse_events_terms__purge() Purges 'struct parse_event_term' entries from a list_head. Some users need this because they don't allocate space for the list head, it maybe on the stack or embedded into some other struct. Next patch will convert users that need just purging and then the perf_events__free_terms() routine will free the list head as well, finally being renamed to perf_events_terms__delete(). Acked-by: Jiri Olsa Cc: Alexei Starovoitov Cc: He Kuang Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Wang Nan Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/n/tip-4w3zl4ifcl0ed0j4bu3tckqp@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 7 ++++++- tools/perf/util/parse-events.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 133c8d28f36c..668afdccfcca 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -2068,7 +2068,7 @@ int parse_events_term__clone(struct parse_events_term **new, term->err_term, term->err_val); } -void parse_events__free_terms(struct list_head *terms) +void parse_events_terms__purge(struct list_head *terms) { struct parse_events_term *term, *h; @@ -2078,6 +2078,11 @@ void parse_events__free_terms(struct list_head *terms) } } +void parse_events__free_terms(struct list_head *terms) +{ + parse_events_terms__purge(terms); +} + void parse_events_evlist_error(struct parse_events_evlist *data, int idx, const char *str) { diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index f1a6db107241..f90a04ccab39 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -116,6 +116,7 @@ int parse_events_term__sym_hw(struct parse_events_term **term, int parse_events_term__clone(struct parse_events_term **new, struct parse_events_term *term); void parse_events__free_terms(struct list_head *terms); +void parse_events_terms__purge(struct list_head *terms); int parse_events__modifier_event(struct list_head *list, char *str, bool add); int parse_events__modifier_group(struct list_head *list, char *event_mod); int parse_events_name(struct list_head *list, char *name); From 682dc24c2a0f13d5a16ac8f4303671eb8f11519f Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 12 Feb 2016 16:48:00 -0300 Subject: [PATCH 09/13] perf tools: Use perf_event_terms__purge() for non-malloced terms In these two cases, a 'perf test' entry and in the PMU code the list_head is on the stack, so we can't use perf_event__free_terms() (soon to be renamed to perf_event_terms__delete()), because it will free the list_head as well. Acked-by: Jiri Olsa Cc: Alexei Starovoitov Cc: He Kuang Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Wang Nan Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/n/tip-i956ryjhz97gnnqe8iqe7m7s@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/parse-events.c | 2 +- tools/perf/util/pmu.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index abe8849d1d70..6648274f4601 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -1666,7 +1666,7 @@ static int test_term(struct terms_test *t) } ret = t->check(&terms); - parse_events__free_terms(&terms); + parse_events_terms__purge(&terms); return ret; } diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index 41a9c875e492..cf59fbaee491 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -354,7 +354,7 @@ static int pmu_alias_terms(struct perf_pmu_alias *alias, list_for_each_entry(term, &alias->terms, list) { ret = parse_events_term__clone(&cloned, term); if (ret) { - parse_events__free_terms(&list); + parse_events_terms__purge(&list); return ret; } list_add_tail(&cloned->list, &list); From d20a5f2b277b2f46548fb60f2bb95ad9a601d3fe Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Fri, 12 Feb 2016 17:01:17 -0300 Subject: [PATCH 10/13] perf tools: Free the terms list_head in parse_events__free_terms() Fixing a leak, since code calling parse_events__free_terms() expect it to free the list_head too. Signed-off-by: Wang Nan Acked-by: Jiri Olsa Cc: Alexei Starovoitov Cc: He Kuang Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Zefan Li Cc: pi3orama@163.com [ Spun off from another patch ] Link: http://lkml.kernel.org/r/1454680939-24963-2-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 668afdccfcca..d1b49ec7ae46 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -2081,6 +2081,7 @@ void parse_events_terms__purge(struct list_head *terms) void parse_events__free_terms(struct list_head *terms) { parse_events_terms__purge(terms); + free(terms); } void parse_events_evlist_error(struct parse_events_evlist *data, From 2146afc6b45b3f1b967f5605d4e6d97dd9e31061 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 12 Feb 2016 17:09:17 -0300 Subject: [PATCH 11/13] perf tools: Rename parse_events__free_terms() to parse_events_terms__delete() To follow convention used in other tools/perf/ areas. Also remove the need to check if it is NULL before calling the destructor, again, to follow convention that goes back to free(). Cc: Alexei Starovoitov Cc: He Kuang cc: Jiri Olsa Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Wang Nan Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/n/tip-w6owu7rb8a46gvunlinxaqwx@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/arch/x86/util/intel-pt.c | 2 +- tools/perf/util/parse-events.c | 7 ++++--- tools/perf/util/parse-events.h | 2 +- tools/perf/util/parse-events.y | 8 ++++---- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c index 6f7d453b0e32..a3395179c9ee 100644 --- a/tools/perf/arch/x86/util/intel-pt.c +++ b/tools/perf/arch/x86/util/intel-pt.c @@ -89,7 +89,7 @@ static int intel_pt_parse_terms_with_default(struct list_head *formats, *config = attr.config; out_free: - parse_events__free_terms(terms); + parse_events_terms__delete(terms); return err; } diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index d1b49ec7ae46..e5583fd4e7bd 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -1386,8 +1386,7 @@ int parse_events_terms(struct list_head *terms, const char *str) return 0; } - if (data.terms) - parse_events__free_terms(data.terms); + parse_events_terms__delete(data.terms); return ret; } @@ -2078,8 +2077,10 @@ void parse_events_terms__purge(struct list_head *terms) } } -void parse_events__free_terms(struct list_head *terms) +void parse_events_terms__delete(struct list_head *terms) { + if (!terms) + return; parse_events_terms__purge(terms); free(terms); } diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index f90a04ccab39..53628bf3da67 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -115,7 +115,7 @@ int parse_events_term__sym_hw(struct parse_events_term **term, char *config, unsigned idx); int parse_events_term__clone(struct parse_events_term **new, struct parse_events_term *term); -void parse_events__free_terms(struct list_head *terms); +void parse_events_terms__delete(struct list_head *terms); void parse_events_terms__purge(struct list_head *terms); int parse_events__modifier_event(struct list_head *list, char *str, bool add); int parse_events__modifier_group(struct list_head *list, char *event_mod); diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index ad379968d4c1..c0eac88ef474 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -218,7 +218,7 @@ PE_NAME '/' event_config '/' ALLOC_LIST(list); ABORT_ON(parse_events_add_pmu(data, list, $1, $3)); - parse_events__free_terms($3); + parse_events_terms__delete($3); $$ = list; } | @@ -246,7 +246,7 @@ PE_KERNEL_PMU_EVENT sep_dc ALLOC_LIST(list); ABORT_ON(parse_events_add_pmu(data, list, "cpu", head)); - parse_events__free_terms(head); + parse_events_terms__delete(head); $$ = list; } | @@ -266,7 +266,7 @@ PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc ALLOC_LIST(list); ABORT_ON(parse_events_add_pmu(data, list, "cpu", head)); - parse_events__free_terms(head); + parse_events_terms__delete(head); $$ = list; } @@ -285,7 +285,7 @@ value_sym '/' event_config '/' ALLOC_LIST(list); ABORT_ON(parse_events_add_numeric(data, list, type, config, $3)); - parse_events__free_terms($3); + parse_events_terms__delete($3); $$ = list; } | From 5141d7350d3d8a12f1f76b1015b937f14d2b97e2 Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Fri, 5 Feb 2016 14:01:30 +0000 Subject: [PATCH 12/13] perf data: Fix releasing event_class A new patch in libbabeltrace [1] reveals a object leak problem in 'perf data' CTF support: perf code never releases the event_class which is allocated in add_event() and stored in evsel's private field. If libbabeltrace has the above patch applied, leaking event_class prevents the writer from being destroyed and flushing metadata. For example: $ perf record ls perf.data [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.012 MB perf.data (12 samples) ] $ perf data convert --to-ctf ./out.ctf [ perf data convert: Converted 'perf.data' into CTF data './out.ctf' ] [ perf data convert: Converted and wrote 0.000 MB (12 samples) ] $ cat ./out.ctf/metadata $ ls -l ./out.ctf/metadata -rw-r----- 1 w00229757 mm 0 Jan 27 10:49 ./out.ctf/metadata The correct result should be: ... $ cat ./out.ctf/metadata /* CTF 1.8 */ trace { [SNIP] $ ls -l ./out.ctf/metadata -rw-r----- 1 w00229757 mm 2446 Jan 27 10:52 ./out.ctf/metadata The full story is: Patch [1] of babeltrace redesigns its reference counting scheme. In that patch: * writer <- trace (bt_ctf_writer_create) * trace <- stream_class (bt_ctf_trace_add_stream_class) * stream_class <- event_class (bt_ctf_stream_class_add_event_class) ('<-' means 'is a parent of') Holding of event_class causes reference count of corresponding 'writer' to increase through parent chain. Perf expects that 'writer' is released (so metadata is flushed) through bt_ctf_writer_put() in ctf_writer__cleanup(). However, since it never releases event_class, the reference of 'writer' won't be dropped, so bt_ctf_writer_put() won't lead to the release of writer. Before this CTF patch, !(writer <- trace). Even with event_class leaking, the writer ends up being released. [1] https://github.com/efficios/babeltrace/commit/e6a8e8e4744633807083a077ff9f101eb97d9801 Signed-off-by: Wang Nan Acked-by: Jiri Olsa Cc: Adrian Hunter Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Cody P Schafer Cc: He Kuang Cc: Jeremie Galarneau Cc: Kirill Smelkov Cc: Li Zefan Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1454680939-24963-6-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/data-convert-bt.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c index 34cd1e4039d3..b722e57d5a87 100644 --- a/tools/perf/util/data-convert-bt.c +++ b/tools/perf/util/data-convert-bt.c @@ -858,6 +858,23 @@ static int setup_events(struct ctf_writer *cw, struct perf_session *session) return 0; } +static void cleanup_events(struct perf_session *session) +{ + struct perf_evlist *evlist = session->evlist; + struct perf_evsel *evsel; + + evlist__for_each(evlist, evsel) { + struct evsel_priv *priv; + + priv = evsel->priv; + bt_ctf_event_class_put(priv->event_class); + zfree(&evsel->priv); + } + + perf_evlist__delete(evlist); + session->evlist = NULL; +} + static int setup_streams(struct ctf_writer *cw, struct perf_session *session) { struct ctf_stream **stream; @@ -1171,6 +1188,7 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force) (double) c.events_size / 1024.0 / 1024.0, c.events_count); + cleanup_events(session); perf_session__delete(session); ctf_writer__cleanup(cw); From 1ad826bad5bd0b6ccfb203f78c70302b764df0be Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 12 Feb 2016 18:30:01 -0300 Subject: [PATCH 13/13] perf tests: Fix build on older systems where 'signal' is reserved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixing the following problems, for instance, on RHEL6.7: CC /tmp/build/perf/tests/bp_signal.o cc1: warnings being treated as errors tests/bp_signal.c: In function ‘__event’: tests/bp_signal.c:106: error: declaration of ‘signal’ shadows a global declaration /usr/include/signal.h:101: error: shadowed declaration is here tests/bp_signal.c: In function ‘bp_event’: tests/bp_signal.c:144: error: declaration of ‘signal’ shadows a global declaration /usr/include/signal.h:101: error: shadowed declaration is here tests/bp_signal.c: In function ‘wp_event’: tests/bp_signal.c:149: error: declaration of ‘signal’ shadows a global declaration /usr/include/signal.h:101: error: shadowed declaration is here mv: cannot stat `/tmp/build/perf/tests/.bp_signal.o.tmp': No such file or directory make[3]: *** [/tmp/build/perf/tests/bp_signal.o] Error 1 make[2]: *** [tests] Error 2 make[1]: *** [/tmp/build/perf/perf-in.o] Error 2 make[1]: *** Waiting for unfinished jobs.... Reported-by: Vinson Lee Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Daniel Borkmann Cc: He Kuang Cc: Li Zefan Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Wang Nan Cc: Will Deacon Cc: pi3orama@163.com Fixes: 8fd34e1cce18 ("perf test: Improve bp_signal") Link: http://lkml.kernel.org/n/tip-wlpx6tik1b0jirlkw64bv400@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/bp_signal.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c index 1d1bb489b4e8..e7664fe3bd33 100644 --- a/tools/perf/tests/bp_signal.c +++ b/tools/perf/tests/bp_signal.c @@ -103,7 +103,7 @@ static void sig_handler(int signum __maybe_unused, } } -static int __event(bool is_x, void *addr, int signal) +static int __event(bool is_x, void *addr, int sig) { struct perf_event_attr pe; int fd; @@ -133,7 +133,7 @@ static int __event(bool is_x, void *addr, int signal) } fcntl(fd, F_SETFL, O_RDWR|O_NONBLOCK|O_ASYNC); - fcntl(fd, F_SETSIG, signal); + fcntl(fd, F_SETSIG, sig); fcntl(fd, F_SETOWN, getpid()); ioctl(fd, PERF_EVENT_IOC_RESET, 0); @@ -141,14 +141,14 @@ static int __event(bool is_x, void *addr, int signal) return fd; } -static int bp_event(void *addr, int signal) +static int bp_event(void *addr, int sig) { - return __event(true, addr, signal); + return __event(true, addr, sig); } -static int wp_event(void *addr, int signal) +static int wp_event(void *addr, int sig) { - return __event(false, addr, signal); + return __event(false, addr, sig); } static long long bp_count(int fd)