2010-08-06 01:47:41

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 00/10] perf/core improvements and fixes

Hi Ingo,

Please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core

Regards,

- Arnaldo

Andrea Gelmini (2):
perf probe: Remove duplicated #include
perf trace: Clean up #includes

Arnaldo Carvalho de Melo (7):
perf symbols: Store the symbol binding
perf ui: Add a map browser
perf ui: Shorten ui_browser->refresh_entries to refresh
perf hists: Handle verbose in hists__sort_list_width
perf hists: Fixup addr snprintf width on 32 bit arches
perf ui: Add search by name/addr to the map__browser
perf report: Speed up exit path

Julia Lawall (1):
perf timechart: Adjust confusing if indentation

tools/perf/builtin-report.c | 31 +++++-
tools/perf/builtin-timechart.c | 4 +-
tools/perf/builtin-trace.c | 19 +--
tools/perf/util/hist.c | 3 +
tools/perf/util/newt.c | 242 +++++++++++++++++++++++++++++++++++++--
tools/perf/util/probe-finder.c | 1 -
tools/perf/util/sort.c | 6 +-
tools/perf/util/symbol.c | 30 ++++--
tools/perf/util/symbol.h | 1 +
9 files changed, 297 insertions(+), 40 deletions(-)


2010-08-06 01:46:37

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 05/10] perf hists: Fixup addr snprintf width on 32 bit arches

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

By using BITS_PER_LONG/4 as the width specifier.

Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/sort.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 1c61a4f..b62a553 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -196,7 +196,8 @@ static int hist_entry__sym_snprintf(struct hist_entry *self, char *bf,

if (verbose) {
char o = self->ms.map ? dso__symtab_origin(self->ms.map->dso) : '!';
- ret += repsep_snprintf(bf, size, "%#018llx %c ", self->ip, o);
+ ret += repsep_snprintf(bf, size, "%*Lx %c ",
+ BITS_PER_LONG / 4, self->ip, o);
}

ret += repsep_snprintf(bf + ret, size - ret, "[%c] ", self->level);
@@ -204,7 +205,8 @@ static int hist_entry__sym_snprintf(struct hist_entry *self, char *bf,
ret += repsep_snprintf(bf + ret, size - ret, "%s",
self->ms.sym->name);
else
- ret += repsep_snprintf(bf + ret, size - ret, "%#016llx", self->ip);
+ ret += repsep_snprintf(bf + ret, size - ret, "%*Lx",
+ BITS_PER_LONG / 4, self->ip);

return ret;
}
--
1.6.2.5

2010-08-06 01:46:46

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 10/10] perf report: Speed up exit path

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

When cmd_record exits the whole perf binary will exit right after,
so no need to traverse lots of complex data structures freeing them.

Sticked a comment for leak detectives and for a experiment with obstacks
to be performed so that we can speed up freeing that memory.

Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Nick Piggin <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-report.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4a7a743..55fc1f4 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -348,7 +348,18 @@ static int __cmd_report(void)
hists__tty_browse_tree(&session->hists_tree, help);

out_delete:
- perf_session__delete(session);
+ /*
+ * Speed up the exit process, for large files this can
+ * take quite a while.
+ *
+ * XXX Enable this when using valgrind or if we ever
+ * librarize this command.
+ *
+ * Also experiment with obstacks to see how much speed
+ * up we'll get here.
+ *
+ * perf_session__delete(session);
+ */
return ret;
}

--
1.6.2.5

2010-08-06 01:46:55

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 07/10] perf probe: Remove duplicated #include

From: Andrea Gelmini <[email protected]>

Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Andrea Gelmini <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-finder.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 840f1aa..6c7750d 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -33,7 +33,6 @@
#include <ctype.h>
#include <dwarf-regs.h>

-#include "string.h"
#include "event.h"
#include "debug.h"
#include "util.h"
--
1.6.2.5

2010-08-06 01:47:12

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 06/10] perf ui: Add search by name/addr to the map__browser

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

Only in verbose mode so as not to bloat struct symbol too much.

The key used is '/', just like in vi, less, etc.

More work is needed to allocate space on the symbol in a more clear way.

This experiment shows how to do it for the hist_browser, in the main
window.

Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Stephane Eranian <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-report.c | 18 +++++++-
tools/perf/util/newt.c | 109 +++++++++++++++++++++++++++++++++++++++----
2 files changed, 117 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2f4b929..4a7a743 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -478,8 +478,24 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
* so don't allocate extra space that won't be used in the stdio
* implementation.
*/
- if (use_browser > 0)
+ if (use_browser > 0) {
symbol_conf.priv_size = sizeof(struct sym_priv);
+ /*
+ * For searching by name on the "Browse map details".
+ * providing it only in verbose mode not to bloat too
+ * much struct symbol.
+ */
+ if (verbose) {
+ /*
+ * XXX: Need to provide a less kludgy way to ask for
+ * more space per symbol, the u32 is for the index on
+ * the ui browser.
+ * See symbol__browser_index.
+ */
+ symbol_conf.priv_size += sizeof(u32);
+ symbol_conf.sort_by_name = true;
+ }
+ }

if (symbol__init() < 0)
return -1;
diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index f98a240..e2deae0 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -128,6 +128,39 @@ static void ui_helpline__puts(const char *msg)
ui_helpline__push(msg);
}

+static int ui_entry__read(const char *title, char *bf, size_t size, int width)
+{
+ struct newtExitStruct es;
+ newtComponent form, entry;
+ const char *result;
+ int err = -1;
+
+ newtCenteredWindow(width, 1, title);
+ form = newtForm(NULL, NULL, 0);
+ if (form == NULL)
+ return -1;
+
+ entry = newtEntry(0, 0, "0x", width, &result, NEWT_FLAG_SCROLL);
+ if (entry == NULL)
+ goto out_free_form;
+
+ newtFormAddComponent(form, entry);
+ newtFormAddHotKey(form, NEWT_KEY_ENTER);
+ newtFormAddHotKey(form, NEWT_KEY_ESCAPE);
+ newtFormAddHotKey(form, NEWT_KEY_LEFT);
+ newtFormAddHotKey(form, CTRL('c'));
+ newtFormRun(form, &es);
+
+ if (result != NULL) {
+ strncpy(bf, result, size);
+ err = 0;
+ }
+out_free_form:
+ newtPopWindow();
+ newtFormDestroy(form);
+ return 0;
+}
+
static char browser__last_msg[1024];

int browser__show_help(const char *format, va_list ap)
@@ -670,6 +703,67 @@ static void map_browser__write(struct ui_browser *self, void *nd, int row)
slsmg_write_nstring(sym->name, mb->namelen);
}

+/* FIXME uber-kludgy, see comment on cmd_report... */
+static u32 *symbol__browser_index(struct symbol *self)
+{
+ return ((void *)self) - sizeof(struct rb_node) - sizeof(u32);
+}
+
+static int map_browser__search(struct map_browser *self)
+{
+ char target[512];
+ struct symbol *sym;
+ int err = ui_entry__read("Search by name/addr", target, sizeof(target), 40);
+
+ if (err)
+ return err;
+
+ if (target[0] == '0' && tolower(target[1]) == 'x') {
+ u64 addr = strtoull(target, NULL, 16);
+ sym = map__find_symbol(self->map, addr, NULL);
+ } else
+ sym = map__find_symbol_by_name(self->map, target, NULL);
+
+ if (sym != NULL) {
+ u32 *idx = symbol__browser_index(sym);
+
+ self->b.first_visible_entry = &sym->rb_node;
+ self->b.index = self->b.first_visible_entry_idx = *idx;
+ } else
+ ui_helpline__fpush("%s not found!", target);
+
+ return 0;
+}
+
+static int map_browser__run(struct map_browser *self, struct newtExitStruct *es)
+{
+ if (ui_browser__show(&self->b, self->map->dso->long_name) < 0)
+ return -1;
+
+ ui_helpline__fpush("Press <- or ESC to exit, %s / to search",
+ verbose ? "" : "restart with -v to use");
+ newtFormAddHotKey(self->b.form, NEWT_KEY_LEFT);
+ newtFormAddHotKey(self->b.form, NEWT_KEY_ENTER);
+ if (verbose)
+ newtFormAddHotKey(self->b.form, '/');
+
+ while (1) {
+ ui_browser__run(&self->b, es);
+
+ if (es->reason != NEWT_EXIT_HOTKEY)
+ break;
+ if (verbose && es->u.key == '/')
+ map_browser__search(self);
+ else
+ break;
+ }
+
+ newtFormDestroy(self->b.form);
+ newtPopWindow();
+ ui_helpline__pop();
+ return 0;
+}
+
static int map__browse(struct map *self)
{
struct map_browser mb = {
@@ -679,14 +773,12 @@ static int map__browse(struct map *self)
.seek = ui_browser__rb_tree_seek,
.write = map_browser__write,
},
+ .map = self,
};
struct newtExitStruct es;
struct rb_node *nd;
char tmp[BITS_PER_LONG / 4];
u64 maxaddr = 0;
- int ret;
-
- ui_helpline__push("Press <- or ESC to exit");

for (nd = rb_first(mb.b.entries); nd; nd = rb_next(nd)) {
struct symbol *pos = rb_entry(nd, struct symbol, rb_node);
@@ -695,17 +787,16 @@ static int map__browse(struct map *self)
mb.namelen = pos->namelen;
if (maxaddr < pos->end)
maxaddr = pos->end;
+ if (verbose) {
+ u32 *idx = symbol__browser_index(pos);
+ *idx = mb.b.nr_entries;
+ }
++mb.b.nr_entries;
}

mb.addrlen = snprintf(tmp, sizeof(tmp), "%llx", maxaddr);
mb.b.width += mb.addrlen * 2 + 4 + mb.namelen;
- ui_browser__show(&mb.b, self->dso->long_name);
- ret = ui_browser__run(&mb.b, &es);
- newtFormDestroy(mb.b.form);
- newtPopWindow();
- ui_helpline__pop();
- return ret;
+ return map_browser__run(&mb, &es);
}

/* -------------------------------------------------------------------- */
--
1.6.2.5

2010-08-06 01:47:09

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 01/10] perf symbols: Store the symbol binding

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

So that tools that wan't to act only on a subset of (weak, global,
local) symbols can do so, such as the upcoming uprobes support in 'perf
probe'.

Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Stephane Eranian <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/symbol.c | 30 ++++++++++++++++++++++--------
tools/perf/util/symbol.h | 1 +
2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 6f0dd90..b6f5970 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -131,7 +131,8 @@ static void map_groups__fixup_end(struct map_groups *self)
__map_groups__fixup_end(self, i);
}

-static struct symbol *symbol__new(u64 start, u64 len, const char *name)
+static struct symbol *symbol__new(u64 start, u64 len, u8 binding,
+ const char *name)
{
size_t namelen = strlen(name) + 1;
struct symbol *self = calloc(1, (symbol_conf.priv_size +
@@ -144,6 +145,7 @@ static struct symbol *symbol__new(u64 start, u64 len, const char *name)

self->start = start;
self->end = len ? start + len - 1 : start;
+ self->binding = binding;
self->namelen = namelen - 1;

pr_debug4("%s: %s %#Lx-%#Lx\n", __func__, name, start, self->end);
@@ -160,8 +162,11 @@ void symbol__delete(struct symbol *self)

static size_t symbol__fprintf(struct symbol *self, FILE *fp)
{
- return fprintf(fp, " %llx-%llx %s\n",
- self->start, self->end, self->name);
+ return fprintf(fp, " %llx-%llx %c %s\n",
+ self->start, self->end,
+ self->binding == STB_GLOBAL ? 'g' :
+ self->binding == STB_LOCAL ? 'l' : 'w',
+ self->name);
}

void dso__set_long_name(struct dso *self, char *name)
@@ -453,6 +458,14 @@ struct process_kallsyms_args {
struct dso *dso;
};

+static u8 kallsyms2elf_type(char type)
+{
+ if (type == 'W')
+ return STB_WEAK;
+
+ return isupper(type) ? STB_GLOBAL : STB_LOCAL;
+}
+
static int map__process_kallsym_symbol(void *arg, const char *name,
char type, u64 start)
{
@@ -466,7 +479,7 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
/*
* Will fix up the end later, when we have all symbols sorted.
*/
- sym = symbol__new(start, 0, name);
+ sym = symbol__new(start, 0, kallsyms2elf_type(type), name);

if (sym == NULL)
return -ENOMEM;
@@ -661,7 +674,7 @@ static int dso__load_perf_map(struct dso *self, struct map *map,
if (len + 2 >= line_len)
continue;

- sym = symbol__new(start, size, line + len);
+ sym = symbol__new(start, size, STB_GLOBAL, line + len);

if (sym == NULL)
goto out_delete_line;
@@ -873,7 +886,7 @@ static int dso__synthesize_plt_symbols(struct dso *self, struct map *map,
"%s@plt", elf_sym__name(&sym, symstrs));

f = symbol__new(plt_offset, shdr_plt.sh_entsize,
- sympltname);
+ STB_GLOBAL, sympltname);
if (!f)
goto out_elf_end;

@@ -895,7 +908,7 @@ static int dso__synthesize_plt_symbols(struct dso *self, struct map *map,
"%s@plt", elf_sym__name(&sym, symstrs));

f = symbol__new(plt_offset, shdr_plt.sh_entsize,
- sympltname);
+ STB_GLOBAL, sympltname);
if (!f)
goto out_elf_end;

@@ -1146,7 +1159,8 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
if (demangled != NULL)
elf_name = demangled;
new_symbol:
- f = symbol__new(sym.st_value, sym.st_size, elf_name);
+ f = symbol__new(sym.st_value, sym.st_size,
+ GELF_ST_BIND(sym.st_info), elf_name);
free(demangled);
if (!f)
goto out_elf_end;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 906be20..b7a8da4 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -53,6 +53,7 @@ struct symbol {
u64 start;
u64 end;
u16 namelen;
+ u8 binding;
char name[0];
};

--
1.6.2.5

2010-08-06 01:46:51

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 08/10] perf trace: Clean up #includes

From: Andrea Gelmini <[email protected]>

Removed duplicated #includes util/trace-event.h and
util/exec_cmd.h.
Grouped and sorted all the #includes.

Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Tom Zanussi <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Thomas Gleixner <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Andrea Gelmini <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-trace.c | 19 +++++++------------
1 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 294da72..40a6a29 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1,13 +1,16 @@
#include "builtin.h"

-#include "util/util.h"
+#include "perf.h"
#include "util/cache.h"
+#include "util/debug.h"
+#include "util/exec_cmd.h"
+#include "util/header.h"
+#include "util/parse-options.h"
+#include "util/session.h"
#include "util/symbol.h"
#include "util/thread.h"
-#include "util/header.h"
-#include "util/exec_cmd.h"
#include "util/trace-event.h"
-#include "util/session.h"
+#include "util/util.h"

static char const *script_name;
static char const *generate_script_lang;
@@ -59,14 +62,6 @@ static int cleanup_scripting(void)
return scripting_ops->stop_script();
}

-#include "util/parse-options.h"
-
-#include "perf.h"
-#include "util/debug.h"
-
-#include "util/trace-event.h"
-#include "util/exec_cmd.h"
-
static char const *input_name = "perf.data";

static int process_sample_event(event_t *event, struct perf_session *session)
--
1.6.2.5

2010-08-06 01:47:05

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 09/10] perf timechart: Adjust confusing if indentation

From: Julia Lawall <[email protected]>

Outdent the code following the if.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r disable braces4@
position p1,p2;
statement S1,S2;
@@

(
if (...) { ... }
|
if (...) S1@p1 S2@p2
)

@script:python@
p1 << r.p1;
p2 << r.p2;
@@

if (p1[0].column == p2[0].column):
cocci.print_main("branch",p1)
cocci.print_secs("after",p2)
// </smpl>

Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ingo Molnar <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Julia Lawall <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-timechart.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 5161619..9bcc38f 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -455,8 +455,8 @@ static void sched_switch(int cpu, u64 timestamp, struct trace_entry *te)
if (p->current->state != TYPE_NONE)
pid_put_sample(sw->next_pid, p->current->state, cpu, p->current->state_since, timestamp);

- p->current->state_since = timestamp;
- p->current->state = TYPE_RUNNING;
+ p->current->state_since = timestamp;
+ p->current->state = TYPE_RUNNING;
}

if (prev_p->current) {
--
1.6.2.5

2010-08-06 01:48:19

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 02/10] perf ui: Add a map browser

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

Press -> and then "Browse map details" to see the DSO long name as the title
and the list of symbols in the DSO used by the map where the current symbol is.

Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Stephane Eranian <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/newt.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 127 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index 91de99b..fc4a2b3 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -13,6 +13,7 @@
#include <slang.h>
#include <signal.h>
#include <stdlib.h>
+#include <elf.h>
#include <newt.h>
#include <sys/ttydefaults.h>

@@ -280,6 +281,7 @@ struct ui_browser {
u16 top, left, width, height;
void *priv;
unsigned int (*refresh_entries)(struct ui_browser *self);
+ void (*write)(struct ui_browser *self, void *entry, int row);
void (*seek)(struct ui_browser *self,
off_t offset, int whence);
u32 nr_entries;
@@ -316,6 +318,58 @@ static void ui_browser__list_head_seek(struct ui_browser *self,
self->first_visible_entry = pos;
}

+static void ui_browser__rb_tree_seek(struct ui_browser *self,
+ off_t offset, int whence)
+{
+ struct rb_root *root = self->entries;
+ struct rb_node *nd;
+
+ switch (whence) {
+ case SEEK_SET:
+ nd = rb_first(root);
+ break;
+ case SEEK_CUR:
+ nd = self->first_visible_entry;
+ break;
+ case SEEK_END:
+ nd = rb_last(root);
+ break;
+ default:
+ return;
+ }
+
+ if (offset > 0) {
+ while (offset-- != 0)
+ nd = rb_next(nd);
+ } else {
+ while (offset++ != 0)
+ nd = rb_prev(nd);
+ }
+
+ self->first_visible_entry = nd;
+}
+
+static unsigned int ui_browser__rb_tree_refresh(struct ui_browser *self)
+{
+ struct rb_node *nd;
+ int row = 0;
+
+ if (self->first_visible_entry == NULL)
+ self->first_visible_entry = rb_first(self->entries);
+
+ nd = self->first_visible_entry;
+
+ while (nd != NULL) {
+ SLsmg_gotorc(self->top + row, self->left);
+ self->write(self, nd, row);
+ if (++row == self->height)
+ break;
+ nd = rb_next(nd);
+ }
+
+ return row;
+}
+
static bool ui_browser__is_current_entry(struct ui_browser *self, unsigned row)
{
return (self->first_visible_entry_idx + row) == self->index;
@@ -592,6 +646,70 @@ int hist_entry__tui_annotate(struct hist_entry *self)
return ret;
}

+/* -------------------------------------------------------------------- */
+
+struct map_browser {
+ struct ui_browser b;
+ struct map *map;
+ u16 namelen;
+ u8 addrlen;
+};
+
+static void map_browser__write(struct ui_browser *self, void *nd, int row)
+{
+ struct symbol *sym = rb_entry(nd, struct symbol, rb_node);
+ struct map_browser *mb = container_of(self, struct map_browser, b);
+ bool current_entry = ui_browser__is_current_entry(self, row);
+ int color = ui_browser__percent_color(0, current_entry);
+
+ SLsmg_set_color(color);
+ slsmg_printf("%*llx %*llx %c ",
+ mb->addrlen, sym->start, mb->addrlen, sym->end,
+ sym->binding == STB_GLOBAL ? 'g' :
+ sym->binding == STB_LOCAL ? 'l' : 'w');
+ slsmg_write_nstring(sym->name, mb->namelen);
+}
+
+static int map__browse(struct map *self)
+{
+ struct map_browser mb = {
+ .b = {
+ .entries = &self->dso->symbols[self->type],
+ .refresh_entries = ui_browser__rb_tree_refresh,
+ .seek = ui_browser__rb_tree_seek,
+ .write = map_browser__write,
+ },
+ };
+ struct newtExitStruct es;
+ struct rb_node *nd;
+ char tmp[BITS_PER_LONG / 4];
+ u64 maxaddr = 0;
+ int ret;
+
+ ui_helpline__push("Press <- or ESC to exit");
+
+ for (nd = rb_first(mb.b.entries); nd; nd = rb_next(nd)) {
+ struct symbol *pos = rb_entry(nd, struct symbol, rb_node);
+
+ if (mb.namelen < pos->namelen)
+ mb.namelen = pos->namelen;
+ if (maxaddr < pos->end)
+ maxaddr = pos->end;
+ ++mb.b.nr_entries;
+ }
+
+ mb.addrlen = snprintf(tmp, sizeof(tmp), "%llx", maxaddr);
+ mb.b.width += mb.addrlen * 2 + 4 + mb.namelen;
+ ui_browser__show(&mb.b, self->dso->long_name);
+ ret = ui_browser__run(&mb.b, &es);
+ newtFormDestroy(mb.b.form);
+ newtPopWindow();
+ ui_helpline__pop();
+ return ret;
+}
+
+/* -------------------------------------------------------------------- */
+
struct hist_browser {
struct ui_browser b;
struct hists *hists;
@@ -680,7 +798,8 @@ int hists__browse(struct hists *self, const char *helpline, const char *ev_name)
const struct dso *dso;
char *options[16];
int nr_options = 0, choice = 0, i,
- annotate = -2, zoom_dso = -2, zoom_thread = -2;
+ annotate = -2, zoom_dso = -2, zoom_thread = -2,
+ browse_map = -2;

if (hist_browser__run(browser, msg, &es))
break;
@@ -771,6 +890,10 @@ do_help:
(dso->kernel ? "the Kernel" : dso->short_name)) > 0)
zoom_dso = nr_options++;

+ if (browser->selection->map != NULL &&
+ asprintf(&options[nr_options], "Browse map details") > 0)
+ browse_map = nr_options++;
+
options[nr_options++] = (char *)"Exit";

choice = popup_menu(nr_options, options);
@@ -800,7 +923,9 @@ do_annotate:
continue;

hist_entry__tui_annotate(he);
- } else if (choice == zoom_dso) {
+ } else if (choice == browse_map)
+ map__browse(browser->selection->map);
+ else if (choice == zoom_dso) {
zoom_dso:
if (dso_filter) {
pstack__remove(fstack, &dso_filter);
--
1.6.2.5

2010-08-06 01:46:33

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 03/10] perf ui: Shorten ui_browser->refresh_entries to refresh

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

LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/newt.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index fc4a2b3..f98a240 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -280,7 +280,7 @@ struct ui_browser {
void *first_visible_entry, *entries;
u16 top, left, width, height;
void *priv;
- unsigned int (*refresh_entries)(struct ui_browser *self);
+ unsigned int (*refresh)(struct ui_browser *self);
void (*write)(struct ui_browser *self, void *entry, int row);
void (*seek)(struct ui_browser *self,
off_t offset, int whence);
@@ -472,12 +472,12 @@ static int objdump_line__show(struct objdump_line *self, struct list_head *head,
return 0;
}

-static int ui_browser__refresh_entries(struct ui_browser *self)
+static int ui_browser__refresh(struct ui_browser *self)
{
int row;

newtScrollbarSet(self->sb, self->index, self->nr_entries - 1);
- row = self->refresh_entries(self);
+ row = self->refresh(self);
SLsmg_set_color(HE_COLORSET_NORMAL);
SLsmg_fill_region(self->top + row, self->left,
self->height - row, self->width, ' ');
@@ -487,7 +487,7 @@ static int ui_browser__refresh_entries(struct ui_browser *self)

static int ui_browser__run(struct ui_browser *self, struct newtExitStruct *es)
{
- if (ui_browser__refresh_entries(self) < 0)
+ if (ui_browser__refresh(self) < 0)
return -1;

while (1) {
@@ -558,7 +558,7 @@ static int ui_browser__run(struct ui_browser *self, struct newtExitStruct *es)
default:
return es->u.key;
}
- if (ui_browser__refresh_entries(self) < 0)
+ if (ui_browser__refresh(self) < 0)
return -1;
}
return 0;
@@ -621,9 +621,9 @@ int hist_entry__tui_annotate(struct hist_entry *self)
ui_helpline__push("Press <- or ESC to exit");

memset(&browser, 0, sizeof(browser));
- browser.entries = &head;
- browser.refresh_entries = hist_entry__annotate_browser_refresh;
- browser.seek = ui_browser__list_head_seek;
+ browser.entries = &head;
+ browser.refresh = hist_entry__annotate_browser_refresh;
+ browser.seek = ui_browser__list_head_seek;
browser.priv = self;
list_for_each_entry(pos, &head, node) {
size_t line_len = strlen(pos->line);
@@ -675,7 +675,7 @@ static int map__browse(struct map *self)
struct map_browser mb = {
.b = {
.entries = &self->dso->symbols[self->type],
- .refresh_entries = ui_browser__rb_tree_refresh,
+ .refresh = ui_browser__rb_tree_refresh,
.seek = ui_browser__rb_tree_seek,
.write = map_browser__write,
},
@@ -720,7 +720,7 @@ struct hist_browser {
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 unsigned int hist_browser__refresh(struct ui_browser *self);
static void ui_browser__hists_seek(struct ui_browser *self,
off_t offset, int whence);

@@ -730,7 +730,7 @@ static struct hist_browser *hist_browser__new(struct hists *hists)

if (self) {
self->hists = hists;
- self->b.refresh_entries = hist_browser__refresh_entries;
+ self->b.refresh = hist_browser__refresh;
self->b.seek = ui_browser__hists_seek;
}

@@ -1338,7 +1338,7 @@ static int hist_browser__show_entry(struct hist_browser *self,
return printed;
}

-static unsigned int hist_browser__refresh_entries(struct ui_browser *self)
+static unsigned int hist_browser__refresh(struct ui_browser *self)
{
unsigned row = 0;
struct rb_node *nd;
--
1.6.2.5

2010-08-06 01:48:34

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 04/10] perf hists: Handle verbose in hists__sort_list_width

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

Otherwise entries will get chopped up on the window.

Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/hist.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index e7263d4..62ec9b0 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -876,6 +876,9 @@ unsigned int hists__sort_list_width(struct hists *self)
if (!se->elide)
ret += 2 + hists__col_len(self, se->se_width_idx);

+ if (verbose) /* Addr + origin */
+ ret += 3 + BITS_PER_LONG / 4;
+
return ret;
}

--
1.6.2.5

2010-08-06 01:58:13

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 05/10] perf hists: Fixup addr snprintf width on 32 bit arches

On Thu, 2010-08-05 at 22:46 -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <[email protected]>
> By using BITS_PER_LONG/4 as the width specifier.
[]
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 1c61a4f..b62a553 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
[]
> - ret += repsep_snprintf(bf, size, "%#018llx %c ", self->ip, o);
> + ret += repsep_snprintf(bf, size, "%*Lx %c ",
> + BITS_PER_LONG / 4, self->ip, o);
[]
> - ret += repsep_snprintf(bf + ret, size - ret, "%#016llx", self->ip);
> + ret += repsep_snprintf(bf + ret, size - ret, "%*Lx",
> + BITS_PER_LONG / 4, self->ip);

This drops leading 0's.

2010-08-06 07:01:26

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL 00/10] perf/core improvements and fixes


* Arnaldo Carvalho de Melo <[email protected]> wrote:

> Hi Ingo,
>
> Please pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core
>
> Regards,
>
> - Arnaldo
>
> Andrea Gelmini (2):
> perf probe: Remove duplicated #include
> perf trace: Clean up #includes
>
> Arnaldo Carvalho de Melo (7):
> perf symbols: Store the symbol binding
> perf ui: Add a map browser
> perf ui: Shorten ui_browser->refresh_entries to refresh
> perf hists: Handle verbose in hists__sort_list_width
> perf hists: Fixup addr snprintf width on 32 bit arches
> perf ui: Add search by name/addr to the map__browser
> perf report: Speed up exit path
>
> Julia Lawall (1):
> perf timechart: Adjust confusing if indentation
>
> tools/perf/builtin-report.c | 31 +++++-
> tools/perf/builtin-timechart.c | 4 +-
> tools/perf/builtin-trace.c | 19 +--
> tools/perf/util/hist.c | 3 +
> tools/perf/util/newt.c | 242 +++++++++++++++++++++++++++++++++++++--
> tools/perf/util/probe-finder.c | 1 -
> tools/perf/util/sort.c | 6 +-
> tools/perf/util/symbol.c | 30 ++++--
> tools/perf/util/symbol.h | 1 +
> 9 files changed, 297 insertions(+), 40 deletions(-)

Pulled, thanks Arnaldo!

Ingo

2010-08-06 14:30:15

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 05/10] perf hists: Fixup addr snprintf width on 32 bit arches

Em Thu, Aug 05, 2010 at 06:58:07PM -0700, Joe Perches escreveu:
> On Thu, 2010-08-05 at 22:46 -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <[email protected]>
> > By using BITS_PER_LONG/4 as the width specifier.
> []
> > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> > index 1c61a4f..b62a553 100644
> > --- a/tools/perf/util/sort.c
> > +++ b/tools/perf/util/sort.c
> []
> > - ret += repsep_snprintf(bf, size, "%#018llx %c ", self->ip, o);
> > + ret += repsep_snprintf(bf, size, "%*Lx %c ",
> > + BITS_PER_LONG / 4, self->ip, o);
> []
> > - ret += repsep_snprintf(bf + ret, size - ret, "%#016llx", self->ip);
> > + ret += repsep_snprintf(bf + ret, size - ret, "%*Lx",
> > + BITS_PER_LONG / 4, self->ip);
>
> This drops leading 0's.

Is this a problem? This removes clutter from the output, right?

- Arnaldo

2010-08-06 15:35:39

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 05/10] perf hists: Fixup addr snprintf width on 32 bit arches

On Fri, 2010-08-06 at 11:29 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Aug 05, 2010 at 06:58:07PM -0700, Joe Perches escreveu:
> > On Thu, 2010-08-05 at 22:46 -0300, Arnaldo Carvalho de Melo wrote:
> > > From: Arnaldo Carvalho de Melo <[email protected]>
> > > By using BITS_PER_LONG/4 as the width specifier.
> > []
> > > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> > > index 1c61a4f..b62a553 100644
> > > --- a/tools/perf/util/sort.c
> > > +++ b/tools/perf/util/sort.c
> > []
> > > - ret += repsep_snprintf(bf, size, "%#018llx %c ", self->ip, o);
> > > + ret += repsep_snprintf(bf, size, "%*Lx %c ",
> > > + BITS_PER_LONG / 4, self->ip, o);
> > []
> > > - ret += repsep_snprintf(bf + ret, size - ret, "%#016llx", self->ip);
> > > + ret += repsep_snprintf(bf + ret, size - ret, "%*Lx",
> > > + BITS_PER_LONG / 4, self->ip);
> >
> > This drops leading 0's.
>
> Is this a problem? This removes clutter from the output, right?

Dunno, you made an output that looked like a pointer address,

64 bit:

"0x0123456789abcdef" now is " 123456789abcdef"

32 bit:

"0x0000000001234567" now is " 1234567"

snprintf(loc, size, "%#0*lx", BITS_PER_LONG / 4 + 2, (long))
might be better.

snprintf(loc, size, "%p", (void)long)
might be best.




2010-08-06 15:40:32

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 05/10] perf hists: Fixup addr snprintf width on 32 bit arches

On Fri, 2010-08-06 at 08:35 -0700, Joe Perches wrote:
> snprintf(loc, size, "%p", (void)long)
> might be best.

Oops.

On the other hand, "%p", (void *) would compile
without warnings and be what I meant in the first
place...

snprintf(loc, size, "%p", (void *)long)