Skip to contentSkip to conversation

perf/urgent fixes

GIT PULL New Arnaldo Carvalho de Melo 2 participants 8 messages Nov 12, 2015 – Nov 13, 2015 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 41ac18ebfc429ce3f4d369ef07447d652999a0cd:

perf/x86/intel/rapl: Remove the unused RAPL_EVENT_DESC() macro (2015-11-12 09:44:25 +0100)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo

for you to fetch changes up to 2059fc7a5a9e667797b8ec503bfb4685afee48d8:

perf symbols: Allow forcing reading of non-root owned files by root (2015-11-12 18:58:18 -0300)

----------------------------------------------------------------
perf/urgent fixes:

- Fix 'd' hotkey for filtering by DSO in the top/report TUI browser (Arnaldo Carvalho de Melo)

- Allow forcing reading of non-root owned /tmp/perf-PID JIT symbol maps (Arnaldo Carvalho de Melo)

- Rebuild rbtree when adjusting symbols for kcore (Adrian Hunter)

- Actually install tmon in the tools/ install rule (Kamal Mostafa)

----------------------------------------------------------------
Adrian Hunter (1):
perf symbols: Rebuild rbtree when adjusting symbols for kcore

Arnaldo Carvalho de Melo (3):
perf hists browser: Fix 'd' hotkey action to filter by DSO perf hists browser: The dso can be obtained from popup_action->ms.map->dso perf symbols: Allow forcing reading of non-root owned files by root

Kamal Mostafa (2):
tools: Actually install tmon in the install rule
tools: Add a "make all" rule

 tools/Makefile                 | 11 ++++++++++-
 tools/perf/builtin-report.c    |  6 +++---
 tools/perf/ui/browsers/hists.c |  7 +------
 tools/perf/util/symbol.c       | 34 ++++++++++++++++------------------
 tools/perf/util/symbol.h       |  1 +
 5 files changed, 31 insertions(+), 28 deletions(-)
author maintainer
#1 lore raw

PATCH 1/6 tools: Actually install tmon in the install rule

From: Kamal Mostafa <[email protected]>

---
 tools/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/Makefile +1 -1
index d6f307dfb1a3..fa9d0a614d97 100644
@@ -101,7 +101,7 @@ freefall_install:
 install: acpi_install cgroup_install cpupower_install hv_install firewire_install lguest_install \
 		perf_install selftests_install turbostat_install usb_install \
 		virtio_install vm_install net_install x86_energy_perf_policy_install \
-		tmon freefall_install
+		tmon_install freefall_install
 
 acpi_clean:
 	$(call descend,power/acpi,clean)
Signature · 1 line
2.1.0
author maintainer
#2 lore raw

PATCH 2/6 tools: Add a "make all" rule

In reply to Arnaldo Carvalho de Melo

From: Kamal Mostafa <[email protected]>

---
 tools/Makefile | 9 +++++++++
 1 file changed, 9 insertions(+)
tools/Makefile +9 -0
index fa9d0a614d97..7dc820a8c1f1 100644
@@ -32,6 +32,10 @@ help:
 	@echo '  from the kernel command line to build and install one of'
 	@echo '  the tools above'
 	@echo ''
+	@echo '  $$ make tools/all'
+	@echo ''
+	@echo '  builds all tools.'
+	@echo ''
 	@echo '  $$ make tools/install'
 	@echo ''
 	@echo '  installs all tools.'
@@ -77,6 +81,11 @@ tmon: FORCE
 freefall: FORCE
 	$(call descend,laptop/$@)
 
+all: acpi cgroup cpupower hv firewire lguest \
+		perf selftests turbostat usb \
+		virtio vm net x86_energy_perf_policy \
+		tmon freefall
+
 acpi_install:
 	$(call descend,power/$(@:_install=),install)
 
Signature · 1 line
2.1.0
author maintainer
#3 lore raw

PATCH 3/6 perf symbols: Rebuild rbtree when adjusting symbols for kcore

In reply to Arnaldo Carvalho de Melo

From: Adrian Hunter <[email protected]>

Normally symbols are read from the DSO and adjusted, if need be, so that the symbol start matches the file offset in the DSO file (we want the file offset because that is what we know from MMAP events). That is done by dso__load_sym() which inserts the symbols *after* adjusting them.

In the case of kcore, the symbols have been read from kallsyms and the symbol start is the memory address. The symbols have to be adjusted to match the kcore file offsets. dso__split_kallsyms_for_kcore() does that, but now the adjustment is being done *after* the symbols have been inserted. It appears dso__split_kallsyms_for_kcore() was assuming that changing the symbol start would not change the order in the rbtree - which is, of course, not guaranteed.

---
 tools/perf/util/symbol.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)
tools/perf/util/symbol.c +14 -16
index b4cc7662677e..09343a880c0b 100644
@@ -654,19 +654,24 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
 	struct map_groups *kmaps = map__kmaps(map);
 	struct map *curr_map;
 	struct symbol *pos;
-	int count = 0, moved = 0;
+	int count = 0;
+	struct rb_root old_root = dso->symbols[map->type];
 	struct rb_root *root = &dso->symbols[map->type];
 	struct rb_node *next = rb_first(root);
 
 	if (!kmaps)
 		return -1;
 
+	*root = RB_ROOT;
+
 	while (next) {
 		char *module;
 
 		pos = rb_entry(next, struct symbol, rb_node);
 		next = rb_next(&pos->rb_node);
 
+		rb_erase_init(&pos->rb_node, &old_root);
+
 		module = strchr(pos->name, '\t');
 		if (module)
 			*module = '\0';
@@ -674,28 +679,21 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
 		curr_map = map_groups__find(kmaps, map->type, pos->start);
 
 		if (!curr_map || (filter && filter(curr_map, pos))) {
-			rb_erase_init(&pos->rb_node, root);
 			symbol__delete(pos);
-		} else {
-			pos->start -= curr_map->start - curr_map->pgoff;
-			if (pos->end)
-				pos->end -= curr_map->start - curr_map->pgoff;
-			if (curr_map->dso != map->dso) {
-				rb_erase_init(&pos->rb_node, root);
-				symbols__insert(
-					&curr_map->dso->symbols[curr_map->type],
-					pos);
-				++moved;
-			} else {
-				++count;
-			}
+			continue;
 		}
+
+		pos->start -= curr_map->start - curr_map->pgoff;
+		if (pos->end)
+			pos->end -= curr_map->start - curr_map->pgoff;
+		symbols__insert(&curr_map->dso->symbols[curr_map->type], pos);
+		++count;
 	}
 
 	/* Symbols have been adjusted */
 	dso->adjust_symbols = 1;
 
-	return count + moved;
+	return count;
 }
 
 /*
Signature · 1 line
2.1.0
author maintainer
#4 lore raw

PATCH 4/6 perf hists browser: Fix 'd' hotkey action to filter by DSO

In reply to Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <[email protected]>

When pressing 'd' the expected action is to filter all entries by the DSO in the current entry, but for that the action->map needs to be set, and only action->dso was being set, fix it.

Cc Adrian Hunter <[email protected]>
Cc David Ahern <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Wang Nan <[email protected]>
Fixes 045b80dd0340 ("perf hists browser: Use the map to determine if a DSO is being used as a kernel")
---
 tools/perf/ui/browsers/hists.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/ui/browsers/hists.c +1 -0
index e5afb8936040..a7ce0cf117e7 100644
@@ -1875,6 +1875,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			continue;
 		case 'd':
 			actions->dso = dso;
+			actions->ms.map = map;
 			do_zoom_dso(browser, actions);
 			continue;
 		case 'V':
Signature · 1 line
2.1.0
author maintainer
#5 lore raw

PATCH 5/6 perf hists browser: The dso can be obtained from popup_action->ms.map->dso

In reply to Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <[email protected]>

So no need to have a 'dso' member in 'popup_action', remove it as no code is using it, already.

---
 tools/perf/ui/browsers/hists.c | 6 ------
 1 file changed, 6 deletions(-)
tools/perf/ui/browsers/hists.c +0 -6
index a7ce0cf117e7..fa9eb92c9e24 100644
@@ -1430,7 +1430,6 @@ close_file_and_continue:
 
 struct popup_action {
 	struct thread 		*thread;
-	struct dso		*dso;
 	struct map_symbol 	ms;
 	int			socket;
 
@@ -1565,7 +1564,6 @@ add_dso_opt(struct hist_browser *browser, struct popup_action *act,
 		return 0;
 
 	act->ms.map = map;
-	act->dso = map->dso;
 	act->fn = do_zoom_dso;
 	return 1;
 }
@@ -1827,7 +1825,6 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 	while (1) {
 		struct thread *thread = NULL;
-		struct dso *dso = NULL;
 		struct map *map = NULL;
 		int choice = 0;
 		int socked_id = -1;
@@ -1839,8 +1836,6 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 		if (browser->he_selection != NULL) {
 			thread = hist_browser__selected_thread(browser);
 			map = browser->selection->map;
-			if (map)
-				dso = map->dso;
 			socked_id = browser->he_selection->socket;
 		}
 		switch (key) {
@@ -1874,7 +1869,6 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			hist_browser__dump(browser);
 			continue;
 		case 'd':
-			actions->dso = dso;
 			actions->ms.map = map;
 			do_zoom_dso(browser, actions);
 			continue;
Signature · 1 line
2.1.0
author maintainer
#6 lore raw

PATCH 6/6 perf symbols: Allow forcing reading of non-root owned files by root

In reply to Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <[email protected]>

When the root user tries to read a file owned by some other user we get:

  # ls -la perf.data
  -rw-------. 1 acme acme 20032 Nov 12 15:50 perf.data
  # perf report
  File perf.data not owned by current user or root (use -f to override)
  # perf report -f | grep -v ^# | head -2
    30.96%  ls       [kernel.vmlinux]  [k] do_set_pte
    28.24%  ls       libc-2.20.so      [.] intel_check_word
  #

That wasn't happening when the symbol code tried to read a JIT map, where the same check was done but no forcing was possible, fix it.

---
 tools/perf/builtin-report.c | 6 +++---
 tools/perf/util/symbol.c    | 4 ++--
 tools/perf/util/symbol.h    | 1 +
 3 files changed, 6 insertions(+), 5 deletions(-)
tools/perf/builtin-report.c +3 -3
index 2853ad2bd435..f256fac1e722 100644
@@ -44,7 +44,7 @@
 struct report {
 	struct perf_tool	tool;
 	struct perf_session	*session;
-	bool			force, use_tui, use_gtk, use_stdio;
+	bool			use_tui, use_gtk, use_stdio;
 	bool			hide_unresolved;
 	bool			dont_use_callchains;
 	bool			show_full_info;
@@ -678,7 +678,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "file", "vmlinux pathname"),
 	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
 		   "file", "kallsyms pathname"),
-	OPT_BOOLEAN('f', "force", &report.force, "don't complain, do it"),
+	OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
 	OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
 		    "load module symbols - WARNING: use only with -k and LIVE kernel"),
 	OPT_BOOLEAN('n', "show-nr-samples", &symbol_conf.show_nr_samples,
@@ -832,7 +832,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	}
 
 	file.path  = input_name;
-	file.force = report.force;
+	file.force = symbol_conf.force;
 
 repeat:
 	session = perf_session__new(&file, false, &report.tool);
tools/perf/util/symbol.c +2 -2
index 09343a880c0b..cd08027a6d2c 100644
@@ -1436,9 +1436,9 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 		if (lstat(dso->name, &st) < 0)
 			goto out;
 
-		if (st.st_uid && (st.st_uid != geteuid())) {
+		if (!symbol_conf.force && st.st_uid && (st.st_uid != geteuid())) {
 			pr_warning("File %s not owned by current user or root, "
-				"ignoring it.\n", dso->name);
+				   "ignoring it (use -f to override).\n", dso->name);
 			goto out;
 		}
 
tools/perf/util/symbol.h +1 -0
index 40073c60b83d..dcd786e364f2 100644
@@ -84,6 +84,7 @@ struct symbol_conf {
 	unsigned short	priv_size;
 	unsigned short	nr_events;
 	bool		try_vmlinux_path,
+			force,
 			ignore_vmlinux,
 			ignore_vmlinux_buildid,
 			show_kernel_path,
Signature · 1 line
2.1.0
#7 lore raw
In reply to Arnaldo Carvalho de Melo
Quoted 48 lines · Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <[email protected]>

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit 41ac18ebfc429ce3f4d369ef07447d652999a0cd:

perf/x86/intel/rapl: Remove the unused RAPL_EVENT_DESC() macro (2015-11-12 09:44:25 +0100)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo

for you to fetch changes up to 2059fc7a5a9e667797b8ec503bfb4685afee48d8:

perf symbols: Allow forcing reading of non-root owned files by root (2015-11-12 18:58:18 -0300)

----------------------------------------------------------------
perf/urgent fixes:

- Fix 'd' hotkey for filtering by DSO in the top/report TUI browser (Arnaldo Carvalho de Melo)

- Allow forcing reading of non-root owned /tmp/perf-PID JIT symbol maps (Arnaldo Carvalho de Melo)

- Rebuild rbtree when adjusting symbols for kcore (Adrian Hunter)

- Actually install tmon in the tools/ install rule (Kamal Mostafa)

----------------------------------------------------------------
Adrian Hunter (1):
perf symbols: Rebuild rbtree when adjusting symbols for kcore

Arnaldo Carvalho de Melo (3):
perf hists browser: Fix 'd' hotkey action to filter by DSO perf hists browser: The dso can be obtained from popup_action->ms.map->dso perf symbols: Allow forcing reading of non-root owned files by root

Kamal Mostafa (2):
tools: Actually install tmon in the install rule
tools: Add a "make all" rule

Quoted 6 lines
 tools/Makefile                 | 11 ++++++++++-
 tools/perf/builtin-report.c    |  6 +++---
 tools/perf/ui/browsers/hists.c |  7 +------
 tools/perf/util/symbol.c       | 34 ++++++++++++++++------------------
 tools/perf/util/symbol.h       |  1 +
 5 files changed, 31 insertions(+), 28 deletions(-)

Pulled, thanks a lot Arnaldo!

Ingo