All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/19] perf hists: Mark entries filtered by parent
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 02/19] perf sort: Make column width code per hists instance Arnaldo Carvalho de Melo
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

And don't consider them in hists__inc_nr_entries.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 7b5848c..d998d1d 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -5,6 +5,12 @@
 #include "sort.h"
 #include <math.h>
 
+enum hist_filter {
+	HIST_FILTER__DSO,
+	HIST_FILTER__THREAD,
+	HIST_FILTER__PARENT,
+};
+
 struct callchain_param	callchain_param = {
 	.mode	= CHAIN_GRAPH_REL,
 	.min_percent = 0.5
@@ -52,11 +58,20 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
 
 static void hists__inc_nr_entries(struct hists *self, struct hist_entry *entry)
 {
+	if (entry->filtered)
+		return;
 	if (entry->ms.sym && self->max_sym_namelen < entry->ms.sym->namelen)
 		self->max_sym_namelen = entry->ms.sym->namelen;
 	++self->nr_entries;
 }
 
+static u8 symbol__parent_filter(const struct symbol *parent)
+{
+	if (symbol_conf.exclude_other && parent == NULL)
+		return 1 << HIST_FILTER__PARENT;
+	return 0;
+}
+
 struct hist_entry *__hists__add_entry(struct hists *self,
 				      struct addr_location *al,
 				      struct symbol *sym_parent, u64 period)
@@ -75,6 +90,7 @@ struct hist_entry *__hists__add_entry(struct hists *self,
 		.level	= al->level,
 		.period	= period,
 		.parent = sym_parent,
+		.filtered = symbol__parent_filter(sym_parent),
 	};
 	int cmp;
 
@@ -790,11 +806,6 @@ print_entries:
 	return ret;
 }
 
-enum hist_filter {
-	HIST_FILTER__DSO,
-	HIST_FILTER__THREAD,
-};
-
 static void hists__remove_entry_filter(struct hists *self, struct hist_entry *h,
 				       enum hist_filter filter)
 {
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 02/19] perf sort: Make column width code per hists instance
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
  2010-08-02  1:08 ` [PATCH 01/19] perf hists: Mark entries filtered by parent Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 03/19] perf ui: Restore SPACE as an alias to PGDN in annotate Arnaldo Carvalho de Melo
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

They were globals, and since we support multiple hists and sessions
at the same time, it doesn't make sense to calculate those values
considering all symbols in all sessions.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c  |   34 ++++++-------
 tools/perf/util/hist.c   |  115 +++++++++++++++++++++++++++++++--------------
 tools/perf/util/hist.h   |   27 ++++++++--
 tools/perf/util/newt.c   |    9 ++--
 tools/perf/util/sort.c   |   17 +++----
 tools/perf/util/sort.h   |    6 +--
 tools/perf/util/symbol.c |    9 ++++
 tools/perf/util/symbol.h |    2 +
 8 files changed, 140 insertions(+), 79 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index d7f21d7..121339f 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -340,30 +340,29 @@ int event__synthesize_kernel_mmap(event__handler_t process,
 	return process(&ev, session);
 }
 
-static void thread__comm_adjust(struct thread *self)
+static void thread__comm_adjust(struct thread *self, struct hists *hists)
 {
 	char *comm = self->comm;
 
 	if (!symbol_conf.col_width_list_str && !symbol_conf.field_sep &&
 	    (!symbol_conf.comm_list ||
 	     strlist__has_entry(symbol_conf.comm_list, comm))) {
-		unsigned int slen = strlen(comm);
+		u16 slen = strlen(comm);
 
-		if (slen > comms__col_width) {
-			comms__col_width = slen;
-			threads__col_width = slen + 6;
-		}
+		if (hists__new_col_len(hists, HISTC_COMM, slen))
+			hists__set_col_len(hists, HISTC_THREAD, slen + 6);
 	}
 }
 
-static int thread__set_comm_adjust(struct thread *self, const char *comm)
+static int thread__set_comm_adjust(struct thread *self, const char *comm,
+				   struct hists *hists)
 {
 	int ret = thread__set_comm(self, comm);
 
 	if (ret)
 		return ret;
 
-	thread__comm_adjust(self);
+	thread__comm_adjust(self, hists);
 
 	return 0;
 }
@@ -374,7 +373,8 @@ int event__process_comm(event_t *self, struct perf_session *session)
 
 	dump_printf(": %s:%d\n", self->comm.comm, self->comm.tid);
 
-	if (thread == NULL || thread__set_comm_adjust(thread, self->comm.comm)) {
+	if (thread == NULL || thread__set_comm_adjust(thread, self->comm.comm,
+						      &session->hists)) {
 		dump_printf("problem processing PERF_RECORD_COMM, skipping event.\n");
 		return -1;
 	}
@@ -641,16 +641,13 @@ void thread__find_addr_location(struct thread *self,
 		al->sym = NULL;
 }
 
-static void dso__calc_col_width(struct dso *self)
+static void dso__calc_col_width(struct dso *self, struct hists *hists)
 {
 	if (!symbol_conf.col_width_list_str && !symbol_conf.field_sep &&
 	    (!symbol_conf.dso_list ||
 	     strlist__has_entry(symbol_conf.dso_list, self->name))) {
-		u16 slen = self->short_name_len;
-		if (verbose)
-			slen = self->long_name_len;
-		if (dsos__col_width < slen)
-			dsos__col_width = slen;
+		u16 slen = dso__name_len(self);
+		hists__new_col_len(hists, HISTC_DSO, slen);
 	}
 
 	self->slen_calculated = 1;
@@ -729,16 +726,17 @@ int event__preprocess_sample(const event_t *self, struct perf_session *session,
 		 * sampled.
 		 */
 		if (!sort_dso.elide && !al->map->dso->slen_calculated)
-			dso__calc_col_width(al->map->dso);
+			dso__calc_col_width(al->map->dso, &session->hists);
 
 		al->sym = map__find_symbol(al->map, al->addr, filter);
 	} else {
 		const unsigned int unresolved_col_width = BITS_PER_LONG / 4;
 
-		if (dsos__col_width < unresolved_col_width &&
+		if (hists__col_len(&session->hists, HISTC_DSO) < unresolved_col_width &&
 		    !symbol_conf.col_width_list_str && !symbol_conf.field_sep &&
 		    !symbol_conf.dso_list)
-			dsos__col_width = unresolved_col_width;
+			hists__set_col_len(&session->hists, HISTC_DSO,
+					   unresolved_col_width);
 	}
 
 	if (symbol_conf.sym_list && al->sym &&
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index d998d1d..0bc6790 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -16,6 +16,50 @@ struct callchain_param	callchain_param = {
 	.min_percent = 0.5
 };
 
+u16 hists__col_len(struct hists *self, enum hist_column col)
+{
+	return self->col_len[col];
+}
+
+void hists__set_col_len(struct hists *self, enum hist_column col, u16 len)
+{
+	self->col_len[col] = len;
+}
+
+bool hists__new_col_len(struct hists *self, enum hist_column col, u16 len)
+{
+	if (len > hists__col_len(self, col)) {
+		hists__set_col_len(self, col, len);
+		return true;
+	}
+	return false;
+}
+
+static void hists__reset_col_len(struct hists *self)
+{
+	enum hist_column col;
+
+	for (col = 0; col < HISTC_NR_COLS; ++col)
+		hists__set_col_len(self, col, 0);
+}
+
+static void hists__calc_col_len(struct hists *self, struct hist_entry *h)
+{
+	u16 len;
+
+	if (h->ms.sym)
+		hists__new_col_len(self, HISTC_SYMBOL, h->ms.sym->namelen);
+
+	len = thread__comm_len(h->thread);
+	if (hists__new_col_len(self, HISTC_COMM, len))
+		hists__set_col_len(self, HISTC_THREAD, len + 6);
+
+	if (h->ms.map) {
+		len = dso__name_len(h->ms.map->dso);
+		hists__new_col_len(self, HISTC_DSO, len);
+	}
+}
+
 static void hist_entry__add_cpumode_period(struct hist_entry *self,
 					   unsigned int cpumode, u64 period)
 {
@@ -56,13 +100,12 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
 	return self;
 }
 
-static void hists__inc_nr_entries(struct hists *self, struct hist_entry *entry)
+static void hists__inc_nr_entries(struct hists *self, struct hist_entry *h)
 {
-	if (entry->filtered)
-		return;
-	if (entry->ms.sym && self->max_sym_namelen < entry->ms.sym->namelen)
-		self->max_sym_namelen = entry->ms.sym->namelen;
-	++self->nr_entries;
+	if (!h->filtered) {
+		hists__calc_col_len(self, h);
+		++self->nr_entries;
+	}
 }
 
 static u8 symbol__parent_filter(const struct symbol *parent)
@@ -208,7 +251,7 @@ void hists__collapse_resort(struct hists *self)
 	tmp = RB_ROOT;
 	next = rb_first(&self->entries);
 	self->nr_entries = 0;
-	self->max_sym_namelen = 0;
+	hists__reset_col_len(self);
 
 	while (next) {
 		n = rb_entry(next, struct hist_entry, rb_node);
@@ -265,7 +308,7 @@ void hists__output_resort(struct hists *self)
 	next = rb_first(&self->entries);
 
 	self->nr_entries = 0;
-	self->max_sym_namelen = 0;
+	hists__reset_col_len(self);
 
 	while (next) {
 		n = rb_entry(next, struct hist_entry, rb_node);
@@ -532,8 +575,9 @@ static size_t hist_entry_callchain__fprintf(FILE *fp, struct hist_entry *self,
 }
 
 int hist_entry__snprintf(struct hist_entry *self, char *s, size_t size,
-			 struct hists *pair_hists, bool show_displacement,
-			 long displacement, bool color, u64 session_total)
+			 struct hists *hists, struct hists *pair_hists,
+			 bool show_displacement, long displacement,
+			 bool color, u64 session_total)
 {
 	struct sort_entry *se;
 	u64 period, total, period_sys, period_us, period_guest_sys, period_guest_us;
@@ -637,24 +681,25 @@ int hist_entry__snprintf(struct hist_entry *self, char *s, size_t size,
 
 		ret += snprintf(s + ret, size - ret, "%s", sep ?: "  ");
 		ret += se->se_snprintf(self, s + ret, size - ret,
-				       se->se_width ? *se->se_width : 0);
+				       hists__col_len(hists, se->se_width_idx));
 	}
 
 	return ret;
 }
 
-int hist_entry__fprintf(struct hist_entry *self, struct hists *pair_hists,
-			bool show_displacement, long displacement, FILE *fp,
-			u64 session_total)
+int hist_entry__fprintf(struct hist_entry *self, struct hists *hists,
+			struct hists *pair_hists, bool show_displacement,
+			long displacement, FILE *fp, u64 session_total)
 {
 	char bf[512];
-	hist_entry__snprintf(self, bf, sizeof(bf), pair_hists,
+	hist_entry__snprintf(self, bf, sizeof(bf), hists, pair_hists,
 			     show_displacement, displacement,
 			     true, session_total);
 	return fprintf(fp, "%s\n", bf);
 }
 
-static size_t hist_entry__fprintf_callchain(struct hist_entry *self, FILE *fp,
+static size_t hist_entry__fprintf_callchain(struct hist_entry *self,
+					    struct hists *hists, FILE *fp,
 					    u64 session_total)
 {
 	int left_margin = 0;
@@ -662,7 +707,7 @@ static size_t hist_entry__fprintf_callchain(struct hist_entry *self, FILE *fp,
 	if (sort__first_dimension == SORT_COMM) {
 		struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
 							 typeof(*se), list);
-		left_margin = se->se_width ? *se->se_width : 0;
+		left_margin = hists__col_len(hists, se->se_width_idx);
 		left_margin -= thread__comm_len(self->thread);
 	}
 
@@ -733,17 +778,17 @@ size_t hists__fprintf(struct hists *self, struct hists *pair,
 			continue;
 		}
 		width = strlen(se->se_header);
-		if (se->se_width) {
-			if (symbol_conf.col_width_list_str) {
-				if (col_width) {
-					*se->se_width = atoi(col_width);
-					col_width = strchr(col_width, ',');
-					if (col_width)
-						++col_width;
-				}
+		if (symbol_conf.col_width_list_str) {
+			if (col_width) {
+				hists__set_col_len(self, se->se_width_idx,
+						   atoi(col_width));
+				col_width = strchr(col_width, ',');
+				if (col_width)
+					++col_width;
 			}
-			width = *se->se_width = max(*se->se_width, width);
 		}
+		if (!hists__new_col_len(self, se->se_width_idx, width))
+			width = hists__col_len(self, se->se_width_idx);
 		fprintf(fp, "  %*s", width, se->se_header);
 	}
 	fprintf(fp, "\n");
@@ -766,9 +811,8 @@ size_t hists__fprintf(struct hists *self, struct hists *pair,
 			continue;
 
 		fprintf(fp, "  ");
-		if (se->se_width)
-			width = *se->se_width;
-		else
+		width = hists__col_len(self, se->se_width_idx);
+		if (width == 0)
 			width = strlen(se->se_header);
 		for (i = 0; i < width; i++)
 			fprintf(fp, ".");
@@ -788,12 +832,12 @@ print_entries:
 				displacement = 0;
 			++position;
 		}
-		ret += hist_entry__fprintf(h, pair, show_displacement,
+		ret += hist_entry__fprintf(h, self, pair, show_displacement,
 					   displacement, fp, self->stats.total_period);
 
 		if (symbol_conf.use_callchain)
-			ret += hist_entry__fprintf_callchain(h, fp, self->stats.total_period);
-
+			ret += hist_entry__fprintf_callchain(h, self, fp,
+							     self->stats.total_period);
 		if (h->ms.map == NULL && verbose > 1) {
 			__map_groups__fprintf_maps(&h->thread->mg,
 						   MAP__FUNCTION, verbose, fp);
@@ -817,8 +861,7 @@ static void hists__remove_entry_filter(struct hists *self, struct hist_entry *h,
 	self->stats.total_period += h->period;
 	self->stats.nr_events[PERF_RECORD_SAMPLE] += h->nr_events;
 
-	if (h->ms.sym && self->max_sym_namelen < h->ms.sym->namelen)
-		self->max_sym_namelen = h->ms.sym->namelen;
+	hists__calc_col_len(self, h);
 }
 
 void hists__filter_by_dso(struct hists *self, const struct dso *dso)
@@ -827,7 +870,7 @@ void hists__filter_by_dso(struct hists *self, const struct dso *dso)
 
 	self->nr_entries = self->stats.total_period = 0;
 	self->stats.nr_events[PERF_RECORD_SAMPLE] = 0;
-	self->max_sym_namelen = 0;
+	hists__reset_col_len(self);
 
 	for (nd = rb_first(&self->entries); nd; nd = rb_next(nd)) {
 		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
@@ -850,7 +893,7 @@ void hists__filter_by_thread(struct hists *self, const struct thread *thread)
 
 	self->nr_entries = self->stats.total_period = 0;
 	self->stats.nr_events[PERF_RECORD_SAMPLE] = 0;
-	self->max_sym_namelen = 0;
+	hists__reset_col_len(self);
 
 	for (nd = rb_first(&self->entries); nd; nd = rb_next(nd)) {
 		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 83fa33a..92962b2 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -56,6 +56,16 @@ struct events_stats {
 	u32 nr_unknown_events;
 };
 
+enum hist_column {
+	HISTC_SYMBOL,
+	HISTC_DSO,
+	HISTC_THREAD,
+	HISTC_COMM,
+	HISTC_PARENT,
+	HISTC_CPU,
+	HISTC_NR_COLS, /* Last entry */
+};
+
 struct hists {
 	struct rb_node		rb_node;
 	struct rb_root		entries;
@@ -64,7 +74,7 @@ struct hists {
 	u64			config;
 	u64			event_stream;
 	u32			type;
-	u32			max_sym_namelen;
+	u16			col_len[HISTC_NR_COLS];
 };
 
 struct hist_entry *__hists__add_entry(struct hists *self,
@@ -72,12 +82,13 @@ struct hist_entry *__hists__add_entry(struct hists *self,
 				      struct symbol *parent, u64 period);
 extern int64_t hist_entry__cmp(struct hist_entry *, struct hist_entry *);
 extern int64_t hist_entry__collapse(struct hist_entry *, struct hist_entry *);
-int hist_entry__fprintf(struct hist_entry *self, struct hists *pair_hists,
-			bool show_displacement, long displacement, FILE *fp,
-			u64 total);
+int hist_entry__fprintf(struct hist_entry *self, struct hists *hists,
+			struct hists *pair_hists, bool show_displacement,
+			long displacement, FILE *fp, u64 total);
 int hist_entry__snprintf(struct hist_entry *self, char *bf, size_t size,
-			 struct hists *pair_hists, bool show_displacement,
-			 long displacement, bool color, u64 total);
+			 struct hists *hists, struct hists *pair_hists,
+			 bool show_displacement, long displacement,
+			 bool color, u64 total);
 void hist_entry__free(struct hist_entry *);
 
 void hists__output_resort(struct hists *self);
@@ -95,6 +106,10 @@ int hist_entry__annotate(struct hist_entry *self, struct list_head *head);
 void hists__filter_by_dso(struct hists *self, const struct dso *dso);
 void hists__filter_by_thread(struct hists *self, const struct thread *thread);
 
+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);
+bool hists__new_col_len(struct hists *self, enum hist_column col, u16 len);
+
 #ifdef NO_NEWT_SUPPORT
 static inline int hists__browse(struct hists *self __used,
 				const char *helpline __used,
diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index 7979003..ab6eb36 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -692,7 +692,8 @@ static void hist_entry__append_callchain_browser(struct hist_entry *self,
 }
 
 static size_t hist_entry__append_browser(struct hist_entry *self,
-					 newtComponent tree, u64 total)
+					 newtComponent tree,
+					 struct hists *hists)
 {
 	char s[256];
 	size_t ret;
@@ -700,8 +701,8 @@ static size_t hist_entry__append_browser(struct hist_entry *self,
 	if (symbol_conf.exclude_other && !self->parent)
 		return 0;
 
-	ret = hist_entry__snprintf(self, s, sizeof(s), NULL,
-				   false, 0, false, total);
+	ret = hist_entry__snprintf(self, s, sizeof(s), hists, NULL,
+				   false, 0, false, hists->stats.total_period);
 	if (symbol_conf.use_callchain) {
 		int indexes[2];
 
@@ -842,7 +843,7 @@ static int hist_browser__populate(struct hist_browser *self, struct hists *hists
 		if (h->filtered)
 			continue;
 
-		len = hist_entry__append_browser(h, self->tree, hists->stats.total_period);
+		len = hist_entry__append_browser(h, self->tree, hists);
 		if (len > max_len)
 			max_len = len;
 		if (symbol_conf.use_callchain)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index c27b4b0..1c61a4f 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1,4 +1,5 @@
 #include "sort.h"
+#include "hist.h"
 
 regex_t		parent_regex;
 const char	default_parent_pattern[] = "^sys_|^do_page_fault";
@@ -10,11 +11,6 @@ int		sort__has_parent = 0;
 
 enum sort_type	sort__first_dimension;
 
-unsigned int dsos__col_width;
-unsigned int comms__col_width;
-unsigned int threads__col_width;
-unsigned int cpus__col_width;
-static unsigned int parent_symbol__col_width;
 char * field_sep;
 
 LIST_HEAD(hist_entry__sort_list);
@@ -36,7 +32,7 @@ struct sort_entry sort_thread = {
 	.se_header	= "Command:  Pid",
 	.se_cmp		= sort__thread_cmp,
 	.se_snprintf	= hist_entry__thread_snprintf,
-	.se_width	= &threads__col_width,
+	.se_width_idx	= HISTC_THREAD,
 };
 
 struct sort_entry sort_comm = {
@@ -44,34 +40,35 @@ struct sort_entry sort_comm = {
 	.se_cmp		= sort__comm_cmp,
 	.se_collapse	= sort__comm_collapse,
 	.se_snprintf	= hist_entry__comm_snprintf,
-	.se_width	= &comms__col_width,
+	.se_width_idx	= HISTC_COMM,
 };
 
 struct sort_entry sort_dso = {
 	.se_header	= "Shared Object",
 	.se_cmp		= sort__dso_cmp,
 	.se_snprintf	= hist_entry__dso_snprintf,
-	.se_width	= &dsos__col_width,
+	.se_width_idx	= HISTC_DSO,
 };
 
 struct sort_entry sort_sym = {
 	.se_header	= "Symbol",
 	.se_cmp		= sort__sym_cmp,
 	.se_snprintf	= hist_entry__sym_snprintf,
+	.se_width_idx	= HISTC_SYMBOL,
 };
 
 struct sort_entry sort_parent = {
 	.se_header	= "Parent symbol",
 	.se_cmp		= sort__parent_cmp,
 	.se_snprintf	= hist_entry__parent_snprintf,
-	.se_width	= &parent_symbol__col_width,
+	.se_width_idx	= HISTC_PARENT,
 };
  
 struct sort_entry sort_cpu = {
 	.se_header      = "CPU",
 	.se_cmp	        = sort__cpu_cmp,
 	.se_snprintf    = hist_entry__cpu_snprintf,
-	.se_width	= &cpus__col_width,
+	.se_width_idx	= HISTC_CPU,
 };
 
 struct sort_dimension {
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 560c855..03a1722 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -36,10 +36,6 @@ extern struct sort_entry sort_comm;
 extern struct sort_entry sort_dso;
 extern struct sort_entry sort_sym;
 extern struct sort_entry sort_parent;
-extern unsigned int dsos__col_width;
-extern unsigned int comms__col_width;
-extern unsigned int threads__col_width;
-extern unsigned int cpus__col_width;
 extern enum sort_type sort__first_dimension;
 
 struct hist_entry {
@@ -87,7 +83,7 @@ struct sort_entry {
 	int64_t (*se_collapse)(struct hist_entry *, struct hist_entry *);
 	int	(*se_snprintf)(struct hist_entry *self, char *bf, size_t size,
 			       unsigned int width);
-	unsigned int *se_width;
+	u8	se_width_idx;
 	bool	elide;
 };
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 971d0a0..bc6e7e8 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -12,6 +12,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include "build-id.h"
+#include "debug.h"
 #include "symbol.h"
 #include "strlist.h"
 
@@ -40,6 +41,14 @@ struct symbol_conf symbol_conf = {
 	.try_vmlinux_path = true,
 };
 
+int dso__name_len(const struct dso *self)
+{
+	if (verbose)
+		return self->long_name_len;
+
+	return self->short_name_len;
+}
+
 bool dso__loaded(const struct dso *self, enum map_type type)
 {
 	return self->loaded & (1 << type);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 80e569b..d436ee3 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -146,6 +146,8 @@ struct dso *dso__new(const char *name);
 struct dso *dso__new_kernel(const char *name);
 void dso__delete(struct dso *self);
 
+int dso__name_len(const struct dso *self);
+
 bool dso__loaded(const struct dso *self, enum map_type type);
 bool dso__sorted_by_name(const struct dso *self, enum map_type type);
 
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 03/19] perf ui: Restore SPACE as an alias to PGDN in annotate
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
  2010-08-02  1:08 ` [PATCH 01/19] perf hists: Mark entries filtered by parent Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 02/19] perf sort: Make column width code per hists instance Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 04/19] perf hist: Introduce routine to measure lenght of formatted entry Arnaldo Carvalho de Melo
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/newt.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index ab6eb36..2f5f7a1 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -750,6 +750,7 @@ int hist_entry__tui_annotate(struct hist_entry *self)
 
 	browser.width += 18; /* Percentage */
 	ui_browser__show(&browser, self->ms.sym->name);
+	newtFormAddHotKey(browser.form, ' ');
 	ret = ui_browser__run(&browser, &es);
 	newtFormDestroy(browser.form);
 	newtPopWindow();
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 04/19] perf hist: Introduce routine to measure lenght of formatted entry
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (2 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 03/19] perf ui: Restore SPACE as an alias to PGDN in annotate Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 05/19] perf ui: Consider the refreshed dimensions in ui_browser__show Arnaldo Carvalho de Melo
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

Will be used to figure out the window width needed in the new tree
widget.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c |   27 +++++++++++++++++++++++++++
 tools/perf/util/hist.h |    3 +++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 0bc6790..f93095f 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -850,6 +850,33 @@ print_entries:
 	return ret;
 }
 
+/*
+ * See hists__fprintf to match the column widths
+ */
+unsigned int hists__sort_list_width(struct hists *self)
+{
+	struct sort_entry *se;
+	int ret = 9; /* total % */
+
+	if (symbol_conf.show_cpu_utilization) {
+		ret += 7; /* count_sys % */
+		ret += 6; /* count_us % */
+		if (perf_guest) {
+			ret += 13; /* count_guest_sys % */
+			ret += 12; /* count_guest_us % */
+		}
+	}
+
+	if (symbol_conf.show_nr_samples)
+		ret += 11;
+
+	list_for_each_entry(se, &hist_entry__sort_list, list)
+		if (!se->elide)
+			ret += 2 + hists__col_len(self, se->se_width_idx);
+
+	return ret;
+}
+
 static void hists__remove_entry_filter(struct hists *self, struct hist_entry *h,
 				       enum hist_filter filter)
 {
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 92962b2..65a48db 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -141,4 +141,7 @@ int hist_entry__tui_annotate(struct hist_entry *self);
 
 int hists__tui_browse_tree(struct rb_root *self, const char *help);
 #endif
+
+unsigned int hists__sort_list_width(struct hists *self);
+
 #endif	/* __PERF_HIST_H */
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 05/19] perf ui: Consider the refreshed dimensions in ui_browser__show
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (3 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 04/19] perf hist: Introduce routine to measure lenght of formatted entry Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 06/19] perf ui: Show the scroll bar over the left window frame Arnaldo Carvalho de Melo
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

When we call ui_browser__show we may have called
ui_browser__refresh_dimensions to check if the maximum lenght for the
contained entries changed, such as when zooming in and out DSOs or
threads in the hist browser.

For that to happen we must delete the old form, that will take care of
deleting the vertical scrollbar, etc, and then recreate them, with the
new dimensions.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/newt.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index 2f5f7a1..aed2149 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -342,8 +342,10 @@ static void ui_browser__reset_index(struct ui_browser *self)
 
 static int ui_browser__show(struct ui_browser *self, const char *title)
 {
-	if (self->form != NULL)
-		return 0;
+	if (self->form != NULL) {
+		newtFormDestroy(self->form);
+		newtPopWindow();
+	}
 	ui_browser__refresh_dimensions(self);
 	newtCenteredWindow(self->width + 2, self->height, title);
 	self->form = newt_form__new();
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 06/19] perf ui: Show the scroll bar over the left window frame
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (4 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 05/19] perf ui: Consider the refreshed dimensions in ui_browser__show Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 07/19] perf ui: New hists tree widget Arnaldo Carvalho de Melo
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

So that we gain two columns and look more like classical (at least in
TUIs) scroll bars bars.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/newt.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index aed2149..cdd958e 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -347,12 +347,12 @@ static int ui_browser__show(struct ui_browser *self, const char *title)
 		newtPopWindow();
 	}
 	ui_browser__refresh_dimensions(self);
-	newtCenteredWindow(self->width + 2, self->height, title);
+	newtCenteredWindow(self->width, self->height, title);
 	self->form = newt_form__new();
 	if (self->form == NULL)
 		return -1;
 
-	self->sb = newtVerticalScrollbar(self->width + 1, 0, self->height,
+	self->sb = newtVerticalScrollbar(self->width, 0, self->height,
 					 HE_COLORSET_NORMAL,
 					 HE_COLORSET_SELECTED);
 	if (self->sb == NULL)
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 07/19] perf ui: New hists tree widget
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (5 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 06/19] perf ui: Show the scroll bar over the left window frame Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 08/19] perf report: Don't abbreviate file paths relative to the cwd Arnaldo Carvalho de Melo
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

The stock newt checkbox tree widget we were using was not really
suitable for hist entry + callchain browsing.

The problems with it were manifold:

- We needed to traverse the whole hist_entry rb_tree to add each entry +
  callchains beforehand.

- No control over the colors used for each row

So a new tree widget, based mostly on slang, was written.

It extends the ui_browser class already used for annotate to allow the
user to fold/unfold branches in the callchains tree, using extra fields
in the map_symbol class that is embedded in hist_entry and
callchain_node instances to store the folding state and when changing
this state calculates the number of rows that are produced when showing
a particular hist_entry instance.

This greatly speeds up browsing as we don't have to upfront touch all
the entries and only calculate callchain related operations when some
callchain branch is actually unfolded.

The memory footprint is also reduced as the data structure is not
duplicated, just some extra fields for controling callchain state and to
simplify the process of seeking thru entries (nr_rows, row_offset) were
added.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c   |    3 +
 tools/perf/util/newt.c   |  972 +++++++++++++++++++++++++++++++---------------
 tools/perf/util/sort.h   |   12 +
 tools/perf/util/symbol.h |    2 +
 4 files changed, 678 insertions(+), 311 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index f93095f..d0f07f7 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -885,6 +885,9 @@ static void hists__remove_entry_filter(struct hists *self, struct hist_entry *h,
 		return;
 
 	++self->nr_entries;
+	if (h->ms.unfolded)
+		self->nr_entries += h->nr_rows;
+	h->row_offset = 0;
 	self->stats.total_period += h->period;
 	self->stats.nr_events[PERF_RECORD_SAMPLE] += h->nr_events;
 
diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index cdd958e..28f74eb 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -509,38 +509,6 @@ static int ui_browser__run(struct ui_browser *self, struct newtExitStruct *es)
 	return 0;
 }
 
-/*
- * When debugging newt problems it was useful to be able to "unroll"
- * the calls to newtCheckBoxTreeAdd{Array,Item}, so that we can generate
- * a source file with the sequence of calls to these methods, to then
- * tweak the arrays to get the intended results, so I'm keeping this code
- * here, may be useful again in the future.
- */
-#undef NEWT_DEBUG
-
-static void newt_checkbox_tree__add(newtComponent tree, const char *str,
-				    void *priv, int *indexes)
-{
-#ifdef NEWT_DEBUG
-	/* Print the newtCheckboxTreeAddArray to tinker with its index arrays */
-	int i = 0, len = 40 - strlen(str);
-
-	fprintf(stderr,
-		"\tnewtCheckboxTreeAddItem(tree, %*.*s\"%s\", (void *)%p, 0, ",
-		len, len, " ", str, priv);
-	while (indexes[i] != NEWT_ARG_LAST) {
-		if (indexes[i] != NEWT_ARG_APPEND)
-			fprintf(stderr, " %d,", indexes[i]);
-		else
-			fprintf(stderr, " %s,", "NEWT_ARG_APPEND");
-		++i;
-	}
-	fprintf(stderr, " %s", " NEWT_ARG_LAST);\n");
-	fflush(stderr);
-#endif
-	newtCheckboxTreeAddArray(tree, str, priv, 0, indexes);
-}
-
 static char *callchain_list__sym_name(struct callchain_list *self,
 				      char *bf, size_t bfsize)
 {
@@ -576,147 +544,6 @@ static unsigned int hist_entry__annotate_browser_refresh(struct ui_browser *self
 	return row;
 }
 
-static void __callchain__append_graph_browser(struct callchain_node *self,
-					      newtComponent tree, u64 total,
-					      int *indexes, int depth)
-{
-	struct rb_node *node;
-	u64 new_total, remaining;
-	int idx = 0;
-
-	if (callchain_param.mode == CHAIN_GRAPH_REL)
-		new_total = self->children_hit;
-	else
-		new_total = total;
-
-	remaining = new_total;
-	node = rb_first(&self->rb_root);
-	while (node) {
-		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
-		struct rb_node *next = rb_next(node);
-		u64 cumul = cumul_hits(child);
-		struct callchain_list *chain;
-		int first = true, printed = 0;
-		int chain_idx = -1;
-		remaining -= cumul;
-
-		indexes[depth] = NEWT_ARG_APPEND;
-		indexes[depth + 1] = NEWT_ARG_LAST;
-
-		list_for_each_entry(chain, &child->val, list) {
-			char ipstr[BITS_PER_LONG / 4 + 1],
-			     *alloc_str = NULL;
-			const char *str = callchain_list__sym_name(chain, ipstr, sizeof(ipstr));
-
-			if (first) {
-				double percent = cumul * 100.0 / new_total;
-
-				first = false;
-				if (asprintf(&alloc_str, "%2.2f%% %s", percent, str) < 0)
-					str = "Not enough memory!";
-				else
-					str = alloc_str;
-			} else {
-				indexes[depth] = idx;
-				indexes[depth + 1] = NEWT_ARG_APPEND;
-				indexes[depth + 2] = NEWT_ARG_LAST;
-				++chain_idx;
-			}
-			newt_checkbox_tree__add(tree, str, &chain->ms, indexes);
-			free(alloc_str);
-			++printed;
-		}
-
-		indexes[depth] = idx;
-		if (chain_idx != -1)
-			indexes[depth + 1] = chain_idx;
-		if (printed != 0)
-			++idx;
-		__callchain__append_graph_browser(child, tree, new_total, indexes,
-						  depth + (chain_idx != -1 ? 2 : 1));
-		node = next;
-	}
-}
-
-static void callchain__append_graph_browser(struct callchain_node *self,
-					    newtComponent tree, u64 total,
-					    int *indexes, int parent_idx)
-{
-	struct callchain_list *chain;
-	int i = 0;
-
-	indexes[1] = NEWT_ARG_APPEND;
-	indexes[2] = NEWT_ARG_LAST;
-
-	list_for_each_entry(chain, &self->val, list) {
-		char ipstr[BITS_PER_LONG / 4 + 1], *str;
-
-		if (chain->ip >= PERF_CONTEXT_MAX)
-			continue;
-
-		if (!i++ && sort__first_dimension == SORT_SYM)
-			continue;
-
-		str = callchain_list__sym_name(chain, ipstr, sizeof(ipstr));
-		newt_checkbox_tree__add(tree, str, &chain->ms, indexes);
-	}
-
-	indexes[1] = parent_idx;
-	indexes[2] = NEWT_ARG_APPEND;
-	indexes[3] = NEWT_ARG_LAST;
-	__callchain__append_graph_browser(self, tree, total, indexes, 2);
-}
-
-static void hist_entry__append_callchain_browser(struct hist_entry *self,
-						 newtComponent tree, u64 total, int parent_idx)
-{
-	struct rb_node *rb_node;
-	int indexes[1024] = { [0] = parent_idx, };
-	int idx = 0;
-	struct callchain_node *chain;
-
-	rb_node = rb_first(&self->sorted_chain);
-	while (rb_node) {
-		chain = rb_entry(rb_node, struct callchain_node, rb_node);
-		switch (callchain_param.mode) {
-		case CHAIN_FLAT:
-			break;
-		case CHAIN_GRAPH_ABS: /* falldown */
-		case CHAIN_GRAPH_REL:
-			callchain__append_graph_browser(chain, tree, total, indexes, idx++);
-			break;
-		case CHAIN_NONE:
-		default:
-			break;
-		}
-		rb_node = rb_next(rb_node);
-	}
-}
-
-static size_t hist_entry__append_browser(struct hist_entry *self,
-					 newtComponent tree,
-					 struct hists *hists)
-{
-	char s[256];
-	size_t ret;
-
-	if (symbol_conf.exclude_other && !self->parent)
-		return 0;
-
-	ret = hist_entry__snprintf(self, s, sizeof(s), hists, NULL,
-				   false, 0, false, hists->stats.total_period);
-	if (symbol_conf.use_callchain) {
-		int indexes[2];
-
-		indexes[0] = NEWT_ARG_APPEND;
-		indexes[1] = NEWT_ARG_LAST;
-		newt_checkbox_tree__add(tree, s, &self->ms, indexes);
-	} else
-		newtListboxAppendEntry(tree, s, &self->ms);
-
-	return ret;
-}
-
 int hist_entry__tui_annotate(struct hist_entry *self)
 {
 	struct ui_browser browser;
@@ -764,157 +591,48 @@ int hist_entry__tui_annotate(struct hist_entry *self)
 	return ret;
 }
 
-static const void *newt__symbol_tree_get_current(newtComponent self)
-{
-	if (symbol_conf.use_callchain)
-		return newtCheckboxTreeGetCurrent(self);
-	return newtListboxGetCurrent(self);
-}
-
-static void hist_browser__selection(newtComponent self, void *data)
-{
-	const struct map_symbol **symbol_ptr = data;
-	*symbol_ptr = newt__symbol_tree_get_current(self);
-}
-
 struct hist_browser {
-	newtComponent		form, tree;
-	const struct map_symbol *selection;
+	struct ui_browser   b;
+	struct hists	    *hists;
+	struct hist_entry   *he_selection;
+	struct map_symbol   *selection;
 };
 
-static struct hist_browser *hist_browser__new(void)
+static void hist_browser__reset(struct hist_browser *self);
+static int hist_browser__run(struct hist_browser *self, const char *title,
+			     struct newtExitStruct *es);
+static unsigned int hist_browser__refresh_entries(struct ui_browser *self);
+static void ui_browser__hists_seek(struct ui_browser *self,
+				   off_t offset, int whence);
+
+static struct hist_browser *hist_browser__new(struct hists *hists)
 {
-	struct hist_browser *self = malloc(sizeof(*self));
+	struct hist_browser *self = zalloc(sizeof(*self));
 
-	if (self != NULL)
-		self->form = NULL;
+	if (self) {
+		self->hists = hists;
+		self->b.refresh_entries = hist_browser__refresh_entries;
+		self->b.seek = ui_browser__hists_seek;
+	}
 
 	return self;
 }
 
 static void hist_browser__delete(struct hist_browser *self)
 {
-	newtFormDestroy(self->form);
+	newtFormDestroy(self->b.form);
 	newtPopWindow();
 	free(self);
 }
 
-static int hist_browser__populate(struct hist_browser *self, struct hists *hists,
-				  const char *title)
-{
-	int max_len = 0, idx, cols, rows;
-	struct ui_progress *progress;
-	struct rb_node *nd;
-	u64 curr_hist = 0;
-	char seq[] = ".", unit;
-	char str[256];
-	unsigned long nr_events = hists->stats.nr_events[PERF_RECORD_SAMPLE];
-
-	if (self->form) {
-		newtFormDestroy(self->form);
-		newtPopWindow();
-	}
-
-	nr_events = convert_unit(nr_events, &unit);
-	snprintf(str, sizeof(str), "Events: %lu%c                            ",
-		 nr_events, unit);
-	newtDrawRootText(0, 0, str);
-
-	newtGetScreenSize(NULL, &rows);
-
-	if (symbol_conf.use_callchain)
-		self->tree = newtCheckboxTreeMulti(0, 0, rows - 5, seq,
-						   NEWT_FLAG_SCROLL);
-	else
-		self->tree = newtListbox(0, 0, rows - 5,
-					(NEWT_FLAG_SCROLL |
-					 NEWT_FLAG_RETURNEXIT));
-
-	newtComponentAddCallback(self->tree, hist_browser__selection,
-				 &self->selection);
-
-	progress = ui_progress__new("Adding entries to the browser...",
-				    hists->nr_entries);
-	if (progress == NULL)
-		return -1;
-
-	idx = 0;
-	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
-		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
-		int len;
-
-		if (h->filtered)
-			continue;
-
-		len = hist_entry__append_browser(h, self->tree, hists);
-		if (len > max_len)
-			max_len = len;
-		if (symbol_conf.use_callchain)
-			hist_entry__append_callchain_browser(h, self->tree,
-							     hists->stats.total_period, idx++);
-		++curr_hist;
-		if (curr_hist % 5)
-			ui_progress__update(progress, curr_hist);
-	}
-
-	ui_progress__delete(progress);
-
-	newtGetScreenSize(&cols, &rows);
-
-	if (max_len > cols)
-		max_len = cols - 3;
-
-	if (!symbol_conf.use_callchain)
-		newtListboxSetWidth(self->tree, max_len);
-
-	newtCenteredWindow(max_len + (symbol_conf.use_callchain ? 5 : 0),
-			   rows - 5, title);
-	self->form = newt_form__new();
-	if (self->form == NULL)
-		return -1;
-
-	newtFormAddHotKey(self->form, 'A');
-	newtFormAddHotKey(self->form, 'a');
-	newtFormAddHotKey(self->form, 'D');
-	newtFormAddHotKey(self->form, 'd');
-	newtFormAddHotKey(self->form, 'T');
-	newtFormAddHotKey(self->form, 't');
-	newtFormAddHotKey(self->form, '?');
-	newtFormAddHotKey(self->form, 'H');
-	newtFormAddHotKey(self->form, 'h');
-	newtFormAddHotKey(self->form, NEWT_KEY_F1);
-	newtFormAddHotKey(self->form, NEWT_KEY_RIGHT);
-	newtFormAddHotKey(self->form, NEWT_KEY_TAB);
-	newtFormAddHotKey(self->form, NEWT_KEY_UNTAB);
-	newtFormAddComponents(self->form, self->tree, NULL);
-	self->selection = newt__symbol_tree_get_current(self->tree);
-
-	return 0;
-}
-
 static struct hist_entry *hist_browser__selected_entry(struct hist_browser *self)
 {
-	int *indexes;
-
-	if (!symbol_conf.use_callchain)
-		goto out;
-
-	indexes = newtCheckboxTreeFindItem(self->tree, (void *)self->selection);
-	if (indexes) {
-		bool is_hist_entry = indexes[1] == NEWT_ARG_LAST;
-		free(indexes);
-		if (is_hist_entry)
-			goto out;
-	}
-	return NULL;
-out:
-	return container_of(self->selection, struct hist_entry, ms);
+	return self->he_selection;
 }
 
 static struct thread *hist_browser__selected_thread(struct hist_browser *self)
 {
-	struct hist_entry *he = hist_browser__selected_entry(self);
-	return he ? he->thread : NULL;
+	return self->he_selection->thread;
 }
 
 static int hist_browser__title(char *bf, size_t size, const char *ev_name,
@@ -936,7 +654,7 @@ static int hist_browser__title(char *bf, size_t size, const char *ev_name,
 
 int hists__browse(struct hists *self, const char *helpline, const char *ev_name)
 {
-	struct hist_browser *browser = hist_browser__new();
+	struct hist_browser *browser = hist_browser__new(self);
 	struct pstack *fstack;
 	const struct thread *thread_filter = NULL;
 	const struct dso *dso_filter = NULL;
@@ -955,8 +673,6 @@ int hists__browse(struct hists *self, const char *helpline, const char *ev_name)
 
 	hist_browser__title(msg, sizeof(msg), ev_name,
 			    dso_filter, thread_filter);
-	if (hist_browser__populate(browser, self, msg) < 0)
-		goto out_free_stack;
 
 	while (1) {
 		const struct thread *thread;
@@ -965,7 +681,8 @@ int hists__browse(struct hists *self, const char *helpline, const char *ev_name)
 		int nr_options = 0, choice = 0, i,
 		    annotate = -2, zoom_dso = -2, zoom_thread = -2;
 
-		newtFormRun(browser->form, &es);
+		if (hist_browser__run(browser, msg, &es))
+			break;
 
 		thread = hist_browser__selected_thread(browser);
 		dso = browser->selection->map ? browser->selection->map->dso : NULL;
@@ -1100,8 +817,7 @@ zoom_out_dso:
 			hists__filter_by_dso(self, dso_filter);
 			hist_browser__title(msg, sizeof(msg), ev_name,
 					    dso_filter, thread_filter);
-			if (hist_browser__populate(browser, self, msg) < 0)
-				goto out;
+			hist_browser__reset(browser);
 		} else if (choice == zoom_thread) {
 zoom_thread:
 			if (thread_filter) {
@@ -1119,8 +835,7 @@ zoom_out_thread:
 			hists__filter_by_thread(self, thread_filter);
 			hist_browser__title(msg, sizeof(msg), ev_name,
 					    dso_filter, thread_filter);
-			if (hist_browser__populate(browser, self, msg) < 0)
-				goto out;
+			hist_browser__reset(browser);
 		}
 	}
 out_free_stack:
@@ -1207,3 +922,638 @@ void exit_browser(bool wait_for_ok)
 		newtFinished();
 	}
 }
+
+static void hist_browser__refresh_dimensions(struct hist_browser *self)
+{
+	/* 3 == +/- toggle symbol before actual hist_entry rendering */
+	self->b.width = 3 + (hists__sort_list_width(self->hists) +
+			     sizeof("[k]"));
+}
+
+static void hist_browser__reset(struct hist_browser *self)
+{
+	self->b.nr_entries = self->hists->nr_entries;
+	hist_browser__refresh_dimensions(self);
+	ui_browser__reset_index(&self->b);
+}
+
+static char tree__folded_sign(bool unfolded)
+{
+	return unfolded ? '-' : '+';
+}
+
+static char map_symbol__folded(const struct map_symbol *self)
+{
+	return self->has_children ? tree__folded_sign(self->unfolded) : ' ';
+}
+
+static char hist_entry__folded(const struct hist_entry *self)
+{
+	return map_symbol__folded(&self->ms);
+}
+
+static char callchain_list__folded(const struct callchain_list *self)
+{
+	return map_symbol__folded(&self->ms);
+}
+
+static bool map_symbol__toggle_fold(struct map_symbol *self)
+{
+	if (!self->has_children)
+		return false;
+
+	self->unfolded = !self->unfolded;
+	return true;
+}
+
+#define LEVEL_OFFSET_STEP 3
+
+static int hist_browser__show_callchain_node_rb_tree(struct hist_browser *self,
+						     struct callchain_node *chain_node,
+						     u64 total, int level,
+						     unsigned short row,
+						     off_t *row_offset,
+						     bool *is_current_entry)
+{
+	struct rb_node *node;
+	int first_row = row, width, offset = level * LEVEL_OFFSET_STEP;
+	u64 new_total, remaining;
+
+	if (callchain_param.mode == CHAIN_GRAPH_REL)
+		new_total = chain_node->children_hit;
+	else
+		new_total = total;
+
+	remaining = new_total;
+	node = rb_first(&chain_node->rb_root);
+	while (node) {
+		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
+		struct rb_node *next = rb_next(node);
+		u64 cumul = cumul_hits(child);
+		struct callchain_list *chain;
+		char folded_sign = ' ';
+		int first = true;
+		int extra_offset = 0;
+
+		remaining -= cumul;
+
+		list_for_each_entry(chain, &child->val, list) {
+			char ipstr[BITS_PER_LONG / 4 + 1], *alloc_str;
+			const char *str;
+			int color;
+			bool was_first = first;
+
+			if (first) {
+				first = false;
+				chain->ms.has_children = chain->list.next != &child->val ||
+							 rb_first(&child->rb_root) != NULL;
+			} else {
+				extra_offset = LEVEL_OFFSET_STEP;
+				chain->ms.has_children = chain->list.next == &child->val &&
+							 rb_first(&child->rb_root) != NULL;
+			}
+
+			folded_sign = callchain_list__folded(chain);
+			if (*row_offset != 0) {
+				--*row_offset;
+				goto do_next;
+			}
+
+			alloc_str = NULL;
+			str = callchain_list__sym_name(chain, ipstr, sizeof(ipstr));
+			if (was_first) {
+				double percent = cumul * 100.0 / new_total;
+
+				if (asprintf(&alloc_str, "%2.2f%% %s", percent, str) < 0)
+					str = "Not enough memory!";
+				else
+					str = alloc_str;
+			}
+
+			color = HE_COLORSET_NORMAL;
+			width = self->b.width - (offset + extra_offset + 2);
+			if (ui_browser__is_current_entry(&self->b, row)) {
+				self->selection = &chain->ms;
+				color = HE_COLORSET_SELECTED;
+				*is_current_entry = true;
+			}
+
+			SLsmg_set_color(color);
+			SLsmg_gotorc(self->b.top + row, self->b.left);
+			slsmg_write_nstring(" ", offset + extra_offset);
+			slsmg_printf("%c ", folded_sign);
+			slsmg_write_nstring(str, width);
+			free(alloc_str);
+
+			if (++row == self->b.height)
+				goto out;
+do_next:
+			if (folded_sign == '+')
+				break;
+		}
+
+		if (folded_sign == '-') {
+			const int new_level = level + (extra_offset ? 2 : 1);
+			row += hist_browser__show_callchain_node_rb_tree(self, child, new_total,
+									 new_level, row, row_offset,
+									 is_current_entry);
+		}
+		if (row == self->b.height)
+			goto out;
+		node = next;
+	}
+out:
+	return row - first_row;
+}
+
+static int hist_browser__show_callchain_node(struct hist_browser *self,
+					     struct callchain_node *node,
+					     int level, unsigned short row,
+					     off_t *row_offset,
+					     bool *is_current_entry)
+{
+	struct callchain_list *chain;
+	int first_row = row,
+	     offset = level * LEVEL_OFFSET_STEP,
+	     width = self->b.width - offset;
+	char folded_sign = ' ';
+
+	list_for_each_entry(chain, &node->val, list) {
+		char ipstr[BITS_PER_LONG / 4 + 1], *s;
+		int color;
+		/*
+		 * FIXME: This should be moved to somewhere else,
+		 * probably when the callchain is created, so as not to
+		 * traverse it all over again
+		 */
+		chain->ms.has_children = rb_first(&node->rb_root) != NULL;
+		folded_sign = callchain_list__folded(chain);
+
+		if (*row_offset != 0) {
+			--*row_offset;
+			continue;
+		}
+
+		color = HE_COLORSET_NORMAL;
+		if (ui_browser__is_current_entry(&self->b, row)) {
+			self->selection = &chain->ms;
+			color = HE_COLORSET_SELECTED;
+			*is_current_entry = true;
+		}
+
+		s = callchain_list__sym_name(chain, ipstr, sizeof(ipstr));
+		SLsmg_gotorc(self->b.top + row, self->b.left);
+		SLsmg_set_color(color);
+		slsmg_write_nstring(" ", offset);
+		slsmg_printf("%c ", folded_sign);
+		slsmg_write_nstring(s, width - 2);
+
+		if (++row == self->b.height)
+			goto out;
+	}
+
+	if (folded_sign == '-')
+		row += hist_browser__show_callchain_node_rb_tree(self, node,
+								 self->hists->stats.total_period,
+								 level + 1, row,
+								 row_offset,
+								 is_current_entry);
+out:
+	return row - first_row;
+}
+
+static int hist_browser__show_callchain(struct hist_browser *self,
+					struct rb_root *chain,
+					int level, unsigned short row,
+					off_t *row_offset,
+					bool *is_current_entry)
+{
+	struct rb_node *nd;
+	int first_row = row;
+
+	for (nd = rb_first(chain); nd; nd = rb_next(nd)) {
+		struct callchain_node *node = rb_entry(nd, struct callchain_node, rb_node);
+
+		row += hist_browser__show_callchain_node(self, node, level,
+							 row, row_offset,
+							 is_current_entry);
+		if (row == self->b.height)
+			break;
+	}
+
+	return row - first_row;
+}
+
+static int hist_browser__show_entry(struct hist_browser *self,
+				    struct hist_entry *entry,
+				    unsigned short row)
+{
+	char s[256];
+	double percent;
+	int printed = 0;
+	int color, width = self->b.width;
+	char folded_sign = ' ';
+	bool current_entry = ui_browser__is_current_entry(&self->b, row);
+	off_t row_offset = entry->row_offset;
+
+	if (current_entry) {
+		self->he_selection = entry;
+		self->selection = &entry->ms;
+	}
+
+	if (symbol_conf.use_callchain) {
+		entry->ms.has_children = !RB_EMPTY_ROOT(&entry->sorted_chain);
+		folded_sign = hist_entry__folded(entry);
+	}
+
+	if (row_offset == 0) {
+		hist_entry__snprintf(entry, s, sizeof(s), self->hists, NULL, false,
+				     0, false, self->hists->stats.total_period);
+		percent = (entry->period * 100.0) / self->hists->stats.total_period;
+
+		color = HE_COLORSET_SELECTED;
+		if (!current_entry) {
+			if (percent >= MIN_RED)
+				color = HE_COLORSET_TOP;
+			else if (percent >= MIN_GREEN)
+				color = HE_COLORSET_MEDIUM;
+			else
+				color = HE_COLORSET_NORMAL;
+		}
+
+		SLsmg_set_color(color);
+		SLsmg_gotorc(self->b.top + row, self->b.left);
+		if (symbol_conf.use_callchain) {
+			slsmg_printf("%c ", folded_sign);
+			width -= 2;
+		}
+		slsmg_write_nstring(s, width);
+		++row;
+		++printed;
+	} else
+		--row_offset;
+
+	if (folded_sign == '-' && row != self->b.height) {
+		printed += hist_browser__show_callchain(self, &entry->sorted_chain,
+							1, row, &row_offset,
+							&current_entry);
+		if (current_entry)
+			self->he_selection = entry;
+	}
+
+	return printed;
+}
+
+static unsigned int hist_browser__refresh_entries(struct ui_browser *self)
+{
+	unsigned row = 0;
+	struct rb_node *nd;
+	struct hist_browser *hb = container_of(self, struct hist_browser, b);
+
+	if (self->first_visible_entry == NULL)
+		self->first_visible_entry = rb_first(&hb->hists->entries);
+
+	for (nd = self->first_visible_entry; nd; nd = rb_next(nd)) {
+		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
+
+		if (h->filtered)
+			continue;
+
+		row += hist_browser__show_entry(hb, h, row);
+		if (row == self->height)
+			break;
+	}
+
+	return row;
+}
+
+static void callchain_node__init_have_children_rb_tree(struct callchain_node *self)
+{
+	struct rb_node *nd = rb_first(&self->rb_root);
+
+	for (nd = rb_first(&self->rb_root); nd; nd = rb_next(nd)) {
+		struct callchain_node *child = rb_entry(nd, struct callchain_node, rb_node);
+		struct callchain_list *chain;
+		int first = true;
+
+		list_for_each_entry(chain, &child->val, list) {
+			if (first) {
+				first = false;
+				chain->ms.has_children = chain->list.next != &child->val ||
+							 rb_first(&child->rb_root) != NULL;
+			} else
+				chain->ms.has_children = chain->list.next == &child->val &&
+							 rb_first(&child->rb_root) != NULL;
+		}
+
+		callchain_node__init_have_children_rb_tree(child);
+	}
+}
+
+static void callchain_node__init_have_children(struct callchain_node *self)
+{
+	struct callchain_list *chain;
+
+	list_for_each_entry(chain, &self->val, list)
+		chain->ms.has_children = rb_first(&self->rb_root) != NULL;
+
+	callchain_node__init_have_children_rb_tree(self);
+}
+
+static void callchain__init_have_children(struct rb_root *self)
+{
+	struct rb_node *nd;
+
+	for (nd = rb_first(self); nd; nd = rb_next(nd)) {
+		struct callchain_node *node = rb_entry(nd, struct callchain_node, rb_node);
+		callchain_node__init_have_children(node);
+	}
+}
+
+static void hist_entry__init_have_children(struct hist_entry *self)
+{
+	if (!self->init_have_children) {
+		callchain__init_have_children(&self->sorted_chain);
+		self->init_have_children = true;
+	}
+}
+
+static struct rb_node *hists__filter_entries(struct rb_node *nd)
+{
+	while (nd != NULL) {
+		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
+		if (!h->filtered)
+			return nd;
+
+		nd = rb_next(nd);
+	}
+
+	return NULL;
+}
+
+static struct rb_node *hists__filter_prev_entries(struct rb_node *nd)
+{
+	while (nd != NULL) {
+		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
+		if (!h->filtered)
+			return nd;
+
+		nd = rb_prev(nd);
+	}
+
+	return NULL;
+}
+
+static void ui_browser__hists_seek(struct ui_browser *self,
+				   off_t offset, int whence)
+{
+	struct hist_entry *h;
+	struct rb_node *nd;
+	bool first = true;
+
+	switch (whence) {
+	case SEEK_SET:
+		nd = hists__filter_entries(rb_first(self->entries));
+		break;
+	case SEEK_CUR:
+		nd = self->first_visible_entry;
+		goto do_offset;
+	case SEEK_END:
+		nd = hists__filter_prev_entries(rb_last(self->entries));
+		first = false;
+		break;
+	default:
+		return;
+	}
+
+	/*
+	 * Moves not relative to the first visible entry invalidates its
+	 * row_offset:
+	 */
+	h = rb_entry(self->first_visible_entry, struct hist_entry, rb_node);
+	h->row_offset = 0;
+
+	/*
+	 * Here we have to check if nd is expanded (+), if it is we can't go
+	 * the next top level hist_entry, instead we must compute an offset of
+	 * what _not_ to show and not change the first visible entry.
+	 *
+	 * This offset increments when we are going from top to bottom and
+	 * decreases when we're going from bottom to top.
+	 *
+	 * As we don't have backpointers to the top level in the callchains
+	 * structure, we need to always print the whole hist_entry callchain,
+	 * skipping the first ones that are before the first visible entry
+	 * and stop when we printed enough lines to fill the screen.
+	 */
+do_offset:
+	if (offset > 0) {
+		do {
+			h = rb_entry(nd, struct hist_entry, rb_node);
+			if (h->ms.unfolded) {
+				u16 remaining = h->nr_rows - h->row_offset;
+				if (offset > remaining) {
+					offset -= remaining;
+					h->row_offset = 0;
+				} else {
+					h->row_offset += offset;
+					offset = 0;
+					self->first_visible_entry = nd;
+					break;
+				}
+			}
+			nd = hists__filter_entries(rb_next(nd));
+			if (nd == NULL)
+				break;
+			--offset;
+			self->first_visible_entry = nd;
+		} while (offset != 0);
+	} else if (offset < 0) {
+		while (1) {
+			h = rb_entry(nd, struct hist_entry, rb_node);
+			if (h->ms.unfolded) {
+				if (first) {
+					if (-offset > h->row_offset) {
+						offset += h->row_offset;
+						h->row_offset = 0;
+					} else {
+						h->row_offset += offset;
+						offset = 0;
+						self->first_visible_entry = nd;
+						break;
+					}
+				} else {
+					if (-offset > h->nr_rows) {
+						offset += h->nr_rows;
+						h->row_offset = 0;
+					} else {
+						h->row_offset = h->nr_rows + offset;
+						offset = 0;
+						self->first_visible_entry = nd;
+						break;
+					}
+				}
+			}
+
+			nd = hists__filter_prev_entries(rb_prev(nd));
+			if (nd == NULL)
+				break;
+			++offset;
+			self->first_visible_entry = nd;
+			if (offset == 0) {
+				/*
+				 * Last unfiltered hist_entry, check if it is
+				 * unfolded, if it is then we should have
+				 * row_offset at its last entry.
+				 */
+				h = rb_entry(nd, struct hist_entry, rb_node);
+				if (h->ms.unfolded)
+					h->row_offset = h->nr_rows;
+				break;
+			}
+			first = false;
+		}
+	} else {
+		self->first_visible_entry = nd;
+		h = rb_entry(nd, struct hist_entry, rb_node);
+		h->row_offset = 0;
+	}
+}
+
+static int callchain_node__count_rows_rb_tree(struct callchain_node *self)
+{
+	int n = 0;
+	struct rb_node *nd;
+
+	for (nd = rb_first(&self->rb_root); nd; nd = rb_next(nd)) {
+		struct callchain_node *child = rb_entry(nd, struct callchain_node, rb_node);
+		struct callchain_list *chain;
+		char folded_sign = ' '; /* No children */
+
+		list_for_each_entry(chain, &child->val, list) {
+			++n;
+			/* We need this because we may not have children */
+			folded_sign = callchain_list__folded(chain);
+			if (folded_sign == '+')
+				break;
+		}
+
+		if (folded_sign == '-') /* Have children and they're unfolded */
+			n += callchain_node__count_rows_rb_tree(child);
+	}
+
+	return n;
+}
+
+static int callchain_node__count_rows(struct callchain_node *node)
+{
+	struct callchain_list *chain;
+	bool unfolded = false;
+	int n = 0;
+
+	list_for_each_entry(chain, &node->val, list) {
+		++n;
+		unfolded = chain->ms.unfolded;
+	}
+
+	if (unfolded)
+		n += callchain_node__count_rows_rb_tree(node);
+
+	return n;
+}
+
+static int callchain__count_rows(struct rb_root *chain)
+{
+	struct rb_node *nd;
+	int n = 0;
+
+	for (nd = rb_first(chain); nd; nd = rb_next(nd)) {
+		struct callchain_node *node = rb_entry(nd, struct callchain_node, rb_node);
+		n += callchain_node__count_rows(node);
+	}
+
+	return n;
+}
+
+static bool hist_browser__toggle_fold(struct hist_browser *self)
+{
+	if (map_symbol__toggle_fold(self->selection)) {
+		struct hist_entry *he = self->he_selection;
+
+		hist_entry__init_have_children(he);
+		self->hists->nr_entries -= he->nr_rows;
+
+		if (he->ms.unfolded)
+			he->nr_rows = callchain__count_rows(&he->sorted_chain);
+		else
+			he->nr_rows = 0;
+		self->hists->nr_entries += he->nr_rows;
+		self->b.nr_entries = self->hists->nr_entries;
+
+		return true;
+	}
+
+	/* If it doesn't have children, no toggling performed */
+	return false;
+}
+
+static int hist_browser__run(struct hist_browser *self, const char *title,
+			     struct newtExitStruct *es)
+{
+	char str[256], unit;
+	unsigned long nr_events = self->hists->stats.nr_events[PERF_RECORD_SAMPLE];
+
+	self->b.entries = &self->hists->entries;
+	self->b.nr_entries = self->hists->nr_entries;
+
+	hist_browser__refresh_dimensions(self);
+
+	nr_events = convert_unit(nr_events, &unit);
+	snprintf(str, sizeof(str), "Events: %lu%c                            ",
+		 nr_events, unit);
+	newtDrawRootText(0, 0, str);
+
+	if (ui_browser__show(&self->b, title) < 0)
+		return -1;
+
+	newtFormAddHotKey(self->b.form, 'A');
+	newtFormAddHotKey(self->b.form, 'a');
+	newtFormAddHotKey(self->b.form, '?');
+	newtFormAddHotKey(self->b.form, 'h');
+	newtFormAddHotKey(self->b.form, 'H');
+	newtFormAddHotKey(self->b.form, 'd');
+
+	newtFormAddHotKey(self->b.form, NEWT_KEY_LEFT);
+	newtFormAddHotKey(self->b.form, NEWT_KEY_RIGHT);
+	newtFormAddHotKey(self->b.form, NEWT_KEY_ENTER);
+
+	while (1) {
+		ui_browser__run(&self->b, es);
+
+		if (es->reason != NEWT_EXIT_HOTKEY)
+			break;
+		switch (es->u.key) {
+		case 'd': { /* Debug */
+			static int seq;
+			struct hist_entry *h = rb_entry(self->b.first_visible_entry,
+							struct hist_entry, rb_node);
+			ui_helpline__pop();
+			ui_helpline__fpush("%d: nr_ent=(%d,%d), height=%d, idx=%d, fve: idx=%d, row_off=%d, nrows=%d",
+					   seq++, self->b.nr_entries,
+					   self->hists->nr_entries,
+					   self->b.height,
+					   self->b.index,
+					   self->b.first_visible_entry_idx,
+					   h->row_offset, h->nr_rows);
+		}
+			continue;
+		case NEWT_KEY_ENTER:
+			if (hist_browser__toggle_fold(self))
+				break;
+			/* fall thru */
+		default:
+			return 0;
+		}
+	}
+	return 0;
+}
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 03a1722..46e531d 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -38,6 +38,12 @@ extern struct sort_entry sort_sym;
 extern struct sort_entry sort_parent;
 extern enum sort_type sort__first_dimension;
 
+/**
+ * struct hist_entry - histogram entry
+ *
+ * @row_offset - offset from the first callchain expanded to appear on screen
+ * @nr_rows - rows expanded in callchain, recalculated on folding/unfolding
+ */
 struct hist_entry {
 	struct rb_node		rb_node;
 	u64			period;
@@ -50,6 +56,12 @@ struct hist_entry {
 	u64			ip;
 	s32			cpu;
 	u32			nr_events;
+
+	/* XXX These two should move to some tree widget lib */
+	u16			row_offset;
+	u16			nr_rows;
+
+	bool			init_have_children;
 	char			level;
 	u8			filtered;
 	struct symbol		*parent;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index d436ee3..bb1aab9 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -102,6 +102,8 @@ struct ref_reloc_sym {
 struct map_symbol {
 	struct map    *map;
 	struct symbol *sym;
+	bool	      unfolded;
+	bool	      has_children;
 };
 
 struct addr_location {
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 08/19] perf report: Don't abbreviate file paths relative to the cwd
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (6 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 07/19] perf ui: New hists tree widget Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 09/19] perf tools: Remove unneeded code for tracking the cwd in perf sessions Arnaldo Carvalho de Melo
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Dave Martin, Arnaldo Carvalho de Melo

From: Dave Martin <dave.martin@linaro.org>

This avoids around some problems where the full path is executables and DSOs it
needed for finding debug symbols on platforms with separated debug symbol files
such as Ubuntu.  This is simpler than tracking an extra name for each image.

The only impact should be that paths in verbose output from the perf tools
become absolute, instead of relative to .

LKML-Reference: <new-submission>
Signed-off-by: Dave Martin <dave.martin@linaro.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c |    2 +-
 tools/perf/util/map.c   |   22 +---------------------
 tools/perf/util/map.h   |    2 +-
 3 files changed, 3 insertions(+), 23 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 121339f..5b81bb2 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -517,7 +517,7 @@ int event__process_mmap(event_t *self, struct perf_session *session)
 	map = map__new(&machine->user_dsos, self->mmap.start,
 			self->mmap.len, self->mmap.pgoff,
 			self->mmap.pid, self->mmap.filename,
-			MAP__FUNCTION, session->cwd, session->cwdlen);
+			MAP__FUNCTION);
 
 	if (thread == NULL || map == NULL)
 		goto out_problem;
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index e672f2f..37cab90 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -17,16 +17,6 @@ static inline int is_anon_memory(const char *filename)
 	return strcmp(filename, "//anon") == 0;
 }
 
-static int strcommon(const char *pathname, char *cwd, int cwdlen)
-{
-	int n = 0;
-
-	while (n < cwdlen && pathname[n] == cwd[n])
-		++n;
-
-	return n;
-}
-
 void map__init(struct map *self, enum map_type type,
 	       u64 start, u64 end, u64 pgoff, struct dso *dso)
 {
@@ -43,7 +33,7 @@ void map__init(struct map *self, enum map_type type,
 
 struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
 		     u64 pgoff, u32 pid, char *filename,
-		     enum map_type type, char *cwd, int cwdlen)
+		     enum map_type type)
 {
 	struct map *self = malloc(sizeof(*self));
 
@@ -52,16 +42,6 @@ struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
 		struct dso *dso;
 		int anon;
 
-		if (cwd) {
-			int n = strcommon(filename, cwd, cwdlen);
-
-			if (n == cwdlen) {
-				snprintf(newfilename, sizeof(newfilename),
-					 ".%s", filename + n);
-				filename = newfilename;
-			}
-		}
-
 		anon = is_anon_memory(filename);
 
 		if (anon) {
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index f391345..3b2f706 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -106,7 +106,7 @@ void map__init(struct map *self, enum map_type type,
 	       u64 start, u64 end, u64 pgoff, struct dso *dso);
 struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
 		     u64 pgoff, u32 pid, char *filename,
-		     enum map_type type, char *cwd, int cwdlen);
+		     enum map_type type);
 void map__delete(struct map *self);
 struct map *map__clone(struct map *self);
 int map__overlap(struct map *l, struct map *r);
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 09/19] perf tools: Remove unneeded code for tracking the cwd in perf sessions
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (7 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 08/19] perf report: Don't abbreviate file paths relative to the cwd Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 10/19] perf man pages: Fix cut'n'paste error Arnaldo Carvalho de Melo
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Dave Martin, Arnaldo Carvalho de Melo

From: Dave Martin <dave.martin@linaro.org>

Tidy-up patch to remove some code and struct perf_session data members
which are no longer needed due to the previous patch: "perf tools: Don't
abbreviate file paths relative to the cwd".

LKML-Reference: <new-submission>
Signed-off-by: Dave Martin <dave.martin@linaro.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-buildid-list.c |    4 +---
 tools/perf/builtin-diff.c         |    2 --
 tools/perf/builtin-report.c       |    2 --
 tools/perf/util/session.c         |   22 +---------------------
 tools/perf/util/symbol.h          |    1 -
 5 files changed, 2 insertions(+), 29 deletions(-)

diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index 9989072..44a47e1 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -43,10 +43,8 @@ static int __cmd_buildid_list(void)
 	if (session == NULL)
 		return -1;
 
-	if (with_hits) {
-		symbol_conf.full_paths = true;
+	if (with_hits)
 		perf_session__process_events(session, &build_id__mark_dso_hit_ops);
-	}
 
 	perf_session__fprintf_dsos_buildid(session, stdout, with_hits);
 
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 39e6627..fca1d44 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -180,8 +180,6 @@ static const struct option options[] = {
 	OPT_BOOLEAN('f', "force", &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('P', "full-paths", &symbol_conf.full_paths,
-		    "Don't shorten the pathnames taking into account the cwd"),
 	OPT_STRING('d', "dsos", &symbol_conf.dso_list_str, "dso[,dso...]",
 		   "only consider symbols in these dsos"),
 	OPT_STRING('C', "comms", &symbol_conf.comm_list_str, "comm[,comm...]",
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index ce42bba..2f4b929 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -441,8 +441,6 @@ static const struct option options[] = {
 		   "pretty printing style key: normal raw"),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
 		   "sort by key(s): pid, comm, dso, symbol, parent"),
-	OPT_BOOLEAN('P', "full-paths", &symbol_conf.full_paths,
-		    "Don't shorten the pathnames taking into account the cwd"),
 	OPT_BOOLEAN(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
 		    "Show sample percentage for different cpu modes"),
 	OPT_STRING('p', "parent", &parent_pattern, "regex",
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 0307918..8cbea12 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -96,8 +96,6 @@ struct perf_session *perf_session__new(const char *filename, int mode, bool forc
 	self->hists_tree = RB_ROOT;
 	self->last_match = NULL;
 	self->mmap_window = 32;
-	self->cwd = NULL;
-	self->cwdlen = 0;
 	self->machines = RB_ROOT;
 	self->repipe = repipe;
 	INIT_LIST_HEAD(&self->ordered_samples.samples_head);
@@ -130,7 +128,6 @@ void perf_session__delete(struct perf_session *self)
 {
 	perf_header__exit(&self->header);
 	close(self->fd);
-	free(self->cwd);
 	free(self);
 }
 
@@ -832,23 +829,6 @@ int perf_session__process_events(struct perf_session *self,
 	if (perf_session__register_idle_thread(self) == NULL)
 		return -ENOMEM;
 
-	if (!symbol_conf.full_paths) {
-		char bf[PATH_MAX];
-
-		if (getcwd(bf, sizeof(bf)) == NULL) {
-			err = -errno;
-out_getcwd_err:
-			pr_err("failed to get the current directory\n");
-			goto out_err;
-		}
-		self->cwd = strdup(bf);
-		if (self->cwd == NULL) {
-			err = -ENOMEM;
-			goto out_getcwd_err;
-		}
-		self->cwdlen = strlen(self->cwd);
-	}
-
 	if (!self->fd_pipe)
 		err = __perf_session__process_events(self,
 						     self->header.data_offset,
@@ -856,7 +836,7 @@ out_getcwd_err:
 						     self->size, ops);
 	else
 		err = __perf_session__process_pipe_events(self, ops);
-out_err:
+
 	return err;
 }
 
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index bb1aab9..6452a07 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -68,7 +68,6 @@ struct symbol_conf {
 			show_nr_samples,
 			use_callchain,
 			exclude_other,
-			full_paths,
 			show_cpu_utilization;
 	const char	*vmlinux_name,
 			*source_prefix,
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 10/19] perf man pages: Fix cut'n'paste error
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (8 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 09/19] perf tools: Remove unneeded code for tracking the cwd in perf sessions Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 11/19] perf record: Release resources at exit Arnaldo Carvalho de Melo
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

We remove files _from_ the cache.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-buildid-cache.txt |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index 5d1a950..c105770 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -12,9 +12,9 @@ SYNOPSIS
 
 DESCRIPTION
 -----------
-This command manages the build-id cache. It can add and remove files to the
-cache. In the future it should as well purge older entries, set upper limits
-for the space used by the cache, etc.
+This command manages the build-id cache. It can add and remove files to/from
+the cache. In the future it should as well purge older entries, set upper
+limits for the space used by the cache, etc.
 
 OPTIONS
 -------
@@ -23,7 +23,7 @@ OPTIONS
         Add specified file to the cache.
 -r::
 --remove=::
-        Remove specified file to the cache.
+        Remove specified file from the cache.
 -v::
 --verbose::
 	Be more verbose.
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 11/19] perf record: Release resources at exit
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (9 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 10/19] perf man pages: Fix cut'n'paste error Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  7:30   ` Nick Piggin
  2010-08-02  8:08   ` Pekka Enberg
  2010-08-02  1:08 ` [PATCH 12/19] perf symbols: Precisely specify if dso->{long,short}_name should be freed Arnaldo Carvalho de Melo
                   ` (8 subsequent siblings)
  19 siblings, 2 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

So that we can reduce the noise on valgrind when looking for memory
leaks.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |   32 ++++++++++++++++++++++++++------
 1 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index b938796..5ae0d93 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -439,6 +439,7 @@ static void atexit_header(void)
 
 		process_buildids();
 		perf_header__write(&session->header, output, true);
+		perf_session__delete(session);
 	}
 }
 
@@ -558,12 +559,15 @@ static int __cmd_record(int argc, const char **argv)
 	if (!file_new) {
 		err = perf_header__read(session, output);
 		if (err < 0)
-			return err;
+			goto out_delete_session;
 	}
 
 	if (have_tracepoints(attrs, nr_counters))
 		perf_header__set_feat(&session->header, HEADER_TRACE_INFO);
 
+	/*
+ 	 * perf_session__delete(session) will be called at atexit_header()
+	 */
 	atexit(atexit_header);
 
 	if (forks) {
@@ -768,6 +772,10 @@ static int __cmd_record(int argc, const char **argv)
 		bytes_written / 24);
 
 	return 0;
+
+out_delete_session:
+	perf_session__delete(session);
+	return err;
 }
 
 static const char * const record_usage[] = {
@@ -824,7 +832,7 @@ static const struct option options[] = {
 
 int cmd_record(int argc, const char **argv, const char *prefix __used)
 {
-	int i,j;
+	int i, j, err = -ENOMEM;
 
 	argc = parse_options(argc, argv, options, record_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
@@ -873,13 +881,13 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		for (j = 0; j < MAX_COUNTERS; j++) {
 			fd[i][j] = malloc(sizeof(int)*thread_num);
 			if (!fd[i][j])
-				return -ENOMEM;
+				goto out_free_fd;
 		}
 	}
 	event_array = malloc(
 		sizeof(struct pollfd)*MAX_NR_CPUS*MAX_COUNTERS*thread_num);
 	if (!event_array)
-		return -ENOMEM;
+		goto out_free_fd;
 
 	if (user_interval != ULLONG_MAX)
 		default_interval = user_interval;
@@ -895,8 +903,20 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		default_interval = freq;
 	} else {
 		fprintf(stderr, "frequency and count are zero, aborting\n");
-		exit(EXIT_FAILURE);
+		err = -EINVAL;
+		goto out_free_event_array;
 	}
 
-	return __cmd_record(argc, argv);
+	err = __cmd_record(argc, argv);
+
+out_free_event_array:
+	free(event_array);
+out_free_fd:
+	for (i = 0; i < MAX_NR_CPUS; i++) {
+		for (j = 0; j < MAX_COUNTERS; j++)
+			free(fd[i][j]);
+	}
+	free(all_tids);
+	all_tids = NULL;
+	return err;
 }
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 12/19] perf symbols: Precisely specify if dso->{long,short}_name should be freed
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (10 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 11/19] perf record: Release resources at exit Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 13/19] perf tools: Factor out buildid reading and make it implicit in dso__load Arnaldo Carvalho de Melo
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c  |    1 +
 tools/perf/util/symbol.c |    5 ++++-
 tools/perf/util/symbol.h |    4 +++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 5b81bb2..8151d23 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -456,6 +456,7 @@ static int event__process_kernel_mmap(event_t *self,
 			goto out_problem;
 
 		map->dso->short_name = name;
+		map->dso->sname_alloc = 1;
 		map->end = map->start + self->mmap.len;
 	} else if (is_kernel_mmap) {
 		const char *symbol_name = (self->mmap.filename +
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index bc6e7e8..242d2b2 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -224,7 +224,9 @@ void dso__delete(struct dso *self)
 	int i;
 	for (i = 0; i < MAP__NR_TYPES; ++i)
 		symbols__delete(&self->symbols[i]);
-	if (self->long_name != self->name)
+	if (self->sname_alloc)
+		free((char *)self->short_name);
+	if (self->lname_alloc)
 		free(self->long_name);
 	free(self);
 }
@@ -1530,6 +1532,7 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
 			if (long_name == NULL)
 				goto failure;
 			dso__set_long_name(map->dso, long_name);
+			map->dso->lname_alloc = 1;
 			dso__kernel_module_get_build_id(map->dso, "");
 		}
 	}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 6452a07..f29f73c 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -126,12 +126,14 @@ struct dso {
 	struct list_head node;
 	struct rb_root	 symbols[MAP__NR_TYPES];
 	struct rb_root	 symbol_names[MAP__NR_TYPES];
+	enum dso_kernel_type	kernel;
 	u8		 adjust_symbols:1;
 	u8		 slen_calculated:1;
 	u8		 has_build_id:1;
-	enum dso_kernel_type	kernel;
 	u8		 hit:1;
 	u8		 annotate_warned:1;
+	u8		 sname_alloc:1;
+	u8		 lname_alloc:1;
 	unsigned char	 origin;
 	u8		 sorted_by_name;
 	u8		 loaded;
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 13/19] perf tools: Factor out buildid reading and make it implicit in dso__load
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (11 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 12/19] perf symbols: Precisely specify if dso->{long,short}_name should be freed Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 14/19] perf tools: remove extra build-id check factored into dso__load Arnaldo Carvalho de Melo
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Dave Martin, Arnaldo Carvalho de Melo

From: Dave Martin <dave.martin@linaro.org>

If we have a buildid, then we never want to load an image which has no buildid,
or which has a different buildid, so it makes sense for the check to be built
into dso__load and not done separately.  This is fine for old distros which
don't use buildid at all since we do no check in that case.

This refactoring also alleviates some subtle race condition issues by not
opening ELF images twice to check the buildid and then load the symbols, which
could lead to weirdness if an image is replaced under our feet.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |   80 +++++++++++++++++++++++++++-------------------
 1 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 242d2b2..b812ace 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -26,6 +26,8 @@
 #define NT_GNU_BUILD_ID 3
 #endif
 
+static bool dso__build_id_equal(const struct dso *self, u8 *build_id);
+static int elf_read_build_id(Elf *elf, void *bf, size_t size);
 static void dsos__add(struct list_head *head, struct dso *dso);
 static struct map *map__new2(u64 start, struct dso *dso, enum map_type type);
 static int dso__load_kernel_sym(struct dso *self, struct map *map,
@@ -993,6 +995,17 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
 		goto out_elf_end;
 	}
 
+	if (self->has_build_id) {
+		u8 build_id[BUILD_ID_SIZE];
+
+		if (elf_read_build_id(elf, build_id,
+				      BUILD_ID_SIZE) != BUILD_ID_SIZE)
+			goto out_elf_end;
+
+		if (!dso__build_id_equal(self, build_id))
+			goto out_elf_end;
+	}
+
 	sec = elf_section_by_name(elf, &ehdr, &shdr, ".symtab", NULL);
 	if (sec == NULL) {
 		sec = elf_section_by_name(elf, &ehdr, &shdr, ".dynsym", NULL);
@@ -1193,37 +1206,26 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits)
  */
 #define NOTE_ALIGN(n) (((n) + 3) & -4U)
 
-int filename__read_build_id(const char *filename, void *bf, size_t size)
+static int elf_read_build_id(Elf *elf, void *bf, size_t size)
 {
-	int fd, err = -1;
+	int err = -1;
 	GElf_Ehdr ehdr;
 	GElf_Shdr shdr;
 	Elf_Data *data;
 	Elf_Scn *sec;
 	Elf_Kind ek;
 	void *ptr;
-	Elf *elf;
 
 	if (size < BUILD_ID_SIZE)
 		goto out;
 
-	fd = open(filename, O_RDONLY);
-	if (fd < 0)
-		goto out;
-
-	elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
-	if (elf == NULL) {
-		pr_debug2("%s: cannot read %s ELF file.\n", __func__, filename);
-		goto out_close;
-	}
-
 	ek = elf_kind(elf);
 	if (ek != ELF_K_ELF)
-		goto out_elf_end;
+		goto out;
 
 	if (gelf_getehdr(elf, &ehdr) == NULL) {
 		pr_err("%s: cannot get elf header.\n", __func__);
-		goto out_elf_end;
+		goto out;
 	}
 
 	sec = elf_section_by_name(elf, &ehdr, &shdr,
@@ -1232,12 +1234,12 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
 		sec = elf_section_by_name(elf, &ehdr, &shdr,
 					  ".notes", NULL);
 		if (sec == NULL)
-			goto out_elf_end;
+			goto out;
 	}
 
 	data = elf_getdata(sec, NULL);
 	if (data == NULL)
-		goto out_elf_end;
+		goto out;
 
 	ptr = data->d_buf;
 	while (ptr < (data->d_buf + data->d_size)) {
@@ -1259,7 +1261,31 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
 		}
 		ptr += descsz;
 	}
-out_elf_end:
+
+out:
+	return err;
+}
+
+int filename__read_build_id(const char *filename, void *bf, size_t size)
+{
+	int fd, err = -1;
+	Elf *elf;
+
+	if (size < BUILD_ID_SIZE)
+		goto out;
+
+	fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		goto out;
+
+	elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
+	if (elf == NULL) {
+		pr_debug2("%s: cannot read %s ELF file.\n", __func__, filename);
+		goto out_close;
+	}
+
+	err = elf_read_build_id(elf, bf, size);
+
 	elf_end(elf);
 out_close:
 	close(fd);
@@ -1335,7 +1361,6 @@ int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
 {
 	int size = PATH_MAX;
 	char *name;
-	u8 build_id[BUILD_ID_SIZE];
 	int ret = -1;
 	int fd;
 	struct machine *machine;
@@ -1382,16 +1407,14 @@ more:
 				 self->long_name);
 			break;
 		case DSO__ORIG_BUILDID:
-			if (filename__read_build_id(self->long_name, build_id,
-						    sizeof(build_id))) {
+			if (self->has_build_id) {
 				char build_id_hex[BUILD_ID_SIZE * 2 + 1];
-				build_id__sprintf(build_id, sizeof(build_id),
+				build_id__sprintf(self->build_id,
+						  sizeof(self->build_id),
 						  build_id_hex);
 				snprintf(name, size,
 					 "/usr/lib/debug/.build-id/%.2s/%s.debug",
 					build_id_hex, build_id_hex + 2);
-				if (self->has_build_id)
-					goto compare_build_id;
 				break;
 			}
 			self->origin++;
@@ -1410,15 +1433,6 @@ more:
 		default:
 			goto out;
 		}
-
-		if (self->has_build_id) {
-			if (filename__read_build_id(name, build_id,
-						    sizeof(build_id)) < 0)
-				goto more;
-compare_build_id:
-			if (!dso__build_id_equal(self, build_id))
-				goto more;
-		}
 open_file:
 		fd = open(name, O_RDONLY);
 	} while (fd < 0);
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 14/19] perf tools: remove extra build-id check factored into dso__load
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (12 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 13/19] perf tools: Factor out buildid reading and make it implicit in dso__load Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 15/19] perf symbols: Improve debug image search when loading symbols Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Dave Martin, Arnaldo Carvalho de Melo

From: Dave Martin <dave.martin@linaro.org>

Signed-off-by: Dave Martin <dave.martin@linaro.org>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |   28 ++--------------------------
 1 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b812ace..e0d9480 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -986,12 +986,12 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
 
 	elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
 	if (elf == NULL) {
-		pr_err("%s: cannot read %s ELF file.\n", __func__, name);
+		pr_debug("%s: cannot read %s ELF file.\n", __func__, name);
 		goto out_close;
 	}
 
 	if (gelf_getehdr(elf, &ehdr) == NULL) {
-		pr_err("%s: cannot get elf header.\n", __func__);
+		pr_debug("%s: cannot get elf header.\n", __func__);
 		goto out_elf_end;
 	}
 
@@ -1710,30 +1710,6 @@ static int dso__load_vmlinux(struct dso *self, struct map *map,
 {
 	int err = -1, fd;
 
-	if (self->has_build_id) {
-		u8 build_id[BUILD_ID_SIZE];
-
-		if (filename__read_build_id(vmlinux, build_id,
-					    sizeof(build_id)) < 0) {
-			pr_debug("No build_id in %s, ignoring it\n", vmlinux);
-			return -1;
-		}
-		if (!dso__build_id_equal(self, build_id)) {
-			char expected_build_id[BUILD_ID_SIZE * 2 + 1],
-			     vmlinux_build_id[BUILD_ID_SIZE * 2 + 1];
-
-			build_id__sprintf(self->build_id,
-					  sizeof(self->build_id),
-					  expected_build_id);
-			build_id__sprintf(build_id, sizeof(build_id),
-					  vmlinux_build_id);
-			pr_debug("build_id in %s is %s while expected is %s, "
-				 "ignoring it\n", vmlinux, vmlinux_build_id,
-				 expected_build_id);
-			return -1;
-		}
-	}
-
 	fd = open(vmlinux, O_RDONLY);
 	if (fd < 0)
 		return -1;
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 15/19] perf symbols: Improve debug image search when loading symbols
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (13 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 14/19] perf tools: remove extra build-id check factored into dso__load Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 16/19] perf tui: Make CTRL+Z suspend perf Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Dave Martin, Arnaldo Carvalho de Melo

From: Dave Martin <dave.martin@linaro.org>

Changes:
	* Simplification of the main search loop on dso__load()
	* Replace the search with a 2-pass search:
		* First, try to find an image with a proper symtab.
		* Second, repeat the search, accepting dynsym.

A second scan should only ever happen when needed debug images are
missing from the buildid cache or stale, i.e., when the cache is out of
sync.

Currently, the second scan also happens when using separated debug
images, since the caching logic doesn't currently know how to cache
those.  Improvements to the cache behaviour ought to solve that.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |   96 +++++++++++++++++++++++++++++-----------------
 1 files changed, 61 insertions(+), 35 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index e0d9480..d99497e 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -966,7 +966,8 @@ static size_t elf_addr_to_index(Elf *elf, GElf_Addr addr)
 }
 
 static int dso__load_sym(struct dso *self, struct map *map, const char *name,
-			 int fd, symbol_filter_t filter, int kmodule)
+			 int fd, symbol_filter_t filter, int kmodule,
+			 int want_symtab)
 {
 	struct kmap *kmap = self->kernel ? map__kmap(map) : NULL;
 	struct map *curr_map = map;
@@ -995,6 +996,7 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
 		goto out_elf_end;
 	}
 
+	/* Always reject images with a mismatched build-id: */
 	if (self->has_build_id) {
 		u8 build_id[BUILD_ID_SIZE];
 
@@ -1008,6 +1010,9 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
 
 	sec = elf_section_by_name(elf, &ehdr, &shdr, ".symtab", NULL);
 	if (sec == NULL) {
+		if (want_symtab)
+			goto out_elf_end;
+
 		sec = elf_section_by_name(elf, &ehdr, &shdr, ".dynsym", NULL);
 		if (sec == NULL)
 			goto out_elf_end;
@@ -1365,6 +1370,7 @@ int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
 	int fd;
 	struct machine *machine;
 	const char *root_dir;
+	int want_symtab;
 
 	dso__set_loaded(self, map->type);
 
@@ -1391,13 +1397,18 @@ int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
 		return ret;
 	}
 
-	self->origin = DSO__ORIG_BUILD_ID_CACHE;
-	if (dso__build_id_filename(self, name, size) != NULL)
-		goto open_file;
-more:
-	do {
-		self->origin++;
+	/* Iterate over candidate debug images.
+	 * On the first pass, only load images if they have a full symtab.
+	 * Failing that, do a second pass where we accept .dynsym also
+	 */
+	for (self->origin = DSO__ORIG_BUILD_ID_CACHE, want_symtab = 1;
+	     self->origin != DSO__ORIG_NOT_FOUND;
+	     self->origin++) {
 		switch (self->origin) {
+		case DSO__ORIG_BUILD_ID_CACHE:
+			if (dso__build_id_filename(self, name, size) == NULL)
+				continue;
+			break;
 		case DSO__ORIG_FEDORA:
 			snprintf(name, size, "/usr/lib/debug%s.debug",
 				 self->long_name);
@@ -1406,19 +1417,20 @@ more:
 			snprintf(name, size, "/usr/lib/debug%s",
 				 self->long_name);
 			break;
-		case DSO__ORIG_BUILDID:
-			if (self->has_build_id) {
-				char build_id_hex[BUILD_ID_SIZE * 2 + 1];
-				build_id__sprintf(self->build_id,
-						  sizeof(self->build_id),
-						  build_id_hex);
-				snprintf(name, size,
-					 "/usr/lib/debug/.build-id/%.2s/%s.debug",
-					build_id_hex, build_id_hex + 2);
-				break;
+		case DSO__ORIG_BUILDID: {
+			char build_id_hex[BUILD_ID_SIZE * 2 + 1];
+
+			if (!self->has_build_id)
+				continue;
+
+			build_id__sprintf(self->build_id,
+					  sizeof(self->build_id),
+					  build_id_hex);
+			snprintf(name, size,
+				 "/usr/lib/debug/.build-id/%.2s/%s.debug",
+				 build_id_hex, build_id_hex + 2);
 			}
-			self->origin++;
-			/* Fall thru */
+			break;
 		case DSO__ORIG_DSO:
 			snprintf(name, size, "%s", self->long_name);
 			break;
@@ -1431,27 +1443,41 @@ more:
 			break;
 
 		default:
-			goto out;
+			/*
+			 * If we wanted a full symtab but no image had one,
+			 * relax our requirements and repeat the search.
+			 */
+			if (want_symtab) {
+				want_symtab = 0;
+				self->origin = DSO__ORIG_BUILD_ID_CACHE;
+			} else
+				continue;
 		}
-open_file:
+
+		/* Name is now the name of the next image to try */
 		fd = open(name, O_RDONLY);
-	} while (fd < 0);
+		if (fd < 0)
+			continue;
 
-	ret = dso__load_sym(self, map, name, fd, filter, 0);
-	close(fd);
+		ret = dso__load_sym(self, map, name, fd, filter, 0,
+				    want_symtab);
+		close(fd);
 
-	/*
-	 * Some people seem to have debuginfo files _WITHOUT_ debug info!?!?
-	 */
-	if (!ret)
-		goto more;
+		/*
+		 * Some people seem to have debuginfo files _WITHOUT_ debug
+		 * info!?!?
+		 */
+		if (!ret)
+			continue;
 
-	if (ret > 0) {
-		int nr_plt = dso__synthesize_plt_symbols(self, map, filter);
-		if (nr_plt > 0)
-			ret += nr_plt;
+		if (ret > 0) {
+			int nr_plt = dso__synthesize_plt_symbols(self, map, filter);
+			if (nr_plt > 0)
+				ret += nr_plt;
+			break;
+		}
 	}
-out:
+
 	free(name);
 	if (ret < 0 && strstr(self->name, " (deleted)") != NULL)
 		return 0;
@@ -1715,7 +1741,7 @@ static int dso__load_vmlinux(struct dso *self, struct map *map,
 		return -1;
 
 	dso__set_loaded(self, map->type);
-	err = dso__load_sym(self, map, vmlinux, fd, filter, 0);
+	err = dso__load_sym(self, map, vmlinux, fd, filter, 0, 0);
 	close(fd);
 
 	if (err > 0)
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 16/19] perf tui: Make CTRL+Z suspend perf
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (14 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 15/19] perf symbols: Improve debug image search when loading symbols Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 17/19] perf probe: Rename common fields/functions from kprobe to probe Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/newt.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index 28f74eb..91de99b 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -11,6 +11,7 @@
 #define HAVE_LONG_LONG __GLIBC_HAVE_LONG_LONG
 #endif
 #include <slang.h>
+#include <signal.h>
 #include <stdlib.h>
 #include <newt.h>
 #include <sys/ttydefaults.h>
@@ -891,6 +892,13 @@ static struct newtPercentTreeColors {
 	"blue",	     "lightgray",
 };
 
+static void newt_suspend(void *d __used)
+{
+	newtSuspend();
+	raise(SIGTSTP);
+	newtResume();
+}
+
 void setup_browser(void)
 {
 	struct newtPercentTreeColors *c = &defaultPercentTreeColors;
@@ -904,6 +912,7 @@ void setup_browser(void)
 	use_browser = 1;
 	newtInit();
 	newtCls();
+	newtSetSuspendCallback(newt_suspend, NULL);
 	ui_helpline__puts(" ");
 	sltt_set_color(HE_COLORSET_TOP, NULL, c->topColorFg, c->topColorBg);
 	sltt_set_color(HE_COLORSET_MEDIUM, NULL, c->mediumColorFg, c->mediumColorBg);
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 17/19] perf probe: Rename common fields/functions from kprobe to probe.
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (15 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 16/19] perf tui: Make CTRL+Z suspend perf Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 18/19] perf tools: Release thread resources on PERF_RECORD_EXIT Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Srikar Dronamraju, Ananth N Mavinakayanahalli,
	Andrew Morton, Christoph Hellwig, Frank Ch. Eigler,
	Frederic Weisbecker, Ingo Molnar, Jim Keniston, Linus Torvalds,
	Mark Wielaard, Mathieu Desnoyers, Naren A Devaiah, Oleg Nesterov,
	Paul E. McKenney, Peter Zijlstra, Randy Dunlap, Steven Rostedt,
	Arnaldo Carvalho de Melo

From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>

As a precursor for perf to support uprobes, rename fields/functions
that had kprobe in their name but can be shared across perf-kprobes
and perf-uprobes to probe.

Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: "Frank Ch. Eigler" <fche@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jim Keniston <jkenisto@linux.vnet.ibm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mark Wielaard <mjw@redhat.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Naren A Devaiah <naren.devaiah@in.ibm.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Randy Dunlap <rdunlap@xenotime.net>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <20100729141351.GG21723@linux.vnet.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-probe.c     |    1 -
 tools/perf/util/probe-event.c  |  135 ++++++++++++++++++++-------------------
 tools/perf/util/probe-event.h  |   27 +++-----
 tools/perf/util/probe-finder.c |   34 +++++-----
 tools/perf/util/probe-finder.h |   10 ++--
 5 files changed, 101 insertions(+), 106 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 5455186..199d5e1 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -267,4 +267,3 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 	}
 	return 0;
 }
-
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 4445a1e..2e665cb 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1,5 +1,5 @@
 /*
- * probe-event.c : perf-probe definition to kprobe_events format converter
+ * probe-event.c : perf-probe definition to probe_events format converter
  *
  * Written by Masami Hiramatsu <mhiramat@redhat.com>
  *
@@ -120,8 +120,11 @@ static int open_vmlinux(void)
 	return open(machine.vmlinux_maps[MAP__FUNCTION]->dso->long_name, O_RDONLY);
 }
 
-/* Convert trace point to probe point with debuginfo */
-static int convert_to_perf_probe_point(struct kprobe_trace_point *tp,
+/*
+ * Convert trace point to probe point with debuginfo
+ * Currently only handles kprobes.
+ */
+static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
 				       struct perf_probe_point *pp)
 {
 	struct symbol *sym;
@@ -151,8 +154,8 @@ static int convert_to_perf_probe_point(struct kprobe_trace_point *tp,
 }
 
 /* Try to find perf_probe_event with debuginfo */
-static int try_to_find_kprobe_trace_events(struct perf_probe_event *pev,
-					   struct kprobe_trace_event **tevs,
+static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
+					   struct probe_trace_event **tevs,
 					   int max_tevs)
 {
 	bool need_dwarf = perf_probe_event_need_dwarf(pev);
@@ -169,11 +172,11 @@ static int try_to_find_kprobe_trace_events(struct perf_probe_event *pev,
 	}
 
 	/* Searching trace events corresponding to probe event */
-	ntevs = find_kprobe_trace_events(fd, pev, tevs, max_tevs);
+	ntevs = find_probe_trace_events(fd, pev, tevs, max_tevs);
 	close(fd);
 
 	if (ntevs > 0) {	/* Succeeded to find trace events */
-		pr_debug("find %d kprobe_trace_events.\n", ntevs);
+		pr_debug("find %d probe_trace_events.\n", ntevs);
 		return ntevs;
 	}
 
@@ -377,8 +380,8 @@ end:
 
 #else	/* !DWARF_SUPPORT */
 
-static int convert_to_perf_probe_point(struct kprobe_trace_point *tp,
-					struct perf_probe_point *pp)
+static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
+				       struct perf_probe_point *pp)
 {
 	pp->function = strdup(tp->symbol);
 	if (pp->function == NULL)
@@ -389,8 +392,8 @@ static int convert_to_perf_probe_point(struct kprobe_trace_point *tp,
 	return 0;
 }
 
-static int try_to_find_kprobe_trace_events(struct perf_probe_event *pev,
-				struct kprobe_trace_event **tevs __unused,
+static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
+				struct probe_trace_event **tevs __unused,
 				int max_tevs __unused)
 {
 	if (perf_probe_event_need_dwarf(pev)) {
@@ -781,16 +784,17 @@ bool perf_probe_event_need_dwarf(struct perf_probe_event *pev)
 	return false;
 }
 
-/* Parse kprobe_events event into struct probe_point */
-int parse_kprobe_trace_command(const char *cmd, struct kprobe_trace_event *tev)
+/* Parse probe_events event into struct probe_point */
+static int parse_probe_trace_command(const char *cmd,
+					struct probe_trace_event *tev)
 {
-	struct kprobe_trace_point *tp = &tev->point;
+	struct probe_trace_point *tp = &tev->point;
 	char pr;
 	char *p;
 	int ret, i, argc;
 	char **argv;
 
-	pr_debug("Parsing kprobe_events: %s\n", cmd);
+	pr_debug("Parsing probe_events: %s\n", cmd);
 	argv = argv_split(cmd, &argc);
 	if (!argv) {
 		pr_debug("Failed to split arguments.\n");
@@ -822,7 +826,7 @@ int parse_kprobe_trace_command(const char *cmd, struct kprobe_trace_event *tev)
 		tp->offset = 0;
 
 	tev->nargs = argc - 2;
-	tev->args = zalloc(sizeof(struct kprobe_trace_arg) * tev->nargs);
+	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
 	if (tev->args == NULL) {
 		ret = -ENOMEM;
 		goto out;
@@ -968,13 +972,13 @@ char *synthesize_perf_probe_command(struct perf_probe_event *pev)
 }
 #endif
 
-static int __synthesize_kprobe_trace_arg_ref(struct kprobe_trace_arg_ref *ref,
+static int __synthesize_probe_trace_arg_ref(struct probe_trace_arg_ref *ref,
 					     char **buf, size_t *buflen,
 					     int depth)
 {
 	int ret;
 	if (ref->next) {
-		depth = __synthesize_kprobe_trace_arg_ref(ref->next, buf,
+		depth = __synthesize_probe_trace_arg_ref(ref->next, buf,
 							 buflen, depth + 1);
 		if (depth < 0)
 			goto out;
@@ -992,10 +996,10 @@ out:
 
 }
 
-static int synthesize_kprobe_trace_arg(struct kprobe_trace_arg *arg,
+static int synthesize_probe_trace_arg(struct probe_trace_arg *arg,
 				       char *buf, size_t buflen)
 {
-	struct kprobe_trace_arg_ref *ref = arg->ref;
+	struct probe_trace_arg_ref *ref = arg->ref;
 	int ret, depth = 0;
 	char *tmp = buf;
 
@@ -1015,7 +1019,7 @@ static int synthesize_kprobe_trace_arg(struct kprobe_trace_arg *arg,
 
 	/* Dereferencing arguments */
 	if (ref) {
-		depth = __synthesize_kprobe_trace_arg_ref(ref, &buf,
+		depth = __synthesize_probe_trace_arg_ref(ref, &buf,
 							  &buflen, 1);
 		if (depth < 0)
 			return depth;
@@ -1051,9 +1055,9 @@ static int synthesize_kprobe_trace_arg(struct kprobe_trace_arg *arg,
 	return buf - tmp;
 }
 
-char *synthesize_kprobe_trace_command(struct kprobe_trace_event *tev)
+char *synthesize_probe_trace_command(struct probe_trace_event *tev)
 {
-	struct kprobe_trace_point *tp = &tev->point;
+	struct probe_trace_point *tp = &tev->point;
 	char *buf;
 	int i, len, ret;
 
@@ -1069,7 +1073,7 @@ char *synthesize_kprobe_trace_command(struct kprobe_trace_event *tev)
 		goto error;
 
 	for (i = 0; i < tev->nargs; i++) {
-		ret = synthesize_kprobe_trace_arg(&tev->args[i], buf + len,
+		ret = synthesize_probe_trace_arg(&tev->args[i], buf + len,
 						  MAX_CMDLEN - len);
 		if (ret <= 0)
 			goto error;
@@ -1082,7 +1086,7 @@ error:
 	return NULL;
 }
 
-int convert_to_perf_probe_event(struct kprobe_trace_event *tev,
+static int convert_to_perf_probe_event(struct probe_trace_event *tev,
 				struct perf_probe_event *pev)
 {
 	char buf[64] = "";
@@ -1095,7 +1099,7 @@ int convert_to_perf_probe_event(struct kprobe_trace_event *tev,
 		return -ENOMEM;
 
 	/* Convert trace_point to probe_point */
-	ret = convert_to_perf_probe_point(&tev->point, &pev->point);
+	ret = kprobe_convert_to_perf_probe(&tev->point, &pev->point);
 	if (ret < 0)
 		return ret;
 
@@ -1108,7 +1112,7 @@ int convert_to_perf_probe_event(struct kprobe_trace_event *tev,
 		if (tev->args[i].name)
 			pev->args[i].name = strdup(tev->args[i].name);
 		else {
-			ret = synthesize_kprobe_trace_arg(&tev->args[i],
+			ret = synthesize_probe_trace_arg(&tev->args[i],
 							  buf, 64);
 			pev->args[i].name = strdup(buf);
 		}
@@ -1159,9 +1163,9 @@ void clear_perf_probe_event(struct perf_probe_event *pev)
 	memset(pev, 0, sizeof(*pev));
 }
 
-void clear_kprobe_trace_event(struct kprobe_trace_event *tev)
+static void clear_probe_trace_event(struct probe_trace_event *tev)
 {
-	struct kprobe_trace_arg_ref *ref, *next;
+	struct probe_trace_arg_ref *ref, *next;
 	int i;
 
 	if (tev->event)
@@ -1222,7 +1226,7 @@ static int open_kprobe_events(bool readwrite)
 }
 
 /* Get raw string list of current kprobe_events */
-static struct strlist *get_kprobe_trace_command_rawlist(int fd)
+static struct strlist *get_probe_trace_command_rawlist(int fd)
 {
 	int ret, idx;
 	FILE *fp;
@@ -1290,7 +1294,7 @@ static int show_perf_probe_event(struct perf_probe_event *pev)
 int show_perf_probe_events(void)
 {
 	int fd, ret;
-	struct kprobe_trace_event tev;
+	struct probe_trace_event tev;
 	struct perf_probe_event pev;
 	struct strlist *rawlist;
 	struct str_node *ent;
@@ -1307,20 +1311,20 @@ int show_perf_probe_events(void)
 	if (fd < 0)
 		return fd;
 
-	rawlist = get_kprobe_trace_command_rawlist(fd);
+	rawlist = get_probe_trace_command_rawlist(fd);
 	close(fd);
 	if (!rawlist)
 		return -ENOENT;
 
 	strlist__for_each(ent, rawlist) {
-		ret = parse_kprobe_trace_command(ent->s, &tev);
+		ret = parse_probe_trace_command(ent->s, &tev);
 		if (ret >= 0) {
 			ret = convert_to_perf_probe_event(&tev, &pev);
 			if (ret >= 0)
 				ret = show_perf_probe_event(&pev);
 		}
 		clear_perf_probe_event(&pev);
-		clear_kprobe_trace_event(&tev);
+		clear_probe_trace_event(&tev);
 		if (ret < 0)
 			break;
 	}
@@ -1330,20 +1334,19 @@ int show_perf_probe_events(void)
 }
 
 /* Get current perf-probe event names */
-static struct strlist *get_kprobe_trace_event_names(int fd, bool include_group)
+static struct strlist *get_probe_trace_event_names(int fd, bool include_group)
 {
 	char buf[128];
 	struct strlist *sl, *rawlist;
 	struct str_node *ent;
-	struct kprobe_trace_event tev;
+	struct probe_trace_event tev;
 	int ret = 0;
 
 	memset(&tev, 0, sizeof(tev));
-
-	rawlist = get_kprobe_trace_command_rawlist(fd);
+	rawlist = get_probe_trace_command_rawlist(fd);
 	sl = strlist__new(true, NULL);
 	strlist__for_each(ent, rawlist) {
-		ret = parse_kprobe_trace_command(ent->s, &tev);
+		ret = parse_probe_trace_command(ent->s, &tev);
 		if (ret < 0)
 			break;
 		if (include_group) {
@@ -1353,7 +1356,7 @@ static struct strlist *get_kprobe_trace_event_names(int fd, bool include_group)
 				ret = strlist__add(sl, buf);
 		} else
 			ret = strlist__add(sl, tev.event);
-		clear_kprobe_trace_event(&tev);
+		clear_probe_trace_event(&tev);
 		if (ret < 0)
 			break;
 	}
@@ -1366,13 +1369,13 @@ static struct strlist *get_kprobe_trace_event_names(int fd, bool include_group)
 	return sl;
 }
 
-static int write_kprobe_trace_event(int fd, struct kprobe_trace_event *tev)
+static int write_probe_trace_event(int fd, struct probe_trace_event *tev)
 {
 	int ret = 0;
-	char *buf = synthesize_kprobe_trace_command(tev);
+	char *buf = synthesize_probe_trace_command(tev);
 
 	if (!buf) {
-		pr_debug("Failed to synthesize kprobe trace event.\n");
+		pr_debug("Failed to synthesize probe trace event.\n");
 		return -EINVAL;
 	}
 
@@ -1425,12 +1428,12 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 	return ret;
 }
 
-static int __add_kprobe_trace_events(struct perf_probe_event *pev,
-				     struct kprobe_trace_event *tevs,
+static int __add_probe_trace_events(struct perf_probe_event *pev,
+				     struct probe_trace_event *tevs,
 				     int ntevs, bool allow_suffix)
 {
 	int i, fd, ret;
-	struct kprobe_trace_event *tev = NULL;
+	struct probe_trace_event *tev = NULL;
 	char buf[64];
 	const char *event, *group;
 	struct strlist *namelist;
@@ -1439,7 +1442,7 @@ static int __add_kprobe_trace_events(struct perf_probe_event *pev,
 	if (fd < 0)
 		return fd;
 	/* Get current event names */
-	namelist = get_kprobe_trace_event_names(fd, false);
+	namelist = get_probe_trace_event_names(fd, false);
 	if (!namelist) {
 		pr_debug("Failed to get current event list.\n");
 		return -EIO;
@@ -1474,7 +1477,7 @@ static int __add_kprobe_trace_events(struct perf_probe_event *pev,
 			ret = -ENOMEM;
 			break;
 		}
-		ret = write_kprobe_trace_event(fd, tev);
+		ret = write_probe_trace_event(fd, tev);
 		if (ret < 0)
 			break;
 		/* Add added event name to namelist */
@@ -1511,21 +1514,21 @@ static int __add_kprobe_trace_events(struct perf_probe_event *pev,
 	return ret;
 }
 
-static int convert_to_kprobe_trace_events(struct perf_probe_event *pev,
-					  struct kprobe_trace_event **tevs,
+static int convert_to_probe_trace_events(struct perf_probe_event *pev,
+					  struct probe_trace_event **tevs,
 					  int max_tevs)
 {
 	struct symbol *sym;
 	int ret = 0, i;
-	struct kprobe_trace_event *tev;
+	struct probe_trace_event *tev;
 
 	/* Convert perf_probe_event with debuginfo */
-	ret = try_to_find_kprobe_trace_events(pev, tevs, max_tevs);
+	ret = try_to_find_probe_trace_events(pev, tevs, max_tevs);
 	if (ret != 0)
 		return ret;
 
 	/* Allocate trace event buffer */
-	tev = *tevs = zalloc(sizeof(struct kprobe_trace_event));
+	tev = *tevs = zalloc(sizeof(struct probe_trace_event));
 	if (tev == NULL)
 		return -ENOMEM;
 
@@ -1538,7 +1541,7 @@ static int convert_to_kprobe_trace_events(struct perf_probe_event *pev,
 	tev->point.offset = pev->point.offset;
 	tev->nargs = pev->nargs;
 	if (tev->nargs) {
-		tev->args = zalloc(sizeof(struct kprobe_trace_arg)
+		tev->args = zalloc(sizeof(struct probe_trace_arg)
 				   * tev->nargs);
 		if (tev->args == NULL) {
 			ret = -ENOMEM;
@@ -1579,7 +1582,7 @@ static int convert_to_kprobe_trace_events(struct perf_probe_event *pev,
 
 	return 1;
 error:
-	clear_kprobe_trace_event(tev);
+	clear_probe_trace_event(tev);
 	free(tev);
 	*tevs = NULL;
 	return ret;
@@ -1587,7 +1590,7 @@ error:
 
 struct __event_package {
 	struct perf_probe_event		*pev;
-	struct kprobe_trace_event	*tevs;
+	struct probe_trace_event	*tevs;
 	int				ntevs;
 };
 
@@ -1610,7 +1613,7 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
 	for (i = 0; i < npevs; i++) {
 		pkgs[i].pev = &pevs[i];
 		/* Convert with or without debuginfo */
-		ret  = convert_to_kprobe_trace_events(pkgs[i].pev,
+		ret  = convert_to_probe_trace_events(pkgs[i].pev,
 						      &pkgs[i].tevs, max_tevs);
 		if (ret < 0)
 			goto end;
@@ -1619,24 +1622,24 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
 
 	/* Loop 2: add all events */
 	for (i = 0; i < npevs && ret >= 0; i++)
-		ret = __add_kprobe_trace_events(pkgs[i].pev, pkgs[i].tevs,
+		ret = __add_probe_trace_events(pkgs[i].pev, pkgs[i].tevs,
 						pkgs[i].ntevs, force_add);
 end:
 	/* Loop 3: cleanup trace events  */
 	for (i = 0; i < npevs; i++)
 		for (j = 0; j < pkgs[i].ntevs; j++)
-			clear_kprobe_trace_event(&pkgs[i].tevs[j]);
+			clear_probe_trace_event(&pkgs[i].tevs[j]);
 
 	return ret;
 }
 
-static int __del_trace_kprobe_event(int fd, struct str_node *ent)
+static int __del_trace_probe_event(int fd, struct str_node *ent)
 {
 	char *p;
 	char buf[128];
 	int ret;
 
-	/* Convert from perf-probe event to trace-kprobe event */
+	/* Convert from perf-probe event to trace-probe event */
 	ret = e_snprintf(buf, 128, "-:%s", ent->s);
 	if (ret < 0)
 		goto error;
@@ -1662,7 +1665,7 @@ error:
 	return ret;
 }
 
-static int del_trace_kprobe_event(int fd, const char *group,
+static int del_trace_probe_event(int fd, const char *group,
 				  const char *event, struct strlist *namelist)
 {
 	char buf[128];
@@ -1679,7 +1682,7 @@ static int del_trace_kprobe_event(int fd, const char *group,
 		strlist__for_each_safe(ent, n, namelist)
 			if (strglobmatch(ent->s, buf)) {
 				found++;
-				ret = __del_trace_kprobe_event(fd, ent);
+				ret = __del_trace_probe_event(fd, ent);
 				if (ret < 0)
 					break;
 				strlist__remove(namelist, ent);
@@ -1688,7 +1691,7 @@ static int del_trace_kprobe_event(int fd, const char *group,
 		ent = strlist__find(namelist, buf);
 		if (ent) {
 			found++;
-			ret = __del_trace_kprobe_event(fd, ent);
+			ret = __del_trace_probe_event(fd, ent);
 			if (ret >= 0)
 				strlist__remove(namelist, ent);
 		}
@@ -1712,7 +1715,7 @@ int del_perf_probe_events(struct strlist *dellist)
 		return fd;
 
 	/* Get current event names */
-	namelist = get_kprobe_trace_event_names(fd, true);
+	namelist = get_probe_trace_event_names(fd, true);
 	if (namelist == NULL)
 		return -EINVAL;
 
@@ -1733,7 +1736,7 @@ int del_perf_probe_events(struct strlist *dellist)
 			event = str;
 		}
 		pr_debug("Group: %s, Event: %s\n", group, event);
-		ret = del_trace_kprobe_event(fd, group, event, namelist);
+		ret = del_trace_probe_event(fd, group, event, namelist);
 		free(str);
 		if (ret < 0)
 			break;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index ed362ac..5af3924 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -7,33 +7,33 @@
 extern bool probe_event_dry_run;
 
 /* kprobe-tracer tracing point */
-struct kprobe_trace_point {
+struct probe_trace_point {
 	char		*symbol;	/* Base symbol */
 	unsigned long	offset;		/* Offset from symbol */
 	bool		retprobe;	/* Return probe flag */
 };
 
-/* kprobe-tracer tracing argument referencing offset */
-struct kprobe_trace_arg_ref {
-	struct kprobe_trace_arg_ref	*next;	/* Next reference */
+/* probe-tracer tracing argument referencing offset */
+struct probe_trace_arg_ref {
+	struct probe_trace_arg_ref	*next;	/* Next reference */
 	long				offset;	/* Offset value */
 };
 
 /* kprobe-tracer tracing argument */
-struct kprobe_trace_arg {
+struct probe_trace_arg {
 	char				*name;	/* Argument name */
 	char				*value;	/* Base value */
 	char				*type;	/* Type name */
-	struct kprobe_trace_arg_ref	*ref;	/* Referencing offset */
+	struct probe_trace_arg_ref	*ref;	/* Referencing offset */
 };
 
 /* kprobe-tracer tracing event (point + arg) */
-struct kprobe_trace_event {
+struct probe_trace_event {
 	char				*event;	/* Event name */
 	char				*group;	/* Group name */
-	struct kprobe_trace_point	point;	/* Trace point */
+	struct probe_trace_point	point;	/* Trace point */
 	int				nargs;	/* Number of args */
-	struct kprobe_trace_arg		*args;	/* Arguments */
+	struct probe_trace_arg		*args;	/* Arguments */
 };
 
 /* Perf probe probing point */
@@ -93,25 +93,18 @@ struct line_range {
 /* Command string to events */
 extern int parse_perf_probe_command(const char *cmd,
 				    struct perf_probe_event *pev);
-extern int parse_kprobe_trace_command(const char *cmd,
-				      struct kprobe_trace_event *tev);
 
 /* Events to command string */
 extern char *synthesize_perf_probe_command(struct perf_probe_event *pev);
-extern char *synthesize_kprobe_trace_command(struct kprobe_trace_event *tev);
+extern char *synthesize_probe_trace_command(struct probe_trace_event *tev);
 extern int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf,
 				     size_t len);
 
 /* Check the perf_probe_event needs debuginfo */
 extern bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
 
-/* Convert from kprobe_trace_event to perf_probe_event */
-extern int convert_to_perf_probe_event(struct kprobe_trace_event *tev,
-				       struct perf_probe_event *pev);
-
 /* Release event contents */
 extern void clear_perf_probe_event(struct perf_probe_event *pev);
-extern void clear_kprobe_trace_event(struct kprobe_trace_event *tev);
 
 /* Command string to line-range */
 extern int parse_line_range_desc(const char *cmd, struct line_range *lr);
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index f88070e..840f1aa 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -366,10 +366,10 @@ static Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
  * Probe finder related functions
  */
 
-static struct kprobe_trace_arg_ref *alloc_trace_arg_ref(long offs)
+static struct probe_trace_arg_ref *alloc_trace_arg_ref(long offs)
 {
-	struct kprobe_trace_arg_ref *ref;
-	ref = zalloc(sizeof(struct kprobe_trace_arg_ref));
+	struct probe_trace_arg_ref *ref;
+	ref = zalloc(sizeof(struct probe_trace_arg_ref));
 	if (ref != NULL)
 		ref->offset = offs;
 	return ref;
@@ -385,7 +385,7 @@ static int convert_variable_location(Dwarf_Die *vr_die, struct probe_finder *pf)
 	Dwarf_Word offs = 0;
 	bool ref = false;
 	const char *regs;
-	struct kprobe_trace_arg *tvar = pf->tvar;
+	struct probe_trace_arg *tvar = pf->tvar;
 	int ret;
 
 	/* TODO: handle more than 1 exprs */
@@ -459,10 +459,10 @@ static int convert_variable_location(Dwarf_Die *vr_die, struct probe_finder *pf)
 }
 
 static int convert_variable_type(Dwarf_Die *vr_die,
-				 struct kprobe_trace_arg *tvar,
+				 struct probe_trace_arg *tvar,
 				 const char *cast)
 {
-	struct kprobe_trace_arg_ref **ref_ptr = &tvar->ref;
+	struct probe_trace_arg_ref **ref_ptr = &tvar->ref;
 	Dwarf_Die type;
 	char buf[16];
 	int ret;
@@ -500,7 +500,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 			while (*ref_ptr)
 				ref_ptr = &(*ref_ptr)->next;
 			/* Add new reference with offset +0 */
-			*ref_ptr = zalloc(sizeof(struct kprobe_trace_arg_ref));
+			*ref_ptr = zalloc(sizeof(struct probe_trace_arg_ref));
 			if (*ref_ptr == NULL) {
 				pr_warning("Out of memory error\n");
 				return -ENOMEM;
@@ -545,10 +545,10 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 
 static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
 				    struct perf_probe_arg_field *field,
-				    struct kprobe_trace_arg_ref **ref_ptr,
+				    struct probe_trace_arg_ref **ref_ptr,
 				    Dwarf_Die *die_mem)
 {
-	struct kprobe_trace_arg_ref *ref = *ref_ptr;
+	struct probe_trace_arg_ref *ref = *ref_ptr;
 	Dwarf_Die type;
 	Dwarf_Word offs;
 	int ret, tag;
@@ -574,7 +574,7 @@ static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
 		pr_debug2("Array real type: (%x)\n",
 			 (unsigned)dwarf_dieoffset(&type));
 		if (tag == DW_TAG_pointer_type) {
-			ref = zalloc(sizeof(struct kprobe_trace_arg_ref));
+			ref = zalloc(sizeof(struct probe_trace_arg_ref));
 			if (ref == NULL)
 				return -ENOMEM;
 			if (*ref_ptr)
@@ -605,7 +605,7 @@ static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
 			return -EINVAL;
 		}
 
-		ref = zalloc(sizeof(struct kprobe_trace_arg_ref));
+		ref = zalloc(sizeof(struct probe_trace_arg_ref));
 		if (ref == NULL)
 			return -ENOMEM;
 		if (*ref_ptr)
@@ -738,7 +738,7 @@ static int find_variable(Dwarf_Die *sp_die, struct probe_finder *pf)
 /* Show a probe point to output buffer */
 static int convert_probe_point(Dwarf_Die *sp_die, struct probe_finder *pf)
 {
-	struct kprobe_trace_event *tev;
+	struct probe_trace_event *tev;
 	Dwarf_Addr eaddr;
 	Dwarf_Die die_mem;
 	const char *name;
@@ -803,7 +803,7 @@ static int convert_probe_point(Dwarf_Die *sp_die, struct probe_finder *pf)
 
 	/* Find each argument */
 	tev->nargs = pf->pev->nargs;
-	tev->args = zalloc(sizeof(struct kprobe_trace_arg) * tev->nargs);
+	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
 	if (tev->args == NULL)
 		return -ENOMEM;
 	for (i = 0; i < pf->pev->nargs; i++) {
@@ -1060,9 +1060,9 @@ static int find_probe_point_by_func(struct probe_finder *pf)
 	return _param.retval;
 }
 
-/* Find kprobe_trace_events specified by perf_probe_event from debuginfo */
-int find_kprobe_trace_events(int fd, struct perf_probe_event *pev,
-			     struct kprobe_trace_event **tevs, int max_tevs)
+/* Find probe_trace_events specified by perf_probe_event from debuginfo */
+int find_probe_trace_events(int fd, struct perf_probe_event *pev,
+			     struct probe_trace_event **tevs, int max_tevs)
 {
 	struct probe_finder pf = {.pev = pev, .max_tevs = max_tevs};
 	struct perf_probe_point *pp = &pev->point;
@@ -1072,7 +1072,7 @@ int find_kprobe_trace_events(int fd, struct perf_probe_event *pev,
 	Dwarf *dbg;
 	int ret = 0;
 
-	pf.tevs = zalloc(sizeof(struct kprobe_trace_event) * max_tevs);
+	pf.tevs = zalloc(sizeof(struct probe_trace_event) * max_tevs);
 	if (pf.tevs == NULL)
 		return -ENOMEM;
 	*tevs = pf.tevs;
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index e1f61dc..4507d51 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -16,9 +16,9 @@ static inline int is_c_varname(const char *name)
 }
 
 #ifdef DWARF_SUPPORT
-/* Find kprobe_trace_events specified by perf_probe_event from debuginfo */
-extern int find_kprobe_trace_events(int fd, struct perf_probe_event *pev,
-				    struct kprobe_trace_event **tevs,
+/* Find probe_trace_events specified by perf_probe_event from debuginfo */
+extern int find_probe_trace_events(int fd, struct perf_probe_event *pev,
+				    struct probe_trace_event **tevs,
 				    int max_tevs);
 
 /* Find a perf_probe_point from debuginfo */
@@ -33,7 +33,7 @@ extern int find_line_range(int fd, struct line_range *lr);
 
 struct probe_finder {
 	struct perf_probe_event	*pev;		/* Target probe event */
-	struct kprobe_trace_event *tevs;	/* Result trace events */
+	struct probe_trace_event *tevs;		/* Result trace events */
 	int			ntevs;		/* Number of trace events */
 	int			max_tevs;	/* Max number of trace events */
 
@@ -50,7 +50,7 @@ struct probe_finder {
 #endif
 	Dwarf_Op		*fb_ops;	/* Frame base attribute */
 	struct perf_probe_arg	*pvar;		/* Current target variable */
-	struct kprobe_trace_arg	*tvar;		/* Current result variable */
+	struct probe_trace_arg	*tvar;		/* Current result variable */
 };
 
 struct line_finder {
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 18/19] perf tools: Release thread resources on PERF_RECORD_EXIT
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (16 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 17/19] perf probe: Rename common fields/functions from kprobe to probe Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  1:08 ` [PATCH 19/19] perf tools: Release session and symbol resources on exit Arnaldo Carvalho de Melo
  2010-08-02  6:26 ` [PATCH 00/19] perf/core improvements and fixes Ingo Molnar
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David S. Miller,
	Frederic Weisbecker, Ingo Molnar, Mike Galbraith, Paul Mackerras,
	Peter Zijlstra, Stephane Eranian, Tom Zanussi

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

For long running sessions with many threads with short lifetimes the
amount of memory that the buildid process takes is too much.

Since we don't have hist_entries that may be pointing to them, we can
just release the resources associated with each thread when the exit
(PERF_RECORD_EXIT) event is received.

For normal processing we need to annotate maps with hits, and thus
hist_entries pointing to it and drop the ones that had none. Will be
done in a followup patch.

Cc: David S. Miller <davem@davemloft.net>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/build-id.c |   18 ++++++++++++++++++
 tools/perf/util/map.c      |   33 +++++++++++++++++++++++++++++++++
 tools/perf/util/map.h      |    1 +
 tools/perf/util/thread.c   |    7 +++++++
 tools/perf/util/thread.h   |    2 ++
 5 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 5c26e2d..e437edb 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -12,6 +12,7 @@
 #include "event.h"
 #include "symbol.h"
 #include <linux/kernel.h>
+#include "debug.h"
 
 static int build_id__mark_dso_hit(event_t *event, struct perf_session *session)
 {
@@ -34,10 +35,27 @@ static int build_id__mark_dso_hit(event_t *event, struct perf_session *session)
 	return 0;
 }
 
+static int event__exit_del_thread(event_t *self, struct perf_session *session)
+{
+	struct thread *thread = perf_session__findnew(session, self->fork.tid);
+
+	dump_printf("(%d:%d):(%d:%d)\n", self->fork.pid, self->fork.tid,
+		    self->fork.ppid, self->fork.ptid);
+
+	if (thread) {
+		rb_erase(&thread->rb_node, &session->threads);
+		session->last_match = NULL;
+		thread__delete(thread);
+	}
+
+	return 0;
+}
+
 struct perf_event_ops build_id__mark_dso_hit_ops = {
 	.sample	= build_id__mark_dso_hit,
 	.mmap	= event__process_mmap,
 	.fork	= event__process_task,
+	.exit	= event__exit_del_thread,
 };
 
 char *dso__build_id_filename(struct dso *self, char *bf, size_t size)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 37cab90..2ddbae3 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -228,6 +228,39 @@ void map_groups__init(struct map_groups *self)
 	self->machine = NULL;
 }
 
+static void maps__delete(struct rb_root *self)
+{
+	struct rb_node *next = rb_first(self);
+
+	while (next) {
+		struct map *pos = rb_entry(next, struct map, rb_node);
+
+		next = rb_next(&pos->rb_node);
+		rb_erase(&pos->rb_node, self);
+		map__delete(pos);
+	}
+}
+
+static void maps__delete_removed(struct list_head *self)
+{
+	struct map *pos, *n;
+
+	list_for_each_entry_safe(pos, n, self, node) {
+		list_del(&pos->node);
+		map__delete(pos);
+	}
+}
+
+void map_groups__exit(struct map_groups *self)
+{
+	int i;
+
+	for (i = 0; i < MAP__NR_TYPES; ++i) {
+		maps__delete(&self->maps[i]);
+		maps__delete_removed(&self->removed_maps[i]);
+	}
+}
+
 void map_groups__flush(struct map_groups *self)
 {
 	int type;
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 3b2f706..20eba42 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -127,6 +127,7 @@ size_t __map_groups__fprintf_maps(struct map_groups *self,
 void maps__insert(struct rb_root *maps, struct map *map);
 struct map *maps__find(struct rb_root *maps, u64 addr);
 void map_groups__init(struct map_groups *self);
+void map_groups__exit(struct map_groups *self);
 int map_groups__clone(struct map_groups *self,
 		      struct map_groups *parent, enum map_type type);
 size_t map_groups__fprintf(struct map_groups *self, int verbose, FILE *fp);
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 9a448b4..8c72d88 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -62,6 +62,13 @@ static struct thread *thread__new(pid_t pid)
 	return self;
 }
 
+void thread__delete(struct thread *self)
+{
+	map_groups__exit(&self->mg);
+	free(self->comm);
+	free(self);
+}
+
 int thread__set_comm(struct thread *self, const char *comm)
 {
 	int err;
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index ee6bbcf..688500f 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -20,6 +20,8 @@ struct thread {
 
 struct perf_session;
 
+void thread__delete(struct thread *self);
+
 int find_all_tid(int pid, pid_t ** all_tid);
 int thread__set_comm(struct thread *self, const char *comm);
 int thread__comm_len(struct thread *self);
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 19/19] perf tools: Release session and symbol resources on exit
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (17 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 18/19] perf tools: Release thread resources on PERF_RECORD_EXIT Arnaldo Carvalho de Melo
@ 2010-08-02  1:08 ` Arnaldo Carvalho de Melo
  2010-08-02  6:26 ` [PATCH 00/19] perf/core improvements and fixes Ingo Molnar
  19 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02  1:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Mike Galbraith, Peter Zijlstra, Stephane Eranian

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

So that we reduce the noise when looking for leaks using tools such as
valgrind.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |    5 ++++-
 tools/perf/util/event.c     |    5 +++--
 tools/perf/util/map.c       |   26 ++++++++++++++++++++++++++
 tools/perf/util/map.h       |    1 +
 tools/perf/util/session.c   |   26 ++++++++++++++++++++++++++
 tools/perf/util/symbol.c    |    9 +++++++++
 tools/perf/util/symbol.h    |    1 +
 7 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 5ae0d93..ff77b80 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -440,6 +440,7 @@ static void atexit_header(void)
 		process_buildids();
 		perf_header__write(&session->header, output, true);
 		perf_session__delete(session);
+		symbol__exit();
 	}
 }
 
@@ -871,7 +872,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 	} else {
 		all_tids=malloc(sizeof(pid_t));
 		if (!all_tids)
-			return -ENOMEM;
+			goto out_symbol_exit;
 
 		all_tids[0] = target_tid;
 		thread_num = 1;
@@ -918,5 +919,7 @@ out_free_fd:
 	}
 	free(all_tids);
 	all_tids = NULL;
+out_symbol_exit:
+	symbol__exit();
 	return err;
 }
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 8151d23..6b0db55 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -515,12 +515,13 @@ int event__process_mmap(event_t *self, struct perf_session *session)
 	if (machine == NULL)
 		goto out_problem;
 	thread = perf_session__findnew(session, self->mmap.pid);
+	if (thread == NULL)
+		goto out_problem;
 	map = map__new(&machine->user_dsos, self->mmap.start,
 			self->mmap.len, self->mmap.pgoff,
 			self->mmap.pid, self->mmap.filename,
 			MAP__FUNCTION);
-
-	if (thread == NULL || map == NULL)
+	if (map == NULL)
 		goto out_problem;
 
 	thread__insert_map(thread, map);
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 2ddbae3..15d6a6d 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -539,6 +539,32 @@ int machine__init(struct machine *self, const char *root_dir, pid_t pid)
 	return self->root_dir == NULL ? -ENOMEM : 0;
 }
 
+static void dsos__delete(struct list_head *self)
+{
+	struct dso *pos, *n;
+
+	list_for_each_entry_safe(pos, n, self, node) {
+		list_del(&pos->node);
+		dso__delete(pos);
+	}
+}
+
+void machine__exit(struct machine *self)
+{
+	struct kmap *kmap = map__kmap(self->vmlinux_maps[MAP__FUNCTION]);
+
+	if (kmap->ref_reloc_sym) {
+		free((char *)kmap->ref_reloc_sym->name);
+		free(kmap->ref_reloc_sym);
+	}
+
+	map_groups__exit(&self->kmaps);
+	dsos__delete(&self->user_dsos);
+	dsos__delete(&self->kernel_dsos);
+	free(self->root_dir);
+	self->root_dir = NULL;
+}
+
 struct machine *machines__add(struct rb_root *self, pid_t pid,
 			      const char *root_dir)
 {
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 20eba42..0e0984e 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -143,6 +143,7 @@ struct machine *machines__find(struct rb_root *self, pid_t pid);
 struct machine *machines__findnew(struct rb_root *self, pid_t pid);
 char *machine__mmap_name(struct machine *self, char *bf, size_t size);
 int machine__init(struct machine *self, const char *root_dir, pid_t pid);
+void machine__exit(struct machine *self);
 
 /*
  * Default guest kernel is defined by parameter --guestkallsyms
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 8cbea12..04a3b3d 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -124,9 +124,35 @@ out_delete:
 	return NULL;
 }
 
+static void perf_session__delete_dead_threads(struct perf_session *self)
+{
+	struct thread *n, *t;
+
+	list_for_each_entry_safe(t, n, &self->dead_threads, node) {
+		list_del(&t->node);
+		thread__delete(t);
+	}
+}
+
+static void perf_session__delete_threads(struct perf_session *self)
+{
+	struct rb_node *nd = rb_first(&self->threads);
+
+	while (nd) {
+		struct thread *t = rb_entry(nd, struct thread, rb_node);
+
+		rb_erase(&t->rb_node, &self->threads);
+		nd = rb_next(nd);
+		thread__delete(t);
+	}
+}
+
 void perf_session__delete(struct perf_session *self)
 {
 	perf_header__exit(&self->header);
+	perf_session__delete_dead_threads(self);
+	perf_session__delete_threads(self);
+	machine__exit(&self->host_machine);
 	close(self->fd);
 	free(self);
 }
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index d99497e..94cdf68 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2245,6 +2245,15 @@ out_free_comm_list:
 	return -1;
 }
 
+void symbol__exit(void)
+{
+	strlist__delete(symbol_conf.sym_list);
+	strlist__delete(symbol_conf.dso_list);
+	strlist__delete(symbol_conf.comm_list);
+	vmlinux_path__exit();
+	symbol_conf.sym_list = symbol_conf.dso_list = symbol_conf.comm_list = NULL;
+}
+
 int machines__create_kernel_maps(struct rb_root *self, pid_t pid)
 {
 	struct machine *machine = machines__findnew(self, pid);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index f29f73c..33d53ce 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -219,6 +219,7 @@ int machines__create_kernel_maps(struct rb_root *self, pid_t pid);
 int machines__create_guest_kernel_maps(struct rb_root *self);
 
 int symbol__init(void);
+void symbol__exit(void);
 bool symbol_type__is_a(char symbol_type, enum map_type map_type);
 
 size_t machine__fprintf_vmlinux_path(struct machine *self, FILE *fp);
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 00/19] perf/core improvements and fixes
       [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
                   ` (18 preceding siblings ...)
  2010-08-02  1:08 ` [PATCH 19/19] perf tools: Release session and symbol resources on exit Arnaldo Carvalho de Melo
@ 2010-08-02  6:26 ` Ingo Molnar
  19 siblings, 0 replies; 27+ messages in thread
From: Ingo Molnar @ 2010-08-02  6:26 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Ananth N Mavinakayanahalli, Andrew Morton,
	Christoph Hellwig, Dave Martin, David S . Miller,
	Frank Ch. Eigler, Frederic Weisbecker, Jim Keniston,
	Linus Torvalds, Mark Wielaard, Mathieu Desnoyers, Mike Galbraith,
	Naren A Devaiah, Oleg Nesterov, Paul E. McKenney, Paul Mackerras,
	Peter Zijlstra, Randy Dunlap, Srikar Dronamraju,
	Stephane Eranian, Steven Rostedt, Tom Zanussi


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

> Hi Ingo,
> 
>         Please pull from:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core
> 
> Regards,
> 
> - Arnaldo
> 
> Arnaldo Carvalho de Melo (13):
>   perf hists: Mark entries filtered by parent
>   perf sort: Make column width code per hists instance
>   perf ui: Restore SPACE as an alias to PGDN in annotate
>   perf hist: Introduce routine to measure lenght of formatted entry
>   perf ui: Consider the refreshed dimensions in ui_browser__show
>   perf ui: Show the scroll bar over the left window frame
>   perf ui: New hists tree widget
>   perf man pages: Fix cut'n'paste error
>   perf record: Release resources at exit
>   perf symbols: Precisely specify if dso->{long,short}_name should be freed
>   perf tui: Make CTRL+Z suspend perf
>   perf tools: Release thread resources on PERF_RECORD_EXIT
>   perf tools: Release session and symbol resources on exit
> 
> Dave Martin (5):
>   perf report: Don't abbreviate file paths relative to the cwd
>   perf tools: Remove unneeded code for tracking the cwd in perf sessions
>   perf tools: Factor out buildid reading and make it implicit in dso__load
>   perf tools: remove extra build-id check factored into dso__load
>   perf symbols: Improve debug image search when loading symbols
> 
> Srikar Dronamraju (1):
>   perf probe: Rename common fields/functions from kprobe to probe.
> 
>  tools/perf/Documentation/perf-buildid-cache.txt |    8 +-
>  tools/perf/builtin-buildid-list.c               |    4 +-
>  tools/perf/builtin-diff.c                       |    2 -
>  tools/perf/builtin-probe.c                      |    1 -
>  tools/perf/builtin-record.c                     |   37 +-
>  tools/perf/builtin-report.c                     |    2 -
>  tools/perf/util/build-id.c                      |   18 +
>  tools/perf/util/event.c                         |   42 +-
>  tools/perf/util/hist.c                          |  160 +++-
>  tools/perf/util/hist.h                          |   30 +-
>  tools/perf/util/map.c                           |   81 ++-
>  tools/perf/util/map.h                           |    4 +-
>  tools/perf/util/newt.c                          |  991 ++++++++++++++++-------
>  tools/perf/util/probe-event.c                   |  135 ++--
>  tools/perf/util/probe-event.h                   |   27 +-
>  tools/perf/util/probe-finder.c                  |   34 +-
>  tools/perf/util/probe-finder.h                  |   10 +-
>  tools/perf/util/session.c                       |   48 +-
>  tools/perf/util/sort.c                          |   17 +-
>  tools/perf/util/sort.h                          |   18 +-
>  tools/perf/util/symbol.c                        |  219 +++---
>  tools/perf/util/symbol.h                        |   10 +-
>  tools/perf/util/thread.c                        |    7 +
>  tools/perf/util/thread.h                        |    2 +
>  24 files changed, 1253 insertions(+), 654 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/19] perf record: Release resources at exit
  2010-08-02  1:08 ` [PATCH 11/19] perf record: Release resources at exit Arnaldo Carvalho de Melo
@ 2010-08-02  7:30   ` Nick Piggin
  2010-08-02  7:54     ` Ingo Molnar
  2010-08-02  8:08   ` Pekka Enberg
  1 sibling, 1 reply; 27+ messages in thread
From: Nick Piggin @ 2010-08-02  7:30 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, linux-kernel, Arnaldo Carvalho de Melo,
	Frederic Weisbecker, Mike Galbraith, Peter Zijlstra,
	Stephane Eranian

On Sun, Aug 01, 2010 at 10:08:46PM -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> So that we can reduce the noise on valgrind when looking for memory
> leaks.

Really? That's rather crappy of valgrind. exit is well defined to
release resources and that's often a more efficient way to do it
It finds and batches things a lot better, eg. it can avoid all
TLB flushing of freeing memory that munmap requires.


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/19] perf record: Release resources at exit
  2010-08-02  7:30   ` Nick Piggin
@ 2010-08-02  7:54     ` Ingo Molnar
  2010-08-02  8:03       ` Nick Piggin
  0 siblings, 1 reply; 27+ messages in thread
From: Ingo Molnar @ 2010-08-02  7:54 UTC (permalink / raw)
  To: Nick Piggin
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Arnaldo Carvalho de Melo,
	Frederic Weisbecker, Mike Galbraith, Peter Zijlstra,
	Stephane Eranian


* Nick Piggin <npiggin@suse.de> wrote:

> On Sun, Aug 01, 2010 at 10:08:46PM -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > So that we can reduce the noise on valgrind when looking for memory
> > leaks.
> 
> Really? That's rather crappy of valgrind. exit is well defined to release 
> resources and that's often a more efficient way to do it It finds and 
> batches things a lot better, eg. it can avoid all TLB flushing of freeing 
> memory that munmap requires.

That's certainly true but there's no valgrind crappiness here: valgrind simply 
can do a better job of finding leaks if there's a well defined "all resources 
the app still knows about are freed now" point.

The _kernel_ obviously can release all resources. The distinction is between 
'resources known to the app' and 'all resources'. That set contains the 
leaking resources.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/19] perf record: Release resources at exit
  2010-08-02  7:54     ` Ingo Molnar
@ 2010-08-02  8:03       ` Nick Piggin
  2010-08-02  9:17         ` Ingo Molnar
  0 siblings, 1 reply; 27+ messages in thread
From: Nick Piggin @ 2010-08-02  8:03 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Nick Piggin, Arnaldo Carvalho de Melo, linux-kernel,
	Arnaldo Carvalho de Melo, Frederic Weisbecker, Mike Galbraith,
	Peter Zijlstra, Stephane Eranian

On Mon, Aug 02, 2010 at 09:54:22AM +0200, Ingo Molnar wrote:
> 
> * Nick Piggin <npiggin@suse.de> wrote:
> 
> > On Sun, Aug 01, 2010 at 10:08:46PM -0300, Arnaldo Carvalho de Melo wrote:
> > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > 
> > > So that we can reduce the noise on valgrind when looking for memory
> > > leaks.
> > 
> > Really? That's rather crappy of valgrind. exit is well defined to release 
> > resources and that's often a more efficient way to do it It finds and 
> > batches things a lot better, eg. it can avoid all TLB flushing of freeing 
> > memory that munmap requires.
> 
> That's certainly true but there's no valgrind crappiness here: valgrind simply 
> can do a better job of finding leaks if there's a well defined "all resources 
> the app still knows about are freed now" point.

"noise" sounds like false positives though. Certainly if this is
instead allows valgrind to run in a particular mode that assumes
no application resources consumed at exit(2) time, I wouldn't
call it crappy :)

But you could equally sprinkle in other valgrind specific annotations
or semantics at various points in the code to improve its coverage,
no?

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/19] perf record: Release resources at exit
  2010-08-02  1:08 ` [PATCH 11/19] perf record: Release resources at exit Arnaldo Carvalho de Melo
  2010-08-02  7:30   ` Nick Piggin
@ 2010-08-02  8:08   ` Pekka Enberg
  1 sibling, 0 replies; 27+ messages in thread
From: Pekka Enberg @ 2010-08-02  8:08 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, linux-kernel, Arnaldo Carvalho de Melo,
	Frederic Weisbecker, Mike Galbraith, Peter Zijlstra,
	Stephane Eranian

On Mon, Aug 2, 2010 at 4:08 AM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> So that we can reduce the noise on valgrind when looking for memory
> leaks.
>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Stephane Eranian <eranian@google.com>
> LKML-Reference: <new-submission>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Yes, please.

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/19] perf record: Release resources at exit
  2010-08-02  8:03       ` Nick Piggin
@ 2010-08-02  9:17         ` Ingo Molnar
  2010-08-02  9:59           ` Nick Piggin
  0 siblings, 1 reply; 27+ messages in thread
From: Ingo Molnar @ 2010-08-02  9:17 UTC (permalink / raw)
  To: Nick Piggin
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Arnaldo Carvalho de Melo,
	Frederic Weisbecker, Mike Galbraith, Peter Zijlstra,
	Stephane Eranian


* Nick Piggin <npiggin@suse.de> wrote:

> On Mon, Aug 02, 2010 at 09:54:22AM +0200, Ingo Molnar wrote:
> > 
> > * Nick Piggin <npiggin@suse.de> wrote:
> > 
> > > On Sun, Aug 01, 2010 at 10:08:46PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > > 
> > > > So that we can reduce the noise on valgrind when looking for memory
> > > > leaks.
> > > 
> > > Really? That's rather crappy of valgrind. exit is well defined to release 
> > > resources and that's often a more efficient way to do it It finds and 
> > > batches things a lot better, eg. it can avoid all TLB flushing of freeing 
> > > memory that munmap requires.
> > 
> > That's certainly true but there's no valgrind crappiness here: valgrind simply 
> > can do a better job of finding leaks if there's a well defined "all resources 
> > the app still knows about are freed now" point.
> 
> "noise" sounds like false positives though. [...]

Every predictive bug detection scheme is open to the potential of false 
positives. I've yet to see a complex one that is 100% false positive free.

> [...] Certainly if this is instead allows valgrind to run in a particular 
> mode that assumes no application resources consumed at exit(2) time, I 
> wouldn't call it crappy :)

Most apps free their stuff before they exit - i do it in all my own C apps.

That is generally useful: for example it makes it easier to thread a program 
later on - when exit() becomes pthread_exit() and a silent leak turns into a 
real leak.

Hence Valgrind checking for exit() by default looks useful to me.

> But you could equally sprinkle in other valgrind specific annotations or 
> semantics at various points in the code to improve its coverage, no?

Yeah, and exit() sounds like a pretty convenient point, right? That's the 
point where all resources are inactive hence a scan for leaks is expected to 
be the most efficient in finding real leaks.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/19] perf record: Release resources at exit
  2010-08-02  9:17         ` Ingo Molnar
@ 2010-08-02  9:59           ` Nick Piggin
  2010-08-02 14:49             ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 27+ messages in thread
From: Nick Piggin @ 2010-08-02  9:59 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Nick Piggin, Arnaldo Carvalho de Melo, linux-kernel,
	Arnaldo Carvalho de Melo, Frederic Weisbecker, Mike Galbraith,
	Peter Zijlstra, Stephane Eranian

On Mon, Aug 02, 2010 at 11:17:14AM +0200, Ingo Molnar wrote:
> 
> * Nick Piggin <npiggin@suse.de> wrote:
> 
> > On Mon, Aug 02, 2010 at 09:54:22AM +0200, Ingo Molnar wrote:
> > > 
> > > * Nick Piggin <npiggin@suse.de> wrote:
> > > 
> > > That's certainly true but there's no valgrind crappiness here: valgrind simply 
> > > can do a better job of finding leaks if there's a well defined "all resources 
> > > the app still knows about are freed now" point.
> > 
> > "noise" sounds like false positives though. [...]
> 
> Every predictive bug detection scheme is open to the potential of false 
> positives. I've yet to see a complex one that is 100% false positive free.

So long as false positive noise can be easily avoided in running
of the automated testing -- ie. not worked around in the software --
then this is totally fine of course.

 
> > [...] Certainly if this is instead allows valgrind to run in a particular 
> > mode that assumes no application resources consumed at exit(2) time, I 
> > wouldn't call it crappy :)
> 
> Most apps free their stuff before they exit - i do it in all my own C apps.
> 
> That is generally useful: for example it makes it easier to thread a program 
> later on - when exit() becomes pthread_exit() and a silent leak turns into a 
> real leak.
> 
> Hence Valgrind checking for exit() by default looks useful to me.

Sure, most of the time I would too (in fact, all the time seeing as
I don't write non-trivial user programs). But when you're doing last
pass optimisations, it would be very reasonable to avoid things like
teardown of complex data structures.

 
> > But you could equally sprinkle in other valgrind specific annotations or 
> > semantics at various points in the code to improve its coverage, no?
> 
> Yeah, and exit() sounds like a pretty convenient point, right? That's the 
> point where all resources are inactive hence a scan for leaks is expected to 
> be the most efficient in finding real leaks.


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/19] perf record: Release resources at exit
  2010-08-02  9:59           ` Nick Piggin
@ 2010-08-02 14:49             ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-02 14:49 UTC (permalink / raw)
  To: Nick Piggin
  Cc: Ingo Molnar, linux-kernel, Frederic Weisbecker, Mike Galbraith,
	Peter Zijlstra, Stephane Eranian

Em Mon, Aug 02, 2010 at 07:59:47PM +1000, Nick Piggin escreveu:
> On Mon, Aug 02, 2010 at 11:17:14AM +0200, Ingo Molnar wrote:
> > Most apps free their stuff before they exit - i do it in all my own C apps.
> > 
> > That is generally useful: for example it makes it easier to thread a program 
> > later on - when exit() becomes pthread_exit() and a silent leak turns into a 
> > real leak.

Right, part of my goal was exactly the potential librarization of
nowaday main routines of builtin commands.

> > Hence Valgrind checking for exit() by default looks useful to me.
> 
> Sure, most of the time I would too (in fact, all the time seeing as
> I don't write non-trivial user programs). But when you're doing last
> pass optimisations, it would be very reasonable to avoid things like
> teardown of complex data structures.

Yeah, I thought about having some test to only do the on-exit
destruction of lots of rbtrees if in debugging mode, and I may end up
doing it at some point.

But on another angle, right now I think we really need to more
aggressively delete stuff (like maps that had no hits on
PERF_RECORD_EXIT'ed threads and thus have no reference on any hist_entry
instance) to support huge perf.data file buildid exit processing. So
part of the goal was that: no leaks left behind policy.

That is, till the day when we stash the 20 byte buildid in the per
binary image loaded kernel data structure so that we can have it in the
PERF_RECORD_MMAP and avoid all this processing at exit, and instead do
it taking advantage of the existing noise at loading time. 8-)
 
- Arnaldo

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2010-08-02 14:50 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
2010-08-02  1:08 ` [PATCH 01/19] perf hists: Mark entries filtered by parent Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 02/19] perf sort: Make column width code per hists instance Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 03/19] perf ui: Restore SPACE as an alias to PGDN in annotate Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 04/19] perf hist: Introduce routine to measure lenght of formatted entry Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 05/19] perf ui: Consider the refreshed dimensions in ui_browser__show Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 06/19] perf ui: Show the scroll bar over the left window frame Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 07/19] perf ui: New hists tree widget Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 08/19] perf report: Don't abbreviate file paths relative to the cwd Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 09/19] perf tools: Remove unneeded code for tracking the cwd in perf sessions Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 10/19] perf man pages: Fix cut'n'paste error Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 11/19] perf record: Release resources at exit Arnaldo Carvalho de Melo
2010-08-02  7:30   ` Nick Piggin
2010-08-02  7:54     ` Ingo Molnar
2010-08-02  8:03       ` Nick Piggin
2010-08-02  9:17         ` Ingo Molnar
2010-08-02  9:59           ` Nick Piggin
2010-08-02 14:49             ` Arnaldo Carvalho de Melo
2010-08-02  8:08   ` Pekka Enberg
2010-08-02  1:08 ` [PATCH 12/19] perf symbols: Precisely specify if dso->{long,short}_name should be freed Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 13/19] perf tools: Factor out buildid reading and make it implicit in dso__load Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 14/19] perf tools: remove extra build-id check factored into dso__load Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 15/19] perf symbols: Improve debug image search when loading symbols Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 16/19] perf tui: Make CTRL+Z suspend perf Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 17/19] perf probe: Rename common fields/functions from kprobe to probe Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 18/19] perf tools: Release thread resources on PERF_RECORD_EXIT Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 19/19] perf tools: Release session and symbol resources on exit Arnaldo Carvalho de Melo
2010-08-02  6:26 ` [PATCH 00/19] perf/core improvements and fixes Ingo Molnar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.