perf/core improvements and fixes
Arnaldo Carvalho de Melo
maintainerFrom: Arnaldo Carvalho de Melo <[email protected]>
Hi Ingo,
Please consider pulling,
- Arnaldo
The following changes since commit a7636d9ecfa3ab7800a7c04c1f89378229eff609:
kprobes: Optimize hot path by using percpu counter to collect 'nhit' statistics (2016-02-09 11:08:58 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo
for you to fetch changes up to 1ad826bad5bd0b6ccfb203f78c70302b764df0be:
perf tests: Fix build on older systems where 'signal' is reserved (2016-02-15 17:33:26 -0300)
----------------------------------------------------------------
perf/core improvements and fixes:
User visible:
- Do not print trailing spaces in the hists browser (top, report) to avoid line wrapping issues when long C++ demangled functions are sampled (Arnaldo Carvalho de Melo)
- Allow 'perf config' to show --system or --user settings (Taeung Song)
- Add better warning about the need to install the audit-lib-python package when using perf python scripts (Taeung Song)
- Fix symbol resolution when kernel modules files are only in the build id cache (~/.debug) (Wang Nan)
Build fixes:
- Fix 'perf test' build on older systems where 'signal' is reserved (Arnaldo Carvalho de Melo)
Infrastructure:
- Free the terms list_head in parse_events__free_terms(), also unlink the entries when deleting them (Wang Nan)
- Fix releasing event_class in 'perf data' fixing integration with libbabeltrace (Wang Nan)
- Add EXTRA_LDFLAGS option to Makefile (Zubair Lutfullah Kakakhel)
----------------------------------------------------------------
Arnaldo Carvalho de Melo (6):
perf tools: Add comment explaining the repsep_snprintf function perf hists: Do column alignment on the format iterator
perf tools: Introduce parse_events_terms__purge()
perf tools: Use perf_event_terms__purge() for non-malloced terms perf tools: Rename parse_events__free_terms() to parse_events_terms__delete() perf tests: Fix build on older systems where 'signal' is reserved
Taeung Song (2):
perf config: Add '--system' and '--user' options to select which config file is used perf python scripting: Append examples to err msg about audit-libs-python
Wang Nan (4):
perf symbols: Fix symbols searching for module in buildid-cache perf tools: Unlink entries from terms list
perf tools: Free the terms list_head in parse_events__free_terms() perf data: Fix releasing event_class
Zubair Lutfullah Kakakhel (1):
perf build: Add EXTRA_LDFLAGS option to makefile
tools/perf/Documentation/perf-config.txt | 14 ++++++- tools/perf/Makefile.perf | 2 + tools/perf/arch/x86/util/intel-pt.c | 2 +- tools/perf/builtin-config.c | 27 +++++++++++-- .../python/Perf-Trace-Util/lib/Perf/Trace/Util.py | 5 ++- tools/perf/tests/bp_signal.c | 12 +++--- tools/perf/tests/parse-events.c | 2 +- tools/perf/ui/browsers/hists.c | 27 ++++++++----- tools/perf/ui/stdio/hist.c | 1 + tools/perf/util/build-id.c | 44 ++++++++++++++++++++++ tools/perf/util/build-id.h | 1 + tools/perf/util/cache.h | 3 ++ tools/perf/util/config.c | 4 +- tools/perf/util/data-convert-bt.c | 18 +++++++++ tools/perf/util/hist.c | 21 +++++++++++ tools/perf/util/hist.h | 5 +++ tools/perf/util/parse-events.c | 17 +++++++-- tools/perf/util/parse-events.h | 3 +- tools/perf/util/parse-events.y | 8 ++-- tools/perf/util/pmu.c | 2 +- tools/perf/util/sort.c | 23 +++++------ tools/perf/util/symbol.c | 4 ++ 22 files changed, 200 insertions(+), 45 deletions(-)
Arnaldo Carvalho de Melo
maintainerPATCH 1/13 perf config: Add '--system' and '--user' options to select which config file is used
From: Taeung Song <[email protected]>
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 [<file-option>] [options]
With an specific config file:
# perf config --user | --system
or both user and system config file:
# perf config
--- 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(-)
tools/perf/Documentation/perf-config.txt +13 -1
@@ -8,7 +8,7 @@ perf-config - Get and set variables in a configuration file. SYNOPSIS -------- [verse] -'perf config' -l | --list +'perf config' [<file-option>] -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 ~~~~~~
tools/perf/builtin-config.c +24 -3
@@ -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 [<file-option>] [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 :
tools/perf/util/cache.h +3 -0
@@ -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);
tools/perf/util/config.c +2 -2
@@ -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 *datreturn ret; } -static const char *perf_etc_perfconfig (void ) +const char *perf_etc_perfconfig (void ) {static const char *system_wide;if (!system_wide)
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 2/13 perf symbols: Fix symbols searching for module in buildid-cache
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
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.
--- 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(+)
tools/perf/util/build-id.c +44 -0
@@ -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) \
tools/perf/util/build-id.h +1 -0
@@ -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,
tools/perf/util/symbol.c +4 -0
@@ -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
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 3/13 perf build: Add EXTRA_LDFLAGS option to makefile
In reply to Arnaldo Carvalho de MeloFrom: Zubair Lutfullah Kakakhel <[email protected]>
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.
--- tools/perf/Makefile.perf | 2 ++ 1 file changed, 2 insertions(+)
tools/perf/Makefile.perf +2 -0
@@ -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
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 4/13 perf python scripting: Append examples to err msg about audit-libs-python
In reply to Arnaldo Carvalho de MeloFrom: Taeung Song <[email protected]>
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.--- tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py +4 -1
@@ -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:
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 5/13 perf tools: Add comment explaining the repsep_snprintf function
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
--- tools/perf/util/sort.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
tools/perf/util/sort.c +9 -1
@@ -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;
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 6/13 perf hists: Do column alignment on the format iterator
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
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'.
--- 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(-)
tools/perf/ui/browsers/hists.c +18 -9
@@ -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);
tools/perf/ui/stdio/hist.c +1 -0
@@ -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); }
tools/perf/util/hist.c +21 -0
@@ -1015,6 +1015,27 @@ void hist_entry__delete(struct hist_entry *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 */
tools/perf/util/hist.h +5 -0
@@ -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);
tools/perf/util/sort.c +3 -10
@@ -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
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 7/13 perf tools: Unlink entries from terms list
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
We were just freeing them, better unlink and init its nodes to catch bugs faster if we keep dangling references to them.
--- tools/perf/util/parse-events.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
tools/perf/util/parse-events.c +3 -1
@@ -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,
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 8/13 perf tools: Introduce parse_events_terms__purge()
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
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().
--- tools/perf/util/parse-events.c | 7 ++++++- tools/perf/util/parse-events.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-)
tools/perf/util/parse-events.c +6 -1
@@ -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) {
tools/perf/util/parse-events.h +1 -0
@@ -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);
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 9/13 perf tools: Use perf_event_terms__purge() for non-malloced terms
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
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.
--- tools/perf/tests/parse-events.c | 2 +- tools/perf/util/pmu.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
tools/perf/tests/parse-events.c +1 -1
@@ -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; }
tools/perf/util/pmu.c +1 -1
@@ -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);
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 10/13 perf tools: Free the terms list_head in parse_events__free_terms()
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
Fixing a leak, since code calling parse_events__free_terms() expect it to free the list_head too.
--- tools/perf/util/parse-events.c | 1 + 1 file changed, 1 insertion(+)
tools/perf/util/parse-events.c +1 -0
@@ -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,
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 11/13 perf tools: Rename parse_events__free_terms() to parse_events_terms__delete()
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
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().
--- 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(-)
tools/perf/arch/x86/util/intel-pt.c +1 -1
@@ -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; }
tools/perf/util/parse-events.c +4 -3
@@ -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); }
tools/perf/util/parse-events.h +1 -1
@@ -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);
tools/perf/util/parse-events.y +4 -4
@@ -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; } |
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 12/13 perf data: Fix releasing event_class
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
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
--- tools/perf/util/data-convert-bt.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
tools/perf/util/data-convert-bt.c +18 -0
@@ -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);
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 13/13 perf tests: Fix build on older systems where 'signal' is reserved
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
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....
--- tools/perf/tests/bp_signal.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
tools/perf/tests/bp_signal.c +6 -6
@@ -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)
Signature · 1 line
2.5.0
Quoted 90 lines · Arnaldo Carvalho de Melo
From: Arnaldo Carvalho de Melo <[email protected]>
Hi Ingo,
Please consider pulling,
- Arnaldo
The following changes since commit
a7636d9ecfa3ab7800a7c04c1f89378229eff609:kprobes: Optimize hot path by using percpu counter to collect 'nhit' statistics (2016-02-09 11:08:58 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.gittags/perf-core-for-mingofor you to fetch changes up to
1ad826bad5bd0b6ccfb203f78c70302b764df0be:perf tests: Fix build on older systems where 'signal' is reserved (2016-02-15 17:33:26 -0300)
----------------------------------------------------------------
perf/core improvements and fixes:User visible:
- Do not print trailing spaces in the hists browser (top, report) to avoid line wrapping issues when long C++ demangled functions are sampled (Arnaldo Carvalho de Melo)
- Allow 'perf config' to show --system or --user settings (Taeung Song)
- Add better warning about the need to install the audit-lib-python package when using perf python scripts (Taeung Song)
- Fix symbol resolution when kernel modules files are only in the build id cache (~/.debug) (Wang Nan)
Build fixes:
- Fix 'perf test' build on older systems where 'signal' is reserved (Arnaldo Carvalho de Melo)
Infrastructure:
- Free the terms
list_headinparse_events__free_terms(), also unlink the entries when deleting them (Wang Nan)- Fix releasing
event_classin 'perf data' fixing integration with libbabeltrace (Wang Nan)- Add
EXTRA_LDFLAGSoption toMakefile(Zubair Lutfullah Kakakhel)Signed-off-by Arnaldo Carvalho de Melo <[email protected]>----------------------------------------------------------------
Arnaldo Carvalho de Melo (6):
perf tools: Add comment explaining therepsep_snprintffunction perf hists: Do column alignment on the format iterator
perf tools: Introduceparse_events_terms__purge()
perf tools: Useperf_event_terms__purge()for non-malloced terms perf tools: Renameparse_events__free_terms()toparse_events_terms__delete()perf tests: Fix build on older systems where 'signal' is reservedTaeung Song (2):
perf config: Add '--system' and '--user' options to select which config file is used perf python scripting: Append examples to err msg about audit-libs-pythonWang Nan (4):
perf symbols: Fix symbols searching for module in buildid-cache perf tools: Unlink entries from terms list
perf tools: Free the termslist_headinparse_events__free_terms()perf data: Fix releasingevent_classZubair Lutfullah Kakakhel (1):
perf build: AddEXTRA_LDFLAGSoption to makefiletools/perf/Documentation/perf-config.txt | 14 ++++++- tools/perf/Makefile.perf | 2 + tools/perf/arch/x86/util/intel-pt.c | 2 +- tools/perf/builtin-config.c | 27 +++++++++++-- .../python/Perf-Trace-Util/lib/Perf/Trace/Util.py | 5 ++- tools/perf/tests/bp_signal.c | 12 +++--- tools/perf/tests/parse-events.c | 2 +- tools/perf/ui/browsers/hists.c | 27 ++++++++----- tools/perf/ui/stdio/hist.c | 1 + tools/perf/util/build-id.c | 44 ++++++++++++++++++++++ tools/perf/util/build-id.h | 1 + tools/perf/util/cache.h | 3 ++ tools/perf/util/config.c | 4 +- tools/perf/util/data-convert-bt.c | 18 +++++++++ tools/perf/util/hist.c | 21 +++++++++++
Quoted 8 lines
tools/perf/util/hist.h | 5 +++ tools/perf/util/parse-events.c | 17 +++++++-- tools/perf/util/parse-events.h | 3 +- tools/perf/util/parse-events.y | 8 ++-- tools/perf/util/pmu.c | 2 +- tools/perf/util/sort.c | 23 +++++------ tools/perf/util/symbol.c | 4 ++ 22 files changed, 200 insertions(+), 45 deletions(-)
Pulled, thanks a lot Arnaldo!
Ingo