Skip to contentSkip to conversation

perf/core improvements and fixes

GIT PULL New Arnaldo Carvalho de Melo 2 participants 15 messages Feb 15, 2016 – Feb 16, 2016 perf events

lore
Share & tools
lore

Arnaldo Carvalho de Melo

author maintainer
#0 lore raw

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.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(-)
author maintainer
#1 lore raw

PATCH 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
index c7158bfb1649..15949e2a7805 100644
@@ -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
index f04e804a9fad..c42448ed5dfe 100644
@@ -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
index 07b5d63947b1..3ca453f0c51f 100644
@@ -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
index d3e12e30e1d5..4e727635476e 100644
@@ -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)
Signature · 1 line
2.5.0
author maintainer
#2 lore raw

PATCH 2/13 perf symbols: Fix symbols searching for module in buildid-cache

In reply to Arnaldo Carvalho de Melo

From: 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.

Cc Adrian Hunter <[email protected]>
Cc Alexei Starovoitov <[email protected]>
Cc Brendan Gregg <[email protected]>
Cc Cody P Schafer <[email protected]>
Cc He Kuang <[email protected]>
Cc Jeremie Galarneau <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Kirill Smelkov <[email protected]>
Cc Li Zefan <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 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
index b28100ee1732..f1479eeef7da 100644
@@ -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
index 27a14a8a945b..64af3e20610d 100644
@@ -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
index 90cedfa30e43..e7588dc91518 100644
@@ -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
author maintainer
#3 lore raw

PATCH 3/13 perf build: Add EXTRA_LDFLAGS option to makefile

In reply to Arnaldo Carvalho de Melo

From: 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
index d404117810a7..4a4fad4182f5 100644
@@ -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
author maintainer
#4 lore raw

PATCH 4/13 perf python scripting: Append examples to err msg about audit-libs-python

In reply to Arnaldo Carvalho de Melo

From: 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
index 15c8400240fd..1d95009592eb 100644
@@ -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
author maintainer
#5 lore raw

PATCH 5/13 perf tools: Add comment explaining the repsep_snprintf function

In reply to Arnaldo Carvalho de Melo

From: 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
index de620f7f40f4..8b54ede7ec1f 100644
@@ -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
author maintainer
#6 lore raw

PATCH 6/13 perf hists: Do column alignment on the format iterator

In reply to Arnaldo Carvalho de Melo

From: 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
index a5a5390476ac..1819771243f9 100644
@@ -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
index 1a6e8f7f38c4..87b022ff03d8 100644
@@ -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
index 12f2d794dc28..561e9473a915 100644
@@ -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
index 1c7544a8fe1a..840b6d6aa44f 100644
@@ -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
index 8b54ede7ec1f..de715756f281 100644
@@ -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
author maintainer
#7 lore raw

PATCH 7/13 perf tools: Unlink entries from terms list

In reply to Arnaldo Carvalho de Melo

From: 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.

Cc Alexei Starovoitov <[email protected]>
Cc He Kuang <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Zefan Li <[email protected]>
[ Spun off from another patch, use list_del_init() instead of list_del() ]
---
 tools/perf/util/parse-events.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tools/perf/util/parse-events.c +3 -1
index 813d9b272c81..133c8d28f36c 100644
@@ -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
author maintainer
#8 lore raw

PATCH 8/13 perf tools: Introduce parse_events_terms__purge()

In reply to Arnaldo Carvalho de Melo

From: 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
index 133c8d28f36c..668afdccfcca 100644
@@ -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
index f1a6db107241..f90a04ccab39 100644
@@ -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
author maintainer
#9 lore raw

PATCH 9/13 perf tools: Use perf_event_terms__purge() for non-malloced terms

In reply to Arnaldo Carvalho de Melo

From: 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
index abe8849d1d70..6648274f4601 100644
@@ -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
index 41a9c875e492..cf59fbaee491 100644
@@ -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
author maintainer
#10 lore raw

PATCH 10/13 perf tools: Free the terms list_head in parse_events__free_terms()

In reply to Arnaldo Carvalho de Melo

From: 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
index 668afdccfcca..d1b49ec7ae46 100644
@@ -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
author maintainer
#11 lore raw

PATCH 11/13 perf tools: Rename parse_events__free_terms() to parse_events_terms__delete()

In reply to Arnaldo Carvalho de Melo

From: 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
index 6f7d453b0e32..a3395179c9ee 100644
@@ -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
index d1b49ec7ae46..e5583fd4e7bd 100644
@@ -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
index f90a04ccab39..53628bf3da67 100644
@@ -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
index ad379968d4c1..c0eac88ef474 100644
@@ -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
author maintainer
#12 lore raw

PATCH 12/13 perf data: Fix releasing event_class

In reply to Arnaldo Carvalho de Melo

From: 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

Cc Adrian Hunter <[email protected]>
Cc Alexei Starovoitov <[email protected]>
Cc Brendan Gregg <[email protected]>
Cc Cody P Schafer <[email protected]>
Cc He Kuang <[email protected]>
Cc Jeremie Galarneau <[email protected]>
Cc Kirill Smelkov <[email protected]>
Cc Li Zefan <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Zefan Li <[email protected]>
---
 tools/perf/util/data-convert-bt.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
tools/perf/util/data-convert-bt.c +18 -0
index 34cd1e4039d3..b722e57d5a87 100644
@@ -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
author maintainer
#13 lore raw

PATCH 13/13 perf tests: Fix build on older systems where 'signal' is reserved

In reply to Arnaldo Carvalho de Melo

From: 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....

Cc Alexei Starovoitov <[email protected]>
Cc Brendan Gregg <[email protected]>
Cc Daniel Borkmann <[email protected]>
Cc He Kuang <[email protected]>
Cc Li Zefan <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Wang Nan <[email protected]>
Cc Will Deacon <[email protected]>
Fixes 8fd34e1cce18 ("perf test: Improve bp_signal")
---
 tools/perf/tests/bp_signal.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
tools/perf/tests/bp_signal.c +6 -6
index 1d1bb489b4e8..e7664fe3bd33 100644
@@ -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
#14 lore raw
In reply to Arnaldo Carvalho de Melo
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.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 +++++++++++
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