Skip to contentSkip to conversation

perf/core improvements and fixes

GIT PULL In review Arnaldo Carvalho de Melo 4 participants 20 messages Jun 23, 2015 – Jun 26, 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 a9a3cd900fbbcbf837d65653105e7bfc583ced09:

Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2015-06-20 01:11:11 +0200)

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 83b2ea257eb1d43e52f76d756722aeb899a2852c:

perf tools: Allow auxtrace data alignment (2015-06-23 18:28:37 -0300)

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

User visible:

- Move toggling event logic from 'perf top' and into hists browser, allowing freeze/unfreeze with event lists with more than one entry (Namhyung Kim)

- Add missing newlines when dumping PERF_RECORD_FINISHED_ROUND and showing the Aggregated stats in 'perf report -D' (Adrian Hunter)

Infrastructure:

- Allow auxtrace data alignment (Adrian Hunter)

- Allow events with dot (Andi Kleen)

- Fix failure to 'perf probe' events on arm (He Kuang)

- Add testing for Makefile.perf (Jiri Olsa)

- Add test for make install with prefix (Jiri Olsa)

- Fix single target build dependency check (Jiri Olsa)

- Access thread_map entries via accessors, prep patch to hold more info per entry, for ongoing 'perf stat --per-thread' work (Jiri Olsa)

- Use __weak definition from compiler.h (Sukadev Bhattiprolu)

- Split perf_pmu__new_alias() (Sukadev Bhattiprolu)

----------------------------------------------------------------
Adrian Hunter (3):
perf session: Print a newline when dumping PERF_RECORD_FINISHED_ROUND perf tools: Print a newline before dumping Aggregated stats perf tools: Allow auxtrace data alignment

Andi Kleen (1):
perf tools: Allow events with dot

He Kuang (1):
perf probe: Fix failure to probe events on arm

Jiri Olsa (5):
perf tests: Add testing for Makefile.perf
perf tests: Add test for make install with prefix perf build: Fix single target build dependency check perf thread_map: Don't access the array entries directly perf thread_map: Change map entries into a struct

Namhyung Kim (1):
perf top: Move toggling event logic into hists browser

Sukadev Bhattiprolu (2):
perf pmu: Use __weak definition from <linux/compiler.h> perf pmu: Split perf_pmu__new_alias()

 tools/perf/Makefile                         |  4 +--
 tools/perf/builtin-top.c                    | 24 ++-------------
 tools/perf/builtin-trace.c                  |  4 +--
 tools/perf/tests/make                       | 31 ++++++++++++++++++--
 tools/perf/tests/openat-syscall-tp-fields.c |  2 +-
 tools/perf/ui/browsers/hists.c              | 19 ++++++++++--
 tools/perf/util/auxtrace.c                  | 11 +++++--
 tools/perf/util/auxtrace.h                  |  1 +
 tools/perf/util/event.c                     |  6 ++--
 tools/perf/util/evlist.c                    |  4 +--
 tools/perf/util/evsel.c                     |  2 +-
 tools/perf/util/parse-events.l              |  5 ++--
 tools/perf/util/pmu.c                       | 45 +++++++++++++++++++----------
 tools/perf/util/probe-event.c               |  6 +++-
 tools/perf/util/session.c                   |  4 ++-
 tools/perf/util/thread_map.c                | 24 ++++++++-------
 tools/perf/util/thread_map.h                | 16 +++++++++-
 17 files changed, 136 insertions(+), 72 deletions(-)
author maintainer
#1 lore raw

PATCH 1/13 perf top: Move toggling event logic into hists browser

From: Namhyung Kim <[email protected]>

Current 'f' key action to enable/disable events won't work if there're more than one event since perf_evsel_menu__run() doesn't return the key. So move it to the hists browser loop so that it can be processed as like other key action, and it's more natural to handle it there IMHO.

---
 tools/perf/builtin-top.c       | 24 +++---------------------
 tools/perf/ui/browsers/hists.c | 19 +++++++++++++++++--
 2 files changed, 20 insertions(+), 23 deletions(-)
tools/perf/builtin-top.c +3 -21
index 619a8696fda7..ecf319728f25 100644
@@ -586,27 +586,9 @@ static void *display_thread_tui(void *arg)
 		hists->uid_filter_str = top->record_opts.target.uid_str;
 	}
 
-	while (true)  {
-		int key = perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
-							top->min_percent,
-							&top->session->header.env);
-
-		if (key != 'f')
-			break;
-
-		perf_evlist__toggle_enable(top->evlist);
-		/*
-		 * No need to refresh, resort/decay histogram entries
-		 * if we are not collecting samples:
-		 */
-		if (top->evlist->enabled) {
-			hbt.refresh = top->delay_secs;
-			help = "Press 'f' to disable the events or 'h' to see other hotkeys";
-		} else {
-			help = "Press 'f' again to re-enable the events";
-			hbt.refresh = 0;
-		}
-	}
+	perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
+				      top->min_percent,
+				      &top->session->header.env);
 
 	done = 1;
 	return NULL;
tools/perf/ui/browsers/hists.c +17 -2
index c42adb600091..7629bef2fd79 100644
@@ -1902,8 +1902,23 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 		case CTRL('c'):
 			goto out_free_stack;
 		case 'f':
-			if (!is_report_browser(hbt))
-				goto out_free_stack;
+			if (!is_report_browser(hbt)) {
+				struct perf_top *top = hbt->arg;
+
+				perf_evlist__toggle_enable(top->evlist);
+				/*
+				 * No need to refresh, resort/decay histogram
+				 * entries if we are not collecting samples:
+				 */
+				if (top->evlist->enabled) {
+					helpline = "Press 'f' to disable the events or 'h' to see other hotkeys";
+					hbt->refresh = delay_secs;
+				} else {
+					helpline = "Press 'f' again to re-enable the events";
+					hbt->refresh = 0;
+				}
+				continue;
+			}
 			/* Fall thru */
 		default:
 			helpline = "Press '?' for help on key bindings";
Signature · 1 line
2.1.0
author maintainer
#2 lore raw

PATCH 2/13 perf tests: Add testing for Makefile.perf

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Currently we test only builds through top level Makefile, but seems like there's a bunch of users using Makefile.perf directly.

Changing the make suite to be run for Makefile.perf as well. It takes now considerable amount of time, but hopefully we catch more issues.

Also fixing the output indentation for make_kernelsrc and make_kernelsrc_tools tests.

---
 tools/perf/tests/make | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)
tools/perf/tests/make +20 -3
index 65280d28662e..bfe1962da0df 100644
@@ -1,5 +1,16 @@
+ifndef MK
+ifeq ($(MAKECMDGOALS),)
+# no target specified, trigger the whole suite
+all:
+	@echo "Testing Makefile";      $(MAKE) -sf tests/make MK=Makefile
+	@echo "Testing Makefile.perf"; $(MAKE) -sf tests/make MK=Makefile.perf
+else
+# run only specific test over 'Makefile'
+%:
+	@echo "Testing Makefile";      $(MAKE) -sf tests/make MK=Makefile $@
+endif
+else
 PERF := .
-MK   := Makefile
 
 include config/Makefile.arch
 
@@ -57,7 +68,12 @@ make_minimal        += NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1
 
 # $(run) contains all available tests
 run := make_pure
+# Targets 'clean all' can be run together only through top level
+# Makefile because we detect clean target in Makefile.perf and
+# disable features detection
+ifeq ($(MK),Makefile)
 run += make_clean_all
+endif
 run += make_python_perf_so
 run += make_debug
 run += make_no_libperl
@@ -226,13 +242,13 @@ tarpkg:
 	( eval $$cmd ) >> $@ 2>&1
 
 make_kernelsrc:
-	@echo " - make -C <kernelsrc> tools/perf"
+	@echo "- make -C <kernelsrc> tools/perf"
 	$(call clean); \
 	(make -C ../.. tools/perf) > $@ 2>&1 && \
 	test -x perf && rm -f $@ || (cat $@ ; false)
 
 make_kernelsrc_tools:
-	@echo " - make -C <kernelsrc>/tools perf"
+	@echo "- make -C <kernelsrc>/tools perf"
 	$(call clean); \
 	(make -C ../../tools perf) > $@ 2>&1 && \
 	test -x perf && rm -f $@ || (cat $@ ; false)
@@ -244,3 +260,4 @@ out: $(run_O)
 	@echo OK
 
 .PHONY: all $(run) $(run_O) tarpkg clean
+endif # ifndef MK
Signature · 1 line
2.1.0
author maintainer
#3 lore raw

PATCH 3/13 perf tests: Add test for make install with prefix

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Lukas Wunner reported issue (and fix[1]) with 'make install prefix=...'. Adding automated test for this, so it wouldn't happen again.

[1]: 75e84ab906ef ("perf tools: Fix build breakage if prefix= is specified")

---
 tools/perf/tests/make | 8 ++++++++
 1 file changed, 8 insertions(+)
tools/perf/tests/make +8 -0
index bfe1962da0df..729112f4cfaa 100644
@@ -58,6 +58,7 @@ make_install_man    := install-man
 make_install_html   := install-html
 make_install_info   := install-info
 make_install_pdf    := install-pdf
+make_install_prefix := install prefix=/tmp/krava
 make_static         := LDFLAGS=-static
 
 # all the NO_* variable combined
@@ -99,6 +100,7 @@ run += make_util_map_o
 run += make_util_pmu_bison_o
 run += make_install
 run += make_install_bin
+run += make_install_prefix
 # FIXME 'install-*' commented out till they're fixed
 # run += make_install_doc
 # run += make_install_man
@@ -173,6 +175,12 @@ test_make_install_O     := $(call test_dest_files,$(installed_files_all))
 test_make_install_bin   := $(call test_dest_files,$(installed_files_bin))
 test_make_install_bin_O := $(call test_dest_files,$(installed_files_bin))
 
+# We prefix all installed files for make_install_prefix
+# with '/tmp/krava' to match installed/prefix-ed files.
+installed_files_all_prefix := $(addprefix /tmp/krava/,$(installed_files_all))
+test_make_install_prefix   := $(call test_dest_files,$(installed_files_all_prefix))
+test_make_install_prefix_O := $(call test_dest_files,$(installed_files_all_prefix))
+
 # FIXME nothing gets installed
 test_make_install_man    := test -f $$TMP_DEST/share/man/man1/perf.1
 test_make_install_man_O  := $(test_make_install_man)
Signature · 1 line
2.1.0
author maintainer
#4 lore raw

PATCH 4/13 perf build: Fix single target build dependency check

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Currently if we build a single target like:

  $ touch util/map.c && make util/map.o

It will not rebuild util/map.o if it already exists and util/map.c is modified.

The reason is that the top-level 'Makefile' processes util/map.o as an implicit rule and if util/map.o exists make considers the 'util/map.o' target as done and will not nest into Makefile.perf.

Adding FORCE for '%', because that's what we want to nest into Makefile.perf for any target.

Adding Makefile into phony targets, because make tries to rebuild it and it's also resolved as '%' target.

---
 tools/perf/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/Makefile +2 -2
index d31a7bbd7cee..480546d5f13b 100644
@@ -83,8 +83,8 @@ build-test:
 #
 # All other targets get passed through:
 #
-%:
+%: FORCE
 	$(print_msg)
 	$(make)
 
-.PHONY: tags TAGS
+.PHONY: tags TAGS FORCE Makefile
Signature · 1 line
2.1.0
author maintainer
#5 lore raw

PATCH 5/13 perf pmu: Use __weak definition from <linux/compiler.h>

In reply to Arnaldo Carvalho de Melo

From: Sukadev Bhattiprolu <[email protected]>

Jiri Olsa pointed out, that the <linux/compiler.h> defines the attribute '__weak'. We might as well use that.

---
 tools/perf/util/pmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
tools/perf/util/pmu.c +2 -1
index 0fcc624eb767..c6b16b1db6d0 100644
@@ -1,4 +1,5 @@
 #include <linux/list.h>
+#include <linux/compiler.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <stdio.h>
@@ -436,7 +437,7 @@ static struct cpu_map *pmu_cpumask(const char *name)
 	return cpus;
 }
 
-struct perf_event_attr *__attribute__((weak))
+struct perf_event_attr * __weak
 perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
 {
 	return NULL;
Signature · 1 line
2.1.0
author maintainer
#6 lore raw

PATCH 6/13 perf pmu: Split perf_pmu__new_alias()

In reply to Arnaldo Carvalho de Melo

From: Sukadev Bhattiprolu <[email protected]>

Separate the event parsing code in perf_pmu__new_alias() out into a separate function __perf_pmu__new_alias() so that code can be called indepdently.

This is based on an earlier patch from Andi Kleen.

---
 tools/perf/util/pmu.c | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)
tools/perf/util/pmu.c +27 -15
index c6b16b1db6d0..7bcb8c315615 100644
@@ -206,17 +206,12 @@ static int perf_pmu__parse_snapshot(struct perf_pmu_alias *alias,
 	return 0;
 }
 
-static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FILE *file)
+static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
+				 char *desc __maybe_unused, char *val)
 {
 	struct perf_pmu_alias *alias;
-	char buf[256];
 	int ret;
 
-	ret = fread(buf, 1, sizeof(buf), file);
-	if (ret == 0)
-		return -EINVAL;
-	buf[ret] = 0;
-
 	alias = malloc(sizeof(*alias));
 	if (!alias)
 		return -ENOMEM;
@@ -226,26 +221,43 @@ static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FI
 	alias->unit[0] = '\0';
 	alias->per_pkg = false;
 
-	ret = parse_events_terms(&alias->terms, buf);
+	ret = parse_events_terms(&alias->terms, val);
 	if (ret) {
+		pr_err("Cannot parse alias %s: %d\n", val, ret);
 		free(alias);
 		return ret;
 	}
 
 	alias->name = strdup(name);
-	/*
-	 * load unit name and scale if available
-	 */
-	perf_pmu__parse_unit(alias, dir, name);
-	perf_pmu__parse_scale(alias, dir, name);
-	perf_pmu__parse_per_pkg(alias, dir, name);
-	perf_pmu__parse_snapshot(alias, dir, name);
+	if (dir) {
+		/*
+		 * load unit name and scale if available
+		 */
+		perf_pmu__parse_unit(alias, dir, name);
+		perf_pmu__parse_scale(alias, dir, name);
+		perf_pmu__parse_per_pkg(alias, dir, name);
+		perf_pmu__parse_snapshot(alias, dir, name);
+	}
 
 	list_add_tail(&alias->list, list);
 
 	return 0;
 }
 
+static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FILE *file)
+{
+	char buf[256];
+	int ret;
+
+	ret = fread(buf, 1, sizeof(buf), file);
+	if (ret == 0)
+		return -EINVAL;
+
+	buf[ret] = 0;
+
+	return __perf_pmu__new_alias(list, dir, name, NULL, buf);
+}
+
 static inline bool pmu_alias_info_file(char *name)
 {
 	size_t len;
Signature · 1 line
2.1.0
author maintainer
#7 lore raw

PATCH 7/13 perf tools: Allow events with dot

In reply to Arnaldo Carvalho de Melo

From: Andi Kleen <[email protected]>

The Intel events use a dot to separate event name and unit mask. Allow dot in names in the scanner, and remove special handling of dot as EOF. Also remove the hack in jevents to replace dot with underscore. This way dotted events can be specified directly by the user.

I'm not fully sure this change to the scanner is correct (what was the dot special case good for?), but I haven't found anything that breaks with it so far at least.

---
 tools/perf/util/parse-events.l | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
tools/perf/util/parse-events.l +2 -3
index 09e738fe9ea2..13cef3c65565 100644
@@ -119,8 +119,8 @@ event		[^,{}/]+
 num_dec		[0-9]+
 num_hex		0x[a-fA-F0-9]+
 num_raw_hex	[a-fA-F0-9]+
-name		[a-zA-Z_*?][a-zA-Z0-9_*?]*
-name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?]*
+name		[a-zA-Z_*?][a-zA-Z0-9_*?.]*
+name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?.]*
 /* If you add a modifier you need to update check_modifier() */
 modifier_event	[ukhpGHSDI]+
 modifier_bp	[rwx]{1,3}
@@ -165,7 +165,6 @@ modifier_bp	[rwx]{1,3}
 			return PE_EVENT_NAME;
 		}
 
-.		|
 <<EOF>>		{
 			BEGIN(INITIAL);
 			REWIND(0);
Signature · 1 line
2.1.0
author maintainer
#8 lore raw

PATCH 8/13 perf session: Print a newline when dumping PERF_RECORD_FINISHED_ROUND

In reply to Arnaldo Carvalho de Melo

From: Adrian Hunter <[email protected]>

With 'perf report -D' the PERF_RECORD_FINISHED_ROUND event was printed without a newline, resulting in:

	0x91a18 [0x8]: PERF_RECORD_FINISHED_ROUNDAggregated stats

Other events print their details, but PERF_RECORD_FINISHED_ROUND doesn't have any so just add a print for a newline.

---
 tools/perf/util/session.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/util/session.c +2 -0
index aa482c10469d..aac1c4cee1e8 100644
@@ -686,6 +686,8 @@ static int process_finished_round(struct perf_tool *tool __maybe_unused,
 				  union perf_event *event __maybe_unused,
 				  struct ordered_events *oe)
 {
+	if (dump_trace)
+		fprintf(stdout, "\n");
 	return ordered_events__flush(oe, OE_FLUSH__ROUND);
 }
 
Signature · 1 line
2.1.0
author maintainer
#9 lore raw

PATCH 9/13 perf tools: Print a newline before dumping Aggregated stats

In reply to Arnaldo Carvalho de Melo

From: Adrian Hunter <[email protected]>

When dumping events with 'perf report -D' the event print always starts with a newline (see dump_event()).

Do the same with the "Aggregated stats" print so that it is not jammed up against the last event print.

---
 tools/perf/util/session.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/session.c +1 -1
index aac1c4cee1e8..b5549b58bb2b 100644
@@ -1728,7 +1728,7 @@ size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp)
 	if (perf_header__has_feat(&session->header, HEADER_AUXTRACE))
 		msg = " (excludes AUX area (e.g. instruction trace) decoded / synthesized events)";
 
-	ret = fprintf(fp, "Aggregated stats:%s\n", msg);
+	ret = fprintf(fp, "\nAggregated stats:%s\n", msg);
 
 	ret += events_stats__fprintf(&session->evlist->stats, fp);
 	return ret;
Signature · 1 line
2.1.0
author maintainer
#10 lore raw

PATCH 10/13 perf probe: Fix failure to probe events on arm

In reply to Arnaldo Carvalho de Melo

From: He Kuang <[email protected]>

Fix failure to probe events on arm, the problem was introduced by commit 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of .text").

For some architectures, the '_etext' label is not in the .text section (in the .notes section for arm/arm64). Labels out of the .text section are not loaded as symbols and we get a zero value when looking up its addresses, which causes all events to be wrongly skipped.

This patch skips checking the text address range when failing to get the address of '_etext' and thus fixes the problem.

The problem can be reproduced on arm as follows:

  # perf probe --add='generic_perform_write'
  generic_perform_write+0 is out of .text, skip it.
  Probe point 'generic_perform_write' not found.
    Error: Failed to add events.

After this patch:

  # perf probe --add='generic_perform_write'
  Added new event:
    probe:generic_perform_write (on generic_perform_write)

You can now use it in all perf tools, such as:

perf record -e probe:generic_perform_write -aR sleep 1

---
 tools/perf/util/probe-event.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
tools/perf/util/probe-event.c +5 -1
index 076527b639bd..381f23a443c7 100644
@@ -249,8 +249,12 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
 static bool kprobe_blacklist__listed(unsigned long address);
 static bool kprobe_warn_out_range(const char *symbol, unsigned long address)
 {
+	u64 etext_addr;
+
 	/* Get the address of _etext for checking non-probable text symbol */
-	if (kernel_get_symbol_address_by_name("_etext", false) < address)
+	etext_addr = kernel_get_symbol_address_by_name("_etext", false);
+
+	if (etext_addr != 0 && etext_addr < address)
 		pr_warning("%s is out of .text, skip it.\n", symbol);
 	else if (kprobe_blacklist__listed(address))
 		pr_warning("%s is blacklisted function, skip it.\n", symbol);
Signature · 1 line
2.1.0
author maintainer
#11 lore raw

PATCH 11/13 perf thread_map: Don't access the array entries directly

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Instead provide a method to set the array entries, and another to access the contents.

Cc Adrian Hunter <[email protected]>
Cc Andi Kleen <[email protected]>
Cc David Ahern <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
[ Split providing the set/get accessors from transforming the entries structs ]
---
 tools/perf/builtin-trace.c                  |  4 ++--
 tools/perf/tests/openat-syscall-tp-fields.c |  2 +-
 tools/perf/util/auxtrace.c                  |  4 ++--
 tools/perf/util/event.c                     |  6 +++---
 tools/perf/util/evlist.c                    |  4 ++--
 tools/perf/util/evsel.c                     |  2 +-
 tools/perf/util/thread_map.c                | 24 +++++++++++++-----------
 tools/perf/util/thread_map.h                | 10 ++++++++++
 8 files changed, 34 insertions(+), 22 deletions(-)
tools/perf/builtin-trace.c +2 -2
index de5d277d1ad7..2bf2ca771ca5 100644
@@ -2325,7 +2325,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 	 */
 	if (trace->filter_pids.nr > 0)
 		err = perf_evlist__set_filter_pids(evlist, trace->filter_pids.nr, trace->filter_pids.entries);
-	else if (evlist->threads->map[0] == -1)
+	else if (thread_map__pid(evlist->threads, 0) == -1)
 		err = perf_evlist__set_filter_pid(evlist, getpid());
 
 	if (err < 0) {
@@ -2343,7 +2343,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 	if (forks)
 		perf_evlist__start_workload(evlist);
 
-	trace->multiple_threads = evlist->threads->map[0] == -1 ||
+	trace->multiple_threads = thread_map__pid(evlist->threads, 0) == -1 ||
 				  evlist->threads->nr > 1 ||
 				  perf_evlist__first(evlist)->attr.inherit;
 again:
tools/perf/tests/openat-syscall-tp-fields.c +1 -1
index 6245221479d7..01a19626c846 100644
@@ -45,7 +45,7 @@ int test__syscall_openat_tp_fields(void)
 
 	perf_evsel__config(evsel, &opts);
 
-	evlist->threads->map[0] = getpid();
+	thread_map__set_pid(evlist->threads, 0, getpid());
 
 	err = perf_evlist__open(evlist);
 	if (err < 0) {
tools/perf/util/auxtrace.c +2 -2
index df66966cfde7..3dab006b4a03 100644
@@ -119,12 +119,12 @@ void auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp,
 	if (per_cpu) {
 		mp->cpu = evlist->cpus->map[idx];
 		if (evlist->threads)
-			mp->tid = evlist->threads->map[0];
+			mp->tid = thread_map__pid(evlist->threads, 0);
 		else
 			mp->tid = -1;
 	} else {
 		mp->cpu = -1;
-		mp->tid = evlist->threads->map[idx];
+		mp->tid = thread_map__pid(evlist->threads, idx);
 	}
 }
 
tools/perf/util/event.c +3 -3
index d7d986d8f23e..67a977e5d0ab 100644
@@ -504,7 +504,7 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
 	for (thread = 0; thread < threads->nr; ++thread) {
 		if (__event__synthesize_thread(comm_event, mmap_event,
 					       fork_event,
-					       threads->map[thread], 0,
+					       thread_map__pid(threads, thread), 0,
 					       process, tool, machine,
 					       mmap_data, proc_map_timeout)) {
 			err = -1;
@@ -515,12 +515,12 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
 		 * comm.pid is set to thread group id by
 		 * perf_event__synthesize_comm
 		 */
-		if ((int) comm_event->comm.pid != threads->map[thread]) {
+		if ((int) comm_event->comm.pid != thread_map__pid(threads, thread)) {
 			bool need_leader = true;
 
 			/* is thread group leader in thread_map? */
 			for (j = 0; j < threads->nr; ++j) {
-				if ((int) comm_event->comm.pid == threads->map[j]) {
+				if ((int) comm_event->comm.pid == thread_map__pid(threads, j)) {
 					need_leader = false;
 					break;
 				}
tools/perf/util/evlist.c +2 -2
index 8366511b45f8..d29df901be3e 100644
@@ -548,7 +548,7 @@ static void perf_evlist__set_sid_idx(struct perf_evlist *evlist,
 	else
 		sid->cpu = -1;
 	if (!evsel->system_wide && evlist->threads && thread >= 0)
-		sid->tid = evlist->threads->map[thread];
+		sid->tid = thread_map__pid(evlist->threads, thread);
 	else
 		sid->tid = -1;
 }
@@ -1475,7 +1475,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist, struct target *tar
 				__func__, __LINE__);
 			goto out_close_pipes;
 		}
-		evlist->threads->map[0] = evlist->workload.pid;
+		thread_map__set_pid(evlist->threads, 0, evlist->workload.pid);
 	}
 
 	close(child_ready_pipe[1]);
tools/perf/util/evsel.c +1 -1
index 33449decf7bd..1b56047af96b 100644
@@ -1167,7 +1167,7 @@ retry_sample_id:
 			int group_fd;
 
 			if (!evsel->cgrp && !evsel->system_wide)
-				pid = threads->map[thread];
+				pid = thread_map__pid(threads, thread);
 
 			group_fd = get_group_fd(evsel, cpu, thread);
 retry_open:
tools/perf/util/thread_map.c +13 -11
index f4822bd03709..8c3c3a0751bd 100644
@@ -45,7 +45,7 @@ struct thread_map *thread_map__new_by_pid(pid_t pid)
 	threads = thread_map__alloc(items);
 	if (threads != NULL) {
 		for (i = 0; i < items; i++)
-			threads->map[i] = atoi(namelist[i]->d_name);
+			thread_map__set_pid(threads, i, atoi(namelist[i]->d_name));
 		threads->nr = items;
 	}
 
@@ -61,8 +61,8 @@ struct thread_map *thread_map__new_by_tid(pid_t tid)
 	struct thread_map *threads = thread_map__alloc(1);
 
 	if (threads != NULL) {
-		threads->map[0] = tid;
-		threads->nr	= 1;
+		thread_map__set_pid(threads, 0, tid);
+		threads->nr = 1;
 	}
 
 	return threads;
@@ -123,8 +123,10 @@ struct thread_map *thread_map__new_by_uid(uid_t uid)
 			threads = tmp;
 		}
 
-		for (i = 0; i < items; i++)
-			threads->map[threads->nr + i] = atoi(namelist[i]->d_name);
+		for (i = 0; i < items; i++) {
+			thread_map__set_pid(threads, threads->nr + i,
+					    atoi(namelist[i]->d_name));
+		}
 
 		for (i = 0; i < items; i++)
 			zfree(&namelist[i]);
@@ -201,7 +203,7 @@ static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
 		threads = nt;
 
 		for (i = 0; i < items; i++) {
-			threads->map[j++] = atoi(namelist[i]->d_name);
+			thread_map__set_pid(threads, j++, atoi(namelist[i]->d_name));
 			zfree(&namelist[i]);
 		}
 		threads->nr = total_tasks;
@@ -227,8 +229,8 @@ struct thread_map *thread_map__new_dummy(void)
 	struct thread_map *threads = thread_map__alloc(1);
 
 	if (threads != NULL) {
-		threads->map[0]	= -1;
-		threads->nr	= 1;
+		thread_map__set_pid(threads, 0, -1);
+		threads->nr = 1;
 	}
 	return threads;
 }
@@ -267,8 +269,8 @@ static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
 			goto out_free_threads;
 
 		threads = nt;
-		threads->map[ntasks - 1] = tid;
-		threads->nr		 = ntasks;
+		thread_map__set_pid(threads, ntasks - 1, tid);
+		threads->nr = ntasks;
 	}
 out:
 	return threads;
@@ -301,7 +303,7 @@ size_t thread_map__fprintf(struct thread_map *threads, FILE *fp)
 	size_t printed = fprintf(fp, "%d thread%s: ",
 				 threads->nr, threads->nr > 1 ? "s" : "");
 	for (i = 0; i < threads->nr; ++i)
-		printed += fprintf(fp, "%s%d", i ? ", " : "", threads->map[i]);
+		printed += fprintf(fp, "%s%d", i ? ", " : "", thread_map__pid(threads, i));
 
 	return printed + fprintf(fp, "\n");
 }
tools/perf/util/thread_map.h +10 -0
index 95313f43cc0f..e22570390470 100644
@@ -27,4 +27,14 @@ static inline int thread_map__nr(struct thread_map *threads)
 	return threads ? threads->nr : 1;
 }
 
+static inline pid_t thread_map__pid(struct thread_map *map, int thread)
+{
+	return map->map[thread];
+}
+
+static inline void
+thread_map__set_pid(struct thread_map *map, int thread, pid_t pid)
+{
+	map->map[thread] = pid;
+}
 #endif	/* __PERF_THREAD_MAP_H */
Signature · 1 line
2.1.0
author maintainer
#12 lore raw

PATCH 12/13 perf thread_map: Change map entries into a struct

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

We need to store command names with the pid. Changing map entries to be a struct holding pid. Process name is coming in shortly.

Cc Adrian Hunter <[email protected]>
Cc Andi Kleen <[email protected]>
Cc David Ahern <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
[ Split providing the set/get accessors from transforming the entries structs ]
---
 tools/perf/util/thread_map.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
tools/perf/util/thread_map.h +7 -3
index e22570390470..b9f40679f589 100644
@@ -4,9 +4,13 @@
 #include <sys/types.h>
 #include <stdio.h>
 
+struct thread_map_data {
+	pid_t    pid;
+};
+
 struct thread_map {
 	int nr;
-	pid_t map[];
+	struct thread_map_data map[];
 };
 
 struct thread_map *thread_map__new_dummy(void);
@@ -29,12 +33,12 @@ static inline int thread_map__nr(struct thread_map *threads)
 
 static inline pid_t thread_map__pid(struct thread_map *map, int thread)
 {
-	return map->map[thread];
+	return map->map[thread].pid;
 }
 
 static inline void
 thread_map__set_pid(struct thread_map *map, int thread, pid_t pid)
 {
-	map->map[thread] = pid;
+	map->map[thread].pid = pid;
 }
 #endif	/* __PERF_THREAD_MAP_H */
Signature · 1 line
2.1.0
#13 lore raw

Re: [PATCH 12/13] perf thread_map: Change map entries into a struct

Quoted 5 lines · Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

We need to store command names with the pid. Changing map entries to be a struct holding pid. Process name is coming in shortly.

Quoted 9 lines
Cc Adrian Hunter <[email protected]>
Cc Andi Kleen <[email protected]>
Cc David Ahern <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
[ Split providing the set/get accessors from transforming the entries structs ]

the comment in [] is wrong..

and you missed following hunk:

---
static struct thread_map *thread_map__realloc(struct thread_map *map, int nr) {
- size_t size = sizeof(*map) + sizeof(pid_t) * nr;
+ size_t size = sizeof(*map) + sizeof(struct thread_map_data) * nr;
---

shouldn't be fatal, it probably has the same or biggger size ATM I'll include the fix in the next version

jirka

Quoted 42 lines
---
 tools/perf/util/thread_map.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
tools/perf/util/thread_map.h +7 -3
index e22570390470..b9f40679f589 100644
@@ -4,9 +4,13 @@
 #include <sys/types.h>
 #include <stdio.h>
 
+struct thread_map_data {
+	pid_t    pid;
+};
+
 struct thread_map {
 	int nr;
-	pid_t map[];
+	struct thread_map_data map[];
 };
 
 struct thread_map *thread_map__new_dummy(void);
@@ -29,12 +33,12 @@ static inline int thread_map__nr(struct thread_map *threads)
 
 static inline pid_t thread_map__pid(struct thread_map *map, int thread)
 {
-	return map->map[thread];
+	return map->map[thread].pid;
 }
 
 static inline void
 thread_map__set_pid(struct thread_map *map, int thread, pid_t pid)
 {
-	map->map[thread] = pid;
+	map->map[thread].pid = pid;
 }
 #endif	/* __PERF_THREAD_MAP_H */
Signature · 1 line
2.1.0
author maintainer
#14 lore raw

Re: [PATCH 12/13] perf thread_map: Change map entries into a struct

Em Thu, Jun 25, 2015 at 10:38:41AM +0200, Jiri Olsa escreveu:

Quoted 27 lines · Arnaldo Carvalho de Melo
Quoted 5 lines

From: Jiri Olsa <[email protected]>

We need to store command names with the pid. Changing map entries to be a struct holding pid. Process name is coming in shortly.

Quoted 9 lines
Cc Adrian Hunter <[email protected]>
Cc Andi Kleen <[email protected]>
Cc David Ahern <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
[ Split providing the set/get accessors from transforming the entries structs ]

the comment in [] is wrong..

and you missed following hunk:

---
static struct thread_map *thread_map__realloc(struct thread_map *map, int nr) {

(quoted diff) +1 -2
-       size_t size = sizeof(*map) + sizeof(pid_t) * nr;
+       size_t size = sizeof(*map) + sizeof(struct thread_map_data) * nr;
---
Quoted 2 lines

shouldn't be fatal, it probably has the same or biggger size ATM I'll include the fix in the next version

Yeah, sorry about that, should have noticed that, but as you said, code wise it is harmless.

- Arnaldo

author maintainer
#15 lore raw

Re: [PATCH 12/13] perf thread_map: Change map entries into a struct

Em Thu, Jun 25, 2015 at 10:45:05AM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 10 lines

Em Thu, Jun 25, 2015 at 10:38:41AM +0200, Jiri Olsa escreveu:

Quoted 8 lines

and you missed following hunk:

---
static struct thread_map *thread_map__realloc(struct thread_map *map, int nr) {

(quoted diff) +1 -2
-       size_t size = sizeof(*map) + sizeof(pid_t) * nr;
+       size_t size = sizeof(*map) + sizeof(struct thread_map_data) * nr;
---
Quoted 5 lines
Quoted 2 lines

shouldn't be fatal, it probably has the same or biggger size ATM I'll include the fix in the next version

Yeah, sorry about that, should have noticed that, but as you said, code wise it is harmless.

Adding this fix for it, should have been there since when I implemented thread_map...

static struct thread_map *thread_map__realloc(struct thread_map *map,
int nr)
{
- size_t size = sizeof(*map) + sizeof(pid_t) * nr;
+ size_t size = sizeof(*map) + sizeof(map->map[0]) * nr;

-----------

[acme@zoo ~]$ cat thread_map.c
#include <sys/types.h>
#include <stdio.h>

struct thread_map_data {
        pid_t    pid;
};
struct thread_map {
        int nr;
        struct thread_map_data map[];
};

int main(void)

{
	struct thread_map *map;

	printf("sizeof(map->map[0]:\t\t%zd\n", sizeof(map->map[0]));
	printf("sizeof(struct thread_map_data):\t%zd\n", sizeof(struct thread_map_data));
	printf("sizeof(pid_t):\t\t\t%zd\n", sizeof(pid_t));
	return 0;
}

[acme@zoo ~]$ make thread_map
cc thread_map.c -o thread_map
[acme@zoo ~]$ ./thread_map
sizeof(map->map[0]: 4
sizeof(struct thread_map_data): 4
sizeof(pid_t): 4
[acme@zoo ~]$ file thread_map
thread_map: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 2.6.32, BuildID[sha1]=a6a9894fb6127e23621ef02e1110ff6635082bcd, not stripped [acme@zoo ~]$

- Arnaldo

#16 lore raw

[tip:perf/urgent] perf tools: Future-proof thread_map allocation size calculation

perf tools: Future-proof thread_map allocation size calculation
Commit-ID:  060664f3b9dff37860e48b5158e8429b2467e526
Gitweb:     http://git.kernel.org/tip/060664f3b9dff37860e48b5158e8429b2467e526
Author:     Arnaldo Carvalho de Melo <[email protected]>
AuthorDate: Thu, 25 Jun 2015 14:48:49 -0300
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 25 Jun 2015 15:15:49 -0300

perf tools: Future-proof thread_map allocation size calculation

---
 tools/perf/util/thread_map.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/thread_map.c +1 -1
index 8c3c3a0..920136d 100644
@@ -22,7 +22,7 @@ static int filter(const struct dirent *dir)
 
 static struct thread_map *thread_map__realloc(struct thread_map *map, int nr)
 {
-	size_t size = sizeof(*map) + sizeof(pid_t) * nr;
+	size_t size = sizeof(*map) + sizeof(map->map[0]) * nr;
 
 	return realloc(map, size);
 }
author maintainer
#17 lore raw

PATCH 13/13 perf tools: Allow auxtrace data alignment

In reply to Arnaldo Carvalho de Melo

From: Adrian Hunter <[email protected]>

Allow auxtrace data to be a multiple of something other than page size. That is needed for BTS where the buffer contains 24-byte records.

---
 tools/perf/util/auxtrace.c | 7 +++++++
 tools/perf/util/auxtrace.h | 1 +
 2 files changed, 8 insertions(+)
tools/perf/util/auxtrace.c +7 -0
index 3dab006b4a03..7e7405c9b936 100644
@@ -1182,6 +1182,13 @@ static int __auxtrace_mmap__read(struct auxtrace_mmap *mm,
 		data2 = NULL;
 	}
 
+	if (itr->alignment) {
+		unsigned int unwanted = len1 % itr->alignment;
+
+		len1 -= unwanted;
+		size -= unwanted;
+	}
+
 	/* padding must be written by fn() e.g. record__process_auxtrace() */
 	padding = size & 7;
 	if (padding)
tools/perf/util/auxtrace.h +1 -0
index a171abbe7301..471aecbc4d68 100644
@@ -303,6 +303,7 @@ struct auxtrace_record {
 				      const char *str);
 	u64 (*reference)(struct auxtrace_record *itr);
 	int (*read_finish)(struct auxtrace_record *itr, int idx);
+	unsigned int alignment;
 };
 
 #ifdef HAVE_AUXTRACE_SUPPORT
Signature · 1 line
2.1.0
#18 lore raw
In reply to Arnaldo Carvalho de Melo
Quoted 88 lines · Arnaldo Carvalho de Melo

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit a9a3cd900fbbcbf837d65653105e7bfc583ced09:

Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2015-06-20 01:11:11 +0200)

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 83b2ea257eb1d43e52f76d756722aeb899a2852c:

perf tools: Allow auxtrace data alignment (2015-06-23 18:28:37 -0300)

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

User visible:

- Move toggling event logic from 'perf top' and into hists browser, allowing freeze/unfreeze with event lists with more than one entry (Namhyung Kim)

- Add missing newlines when dumping PERF_RECORD_FINISHED_ROUND and showing the Aggregated stats in 'perf report -D' (Adrian Hunter)

Infrastructure:

- Allow auxtrace data alignment (Adrian Hunter)

- Allow events with dot (Andi Kleen)

- Fix failure to 'perf probe' events on arm (He Kuang)

- Add testing for Makefile.perf (Jiri Olsa)

- Add test for make install with prefix (Jiri Olsa)

- Fix single target build dependency check (Jiri Olsa)

- Access thread_map entries via accessors, prep patch to hold more info per entry, for ongoing 'perf stat --per-thread' work (Jiri Olsa)

- Use __weak definition from compiler.h (Sukadev Bhattiprolu)

- Split perf_pmu__new_alias() (Sukadev Bhattiprolu)

----------------------------------------------------------------
Adrian Hunter (3):
perf session: Print a newline when dumping PERF_RECORD_FINISHED_ROUND perf tools: Print a newline before dumping Aggregated stats perf tools: Allow auxtrace data alignment

Andi Kleen (1):
perf tools: Allow events with dot

He Kuang (1):
perf probe: Fix failure to probe events on arm

Jiri Olsa (5):
perf tests: Add testing for Makefile.perf
perf tests: Add test for make install with prefix perf build: Fix single target build dependency check perf thread_map: Don't access the array entries directly perf thread_map: Change map entries into a struct

Namhyung Kim (1):
perf top: Move toggling event logic into hists browser

Sukadev Bhattiprolu (2):
perf pmu: Use __weak definition from <linux/compiler.h> perf pmu: Split perf_pmu__new_alias()

 tools/perf/Makefile                         |  4 +--
 tools/perf/builtin-top.c                    | 24 ++-------------
 tools/perf/builtin-trace.c                  |  4 +--
 tools/perf/tests/make                       | 31 ++++++++++++++++++--
 tools/perf/tests/openat-syscall-tp-fields.c |  2 +-
 tools/perf/ui/browsers/hists.c              | 19 ++++++++++--
 tools/perf/util/auxtrace.c                  | 11 +++++--
 tools/perf/util/auxtrace.h                  |  1 +
 tools/perf/util/event.c                     |  6 ++--
 tools/perf/util/evlist.c                    |  4 +--
Quoted 8 lines
 tools/perf/util/evsel.c                     |  2 +-
 tools/perf/util/parse-events.l              |  5 ++--
 tools/perf/util/pmu.c                       | 45 +++++++++++++++++++----------
 tools/perf/util/probe-event.c               |  6 +++-
 tools/perf/util/session.c                   |  4 ++-
 tools/perf/util/thread_map.c                | 24 ++++++++-------
 tools/perf/util/thread_map.h                | 16 +++++++++-
 17 files changed, 136 insertions(+), 72 deletions(-)

Pulled, thanks a lot Arnaldo!

Btw., one small thing I noticed about the status line in perf top: if I ever use 'f' to freeze/unfreeze events, the following message:

Press 'f' to disable the events or 'h' to see other hotkeys

sticks around forever, even after I look into annotation and exit it, etc.

So I don't mind some default, helpful message there (such as 'Press 'h' to see hotkeys'), but it appears this particular message is context and usage sensitive, which wasn't really the goal, right?

Thanks,

Ingo

author maintainer
#19 lore raw

Em Thu, Jun 25, 2015 at 09:31:41AM +0200, Ingo Molnar escreveu:

Quoted 5 lines

Pulled, thanks a lot Arnaldo!

Btw., one small thing I noticed about the status line in perf top: if I ever use 'f' to freeze/unfreeze events, the following message:

Quoted 7 lines

Press 'f' to disable the events or 'h' to see other hotkeys

sticks around forever, even after I look into annotation and exit it, etc.

So I don't mind some default, helpful message there (such as 'Press 'h' to see hotkeys'), but it appears this particular message is context and usage sensitive, which wasn't really the goal, right?

Agreed, some more work is needed to change that message in more places, will do it eventually.

- Arnaldo