Skip to contentSkip to conversation

perf/core improvements and fixes

GIT PULL New Arnaldo Carvalho de Melo 2 participants 13 messages Dec 9, 2015 – Dec 10, 2015 perf events

lore
Share & tools
lore

Arnaldo Carvalho de Melo

author maintainer
#0 lore raw

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit a30c99a0beb3030ba42dab38cad6273cd090805d:

Merge branch 'perf/urgent' into perf/core, to pick up fixes (2015-12-08 06:06:20 +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 0a4bb5da957b83ece8b4723c5bac7a5d29fbfb33:

perf tools: Move cmd_version() to builtin-version.c (2015-12-09 13:42:03 -0300)

----------------------------------------------------------------
perf/core improvements and fixes:

User visible:

- Change default selection TUI background color to yellow (Ingo Molnar)

Infrastructure:

- Start paving the way to reuse some cmdline functions with other tools/ living utilities (Josh Poimboeuf)

- Reference count fixes using the refcount debugger, unleaking some objects (Masami Hiramatsu)

----------------------------------------------------------------
Ingo Molnar (1):
perf tui: Change default selection background color to yellow

Josh Poimboeuf (4):
perf tools: Remove unused pager_use_color variable perf tools: Move term functions out of util.c perf tools: Save cmdline arguments earlier perf tools: Move cmd_version() to builtin-version.c

Masami Hiramatsu (6):
perf tools: Fix map_groups__clone to put cloned map
perf stat: Fix cmd_stat to release cpu_map
perf hists: Fix hists_evsel to release hists
perf tools: Fix maps__fixup_overlappings to put used maps perf machine: Fix machine.vmlinux_maps to make sure to clear the old one perf tools: Fix write_numa_topology to put cpu_map instead of free

 tools/perf/Build                |  1 +
 tools/perf/builtin-stat.c       |  9 +++++++++
 tools/perf/builtin-version.c    | 10 ++++++++++
 tools/perf/perf.c               |  1 +
 tools/perf/ui/browser.c         |  2 +-
 tools/perf/util/Build           |  2 +-
 tools/perf/util/cache.h         |  1 -
 tools/perf/util/color.c         |  2 +-
 tools/perf/util/env.c           |  9 ---------
 tools/perf/util/environment.c   |  8 --------
 tools/perf/util/header.c        |  2 +-
 tools/perf/util/help.c          |  7 -------
 tools/perf/util/hist.c          | 10 +++++++++-
 tools/perf/util/machine.c       |  5 +++++
 tools/perf/util/map.c           |  3 +++
 tools/perf/util/parse-options.c |  2 --
 tools/perf/util/term.c          | 35 +++++++++++++++++++++++++++++++++++
 tools/perf/util/term.h          | 10 ++++++++++
 tools/perf/util/util.c          | 34 ----------------------------------
 tools/perf/util/util.h          |  4 +---
 20 files changed, 88 insertions(+), 69 deletions(-)
 create mode 100644 tools/perf/builtin-version.c
 delete mode 100644 tools/perf/util/environment.c
 create mode 100644 tools/perf/util/term.c
 create mode 100644 tools/perf/util/term.h
author maintainer
#1 lore raw

PATCH 1/11 perf tui: Change default selection background color to yellow

From: Ingo Molnar <[email protected]>

Boris reported that 'perf top' is unusable on his default 'black on white' terminal, which uses (eye friendly) light-grey as a background color.

The reason is that the TUI cursor for the current selection line uses HE_COLORSET_SELECTED, and that has a default background color of 'lightgrey' - which is a common terminal background choice and thus the colors conflict.

Use yellow as the background color instead: that should be an uncommon terminal background, yet it's still ergonomic on both black and white/grey terminals.

[ It would be a better solution to straight out detect color
collisions and resolve them reasonably by converting them to RGB and calculating color space distances, but I was unable to find proper documentation for SLtt_get_color_object() to recover the current color scheme so I gave up ... Yellow works well enough. ]

---
 tools/perf/ui/browser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/ui/browser.c +1 -1
index e9703c0829f1..d37202121689 100644
@@ -528,7 +528,7 @@ static struct ui_browser_colorset {
 		.colorset = HE_COLORSET_SELECTED,
 		.name	  = "selected",
 		.fg	  = "black",
-		.bg	  = "lightgray",
+		.bg	  = "yellow",
 	},
 	{
 		.colorset = HE_COLORSET_CODE,
Signature · 1 line
2.1.0
author maintainer
#2 lore raw

PATCH 2/11 perf tools: Fix map_groups__clone to put cloned map

In reply to Arnaldo Carvalho de Melo

From: Masami Hiramatsu <[email protected]>

Fix map_groups__clone to put cloned map after inserting it to the map_groups.

Refcnt debugger shows:
----
==== [0] ====
Unreclaimed map: 0x2a27ee0
Refcount +1 => 1 at

    ./perf(map_groups__clone+0x8d) [0x4bb7ed]
    ./perf(thread__fork+0xbe) [0x4c1f9e]
    ./perf(machine__process_fork_event+0x216) [0x4b79a6]
    ./perf(perf_event__synthesize_threads+0x38b) [0x48135b]
    ./perf(cmd_top+0xdc6) [0x43cb76]
    ./perf() [0x477223]
    ./perf(main+0x617) [0x422077]
    /lib64/libc.so.6(__libc_start_main+0xf0) [0x7ff806af8fe0]
    ./perf() [0x4221ed]
  Refcount +1 => 2 at
    ./perf(map_groups__clone+0x128) [0x4bb888]
    ./perf(thread__fork+0xbe) [0x4c1f9e]
    ./perf(machine__process_fork_event+0x216) [0x4b79a6]
    ./perf(perf_event__synthesize_threads+0x38b) [0x48135b]
    ./perf(cmd_top+0xdc6) [0x43cb76]
    ./perf() [0x477223]
    ./perf(main+0x617) [0x422077]
    /lib64/libc.so.6(__libc_start_main+0xf0) [0x7ff806af8fe0]
    ./perf() [0x4221ed]
  Refcount -1 => 1 at
    ./perf(map_groups__exit+0x87) [0x4ba757]
    ./perf(map_groups__put+0x68) [0x4ba9a8]
    ./perf(thread__put+0x8b) [0x4c1aeb]
    ./perf(machine__delete_threads+0x81) [0x4b48f1]
    ./perf(perf_session__delete+0x4f) [0x4be63f]
    ./perf(cmd_top+0x1094) [0x43ce44]
    ./perf() [0x477223]
    ./perf(main+0x617) [0x422077]
    /lib64/libc.so.6(__libc_start_main+0xf0) [0x7ff806af8fe0]
    ./perf() [0x4221ed]

----

This shows map_groups__clone get the map twice and put it when map_groups__exit.

---
 tools/perf/util/map.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/util/map.c +1 -0
index 93d9f1ce3baa..7b1c720976fc 100644
@@ -742,6 +742,7 @@ int map_groups__clone(struct map_groups *mg,
 		if (new == NULL)
 			goto out_unlock;
 		map_groups__insert(mg, new);
+		map__put(new);
 	}
 
 	err = 0;
Signature · 1 line
2.1.0
author maintainer
#3 lore raw

PATCH 3/11 perf stat: Fix cmd_stat to release cpu_map

In reply to Arnaldo Carvalho de Melo

From: Masami Hiramatsu <[email protected]>

Fix cmd_stat() to release cpu_map objects (aggr_map and cpus_aggr_map) afterwards.

refcnt debugger shows that the cmd_stat initializes cpu_map but not puts it.
----

  # ./perf stat -v ls
  ....
  REFCNT: BUG: Unreclaimed objects found.
  ==== [0] ====
  Unreclaimed cpu_map@0x29339c0
  Refcount +1 => 1 at
    ./perf(cpu_map__empty_new+0x6d) [0x4e64bd]
    ./perf(cmd_stat+0x5fe) [0x43594e]
    ./perf() [0x47b785]
    ./perf(main+0x617) [0x422587]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f2dff420af5]
    ./perf() [0x4226fd]
  REFCNT: Total 1 objects are not reclaimed.
    "cpu_map" leaks 1 objects
  ----
Cc Adrian Hunter <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
[ Remove NULL checks before calling the put operation, it checks it already ]
---
 tools/perf/builtin-stat.c | 9 +++++++++
 1 file changed, 9 insertions(+)
tools/perf/builtin-stat.c +9 -0
index e74712dee242..25a95f49c36e 100644
@@ -1094,6 +1094,14 @@ static int perf_stat_init_aggr_mode(void)
 	return cpus_aggr_map ? 0 : -ENOMEM;
 }
 
+static void perf_stat__exit_aggr_mode(void)
+{
+	cpu_map__put(aggr_map);
+	cpu_map__put(cpus_aggr_map);
+	aggr_map = NULL;
+	cpus_aggr_map = NULL;
+}
+
 /*
  * Add default attributes, if there were no attributes specified or
  * if -d/--detailed, -d -d or -d -d -d is used:
@@ -1442,6 +1450,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (!forever && status != -1 && !interval)
 		print_counters(NULL, argc, argv);
 
+	perf_stat__exit_aggr_mode();
 	perf_evlist__free_stats(evsel_list);
 out:
 	perf_evlist__delete(evsel_list);
Signature · 1 line
2.1.0
author maintainer
#4 lore raw

PATCH 4/11 perf hists: Fix hists_evsel to release hists

In reply to Arnaldo Carvalho de Melo

From: Masami Hiramatsu <[email protected]>

Since hists__init doesn't set the destructor of hists_evsel (which is an extended evsel structure), when hists_evsel is released, the extended part of the hists_evsel is not deleted (note that the hists_evsel object itself is freed).

This fixes it to add a destructor for hists__evsel and to set it up.

---
 tools/perf/util/hist.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
tools/perf/util/hist.c +9 -1
index 6e8e0ee9ec37..565ea3549894 100644
@@ -1567,6 +1567,13 @@ static int hists_evsel__init(struct perf_evsel *evsel)
 	return 0;
 }
 
+static void hists_evsel__exit(struct perf_evsel *evsel)
+{
+	struct hists *hists = evsel__hists(evsel);
+
+	hists__delete_entries(hists);
+}
+
 /*
  * XXX We probably need a hists_evsel__exit() to free the hist_entries
  * stored in the rbtree...
@@ -1575,7 +1582,8 @@ static int hists_evsel__init(struct perf_evsel *evsel)
 int hists__init(void)
 {
 	int err = perf_evsel__object_config(sizeof(struct hists_evsel),
-					    hists_evsel__init, NULL);
+					    hists_evsel__init,
+					    hists_evsel__exit);
 	if (err)
 		fputs("FATAL ERROR: Couldn't setup hists class\n", stderr);
 
Signature · 1 line
2.1.0
author maintainer
#5 lore raw

PATCH 5/11 perf tools: Fix maps__fixup_overlappings to put used maps

In reply to Arnaldo Carvalho de Melo

From: Masami Hiramatsu <[email protected]>

Since the __map_groups__insert got the given map, we don't need to keep it. So put the maps.

Refcnt debugger shows that map_groups__fixup_overlappings() got a map twice but the group released it just once. This pattern usually indicates the leak happens in caller site.

----
==== [0] ====
Unreclaimed map@0x39d3ae0
Refcount +1 => 1 at

    ./perf(map_groups__fixup_overlappings+0x335) [0x4c1865]
    ./perf(thread__insert_map+0x30) [0x4c8e00]
    ./perf(machine__process_mmap2_event+0x106) [0x4bd876]
    ./perf() [0x4c378e]
    ./perf() [0x4c4393]
    ./perf(perf_session__process_events+0x38a) [0x4c654a]
    ./perf(cmd_record+0xe24) [0x42fc94]
    ./perf() [0x47b745]
    ./perf(main+0x617) [0x422547]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f2eca2deaf5]
    ./perf() [0x4226bd]
  Refcount +1 => 2 at
    ./perf(map_groups__fixup_overlappings+0x3c5) [0x4c18f5]
    ./perf(thread__insert_map+0x30) [0x4c8e00]
    ./perf(machine__process_mmap2_event+0x106) [0x4bd876]
    ./perf() [0x4c378e]
    ./perf() [0x4c4393]
    ./perf(perf_session__process_events+0x38a) [0x4c654a]
    ./perf(cmd_record+0xe24) [0x42fc94]
    ./perf() [0x47b745]
    ./perf(main+0x617) [0x422547]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f2eca2deaf5]
    ./perf() [0x4226bd]
  Refcount -1 => 1 at
    ./perf(map_groups__exit+0x92) [0x4c0962]
    ./perf(map_groups__put+0x60) [0x4c0bc0]
    ./perf(thread__put+0x90) [0x4c8a40]
    ./perf(machine__delete_threads+0x7e) [0x4bad9e]
    ./perf(perf_session__delete+0x4f) [0x4c499f]
    ./perf(cmd_record+0xb6d) [0x42f9dd]
    ./perf() [0x47b745]
    ./perf(main+0x617) [0x422547]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f2eca2deaf5]
    ./perf() [0x4226bd]

----

---
 tools/perf/util/map.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/util/map.c +2 -0
index 7b1c720976fc..171b6d10a04b 100644
@@ -691,6 +691,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
 			__map_groups__insert(pos->groups, before);
 			if (verbose >= 2)
 				map__fprintf(before, fp);
+			map__put(before);
 		}
 
 		if (map->end < pos->end) {
@@ -705,6 +706,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
 			__map_groups__insert(pos->groups, after);
 			if (verbose >= 2)
 				map__fprintf(after, fp);
+			map__put(after);
 		}
 put_map:
 		map__put(pos);
Signature · 1 line
2.1.0
author maintainer
#6 lore raw

PATCH 6/11 perf machine: Fix machine.vmlinux_maps to make sure to clear the old one

In reply to Arnaldo Carvalho de Melo

From: Masami Hiramatsu <[email protected]>

Fix machine.vmlinux_maps to make sure to clear the old one if it is renewal. This can leak the previous maps on the vmlinux_maps because those are just overwritten.

Cc Adrian Hunter <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
[ Simplified the memset, same end result ]
---
 tools/perf/util/machine.c | 5 +++++
 1 file changed, 5 insertions(+)
tools/perf/util/machine.c +5 -0
index bfc289c73c22..f5882b8c8db9 100644
@@ -44,6 +44,8 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
 	machine->comm_exec = false;
 	machine->kernel_start = 0;
 
+	memset(machine->vmlinux_maps, 0, sizeof(machine->vmlinux_maps));
+
 	machine->root_dir = strdup(root_dir);
 	if (machine->root_dir == NULL)
 		return -ENOMEM;
@@ -770,6 +772,9 @@ int __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
 	enum map_type type;
 	u64 start = machine__get_running_kernel_start(machine, NULL);
 
+	/* In case of renewal the kernel map, destroy previous one */
+	machine__destroy_kernel_maps(machine);
+
 	for (type = 0; type < MAP__NR_TYPES; ++type) {
 		struct kmap *kmap;
 		struct map *map;
Signature · 1 line
2.1.0
author maintainer
#7 lore raw

PATCH 7/11 perf tools: Fix write_numa_topology to put cpu_map instead of free

In reply to Arnaldo Carvalho de Melo

From: Masami Hiramatsu <[email protected]>

Fix write_numa_topology to put cpu_map instead of free because cpu_map is managed based on refcnt.

---
 tools/perf/util/header.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/header.c +1 -1
index 43838003c1a1..5ac7bdb0dff7 100644
@@ -724,7 +724,7 @@ static int write_numa_topology(int fd, struct perf_header *h __maybe_unused,
 done:
 	free(buf);
 	fclose(fp);
-	free(node_map);
+	cpu_map__put(node_map);
 	return ret;
 }
 
Signature · 1 line
2.1.0
author maintainer
#8 lore raw

PATCH 8/11 perf tools: Remove unused pager_use_color variable

In reply to Arnaldo Carvalho de Melo

From: Josh Poimboeuf <[email protected]>

---
 tools/perf/util/Build         | 1 -
 tools/perf/util/cache.h       | 1 -
 tools/perf/util/color.c       | 2 +-
 tools/perf/util/environment.c | 8 --------
 4 files changed, 1 insertion(+), 11 deletions(-)
 delete mode 100644 tools/perf/util/environment.c
tools/perf/util/Build +0 -1
index 0513dd525d87..62392ab234f8 100644
@@ -6,7 +6,6 @@ libperf-y += config.o
 libperf-y += ctype.o
 libperf-y += db-export.o
 libperf-y += env.o
-libperf-y += environment.o
 libperf-y += event.o
 libperf-y += evlist.o
 libperf-y += evsel.o
tools/perf/util/cache.h +0 -1
index c861373aaed3..4c2b76499dd5 100644
@@ -31,7 +31,6 @@ extern const char *perf_config_dirname(const char *, const char *);
 /* pager.c */
 extern void setup_pager(void);
 extern int pager_in_use(void);
-extern int pager_use_color;
 
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);
tools/perf/util/color.c +1 -1
index 9b9565416f90..e5fb88bab9e1 100644
@@ -24,7 +24,7 @@ int perf_config_colorbool(const char *var, const char *value, int stdout_is_tty)
  auto_color:
 	if (stdout_is_tty < 0)
 		stdout_is_tty = isatty(1);
-	if (stdout_is_tty || (pager_in_use() && pager_use_color)) {
+	if (stdout_is_tty || pager_in_use()) {
 		char *term = getenv("TERM");
 		if (term && strcmp(term, "dumb"))
 			return 1;
tools/perf/util/environment.c deleted +0 -8
deleted file mode 100644
index 7405123692f1..000000000000
@@ -1,8 +0,0 @@
-/*
- * We put all the perf config variables in this same object
- * file, so that programs can link against the config parser
- * without having to link against all the rest of perf.
- */
-#include "cache.h"
-
-int pager_use_color = 1;
Signature · 1 line
2.1.0
author maintainer
#9 lore raw

PATCH 9/11 perf tools: Move term functions out of util.c

In reply to Arnaldo Carvalho de Melo

From: Josh Poimboeuf <[email protected]>

The term functions are needed by help.c which is going to be moved into a separate library. Move them out of util.c and into their own file.

---
 tools/perf/util/Build  |  1 +
 tools/perf/util/term.c | 35 +++++++++++++++++++++++++++++++++++
 tools/perf/util/term.h | 10 ++++++++++
 tools/perf/util/util.c | 34 ----------------------------------
 tools/perf/util/util.h |  4 +---
 5 files changed, 47 insertions(+), 37 deletions(-)
 create mode 100644 tools/perf/util/term.c
 create mode 100644 tools/perf/util/term.h
tools/perf/util/Build +1 -0
index 62392ab234f8..65fef5951c7d 100644
@@ -86,6 +86,7 @@ libperf-$(CONFIG_AUXTRACE) += intel-pt.o
 libperf-$(CONFIG_AUXTRACE) += intel-bts.o
 libperf-y += parse-branch-options.o
 libperf-y += parse-regs-options.o
+libperf-y += term.o
 
 libperf-$(CONFIG_LIBBPF) += bpf-loader.o
 libperf-$(CONFIG_BPF_PROLOGUE) += bpf-prologue.o
tools/perf/util/term.c new file +35 -0
new file mode 100644
index 000000000000..90b47d8aa19c
@@ -0,0 +1,35 @@
+#include "util.h"
+
+void get_term_dimensions(struct winsize *ws)
+{
+	char *s = getenv("LINES");
+
+	if (s != NULL) {
+		ws->ws_row = atoi(s);
+		s = getenv("COLUMNS");
+		if (s != NULL) {
+			ws->ws_col = atoi(s);
+			if (ws->ws_row && ws->ws_col)
+				return;
+		}
+	}
+#ifdef TIOCGWINSZ
+	if (ioctl(1, TIOCGWINSZ, ws) == 0 &&
+	    ws->ws_row && ws->ws_col)
+		return;
+#endif
+	ws->ws_row = 25;
+	ws->ws_col = 80;
+}
+
+void set_term_quiet_input(struct termios *old)
+{
+	struct termios tc;
+
+	tcgetattr(0, old);
+	tc = *old;
+	tc.c_lflag &= ~(ICANON | ECHO);
+	tc.c_cc[VMIN] = 0;
+	tc.c_cc[VTIME] = 0;
+	tcsetattr(0, TCSANOW, &tc);
+}
tools/perf/util/term.h new file +10 -0
new file mode 100644
index 000000000000..2c06a61846a1
@@ -0,0 +1,10 @@
+#ifndef __PERF_TERM_H
+#define __PERF_TERM_H
+
+struct termios;
+struct winsize;
+
+void get_term_dimensions(struct winsize *ws);
+void set_term_quiet_input(struct termios *old);
+
+#endif /* __PERF_TERM_H */
tools/perf/util/util.c +0 -34
index 75759aebc7b8..07da970a62a3 100644
@@ -355,40 +355,6 @@ void sighandler_dump_stack(int sig)
 	exit(sig);
 }
 
-void get_term_dimensions(struct winsize *ws)
-{
-	char *s = getenv("LINES");
-
-	if (s != NULL) {
-		ws->ws_row = atoi(s);
-		s = getenv("COLUMNS");
-		if (s != NULL) {
-			ws->ws_col = atoi(s);
-			if (ws->ws_row && ws->ws_col)
-				return;
-		}
-	}
-#ifdef TIOCGWINSZ
-	if (ioctl(1, TIOCGWINSZ, ws) == 0 &&
-	    ws->ws_row && ws->ws_col)
-		return;
-#endif
-	ws->ws_row = 25;
-	ws->ws_col = 80;
-}
-
-void set_term_quiet_input(struct termios *old)
-{
-	struct termios tc;
-
-	tcgetattr(0, old);
-	tc = *old;
-	tc.c_lflag &= ~(ICANON | ECHO);
-	tc.c_cc[VMIN] = 0;
-	tc.c_cc[VTIME] = 0;
-	tcsetattr(0, TCSANOW, &tc);
-}
-
 int parse_nsec_time(const char *str, u64 *ptime)
 {
 	u64 time_sec, time_nsec;
tools/perf/util/util.h +1 -3
index dcc659017976..150858f3b4f0 100644
@@ -53,6 +53,7 @@
 #include <stdlib.h>
 #include <stdarg.h>
 #include <string.h>
+#include <term.h>
 #include <errno.h>
 #include <limits.h>
 #include <sys/param.h>
@@ -282,9 +283,6 @@ void sighandler_dump_stack(int sig);
 extern unsigned int page_size;
 extern int cacheline_size;
 
-void get_term_dimensions(struct winsize *ws);
-void set_term_quiet_input(struct termios *old);
-
 struct parse_tag {
 	char tag;
 	int mult;
Signature · 1 line
2.1.0
author maintainer
#10 lore raw

PATCH 10/11 perf tools: Save cmdline arguments earlier

In reply to Arnaldo Carvalho de Melo

From: Josh Poimboeuf <[email protected]>

perf_env__set_cmdline() only saves the arguments the first time it's called. It doesn't need to be called every time the options and suboptions are parsed. Instead it can just be called once.

This also has the advantage of making the option parsing code less perf-specific so it can be moved out to a library.

---
 tools/perf/perf.c               | 1 +
 tools/perf/util/env.c           | 9 ---------
 tools/perf/util/parse-options.c | 2 --
 3 files changed, 1 insertion(+), 11 deletions(-)
tools/perf/perf.c +1 -0
index 4bee53c3f796..59ea48c7e26c 100644
@@ -384,6 +384,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 		use_pager = 1;
 	commit_pager_choice();
 
+	perf_env__set_cmdline(&perf_env, argc, argv);
 	status = p->fn(argc, argv, prefix);
 	exit_browser(status);
 	perf_env__exit(&perf_env);
tools/perf/util/env.c +0 -9
index 6af4f7c36820..7dd5939dea2e 100644
@@ -25,15 +25,6 @@ int perf_env__set_cmdline(struct perf_env *env, int argc, const char *argv[])
 {
 	int i;
 
-	/*
-	 * If env->cmdline_argv has already been set, do not override it.  This allows
-	 * a command to set the cmdline, parse args and then call another
-	 * builtin function that implements a command -- e.g, cmd_kvm calling
-	 * cmd_record.
-	 */
-	if (env->cmdline_argv != NULL)
-		return 0;
-
 	/* do not include NULL termination */
 	env->cmdline_argv = calloc(argc, sizeof(char *));
 	if (env->cmdline_argv == NULL)
tools/perf/util/parse-options.c +0 -2
index 9fca09296eb3..d09aff983581 100644
@@ -501,8 +501,6 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
 {
 	struct parse_opt_ctx_t ctx;
 
-	perf_env__set_cmdline(&perf_env, argc, argv);
-
 	/* build usage string if it's not provided */
 	if (subcommands && !usagestr[0]) {
 		struct strbuf buf = STRBUF_INIT;
Signature · 1 line
2.1.0
author maintainer
#11 lore raw

PATCH 11/11 perf tools: Move cmd_version() to builtin-version.c

In reply to Arnaldo Carvalho de Melo

From: Josh Poimboeuf <[email protected]>

Move cmd_version() to its own file so that help.c can be moved to a library.

---
 tools/perf/Build             |  1 +
 tools/perf/builtin-version.c | 10 ++++++++++
 tools/perf/util/help.c       |  7 -------
 3 files changed, 11 insertions(+), 7 deletions(-)
 create mode 100644 tools/perf/builtin-version.c
tools/perf/Build +1 -0
index 2c7aaf2ba119..2a41217e9d88 100644
@@ -20,6 +20,7 @@ perf-y += builtin-kvm.o
 perf-y += builtin-inject.o
 perf-y += builtin-mem.o
 perf-y += builtin-data.o
+perf-y += builtin-version.o
 
 perf-$(CONFIG_AUDIT) += builtin-trace.o
 perf-$(CONFIG_LIBELF) += builtin-probe.o
tools/perf/builtin-version.c new file +10 -0
new file mode 100644
index 000000000000..9b10cda6b6dc
@@ -0,0 +1,10 @@
+#include "util/util.h"
+#include "builtin.h"
+#include "perf.h"
+
+int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused,
+		const char *prefix __maybe_unused)
+{
+	printf("perf version %s\n", perf_version_string);
+	return 0;
+}
tools/perf/util/help.c +0 -7
index fa1fc4acb8a4..929c93f2c333 100644
@@ -332,10 +332,3 @@ const char *help_unknown_cmd(const char *cmd)
 
 	exit(1);
 }
-
-int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused,
-		const char *prefix __maybe_unused)
-{
-	printf("perf version %s\n", perf_version_string);
-	return 0;
-}
Signature · 1 line
2.1.0
#12 lore raw
In reply to Arnaldo Carvalho de Melo
Quoted 70 lines · Arnaldo Carvalho de Melo

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit a30c99a0beb3030ba42dab38cad6273cd090805d:

Merge branch 'perf/urgent' into perf/core, to pick up fixes (2015-12-08 06:06:20 +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 0a4bb5da957b83ece8b4723c5bac7a5d29fbfb33:

perf tools: Move cmd_version() to builtin-version.c (2015-12-09 13:42:03 -0300)

----------------------------------------------------------------
perf/core improvements and fixes:

User visible:

- Change default selection TUI background color to yellow (Ingo Molnar)

Infrastructure:

- Start paving the way to reuse some cmdline functions with other tools/ living utilities (Josh Poimboeuf)

- Reference count fixes using the refcount debugger, unleaking some objects (Masami Hiramatsu)

----------------------------------------------------------------
Ingo Molnar (1):
perf tui: Change default selection background color to yellow

Josh Poimboeuf (4):
perf tools: Remove unused pager_use_color variable perf tools: Move term functions out of util.c perf tools: Save cmdline arguments earlier perf tools: Move cmd_version() to builtin-version.c

Masami Hiramatsu (6):
perf tools: Fix map_groups__clone to put cloned map
perf stat: Fix cmd_stat to release cpu_map
perf hists: Fix hists_evsel to release hists
perf tools: Fix maps__fixup_overlappings to put used maps perf machine: Fix machine.vmlinux_maps to make sure to clear the old one perf tools: Fix write_numa_topology to put cpu_map instead of free

 tools/perf/Build                |  1 +
 tools/perf/builtin-stat.c       |  9 +++++++++
 tools/perf/builtin-version.c    | 10 ++++++++++
 tools/perf/perf.c               |  1 +
 tools/perf/ui/browser.c         |  2 +-
 tools/perf/util/Build           |  2 +-
 tools/perf/util/cache.h         |  1 -
 tools/perf/util/color.c         |  2 +-
 tools/perf/util/env.c           |  9 ---------
 tools/perf/util/environment.c   |  8 --------
 tools/perf/util/header.c        |  2 +-
 tools/perf/util/help.c          |  7 -------
 tools/perf/util/hist.c          | 10 +++++++++-
 tools/perf/util/machine.c       |  5 +++++
 tools/perf/util/map.c           |  3 +++
 tools/perf/util/parse-options.c |  2 --
 tools/perf/util/term.c          | 35 +++++++++++++++++++++++++++++++++++
Quoted 8 lines
 tools/perf/util/term.h          | 10 ++++++++++
 tools/perf/util/util.c          | 34 ----------------------------------
 tools/perf/util/util.h          |  4 +---
 20 files changed, 88 insertions(+), 69 deletions(-)
 create mode 100644 tools/perf/builtin-version.c
 delete mode 100644 tools/perf/util/environment.c
 create mode 100644 tools/perf/util/term.c
 create mode 100644 tools/perf/util/term.h

Pulled, thanks a lot Arnaldo!

Ingo