[GIT PULL 0/7] perf/core fixes and improvements

[GIT PULL 0/7] perf/core fixes and improvements

[Arnaldo Carvalho de Melo]

Hi Ingo,

        Please consider pulling from:

git://github.com/acmel/linux.git perf/core

	I noticed problems with the filtering by dso/thread code that I'm still
investigating, as well as working suggestions made since the last pull req.

Regards,

- Arnaldo

Arnaldo Carvalho de Melo (3):
  perf hists: Move the dso and thread filters from hist_browser
  perf hists browser: Apply the dso and thread filters when merging new batches
  perf annotate browser: Don't change selection line when returning from callq

David Ahern (4):
  perf script: Fix unknown feature comment
  perf tools: Add prelink suggestion to dso update message
  perf tools: handle endianness of feature bitmap
  perf hists browser: Do not exit on tab key with single event

 tools/perf/util/header.c               |   57 +++++++++++++++++++++++++------
 tools/perf/util/hist.c                 |   57 +++++++++++++++++++++++++++-----
 tools/perf/util/hist.h                 |    9 ++++-
 tools/perf/util/map.c                  |    4 +-
 tools/perf/util/ui/browsers/annotate.c |    2 +-
 tools/perf/util/ui/browsers/hists.c    |   50 +++++++++++++--------------
 6 files changed, 128 insertions(+), 51 deletions(-)

[PATCH 1/7] perf hists: Move the dso and thread filters from hist_browser

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Since with dynamic addition of new hist entries we need to apply those
filters as we merge new batches of hist_entry instances, for instance in
perf top.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-zjhhf8kh9w1buty9p10od6rz@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c              |   10 ++++---
 tools/perf/util/hist.h              |    9 +++++-
 tools/perf/util/ui/browsers/hists.c |   48 ++++++++++++++++-------------------
 3 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 8d15e9f..fdff2a8 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1087,7 +1087,7 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
 	hists__calc_col_len(hists, h);
 }
 
-void hists__filter_by_dso(struct hists *hists, const struct dso *dso)
+void hists__filter_by_dso(struct hists *hists)
 {
 	struct rb_node *nd;
 
@@ -1101,7 +1101,8 @@ void hists__filter_by_dso(struct hists *hists, const struct dso *dso)
 		if (symbol_conf.exclude_other && !h->parent)
 			continue;
 
-		if (dso != NULL && (h->ms.map == NULL || h->ms.map->dso != dso)) {
+		if (hists->dso_filter != NULL &&
+		    (h->ms.map == NULL || h->ms.map->dso != hists->dso_filter)) {
 			h->filtered |= (1 << HIST_FILTER__DSO);
 			continue;
 		}
@@ -1110,7 +1111,7 @@ void hists__filter_by_dso(struct hists *hists, const struct dso *dso)
 	}
 }
 
-void hists__filter_by_thread(struct hists *hists, const struct thread *thread)
+void hists__filter_by_thread(struct hists *hists)
 {
 	struct rb_node *nd;
 
@@ -1121,7 +1122,8 @@ void hists__filter_by_thread(struct hists *hists, const struct thread *thread)
 	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
 		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
 
-		if (thread != NULL && h->thread != thread) {
+		if (hists->thread_filter != NULL &&
+		    h->thread != hists->thread_filter) {
 			h->filtered |= (1 << HIST_FILTER__THREAD);
 			continue;
 		}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index f338cb0..575bcbc 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -43,12 +43,17 @@ enum hist_column {
 	HISTC_NR_COLS, /* Last entry */
 };
 
+struct thread;
+struct dso;
+
 struct hists {
 	struct rb_root		entries_in_array[2];
 	struct rb_root		*entries_in;
 	struct rb_root		entries;
 	struct rb_root		entries_collapsed;
 	u64			nr_entries;
+	const struct thread	*thread_filter;
+	const struct dso	*dso_filter;
 	pthread_mutex_t		lock;
 	struct events_stats	stats;
 	u64			event_stream;
@@ -91,8 +96,8 @@ size_t hists__fprintf(struct hists *self, struct hists *pair,
 int hist_entry__inc_addr_samples(struct hist_entry *self, int evidx, u64 addr);
 int hist_entry__annotate(struct hist_entry *self, size_t privsize);
 
-void hists__filter_by_dso(struct hists *self, const struct dso *dso);
-void hists__filter_by_thread(struct hists *self, const struct thread *thread);
+void hists__filter_by_dso(struct hists *hists);
+void hists__filter_by_thread(struct hists *hists);
 
 u16 hists__col_len(struct hists *self, enum hist_column col);
 void hists__set_col_len(struct hists *self, enum hist_column col, u16 len);
diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index 936e641..94e1ab0 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -24,14 +24,11 @@ struct hist_browser {
 	struct hists	    *hists;
 	struct hist_entry   *he_selection;
 	struct map_symbol   *selection;
-	const struct thread *thread_filter;
-	const struct dso    *dso_filter;
 	bool		     has_symbols;
 };
 
 static int hists__browser_title(struct hists *self, char *bf, size_t size,
-				const char *ev_name, const struct dso *dso,
-				const struct thread *thread);
+				const char *ev_name);
 
 static void hist_browser__refresh_dimensions(struct hist_browser *self)
 {
@@ -307,8 +304,7 @@ static int hist_browser__run(struct hist_browser *self, const char *ev_name,
 	self->b.nr_entries = self->hists->nr_entries;
 
 	hist_browser__refresh_dimensions(self);
-	hists__browser_title(self->hists, title, sizeof(title), ev_name,
-			     self->dso_filter, self->thread_filter);
+	hists__browser_title(self->hists, title, sizeof(title), ev_name);
 
 	if (ui_browser__show(&self->b, title,
 			     "Press '?' for help on key bindings") < 0)
@@ -323,8 +319,7 @@ static int hist_browser__run(struct hist_browser *self, const char *ev_name,
 			timer(arg);
 			ui_browser__update_nr_entries(&self->b, self->hists->nr_entries);
 			hists__browser_title(self->hists, title, sizeof(title),
-					     ev_name, self->dso_filter,
-					     self->thread_filter);
+					     ev_name);
 			ui_browser__show_title(&self->b, title);
 			continue;
 		case 'D': { /* Debug */
@@ -809,11 +804,12 @@ static struct thread *hist_browser__selected_thread(struct hist_browser *self)
 }
 
 static int hists__browser_title(struct hists *self, char *bf, size_t size,
-				const char *ev_name, const struct dso *dso,
-				const struct thread *thread)
+				const char *ev_name)
 {
 	char unit;
 	int printed;
+	const struct dso *dso = self->dso_filter;
+	const struct thread *thread = self->thread_filter;
 	unsigned long nr_events = self->stats.nr_events[PERF_RECORD_SAMPLE];
 
 	nr_events = convert_unit(nr_events, &unit);
@@ -917,9 +913,9 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 				continue;
 			}
 			top = pstack__pop(fstack);
-			if (top == &browser->dso_filter)
+			if (top == &browser->hists->dso_filter)
 				goto zoom_out_dso;
-			if (top == &browser->thread_filter)
+			if (top == &browser->hists->thread_filter)
 				goto zoom_out_thread;
 			continue;
 		}
@@ -947,14 +943,14 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 		if (thread != NULL &&
 		    asprintf(&options[nr_options], "Zoom %s %s(%d) thread",
-			     (browser->thread_filter ? "out of" : "into"),
+			     (browser->hists->thread_filter ? "out of" : "into"),
 			     (thread->comm_set ? thread->comm : ""),
 			     thread->pid) > 0)
 			zoom_thread = nr_options++;
 
 		if (dso != NULL &&
 		    asprintf(&options[nr_options], "Zoom %s %s DSO",
-			     (browser->dso_filter ? "out of" : "into"),
+			     (browser->hists->dso_filter ? "out of" : "into"),
 			     (dso->kernel ? "the Kernel" : dso->short_name)) > 0)
 			zoom_dso = nr_options++;
 
@@ -994,36 +990,36 @@ do_annotate:
 			map__browse(browser->selection->map);
 		else if (choice == zoom_dso) {
 zoom_dso:
-			if (browser->dso_filter) {
-				pstack__remove(fstack, &browser->dso_filter);
+			if (browser->hists->dso_filter) {
+				pstack__remove(fstack, &browser->hists->dso_filter);
 zoom_out_dso:
 				ui_helpline__pop();
-				browser->dso_filter = NULL;
+				browser->hists->dso_filter = NULL;
 			} else {
 				if (dso == NULL)
 					continue;
 				ui_helpline__fpush("To zoom out press <- or -> + \"Zoom out of %s DSO\"",
 						   dso->kernel ? "the Kernel" : dso->short_name);
-				browser->dso_filter = dso;
-				pstack__push(fstack, &browser->dso_filter);
+				browser->hists->dso_filter = dso;
+				pstack__push(fstack, &browser->hists->dso_filter);
 			}
-			hists__filter_by_dso(self, browser->dso_filter);
+			hists__filter_by_dso(self);
 			hist_browser__reset(browser);
 		} else if (choice == zoom_thread) {
 zoom_thread:
-			if (browser->thread_filter) {
-				pstack__remove(fstack, &browser->thread_filter);
+			if (browser->hists->thread_filter) {
+				pstack__remove(fstack, &browser->hists->thread_filter);
 zoom_out_thread:
 				ui_helpline__pop();
-				browser->thread_filter = NULL;
+				browser->hists->thread_filter = NULL;
 			} else {
 				ui_helpline__fpush("To zoom out press <- or -> + \"Zoom out of %s(%d) thread\"",
 						   thread->comm_set ? thread->comm : "",
 						   thread->pid);
-				browser->thread_filter = thread;
-				pstack__push(fstack, &browser->thread_filter);
+				browser->hists->thread_filter = thread;
+				pstack__push(fstack, &browser->hists->thread_filter);
 			}
-			hists__filter_by_thread(self, browser->thread_filter);
+			hists__filter_by_thread(self);
 			hist_browser__reset(browser);
 		}
 	}
-- 
1.6.2.5

[PATCH 2/7] perf hists browser: Apply the dso and thread filters when merging new batches

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Now that we dynamicly add entries on the timer we need to not only
traverse all entries when the user zooms into threads and/or DSOs, but
as well after that apply it to the new batches of hist entries in
hists__collapse_resort.

Reported-by: Mike Galbraith <efault@gmx.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-zustn633c7hnrae94x6nld1p@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c |   55 ++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index fdff2a8..75526d1 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -6,6 +6,11 @@
 #include "sort.h"
 #include <math.h>
 
+static bool hists__filter_entry_by_dso(struct hists *hists,
+				       struct hist_entry *he);
+static bool hists__filter_entry_by_thread(struct hists *hists,
+					  struct hist_entry *he);
+
 enum hist_filter {
 	HIST_FILTER__DSO,
 	HIST_FILTER__THREAD,
@@ -338,6 +343,12 @@ static struct rb_root *hists__get_rotate_entries_in(struct hists *hists)
 	return root;
 }
 
+static void hists__apply_filters(struct hists *hists, struct hist_entry *he)
+{
+	hists__filter_entry_by_dso(hists, he);
+	hists__filter_entry_by_thread(hists, he);
+}
+
 static void __hists__collapse_resort(struct hists *hists, bool threaded)
 {
 	struct rb_root *root;
@@ -356,8 +367,15 @@ static void __hists__collapse_resort(struct hists *hists, bool threaded)
 		next = rb_next(&n->rb_node_in);
 
 		rb_erase(&n->rb_node_in, root);
-		if (hists__collapse_insert_entry(hists, &hists->entries_collapsed, n))
+		if (hists__collapse_insert_entry(hists, &hists->entries_collapsed, n)) {
+			/*
+			 * If it wasn't combined with one of the entries already
+			 * collapsed, we need to apply the filters that may have
+			 * been set by, say, the hist_browser.
+			 */
+			hists__apply_filters(hists, n);
 			hists__inc_nr_entries(hists, n);
+		}
 	}
 }
 
@@ -1087,6 +1105,19 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
 	hists__calc_col_len(hists, h);
 }
 
+
+static bool hists__filter_entry_by_dso(struct hists *hists,
+				       struct hist_entry *he)
+{
+	if (hists->dso_filter != NULL &&
+	    (he->ms.map == NULL || he->ms.map->dso != hists->dso_filter)) {
+		he->filtered |= (1 << HIST_FILTER__DSO);
+		return true;
+	}
+
+	return false;
+}
+
 void hists__filter_by_dso(struct hists *hists)
 {
 	struct rb_node *nd;
@@ -1101,16 +1132,25 @@ void hists__filter_by_dso(struct hists *hists)
 		if (symbol_conf.exclude_other && !h->parent)
 			continue;
 
-		if (hists->dso_filter != NULL &&
-		    (h->ms.map == NULL || h->ms.map->dso != hists->dso_filter)) {
-			h->filtered |= (1 << HIST_FILTER__DSO);
+		if (hists__filter_entry_by_dso(hists, h))
 			continue;
-		}
 
 		hists__remove_entry_filter(hists, h, HIST_FILTER__DSO);
 	}
 }
 
+static bool hists__filter_entry_by_thread(struct hists *hists,
+					  struct hist_entry *he)
+{
+	if (hists->thread_filter != NULL &&
+	    he->thread != hists->thread_filter) {
+		he->filtered |= (1 << HIST_FILTER__THREAD);
+		return true;
+	}
+
+	return false;
+}
+
 void hists__filter_by_thread(struct hists *hists)
 {
 	struct rb_node *nd;
@@ -1122,11 +1162,8 @@ void hists__filter_by_thread(struct hists *hists)
 	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
 		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
 
-		if (hists->thread_filter != NULL &&
-		    h->thread != hists->thread_filter) {
-			h->filtered |= (1 << HIST_FILTER__THREAD);
+		if (hists__filter_entry_by_thread(hists, h))
 			continue;
-		}
 
 		hists__remove_entry_filter(hists, h, HIST_FILTER__THREAD);
 	}
-- 
1.6.2.5

[PATCH 3/7] perf script: Fix unknown feature comment

[Arnaldo Carvalho de Melo]

From: David Ahern <dsahern@gmail.com>

"perf script -v" emits:

unknown feature 3, continuing...
unknown feature 4, continuing...
unknown feature 5, continuing...
unknown feature 6, continuing...
unknown feature 7, continuing...
unknown feature 8, continuing...
unknown feature 9, continuing...
unknown feature 10, continuing...
unknown feature 11, continuing...
unknown feature 12, continuing...
unknown feature 13, continuing...
unknown feature 14, continuing...

These are all new features added by fbe96f2. Update
perf_file_section__process to know they are valid feature ids.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1318984464-20650-1-git-send-email-dsahern@gmail.com
Signed-off-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 2143a32..3724707 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1895,6 +1895,21 @@ static int perf_file_section__process(struct perf_file_section *section,
 		if (perf_header__read_build_ids(ph, fd, section->offset, section->size))
 			pr_debug("Failed to read buildids, continuing...\n");
 		break;
+
+	case HEADER_HOSTNAME:
+	case HEADER_OSRELEASE:
+	case HEADER_VERSION:
+	case HEADER_ARCH:
+	case HEADER_NRCPUS:
+	case HEADER_CPUDESC:
+	case HEADER_CPUID:
+	case HEADER_TOTAL_MEM:
+	case HEADER_CMDLINE:
+	case HEADER_EVENT_DESC:
+	case HEADER_CPU_TOPOLOGY:
+	case HEADER_NUMA_TOPOLOGY:
+		break;
+
 	default:
 		pr_debug("unknown feature %d, continuing...\n", feat);
 	}
-- 
1.6.2.5

[PATCH 4/7] perf tools: Add prelink suggestion to dso update message

[Arnaldo Carvalho de Melo]

From: David Ahern <dsahern@gmail.com>

Following a prelink run mapped files for long running processes can show
as deleted. The current message suggests restarting long running
processes. Add to that a suggestion that prelink might be the cause.

Old message:
/lib64/libc-2.14.so was updated, restart the long running
 apps that use it!

New message:
/lib64/libc-2.14.so was updated (is prelink enabled?).
  Restart the long running apps that use it!

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1318985085-20776-1-git-send-email-dsahern@gmail.com
Signed-off-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/map.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 9cf0d43..78284b1 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -139,8 +139,8 @@ int map__load(struct map *self, symbol_filter_t filter)
 
 		if (len > sizeof(DSO__DELETED) &&
 		    strcmp(name + real_len + 1, DSO__DELETED) == 0) {
-			pr_warning("%.*s was updated, restart the long "
-				   "running apps that use it!\n",
+			pr_warning("%.*s was updated (is prelink enabled?). "
+				"Restart the long running apps that use it!\n",
 				   (int)real_len, name);
 		} else {
 			pr_warning("no symbols found in %s, maybe install "
-- 
1.6.2.5

[PATCH 5/7] perf tools: handle endianness of feature bitmap

[Arnaldo Carvalho de Melo]

From: David Ahern <dsahern@gmail.com>

Feature bitmap is declared as an array of unsigned longs -- not good
since its size can differ between the host that generated the data file
and the host analyzing the file.

We need to handle endianness, but we don't know the size of the unsigned
long where the file was generated. Take a best guess at determining it:
try 64-bit swap first (ie., file created on a 64-bit host), and check if
the hostname feature bit is set (this feature bit is forced on as of
fbe96f2).  If the bit is not, undo the 64-bit swap and try a 32-bit
swap. If the hostname bit is still not set (e.g., older data file), punt
and fallback to the original behavior -- clearing all feature bits and
setting buildid.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1318980841-12616-1-git-send-email-dsahern@gmail.com
Signed-off-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c |   42 +++++++++++++++++++++++++++++++-----------
 1 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 3724707..6a9c041 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1703,21 +1703,41 @@ int perf_file_header__read(struct perf_file_header *header,
 			bitmap_zero(header->adds_features, HEADER_FEAT_BITS);
 		else
 			return -1;
+	} else if (ph->needs_swap) {
+		unsigned int i;
+		/*
+		 * feature bitmap is declared as an array of unsigned longs --
+		 * not good since its size can differ between the host that
+		 * generated the data file and the host analyzing the file.
+		 *
+		 * We need to handle endianness, but we don't know the size of
+		 * the unsigned long where the file was generated. Take a best
+		 * guess at determining it: try 64-bit swap first (ie., file
+		 * created on a 64-bit host), and check if the hostname feature
+		 * bit is set (this feature bit is forced on as of fbe96f2).
+		 * If the bit is not, undo the 64-bit swap and try a 32-bit
+		 * swap. If the hostname bit is still not set (e.g., older data
+		 * file), punt and fallback to the original behavior --
+		 * clearing all feature bits and setting buildid.
+		 */
+		for (i = 0; i < BITS_TO_LONGS(HEADER_FEAT_BITS); ++i)
+			header->adds_features[i] = bswap_64(header->adds_features[i]);
+
+		if (!test_bit(HEADER_HOSTNAME, header->adds_features)) {
+			for (i = 0; i < BITS_TO_LONGS(HEADER_FEAT_BITS); ++i) {
+				header->adds_features[i] = bswap_64(header->adds_features[i]);
+				header->adds_features[i] = bswap_32(header->adds_features[i]);
+			}
+		}
+
+		if (!test_bit(HEADER_HOSTNAME, header->adds_features)) {
+			bitmap_zero(header->adds_features, HEADER_FEAT_BITS);
+			set_bit(HEADER_BUILD_ID, header->adds_features);
+		}
 	}
 
 	memcpy(&ph->adds_features, &header->adds_features,
 	       sizeof(ph->adds_features));
-	/*
-	 * FIXME: hack that assumes that if we need swap the perf.data file
-	 * may be coming from an arch with a different word-size, ergo different
-	 * DEFINE_BITMAP format, investigate more later, but for now its mostly
-	 * safe to assume that we have a build-id section. Trace files probably
-	 * have several other issues in this realm anyway...
-	 */
-	if (ph->needs_swap) {
-		memset(&ph->adds_features, 0, sizeof(ph->adds_features));
-		perf_header__set_feat(ph, HEADER_BUILD_ID);
-	}
 
 	ph->event_offset = header->event_types.offset;
 	ph->event_size   = header->event_types.size;
-- 
1.6.2.5

[PATCH 6/7] perf annotate browser: Don't change selection line when returning from callq

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

When the user navigates to another annotation browser pressing -> on a
'callq' line, on exit (<-) return to the originating 'callq' line.

Reported-by: Ingo Molnar <mingo@elte.hu>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-z5vgver0jgevbiicfndqni5g@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ui/browsers/annotate.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/ui/browsers/annotate.c b/tools/perf/util/ui/browsers/annotate.c
index a2c351c..1a12d8f 100644
--- a/tools/perf/util/ui/browsers/annotate.c
+++ b/tools/perf/util/ui/browsers/annotate.c
@@ -349,7 +349,7 @@ static int annotate_browser__run(struct annotate_browser *self, int evidx,
 				symbol__tui_annotate(target, ms->map, evidx, nr_events,
 						     timer, arg, delay_secs);
 			}
-			break;
+			continue;
 		case NEWT_KEY_LEFT:
 		case NEWT_KEY_ESCAPE:
 		case 'q':
-- 
1.6.2.5

[PATCH 7/7] perf hists browser: Do not exit on tab key with single event

[Arnaldo Carvalho de Melo]

From: David Ahern <dsahern@gmail.com>

TUI help states for multiple event sessions the TAB/UNTAB keys are used
to switch events. For single event sessions (e.g., the default) the tab
key currently causes the tui to exit. Change that to do nothing since
there is not no second event to switch to.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1319045867-12728-1-git-send-email-dsahern@gmail.com
Signed-off-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ui/browsers/hists.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index 94e1ab0..d762875 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -864,6 +864,8 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 		switch (key) {
 		case NEWT_KEY_TAB:
 		case NEWT_KEY_UNTAB:
+			if (nr_events == 1)
+				continue;
 			/*
 			 * Exit the browser, let hists__browser_tree
 			 * go to the next or previous
-- 
1.6.2.5

Re: [GIT PULL 0/7] perf/core fixes and improvements

[Arnaldo Carvalho de Melo]

Em Wed, Oct 19, 2011 at 09:14:57AM +0200, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> > 	The TUI now should be much closer to the old 'perf top' stdio
> > visual/experience, more on that vein in the next pull req.

> Pulled, thanks Arnaldo.
> 
> The way the TUI now follows terminal colors is really nice. There's a 
> few regressions that sneaked in, and there's also still a few rough 
> edges as well:

>  - in callq following if i exit a secondary annotation screen via 'q' 
>    or left-arrow, it does not jump back to the callq line as it did 
>    earlier.

Oh, I noticed this at some point, but couldn't quickly reproduce as I
was concentrated on something else, but I think its case of using
'continue' versus 'break' when coming back from the nested annotation
browser, so that we don't change the current selection, will check that
now.
 
>  - it's still hard to find all the callq's in the stream of assembly,
>    i think it should be highlighted in a minimal fashion.

I'll use a graphic '->', like » or something, have to look at the
libslang docs.
 
>  - the mixed assembly and source code output for annotation now 
>    became *harder* to follow, that the instruction opcodes are not 
>    embedded. The reason is that there's now fewer visual patterns 
>    that set apart the two types of lines.
> 
>    Not sure what to do about it, but it's not really usable this way, 
>    to me at least. Color differentiation would certainly help, if 
>    it's not too intrusive - could assembly be drawn grey? That would 
>    put it into the 'visual background' on most terminal color 
>    schemes.

I'll make that configurable so that we can play a bit with it and then
decide on some sane default.
 
>    I tried a few mockup screens of splitting the screen 
>    intelligently, and found one variant that works pretty well for 
>    me. The main UI design complication is that the assembly opcodes 
>    look so C source code-ish when put next to each other.
> 
>    So this is the original output:
> 
>          :        static u8 kallsyms2elf_type(char type)                                            ▒
>          :        {                                                                                 ▒
>          :                if (type == 'W')                                                          ▒
>     0.00 :          43fd18:       mov    %rdx,%rdi                                                  ▒
>          :                struct rb_node **p = &symbols->rb_node;                                   ▒
>          :                struct rb_node *parent = NULL;                                            ▒
>          :                const u64 ip = sym->start;                                                ▒
>          :                struct symbol *s;                                                         ▒
>          :                                                                                          ▒
>          :                while (*p != NULL) {                                                      ▒
>     0.00 :          43fd1b:       mov    (%rcx),%rdx                                                ▒
>     0.00 :          43fd1e:       test   %rdx,%rdx                                                  ▒
>     0.00 :          43fd21:       jne    43fd08 <map__process_kallsym_symbol+0xc8>                  ▒
> 
> 
>    and here's the mockup:
> 
>         .                              | static u8 kallsyms2elf_type(char type)                     ▒
>         .                              | {                                                          ▒
>         .                              |         if (type == 'W')                                   ▒
>    0.00 #  43fd18: mov    %rdx,%rdi                                                                 ▒
>         .                              |         struct rb_node **p = &symbols->rb_node;            ▒
>         .                              |         struct rb_node *parent = NULL;                     ▒
>         .                              |         const u64 ip = sym->start;                         ▒
>         .                              |         struct symbol *s;                                  ▒
>         .                              |                                                            ▒
>         .                              |         while (*p != NULL) {                               ▒
>    0.00 #  43fd1b: mov    (%rcx),%rdx                                                               ▒
>    0.00 #  43fd1e: test   %rdx,%rdx                                                                 ▒
>    0.00 #  43fd21: jne    43fd08 <map__process_kallsym_symbol+0xc8>                                 ▒
> 
> There's several UI tricks:
> 
>  - typical short opcodes (80% of assembly) will fit on the left side 
>    of the screen.

right
 
>  - lines can still be arbitrarily long and overlap, so it's not a 
>    true split screen - but the vertical helper line prefixing source 
>    code lines keeps the eye focused on whichever side one intends to 
>    concentrate on.

Ok, I'll play with that.

	
>  - the first column separator uses two types of characters, '.' and 
>    '#', to help the eye find the blocks of assembly.

>  - we could, in addition, print assembly in grey.
> 
>  - i cut one character from the percentage column - the maximum value 
>    is 100.00 so we don't need the original 7.2 format, 6.2 is enough.

OK, after the first there was some 8 spaces that came from how it was
done long ago.
 
> We could eventually further compress the assembly display later on, 
> but auto-labeling function-local labels (which are 99% of the jump 
> targets). This would compress such jumps:
> 
>    0.00 #  43fd21: jne    43fd08 <map__process_kallsym_symbol+0xc8>
> 
> into:
> 
>    0.00 #  43fd21: jne    43fd08 <L3>

Yeah, this is something definetely in the plans, together with jumping
to labels, etc.
 
> Thanks,
> 
> 	Ingo

Re: [GIT PULL 0/7] perf/core fixes and improvements

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:

>         Please consider pulling from:
> 
> git://github.com/acmel/linux.git perf/core
> 
> 	The TUI now should be much closer to the old 'perf top' stdio
> visual/experience, more on that vein in the next pull req.
> 
> - Arnaldo
> 
> Arnaldo Carvalho de Melo (7):
>   perf hists browser: Add missing hotkeys to the help window
>   perf tui: Catch signals to exit gracefully
>   perf ui browser: Allow initial use without navigation UI elements
>   perf hists: Don't format the percentage on hist_entry__snprintf
>   perf tui: Remove unneeded call to newtCls on startup
>   perf ui browser: Make the colors configurable and change the defaults
>   perf top tui: Give color hints just on the percentage, like on --stdio
> 
>  tools/perf/Documentation/perfconfig.example |   20 ++++
>  tools/perf/util/hist.c                      |   29 ++++--
>  tools/perf/util/hist.h                      |    4 +-
>  tools/perf/util/ui/browser.c                |  127 ++++++++++++++++++++++-----
>  tools/perf/util/ui/browser.h                |    2 +
>  tools/perf/util/ui/browsers/annotate.c      |    6 ++
>  tools/perf/util/ui/browsers/hists.c         |   51 ++++++-----
>  tools/perf/util/ui/setup.c                  |   25 +++++-
>  8 files changed, 203 insertions(+), 61 deletions(-)
>  create mode 100644 tools/perf/Documentation/perfconfig.example

Pulled, thanks Arnaldo.

The way the TUI now follows terminal colors is really nice. There's a 
few regressions that sneaked in, and there's also still a few rough 
edges as well:

 - in callq following if i exit a secondary annotation screen via 'q' 
   or left-arrow, it does not jump back to the callq line as it did 
   earlier.

 - it's still hard to find all the callq's in the stream of assembly,
   i think it should be highlighted in a minimal fashion.

 - the mixed assembly and source code output for annotation now 
   became *harder* to follow, that the instruction opcodes are not 
   embedded. The reason is that there's now fewer visual patterns 
   that set apart the two types of lines.

   Not sure what to do about it, but it's not really usable this way, 
   to me at least. Color differentiation would certainly help, if 
   it's not too intrusive - could assembly be drawn grey? That would 
   put it into the 'visual background' on most terminal color 
   schemes.

   I tried a few mockup screens of splitting the screen 
   intelligently, and found one variant that works pretty well for 
   me. The main UI design complication is that the assembly opcodes 
   look so C source code-ish when put next to each other.

   So this is the original output:

         :        static u8 kallsyms2elf_type(char type)                                            ▒
         :        {                                                                                 ▒
         :                if (type == 'W')                                                          ▒
    0.00 :          43fd18:       mov    %rdx,%rdi                                                  ▒
         :                struct rb_node **p = &symbols->rb_node;                                   ▒
         :                struct rb_node *parent = NULL;                                            ▒
         :                const u64 ip = sym->start;                                                ▒
         :                struct symbol *s;                                                         ▒
         :                                                                                          ▒
         :                while (*p != NULL) {                                                      ▒
    0.00 :          43fd1b:       mov    (%rcx),%rdx                                                ▒
    0.00 :          43fd1e:       test   %rdx,%rdx                                                  ▒
    0.00 :          43fd21:       jne    43fd08 <map__process_kallsym_symbol+0xc8>                  ▒


   and here's the mockup:

        .                              | static u8 kallsyms2elf_type(char type)                     ▒
        .                              | {                                                          ▒
        .                              |         if (type == 'W')                                   ▒
   0.00 #  43fd18: mov    %rdx,%rdi                                                                 ▒
        .                              |         struct rb_node **p = &symbols->rb_node;            ▒
        .                              |         struct rb_node *parent = NULL;                     ▒
        .                              |         const u64 ip = sym->start;                         ▒
        .                              |         struct symbol *s;                                  ▒
        .                              |                                                            ▒
        .                              |         while (*p != NULL) {                               ▒
   0.00 #  43fd1b: mov    (%rcx),%rdx                                                               ▒
   0.00 #  43fd1e: test   %rdx,%rdx                                                                 ▒
   0.00 #  43fd21: jne    43fd08 <map__process_kallsym_symbol+0xc8>                                 ▒

There's several UI tricks:

 - typical short opcodes (80% of assembly) will fit on the left side 
   of the screen.

 - lines can still be arbitrarily long and overlap, so it's not a 
   true split screen - but the vertical helper line prefixing source 
   code lines keeps the eye focused on whichever side one intends to 
   concentrate on.

 - the first column separator uses two types of characters, '.' and 
   '#', to help the eye find the blocks of assembly.

 - we could, in addition, print assembly in grey.

 - i cut one character from the percentage column - the maximum value 
   is 100.00 so we don't need the original 7.2 format, 6.2 is enough.

We could eventually further compress the assembly display later on, 
but auto-labeling function-local labels (which are 99% of the jump 
targets). This would compress such jumps:

   0.00 #  43fd21: jne    43fd08 <map__process_kallsym_symbol+0xc8>

into:

   0.00 #  43fd21: jne    43fd08 <L3>

Thanks,

	Ingo

[GIT PULL 0/7] perf/core fixes and improvements

[Arnaldo Carvalho de Melo]

        Please consider pulling from:

git://github.com/acmel/linux.git perf/core

	The TUI now should be much closer to the old 'perf top' stdio
visual/experience, more on that vein in the next pull req.

- Arnaldo

Arnaldo Carvalho de Melo (7):
  perf hists browser: Add missing hotkeys to the help window
  perf tui: Catch signals to exit gracefully
  perf ui browser: Allow initial use without navigation UI elements
  perf hists: Don't format the percentage on hist_entry__snprintf
  perf tui: Remove unneeded call to newtCls on startup
  perf ui browser: Make the colors configurable and change the defaults
  perf top tui: Give color hints just on the percentage, like on --stdio

 tools/perf/Documentation/perfconfig.example |   20 ++++
 tools/perf/util/hist.c                      |   29 ++++--
 tools/perf/util/hist.h                      |    4 +-
 tools/perf/util/ui/browser.c                |  127 ++++++++++++++++++++++-----
 tools/perf/util/ui/browser.h                |    2 +
 tools/perf/util/ui/browsers/annotate.c      |    6 ++
 tools/perf/util/ui/browsers/hists.c         |   51 ++++++-----
 tools/perf/util/ui/setup.c                  |   25 +++++-
 8 files changed, 203 insertions(+), 61 deletions(-)
 create mode 100644 tools/perf/Documentation/perfconfig.example