* [PATCH 01/13] perf config: Add '--system' and '--user' options to select which config file is used
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 02/13] perf symbols: Fix symbols searching for module in buildid-cache Arnaldo Carvalho de Melo
` (12 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Taeung Song, Jiri Olsa, Namhyung Kim,
Arnaldo Carvalho de Melo
From: Taeung Song <treeze.taeung@gmail.com>
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
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1455126685-32367-2-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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' [<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
~~~~~~
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 [<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:
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)
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 02/13] perf symbols: Fix symbols searching for module in buildid-cache
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 01/13] perf config: Add '--system' and '--user' options to select which config file is used Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 03/13] perf build: Add EXTRA_LDFLAGS option to makefile Arnaldo Carvalho de Melo
` (11 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Wang Nan, Adrian Hunter, Alexei Starovoitov,
Brendan Gregg, Cody P Schafer, He Kuang, Jeremie Galarneau,
Jiri Olsa, Kirill Smelkov, Li Zefan, Masami Hiramatsu,
Namhyung Kim, Peter Zijlstra, pi3orama, Arnaldo Carvalho de Melo
From: Wang Nan <wangnan0@huawei.com>
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 <wangnan0@huawei.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Cody P Schafer <dev@codyps.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Jeremie Galarneau <jeremie.galarneau@efficios.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kirill Smelkov <kirr@nexedi.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
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 <acme@redhat.com>
---
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
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 03/13] perf build: Add EXTRA_LDFLAGS option to makefile
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 01/13] perf config: Add '--system' and '--user' options to select which config file is used Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 02/13] perf symbols: Fix symbols searching for module in buildid-cache Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 04/13] perf python scripting: Append examples to err msg about audit-libs-python Arnaldo Carvalho de Melo
` (10 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Zubair Lutfullah Kakakhel, Peter Zijlstra,
Arnaldo Carvalho de Melo
From: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
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 <Zubair.Kakakhel@imgtec.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1455024818-15842-1-git-send-email-Zubair.Kakakhel@imgtec.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 04/13] perf python scripting: Append examples to err msg about audit-libs-python
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2016-02-15 21:01 ` [PATCH 03/13] perf build: Add EXTRA_LDFLAGS option to makefile Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 05/13] perf tools: Add comment explaining the repsep_snprintf function Arnaldo Carvalho de Melo
` (9 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Taeung Song, Jiri Olsa, Namhyung Kim,
Arnaldo Carvalho de Melo
From: Taeung Song <treeze.taeung@gmail.com>
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 <treeze.taeung@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1455018790-13425-1-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/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:
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 05/13] perf tools: Add comment explaining the repsep_snprintf function
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (3 preceding siblings ...)
2016-02-15 21:01 ` [PATCH 04/13] perf python scripting: Append examples to err msg about audit-libs-python Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 06/13] perf hists: Do column alignment on the format iterator Arnaldo Carvalho de Melo
` (8 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter, Jiri Olsa,
Namhyung Kim, Wang Nan
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-4j67nvlfwbnkg85b969ewnkr@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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;
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 06/13] perf hists: Do column alignment on the format iterator
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (4 preceding siblings ...)
2016-02-15 21:01 ` [PATCH 05/13] perf tools: Add comment explaining the repsep_snprintf function Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 07/13] perf tools: Unlink entries from terms list Arnaldo Carvalho de Melo
` (7 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim
From: Arnaldo Carvalho de Melo <acme@redhat.com>
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 <davej@codemonkey.org.uk>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/n/tip-s7iwl2gj23w92l6tibnrcqzr@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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
@@ -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
*/
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
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 07/13] perf tools: Unlink entries from terms list
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (5 preceding siblings ...)
2016-02-15 21:01 ` [PATCH 06/13] perf hists: Do column alignment on the format iterator Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 08/13] perf tools: Introduce parse_events_terms__purge() Arnaldo Carvalho de Melo
` (6 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Wang Nan, Alexei Starovoitov, He Kuang,
Masami Hiramatsu, Namhyung Kim, Zefan Li, pi3orama,
Arnaldo Carvalho de Melo
From: Wang Nan <wangnan0@huawei.com>
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 <wangnan0@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: He Kuang <hekuang@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
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 <acme@redhat.com>
---
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,
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 08/13] perf tools: Introduce parse_events_terms__purge()
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (6 preceding siblings ...)
2016-02-15 21:01 ` [PATCH 07/13] perf tools: Unlink entries from terms list Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 09/13] perf tools: Use perf_event_terms__purge() for non-malloced terms Arnaldo Carvalho de Melo
` (5 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Alexei Starovoitov,
He Kuang, Masami Hiramatsu, Namhyung Kim, Wang Nan, Zefan Li,
pi3orama
From: Arnaldo Carvalho de Melo <acme@redhat.com>
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 <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: He Kuang <hekuang@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/n/tip-4w3zl4ifcl0ed0j4bu3tckqp@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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);
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 09/13] perf tools: Use perf_event_terms__purge() for non-malloced terms
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (7 preceding siblings ...)
2016-02-15 21:01 ` [PATCH 08/13] perf tools: Introduce parse_events_terms__purge() Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 10/13] perf tools: Free the terms list_head in parse_events__free_terms() Arnaldo Carvalho de Melo
` (4 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Alexei Starovoitov,
He Kuang, Masami Hiramatsu, Namhyung Kim, Wang Nan, Zefan Li,
pi3orama
From: Arnaldo Carvalho de Melo <acme@redhat.com>
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 <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: He Kuang <hekuang@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/n/tip-i956ryjhz97gnnqe8iqe7m7s@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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);
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 10/13] perf tools: Free the terms list_head in parse_events__free_terms()
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (8 preceding siblings ...)
2016-02-15 21:01 ` [PATCH 09/13] perf tools: Use perf_event_terms__purge() for non-malloced terms Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 11/13] perf tools: Rename parse_events__free_terms() to parse_events_terms__delete() Arnaldo Carvalho de Melo
` (3 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Wang Nan, Alexei Starovoitov, He Kuang,
Masami Hiramatsu, Namhyung Kim, Zefan Li, pi3orama,
Arnaldo Carvalho de Melo
From: Wang Nan <wangnan0@huawei.com>
Fixing a leak, since code calling parse_events__free_terms() expect it
to free the list_head too.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: He Kuang <hekuang@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
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 <acme@redhat.com>
---
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,
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 11/13] perf tools: Rename parse_events__free_terms() to parse_events_terms__delete()
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (9 preceding siblings ...)
2016-02-15 21:01 ` [PATCH 10/13] perf tools: Free the terms list_head in parse_events__free_terms() Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 12/13] perf data: Fix releasing event_class Arnaldo Carvalho de Melo
` (2 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Alexei Starovoitov,
He Kuang, Jiri Olsa, Masami Hiramatsu, Namhyung Kim, Wang Nan,
Zefan Li, pi3orama
From: Arnaldo Carvalho de Melo <acme@redhat.com>
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 <ast@kernel.org>
Cc: He Kuang <hekuang@huawei.com>
cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/n/tip-w6owu7rb8a46gvunlinxaqwx@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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;
}
|
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 12/13] perf data: Fix releasing event_class
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (10 preceding siblings ...)
2016-02-15 21:01 ` [PATCH 11/13] perf tools: Rename parse_events__free_terms() to parse_events_terms__delete() Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-15 21:01 ` [PATCH 13/13] perf tests: Fix build on older systems where 'signal' is reserved Arnaldo Carvalho de Melo
2016-02-16 7:48 ` [GIT PULL 00/13] perf/core improvements and fixes Ingo Molnar
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Wang Nan, Adrian Hunter, Alexei Starovoitov,
Brendan Gregg, Cody P Schafer, He Kuang, Jeremie Galarneau,
Kirill Smelkov, Li Zefan, Masami Hiramatsu, Namhyung Kim,
Peter Zijlstra, pi3orama, Arnaldo Carvalho de Melo
From: Wang Nan <wangnan0@huawei.com>
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 <wangnan0@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Cody P Schafer <dev@codyps.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Jeremie Galarneau <jeremie.galarneau@efficios.com>
Cc: Kirill Smelkov <kirr@nexedi.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Zefan Li <lizefan@huawei.com>
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 <acme@redhat.com>
---
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);
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 13/13] perf tests: Fix build on older systems where 'signal' is reserved
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (11 preceding siblings ...)
2016-02-15 21:01 ` [PATCH 12/13] perf data: Fix releasing event_class Arnaldo Carvalho de Melo
@ 2016-02-15 21:01 ` Arnaldo Carvalho de Melo
2016-02-16 7:48 ` [GIT PULL 00/13] perf/core improvements and fixes Ingo Molnar
13 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-15 21:01 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Alexei Starovoitov,
Brendan Gregg, Daniel Borkmann, He Kuang, Li Zefan,
Masami Hiramatsu, Namhyung Kim, Peter Zijlstra, Wang Nan,
Will Deacon, pi3orama
From: Arnaldo Carvalho de Melo <acme@redhat.com>
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 <vlee@freedesktop.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: He Kuang <hekuang@huawei.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Will Deacon <will.deacon@arm.com>
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 <acme@redhat.com>
---
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)
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [GIT PULL 00/13] perf/core improvements and fixes
2016-02-15 21:01 [GIT PULL 00/13] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (12 preceding siblings ...)
2016-02-15 21:01 ` [PATCH 13/13] perf tests: Fix build on older systems where 'signal' is reserved Arnaldo Carvalho de Melo
@ 2016-02-16 7:48 ` Ingo Molnar
13 siblings, 0 replies; 15+ messages in thread
From: Ingo Molnar @ 2016-02-16 7:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
Alexei Starovoitov, Brendan Gregg, Cody P Schafer,
Daniel Borkmann, Dave Jones, He Kuang, Jeremie Galarneau,
Jiri Olsa, Kirill Smelkov, Li Zefan, Masami Hiramatsu,
Namhyung Kim, Peter Zijlstra, pi3orama, Taeung Song, Vinson Lee,
Wang Nan, Will Deacon, Zubair Lutfullah Kakakhel
* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> 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)
>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> ----------------------------------------------------------------
> 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(-)
Pulled, thanks a lot Arnaldo!
Ingo
^ permalink raw reply [flat|nested] 15+ messages in thread