Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 6/6] perf ui/tui: Implement header window

0 views
Skip to first unread message

Namhyung Kim

unread,
Dec 19, 2013, 2:10:01 AM12/19/13
to
From: Namhyung Kim <namhyu...@lge.com>

Implement a simple, full-screen header window which shows session
header (metadata) information. Press 'i' key to display the header
window.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/Makefile.perf | 1 +
tools/perf/ui/browser.h | 2 +
tools/perf/ui/browsers/header.c | 116 ++++++++++++++++++++++++++++++++++++++++
tools/perf/ui/browsers/hists.c | 4 ++
4 files changed, 123 insertions(+)
create mode 100644 tools/perf/ui/browsers/header.c

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 30aabace33a0..4d046e97d0e7 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -491,6 +491,7 @@ ifndef NO_SLANG
LIB_OBJS += $(OUTPUT)ui/browsers/map.o
LIB_OBJS += $(OUTPUT)ui/browsers/scripts.o
LIB_OBJS += $(OUTPUT)ui/browsers/log.o
+ LIB_OBJS += $(OUTPUT)ui/browsers/header.o
LIB_OBJS += $(OUTPUT)ui/tui/setup.o
LIB_OBJS += $(OUTPUT)ui/tui/util.o
LIB_OBJS += $(OUTPUT)ui/tui/helpline.o
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index fff46942a8c7..b77d8728b3f0 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -60,6 +60,8 @@ bool ui_browser__dialog_yesno(struct ui_browser *browser, const char *text);
int ui_browser__input_window(const char *title, const char *text, char *input,
const char *exit_msg, int delay_sec);
int tui__log_window(void);
+struct perf_session_env;
+int tui__header_window(struct perf_session_env *env);

void ui_browser__argv_seek(struct ui_browser *browser, off_t offset, int whence);
unsigned int ui_browser__argv_refresh(struct ui_browser *browser);
diff --git a/tools/perf/ui/browsers/header.c b/tools/perf/ui/browsers/header.c
new file mode 100644
index 000000000000..b5f0961bf793
--- /dev/null
+++ b/tools/perf/ui/browsers/header.c
@@ -0,0 +1,116 @@
+#include "util/cache.h"
+#include "util/debug.h"
+#include "ui/browser.h"
+#include "ui/ui.h"
+#include "ui/util.h"
+#include "ui/libslang.h"
+#include "util/header.h"
+#include "util/session.h"
+
+static void ui_browser__argv_write(struct ui_browser *browser,
+ void *entry, int row)
+{
+ char **arg = entry;
+ char *str = *arg;
+ char empty[] = " ";
+ bool current_entry = ui_browser__is_current_entry(browser, row);
+ unsigned long offset = (unsigned long)browser->priv;
+
+ if (offset >= strlen(str))
+ str = empty;
+ else
+ str = str + offset;
+
+ ui_browser__set_color(browser, current_entry ? HE_COLORSET_SELECTED :
+ HE_COLORSET_NORMAL);
+
+ slsmg_write_nstring(str, browser->width);
+}
+
+static int list_menu__run(struct ui_browser *menu)
+{
+ int key;
+ unsigned long offset;
+
+ if (ui_browser__show(menu, "Header information", "Press 'q' to exit") < 0)
+ return -1;
+
+ while (1) {
+ key = ui_browser__run(menu, 0);
+
+ switch (key) {
+ case K_RIGHT:
+ offset = (unsigned long)menu->priv;
+ offset += 10;
+ menu->priv = (void *)offset;
+ continue;
+ case K_LEFT:
+ offset = (unsigned long)menu->priv;
+ if (offset >= 10)
+ offset -= 10;
+ menu->priv = (void *)offset;
+ continue;
+ case K_ESC:
+ case 'q':
+ case CTRL('c'):
+ key = -1;
+ break;
+ default:
+ continue;
+ }
+
+ break;
+ }
+
+ ui_browser__hide(menu);
+ return key;
+}
+
+static int ui__list_menu(int argc, char * const argv[])
+{
+ struct ui_browser menu = {
+ .entries = (void *)argv,
+ .refresh = ui_browser__argv_refresh,
+ .seek = ui_browser__argv_seek,
+ .write = ui_browser__argv_write,
+ .nr_entries = argc,
+ };
+
+ return list_menu__run(&menu);
+}
+
+int tui__header_window(struct perf_session_env *env)
+{
+ int i, argc = 0;
+ char **argv;
+ struct perf_session *session;
+ char *ptr, *pos;
+ size_t size;
+ FILE *fp = open_memstream(&ptr, &size);
+
+ session = container_of(env, struct perf_session, header.env);
+ perf_header__fprintf_info(session, fp, true);
+ fclose(fp);
+
+ for (pos = ptr, argc = 0; (pos = strchr(pos, '\n')) != NULL; pos++)
+ argc++;
+
+ argv = calloc(argc + 1, sizeof(*argv));
+ if (argv == NULL)
+ goto out;
+
+ argv[0] = pos = ptr;
+ for (i = 1; (pos = strchr(pos, '\n')) != NULL; i++) {
+ *pos++ = '\0';
+ argv[i] = pos;
+ }
+
+ BUG_ON(i != argc + 1);
+
+ ui__list_menu(argc, argv);
+
+out:
+ free(argv);
+ free(ptr);
+ return 0;
+}
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 8cc6f5eaa2cf..d710c6403a14 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1487,6 +1487,9 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
case 'l':
tui__log_window();
continue;
+ case 'i':
+ tui__header_window(env);
+ continue;
case K_F1:
case 'h':
case '?':
@@ -1510,6 +1513,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
"P Print histograms to perf.hist.N\n"
"V Verbose (DSO names in callchains, etc)\n"
"l Show log messages\n"
+ "i Show header information\n"
"/ Filter symbol by name");
continue;
case K_ENTER:
--
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Namhyung Kim

unread,
Dec 19, 2013, 2:10:01 AM12/19/13
to
From: Namhyung Kim <namhyu...@lge.com>

Implement a simple, full-screen log window which shows error messages
saved so far. Press 'l' (lower-case 'L') key to display the log
window. It'll be used usually with -v option.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/Makefile.perf | 1 +
tools/perf/ui/browser.h | 1 +
tools/perf/ui/browsers/hists.c | 4 ++
tools/perf/ui/browsers/log.c | 92 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 98 insertions(+)
create mode 100644 tools/perf/ui/browsers/log.c

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index ec8184014a76..30aabace33a0 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -490,6 +490,7 @@ ifndef NO_SLANG
LIB_OBJS += $(OUTPUT)ui/browsers/hists.o
LIB_OBJS += $(OUTPUT)ui/browsers/map.o
LIB_OBJS += $(OUTPUT)ui/browsers/scripts.o
+ LIB_OBJS += $(OUTPUT)ui/browsers/log.o
LIB_OBJS += $(OUTPUT)ui/tui/setup.o
LIB_OBJS += $(OUTPUT)ui/tui/util.o
LIB_OBJS += $(OUTPUT)ui/tui/helpline.o
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index 7d45d2f53601..fff46942a8c7 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -59,6 +59,7 @@ int ui_browser__help_window(struct ui_browser *browser, const char *text);
bool ui_browser__dialog_yesno(struct ui_browser *browser, const char *text);
int ui_browser__input_window(const char *title, const char *text, char *input,
const char *exit_msg, int delay_sec);
+int tui__log_window(void);

void ui_browser__argv_seek(struct ui_browser *browser, off_t offset, int whence);
unsigned int ui_browser__argv_refresh(struct ui_browser *browser);
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index a440e03cd8c2..8cc6f5eaa2cf 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1484,6 +1484,9 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
if (is_report_browser(hbt))
goto do_data_switch;
continue;
+ case 'l':
+ tui__log_window();
+ continue;
case K_F1:
case 'h':
case '?':
@@ -1506,6 +1509,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
"s Switch to another data file in PWD ('perf report' only)\n"
"P Print histograms to perf.hist.N\n"
"V Verbose (DSO names in callchains, etc)\n"
+ "l Show log messages\n"
"/ Filter symbol by name");
continue;
case K_ENTER:
diff --git a/tools/perf/ui/browsers/log.c b/tools/perf/ui/browsers/log.c
new file mode 100644
index 000000000000..f1610d6bb9d2
--- /dev/null
+++ b/tools/perf/ui/browsers/log.c
@@ -0,0 +1,92 @@
+#include <stdio.h>
+
+#include "perf.h"
+#include "util/util.h"
+#include "util/cache.h"
+#include "util/debug.h"
+#include "ui/ui.h"
+#include "ui/util.h"
+#include "ui/browser.h"
+#include "ui/libslang.h"
+#include "ui/keysyms.h"
+#include "ui/tui/tui.h"
+
+static void ui_browser__file_seek(struct ui_browser *browser __maybe_unused,
+ off_t offset __maybe_unused,
+ int whence __maybe_unused)
+{
+ /* do nothing */
+}
+
+static void ui_browser__file_write(struct ui_browser *browser,
+ void *entry, int row)
+{
+ char buf[1024];
+ FILE *fp = perf_log.fp;
+ bool current_entry = ui_browser__is_current_entry(browser, row);
+ off_t *linemap = perf_log.linemap;
+ unsigned int idx = *(unsigned int *)entry;
+
+ fseek(fp, linemap[idx], SEEK_SET);
+ if (fgets(buf, sizeof(buf), fp) == NULL)
+ return;
+
+ ui_browser__set_color(browser, current_entry ? HE_COLORSET_SELECTED :
+ HE_COLORSET_NORMAL);
+ slsmg_write_nstring(buf, browser->width);
+}
+
+static unsigned int ui_browser__file_refresh(struct ui_browser *browser)
+{
+ unsigned int row = 0;
+ unsigned int idx = browser->top_idx;
+
+ while (idx < browser->nr_entries) {
+ ui_browser__gotorc(browser, row, 0);
+ browser->write(browser, &idx, row);
+ if (++row == browser->height)
+ break;
+
+ ++idx;
+ }
+
+ return row;
+}
+
+static int log_menu__run(struct ui_browser *menu)
+{
+ int key;
+
+ if (ui_browser__show(menu, "Log messages", "Press 'q' to exit") < 0)
+ return -1;
+
+ while (1) {
+ key = ui_browser__run(menu, 0);
+ switch (key) {
+ case K_ESC:
+ case 'q':
+ case CTRL('c'):
+ key = -1;
+ break;
+ default:
+ continue;
+ }
+
+ break;
+ }
+
+ ui_browser__hide(menu);
+ return key;
+}
+
+int tui__log_window(void)
+{
+ struct ui_browser log_menu = {
+ .refresh = ui_browser__file_refresh,
+ .seek = ui_browser__file_seek,
+ .write = ui_browser__file_write,
+ .nr_entries = perf_log.lines,
+ };
+
+ return log_menu__run(&log_menu);
+}

Namhyung Kim

unread,
Dec 19, 2013, 2:10:02 AM12/19/13
to
From: Namhyung Kim <namhyu...@lge.com>

The va_end() in _eprintf() should be removed since the caller also
invokes va_end().

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/util/debug.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 8640a9121e72..299b55586502 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -25,7 +25,6 @@ static int _eprintf(int level, const char *fmt, va_list args)
ui_helpline__vshow(fmt, args);
else
ret = vfprintf(stderr, fmt, args);
- va_end(args);
}

return ret;

Namhyung Kim

unread,
Dec 19, 2013, 2:10:02 AM12/19/13
to
Hello,

I was playing with TUI code and added two new windows. One for
showing log messages and another for showing header information.
(Maybe they can be implemented on the GTK code too someday.)

Please try to use it! :)

I put the patches on 'perf/tui-v1' branch in my tree:

git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Any feedbacks are more than welcome, thanks
Namhyung


Namhyung Kim (6):
perf report: Use pr_*() functions if possible
perf tools: Introduce struct perf_log
perf tools: Get rid of a duplicate va_end()
perf tools: Save message when pr_*() was called
perf ui/tui: Implement log window
perf ui/tui: Implement header window

tools/perf/Makefile.perf | 3 ++
tools/perf/builtin-report.c | 24 +++++----
tools/perf/perf.c | 3 ++
tools/perf/ui/browser.h | 3 ++
tools/perf/ui/browsers/header.c | 116 ++++++++++++++++++++++++++++++++++++++++
tools/perf/ui/browsers/hists.c | 8 +++
tools/perf/ui/browsers/log.c | 92 +++++++++++++++++++++++++++++++
tools/perf/util/debug.c | 7 ++-
tools/perf/util/debug.h | 15 ++++++
tools/perf/util/log.c | 105 ++++++++++++++++++++++++++++++++++++
10 files changed, 364 insertions(+), 12 deletions(-)
create mode 100644 tools/perf/ui/browsers/header.c
create mode 100644 tools/perf/ui/browsers/log.c
create mode 100644 tools/perf/util/log.c

Namhyung Kim

unread,
Dec 19, 2013, 2:10:02 AM12/19/13
to
From: Namhyung Kim <namhyu...@lge.com>

Add new functions to save error messages in a temp file. It'll be
used by some UI front-ends to see the messages.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/Makefile.perf | 1 +
tools/perf/perf.c | 3 ++
tools/perf/util/debug.h | 15 +++++++
tools/perf/util/log.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 124 insertions(+)
create mode 100644 tools/perf/util/log.c

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 97a2145e4cc6..ec8184014a76 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -372,6 +372,7 @@ LIB_OBJS += $(OUTPUT)util/stat.o
LIB_OBJS += $(OUTPUT)util/record.o
LIB_OBJS += $(OUTPUT)util/srcline.o
LIB_OBJS += $(OUTPUT)util/data.o
+LIB_OBJS += $(OUTPUT)util/log.o

LIB_OBJS += $(OUTPUT)ui/setup.o
LIB_OBJS += $(OUTPUT)ui/helpline.o
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 431798a4110d..bdf7bd8e4394 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -10,6 +10,7 @@

#include "util/exec_cmd.h"
#include "util/cache.h"
+#include "util/debug.h"
#include "util/quote.h"
#include "util/run-command.h"
#include "util/parse-events.h"
@@ -524,6 +525,7 @@ int main(int argc, const char **argv)
*/
pthread__block_sigwinch();

+ perf_log_init();
while (1) {
static int done_help;
int was_alias = run_argv(&argc, &argv);
@@ -543,6 +545,7 @@ int main(int argc, const char **argv)
} else
break;
}
+ perf_log_exit();

fprintf(stderr, "Failed to run command '%s': %s\n",
cmd, strerror(errno));
diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
index 443694c36b03..ea160abc2ae0 100644
--- a/tools/perf/util/debug.h
+++ b/tools/perf/util/debug.h
@@ -19,4 +19,19 @@ int ui__warning(const char *format, ...) __attribute__((format(printf, 1, 2)));

void pr_stat(const char *fmt, ...);

+struct perf_log {
+ FILE *fp;
+ off_t *linemap;
+ u32 lines;
+ u32 nr_alloc;
+ bool seen_newline;
+};
+
+extern struct perf_log perf_log;
+
+int perf_log_init(void);
+int perf_log_exit(void);
+void perf_log_add(const char *msg);
+void perf_log_addv(const char *fmt, va_list ap);
+
#endif /* __PERF_DEBUG_H */
diff --git a/tools/perf/util/log.c b/tools/perf/util/log.c
new file mode 100644
index 000000000000..4a34bed2af44
--- /dev/null
+++ b/tools/perf/util/log.c
@@ -0,0 +1,105 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include "util/debug.h"
+
+#define LINEMAP_GROW 128
+
+struct perf_log perf_log = {
+ .seen_newline = true,
+};
+
+int perf_log_init(void)
+{
+ FILE *fp;
+ char name[] = "/tmp/perf-tui-log-XXXXXX";
+ int fd = mkstemp(name);
+
+ if (fd < 0)
+ return -1;
+
+ fp = fdopen(fd, "r+");
+ if (fp == NULL) {
+ close(fd);
+ return -1;
+ }
+
+ perf_log.fp = fp;
+
+ return 0;
+}
+
+int perf_log_exit(void)
+{
+ FILE *fp = perf_log.fp;
+ if (fp)
+ fclose(fp);
+
+ free(perf_log.linemap);
+
+ perf_log.fp = NULL;
+ perf_log.linemap = NULL;
+ return 0;
+}
+
+static int grow_linemap(struct perf_log *log)
+{
+ off_t *newmap;
+ int newsize = log->nr_alloc + LINEMAP_GROW;
+
+ newmap = realloc(log->linemap, newsize * sizeof(*log->linemap));
+ if (newmap == NULL)
+ return -1;
+
+ log->nr_alloc = newsize;
+ log->linemap = newmap;
+ return 0;
+}
+
+static int __add_to_linemap(struct perf_log *log, off_t index)
+{
+ if (log->lines == log->nr_alloc)
+ if (grow_linemap(log) < 0)
+ return -1;
+
+ log->linemap[log->lines++] = index;
+ return 0;
+}
+
+static void add_to_linemap(struct perf_log *log, const char *msg, off_t base)
+{
+ const char *pos;
+
+ if (strlen(msg) == 0)
+ return;
+
+ if (log->seen_newline) {
+ if (__add_to_linemap(log, base) < 0)
+ return;
+ }
+
+ if ((pos = strchr(msg, '\n')) != NULL) {
+ log->seen_newline = true;
+ pos++;
+ add_to_linemap(log, pos, base + (pos - msg));
+ } else {
+ log->seen_newline = false;
+ }
+}
+
+void perf_log_add(const char *msg)
+{
+ FILE *fp = perf_log.fp;
+ off_t offset = ftello(fp);
+
+ add_to_linemap(&perf_log, msg, offset);
+
+ fwrite(msg, 1, strlen(msg), fp);
+}
+
+void perf_log_addv(const char *fmt, va_list ap)
+{
+ char buf[4096];
+
+ vsnprintf(buf, sizeof(buf), fmt, ap);
+ perf_log_add(buf);
+}

Namhyung Kim

unread,
Dec 19, 2013, 2:10:02 AM12/19/13
to
From: Namhyung Kim <namhyu...@lge.com>

There're some places printing a message to stdout/err directly. It
should be converted to use proper error printing functions instead.

If it's not possible, just do it when --stdio was enabled only.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/builtin-report.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3a14dbed387c..a1f72b471a96 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -321,8 +321,8 @@ static int process_sample_event(struct perf_tool *tool,
int ret;

if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
- fprintf(stderr, "problem processing %d event, skipping it.\n",
- event->header.type);
+ pr_debug("problem processing %d event, skipping it.\n",
+ event->header.type);
return -1;
}

@@ -552,15 +552,17 @@ static int __cmd_report(struct perf_report *rep)
desc);
}

- if (verbose > 3)
- perf_session__fprintf(session, stdout);
+ if (use_browser == 0) {
+ if (verbose > 3)
+ perf_session__fprintf(session, stdout);

- if (verbose > 2)
- perf_session__fprintf_dsos(session, stdout);
+ if (verbose > 2)
+ perf_session__fprintf_dsos(session, stdout);

- if (dump_trace) {
- perf_session__fprintf_nr_events(session, stdout);
- return 0;
+ if (dump_trace) {
+ perf_session__fprintf_nr_events(session, stdout);
+ return 0;
+ }
}

nr_samples = 0;
@@ -720,7 +722,7 @@ parse_callchain_opt(const struct option *opt, const char *arg, int unset)
return -1;
setup:
if (callchain_register_param(&callchain_param) < 0) {
- fprintf(stderr, "Can't register callchain params\n");
+ pr_err("Can't register callchain params\n");
return -1;
}
return 0;
@@ -942,7 +944,7 @@ repeat:
}
if (report.mem_mode) {
if (sort__mode == SORT_MODE__BRANCH) {
- fprintf(stderr, "branch and mem mode incompatible\n");
+ pr_err("branch and mem mode incompatible\n");
goto error;
}
sort__mode = SORT_MODE__MEMORY;

Namhyung Kim

unread,
Dec 19, 2013, 2:10:02 AM12/19/13
to
From: Namhyung Kim <namhyu...@lge.com>

The message will be saved in a temp file so that it can be shown at a
UI dialog at any time.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/util/debug.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 299b55586502..f55b499c93fb 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -19,12 +19,18 @@ bool dump_trace = false, quiet = false;
static int _eprintf(int level, const char *fmt, va_list args)
{
int ret = 0;
+ va_list ap;

if (verbose >= level) {
+ va_copy(ap, args);
+
if (use_browser >= 1)
ui_helpline__vshow(fmt, args);
else
ret = vfprintf(stderr, fmt, args);
+
+ perf_log_addv(fmt, ap);
+ va_end(ap);
}

return ret;

Ingo Molnar

unread,
Dec 19, 2013, 7:20:04 AM12/19/13
to

* Namhyung Kim <namh...@kernel.org> wrote:

> Hello,
>
> I was playing with TUI code and added two new windows. One for
> showing log messages and another for showing header information.
> (Maybe they can be implemented on the GTK code too someday.)
>
> Please try to use it! :)
>
> I put the patches on 'perf/tui-v1' branch in my tree:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> Any feedbacks are more than welcome, thanks
> Namhyung

In the morning haze it took me some time to figure out that typing 'l'
gives leads to the log window, 'i' to the header information.

'l' seems to work (no log messages though :-), but in perf-top 'i'
segfaulted:

comet:~/tip/tools/perf> perf top
perf: Segmentation fault

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff0d39700 (LWP 14785)]
perf_header__fprintf_info (session=session@entry=0x957bf0, fp=fp@entry=0x7fffec000a30, full=full@entry=true) at util/header.c:2195
2195 int fd = perf_data_file__fd(session->file);
Missing separate debuginfos, use: debuginfo-install audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64 elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64 glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64 libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64 numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64 python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64 xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64
(gdb) bt
#0 perf_header__fprintf_info (session=session@entry=0x957bf0, fp=fp@entry=0x7fffec000a30, full=full@entry=true) at util/header.c:2195
#1 0x00000000004ce3fe in tui__header_window (env=env@entry=0x957c30) at ui/browsers/header.c:92
#2 0x00000000004cbdfd in perf_evsel__hists_browse (evsel=evsel@entry=0x957560, nr_events=nr_events@entry=1,
helpline=helpline@entry=0x57db88 "For a higher level overview, try: perf top --sort comm,dso", ev_name=0x957870 "cycles", left_exits=left_exits@entry=false,
hbt=hbt@entry=0x7ffff0d38ea0, min_pcnt=min_pcnt@entry=0, env=env@entry=0x957c30) at ui/browsers/hists.c:1491
#3 0x00000000004cd5a5 in perf_evlist__tui_browse_hists (evlist=0x8e5930, help=help@entry=0x57db88 "For a higher level overview, try: perf top --sort comm,dso",
hbt=hbt@entry=0x7ffff0d38ea0, min_pcnt=0, env=0x957c30) at ui/browsers/hists.c:1957
#4 0x0000000000433b48 in display_thread_tui (arg=0x7fffffffb150) at builtin-top.c:584
#5 0x0000003e59407c53 in start_thread () from /lib64/libpthread.so.0
#6 0x0000003e590f5dbd in clone () from /lib64/libc.so.6
(gdb)

I suspect it got surprised by perf top not having a header per se?
Still it might make sense to also robustify
perf_header__fprintf_info() against segfaulting and such.

But it's a nice feature nevertheless!

Btw., it would be nice if 'P' worked on these screens, so any
interesting data can be extracted! Cut & paste is usually a PITA due
to the graphical TUI characters.

Btw., in case you are taking TUI usability bugreports, here's a few I
noticed while playing with your changes:

1)

In histogram view it would be nice if 'P' gave some status bar
indication that it just wrote to perf.hist.0 or so - otherwise the
user is kept in the dark.

2)

Likewise, in a TUI every keypress must produce some tangible feedback
to the user. Try hitting 'o' for example - it should probably output
into the status bar that 'o' is not a bound keypress or so.

3)

Same goes for page up / page down in histogram view if we are at the
end of the list: some low-key, single-character feedback should be
given that the keypress was seen but we are at the end of the list.
For exampe the scrollbar 'diamond' character could briefly
inverse-flash or so.

Thanks,

Ingo

Arnaldo Carvalho de Melo

unread,
Dec 19, 2013, 8:30:02 AM12/19/13
to
Em Thu, Dec 19, 2013 at 04:00:07PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyu...@lge.com>
>
> Add new functions to save error messages in a temp file. It'll be
> used by some UI front-ends to see the messages.

Do we really have to grow a logging facility like this? Can't we somehow
openlog/syslog/closelog? Just a first gut reaction, continuing to read
the series...

- Arnaldo

Jiri Olsa

unread,
Dec 19, 2013, 8:40:03 AM12/19/13
to
On Thu, Dec 19, 2013 at 04:00:05PM +0900, Namhyung Kim wrote:
> Hello,
>
> I was playing with TUI code and added two new windows. One for
> showing log messages and another for showing header information.
> (Maybe they can be implemented on the GTK code too someday.)
>
> Please try to use it! :)

hi,
I'm sure the 'i' header display will be appreciated!

Any chance of searching capability in the 'l' log window?
mutt-like limit('l') command would be great ;-)

thanks,
jirka

Arnaldo Carvalho de Melo

unread,
Dec 19, 2013, 8:40:03 AM12/19/13
to
This should be in a different patch, one that is related just to
dump_trace, i.e. if -D, aka dump_trace (right?) is enabled, then
probably we won't be initializing the TUI/GUI, no?

So having a patch that deals just with conversions of fprintf ->
pr_{err,etc} and another that deals with dump_trace related seems to be
the best course of action.

- Arnaldo

Arnaldo Carvalho de Melo

unread,
Dec 19, 2013, 8:40:04 AM12/19/13
to
Em Thu, Dec 19, 2013 at 04:00:08PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyu...@lge.com>
>
> The va_end() in _eprintf() should be removed since the caller also
> invokes va_end().

Thanks, applied.

Jiri Olsa

unread,
Dec 19, 2013, 8:40:04 AM12/19/13
to
On Thu, Dec 19, 2013 at 04:00:08PM +0900, Namhyung Kim wrote:
> From: Namhyung Kim <namhyu...@lge.com>
>
> The va_end() in _eprintf() should be removed since the caller also
> invokes va_end().
>
> Signed-off-by: Namhyung Kim <namh...@kernel.org>

Acked-by: Jiri Olsa <jo...@redhat.com>

jirka

Jiri Olsa

unread,
Dec 19, 2013, 8:40:04 AM12/19/13
to
On Thu, Dec 19, 2013 at 04:00:06PM +0900, Namhyung Kim wrote:
> From: Namhyung Kim <namhyu...@lge.com>
>
> There're some places printing a message to stdout/err directly. It
> should be converted to use proper error printing functions instead.
>
> If it's not possible, just do it when --stdio was enabled only.
>
> Signed-off-by: Namhyung Kim <namh...@kernel.org>

Maybe it'd be worthwhile to factor perf_session__fprintf*
functions so the info could end up in the log window.

Acked-by: Jiri Olsa <jo...@redhat.com>

jirka

Ingo Molnar

unread,
Dec 19, 2013, 11:00:04 AM12/19/13
to

* Jiri Olsa <jo...@redhat.com> wrote:

> On Thu, Dec 19, 2013 at 04:00:05PM +0900, Namhyung Kim wrote:
> > Hello,
> >
> > I was playing with TUI code and added two new windows. One for
> > showing log messages and another for showing header information.
> > (Maybe they can be implemented on the GTK code too someday.)
> >
> > Please try to use it! :)
>
> hi,
> I'm sure the 'i' header display will be appreciated!
>
> Any chance of searching capability in the 'l' log window?
> mutt-like limit('l') command would be great ;-)

A bit like how '/' works in histogram view, it will filter on partial
matches as well, making it really easy to filter down to larger
classes of functions (in projects that have good function name
namespaces).

Thanks,

Ingo

Ingo Molnar

unread,
Dec 19, 2013, 11:10:04 AM12/19/13
to

* Jiri Olsa <jo...@redhat.com> wrote:

> On Thu, Dec 19, 2013 at 04:00:05PM +0900, Namhyung Kim wrote:
> > Hello,
> >
> > I was playing with TUI code and added two new windows. One for
> > showing log messages and another for showing header information.
> > (Maybe they can be implemented on the GTK code too someday.)
> >
> > Please try to use it! :)
>
> hi,
> I'm sure the 'i' header display will be appreciated!
>
> Any chance of searching capability in the 'l' log window?
> mutt-like limit('l') command would be great ;-)

+1 :-)

Ingo

Namhyung Kim

unread,
Dec 19, 2013, 8:30:02 PM12/19/13
to
On Thu, 19 Dec 2013 16:53:16 +0100, Ingo Molnar wrote:
> * Jiri Olsa <jo...@redhat.com> wrote:
>
>> On Thu, Dec 19, 2013 at 04:00:05PM +0900, Namhyung Kim wrote:
>> > Hello,
>> >
>> > I was playing with TUI code and added two new windows. One for
>> > showing log messages and another for showing header information.
>> > (Maybe they can be implemented on the GTK code too someday.)
>> >
>> > Please try to use it! :)
>>
>> hi,
>> I'm sure the 'i' header display will be appreciated!
>>
>> Any chance of searching capability in the 'l' log window?
>> mutt-like limit('l') command would be great ;-)
>
> A bit like how '/' works in histogram view, it will filter on partial
> matches as well, making it really easy to filter down to larger
> classes of functions (in projects that have good function name
> namespaces).

Yes, I was thinking about that too. Will send a patch soon.

Thanks,
Namhyung

Namhyung Kim

unread,
Dec 19, 2013, 8:30:02 PM12/19/13
to
Hi Arnaldo,

On Thu, 19 Dec 2013 10:28:06 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Dec 19, 2013 at 04:00:07PM +0900, Namhyung Kim escreveu:
>> From: Namhyung Kim <namhyu...@lge.com>
>>
>> Add new functions to save error messages in a temp file. It'll be
>> used by some UI front-ends to see the messages.
>
> Do we really have to grow a logging facility like this? Can't we somehow
> openlog/syslog/closelog? Just a first gut reaction, continuing to read
> the series...

I'm not sure. It's not really a logging facility - I just wanted a way to
save log messages somewhere in order to show them in a TUI window. My
feeling is that it doesn't need to grow since perf is not a service or so..

Thanks,
Namhyung

Namhyung Kim

unread,
Dec 19, 2013, 8:30:02 PM12/19/13
to
Hi Ingo,

On Thu, 19 Dec 2013 13:14:35 +0100, Ingo Molnar wrote:
> * Namhyung Kim <namh...@kernel.org> wrote:
>
>> Hello,
>>
>> I was playing with TUI code and added two new windows. One for
>> showing log messages and another for showing header information.
>> (Maybe they can be implemented on the GTK code too someday.)
>>
>> Please try to use it! :)
>>
>> I put the patches on 'perf/tui-v1' branch in my tree:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>>
>> Any feedbacks are more than welcome, thanks
>> Namhyung
>
> In the morning haze it took me some time to figure out that typing 'l'
> gives leads to the log window, 'i' to the header information.
>
> 'l' seems to work (no log messages though :-), but in perf-top 'i'
> segfaulted:
>
> comet:~/tip/tools/perf> perf top
> perf: Segmentation fault

Ah, forgot to test perf top - I just played with perf report at this
time. :-/

>
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7ffff0d39700 (LWP 14785)]
> perf_header__fprintf_info (session=session@entry=0x957bf0, fp=fp@entry=0x7fffec000a30, full=full@entry=true) at util/header.c:2195
> 2195 int fd = perf_data_file__fd(session->file);
> Missing separate debuginfos, use: debuginfo-install audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64 elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64 glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64 libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64 numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64 python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64 xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64
> (gdb) bt
> #0 perf_header__fprintf_info (session=session@entry=0x957bf0, fp=fp@entry=0x7fffec000a30, full=full@entry=true) at util/header.c:2195
> #1 0x00000000004ce3fe in tui__header_window (env=env@entry=0x957c30) at ui/browsers/header.c:92
> #2 0x00000000004cbdfd in perf_evsel__hists_browse (evsel=evsel@entry=0x957560, nr_events=nr_events@entry=1,
> helpline=helpline@entry=0x57db88 "For a higher level overview, try: perf top --sort comm,dso", ev_name=0x957870 "cycles", left_exits=left_exits@entry=false,
> hbt=hbt@entry=0x7ffff0d38ea0, min_pcnt=min_pcnt@entry=0, env=env@entry=0x957c30) at ui/browsers/hists.c:1491
> #3 0x00000000004cd5a5 in perf_evlist__tui_browse_hists (evlist=0x8e5930, help=help@entry=0x57db88 "For a higher level overview, try: perf top --sort comm,dso",
> hbt=hbt@entry=0x7ffff0d38ea0, min_pcnt=0, env=0x957c30) at ui/browsers/hists.c:1957
> #4 0x0000000000433b48 in display_thread_tui (arg=0x7fffffffb150) at builtin-top.c:584
> #5 0x0000003e59407c53 in start_thread () from /lib64/libpthread.so.0
> #6 0x0000003e590f5dbd in clone () from /lib64/libc.so.6
> (gdb)
>
> I suspect it got surprised by perf top not having a header per se?
> Still it might make sense to also robustify
> perf_header__fprintf_info() against segfaulting and such.

Right. To be precise, perf top do have header.env but not set the
actual info in it. I had to check it, sorry. The patch below will fix
the problem.


From 451a267fb2243860f62390151cc156a9eab7d317 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyu...@lge.com>
Date: Fri, 20 Dec 2013 09:14:23 +0900
Subject: [PATCH 1/3] perf ui/tui: fixup for header window on perf-top

It should check whether header.env is valid.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/ui/browsers/hists.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d710c6403a14..c25859cae32d 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1488,7 +1488,9 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
tui__log_window();
continue;
case 'i':
- tui__header_window(env);
+ /* env->arch is NULL for live-mode (i.e. perf top) */
+ if (env->arch)
+ tui__header_window(env);
continue;
case K_F1:
case 'h':
--
1.7.11.7


I'll fold it to the original patch.

>
> But it's a nice feature nevertheless!

Thanks! :)

>
> Btw., it would be nice if 'P' worked on these screens, so any
> interesting data can be extracted! Cut & paste is usually a PITA due
> to the graphical TUI characters.

Ah, okay. I think it should be easy, will cook a patch.

>
> Btw., in case you are taking TUI usability bugreports, here's a few I
> noticed while playing with your changes:

Hmm.. I'll take a look at them next week too. :)

Namhyung Kim

unread,
Dec 19, 2013, 8:40:02 PM12/19/13
to
On Thu, 19 Dec 2013 10:30:47 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Dec 19, 2013 at 04:00:06PM +0900, Namhyung Kim escreveu:
>> - if (verbose > 3)
>> - perf_session__fprintf(session, stdout);
>> + if (use_browser == 0) {
>> + if (verbose > 3)
>> + perf_session__fprintf(session, stdout);
>>
>> - if (verbose > 2)
>> - perf_session__fprintf_dsos(session, stdout);
>> + if (verbose > 2)
>> + perf_session__fprintf_dsos(session, stdout);
>>
>> - if (dump_trace) {
>> - perf_session__fprintf_nr_events(session, stdout);
>> - return 0;
>> + if (dump_trace) {
>> + perf_session__fprintf_nr_events(session, stdout);
>> + return 0;
>> + }
>> }
>
> This should be in a different patch, one that is related just to
> dump_trace, i.e. if -D, aka dump_trace (right?) is enabled, then
> probably we won't be initializing the TUI/GUI, no?

Yes, the dump_trace will make use_browser = 0. So I just moved it under
the if (use_browser == 0).

>
> So having a patch that deals just with conversions of fprintf ->
> pr_{err,etc} and another that deals with dump_trace related seems to be
> the best course of action.

Okay, I'll split this into two.

Thanks,
Namhyung

Namhyung Kim

unread,
Dec 19, 2013, 8:40:02 PM12/19/13
to
Hi Jiri,

On Thu, 19 Dec 2013 14:31:54 +0100, Jiri Olsa wrote:
> On Thu, Dec 19, 2013 at 04:00:06PM +0900, Namhyung Kim wrote:
>> From: Namhyung Kim <namhyu...@lge.com>
>>
>> There're some places printing a message to stdout/err directly. It
>> should be converted to use proper error printing functions instead.
>>
>> If it's not possible, just do it when --stdio was enabled only.
>>
>> Signed-off-by: Namhyung Kim <namh...@kernel.org>
>
> Maybe it'd be worthwhile to factor perf_session__fprintf*
> functions so the info could end up in the log window.

I thought about it too. And then considered something like below:

FILE *sfp = open_memstream(&ptr, &size);
perf_session__fprintf*(session, sfp, ...);
fclose(sfp);

perf_log_add(ptr);
fprintf(orig_fp, "%s", ptr);

free(ptr);


What do you think?

>
> Acked-by: Jiri Olsa <jo...@redhat.com>

Thanks!
Namhyung

Namhyung Kim

unread,
Dec 20, 2013, 12:20:01 AM12/20/13
to
From: Namhyung Kim <namhyu...@lge.com>

The message will be saved in a temp file so that it can be shown at a
UI dialog at any time.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/util/debug.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 299b55586502..f55b499c93fb 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -19,12 +19,18 @@ bool dump_trace = false, quiet = false;
static int _eprintf(int level, const char *fmt, va_list args)
{
int ret = 0;
+ va_list ap;

if (verbose >= level) {
+ va_copy(ap, args);
+
if (use_browser >= 1)
ui_helpline__vshow(fmt, args);
else
ret = vfprintf(stderr, fmt, args);
+
+ perf_log_addv(fmt, ap);
+ va_end(ap);
}

return ret;
--
1.7.11.7

Namhyung Kim

unread,
Dec 20, 2013, 12:20:01 AM12/20/13
to
From: Namhyung Kim <namhyu...@lge.com>

There're some places printing a message to stdout/err directly. It
should be converted to use proper error printing functions instead.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/builtin-report.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 8424053b399a..dcaab5e0bfa7 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -251,8 +251,8 @@ static int process_sample_event(struct perf_tool *tool,
int ret;

if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
- fprintf(stderr, "problem processing %d event, skipping it.\n",
- event->header.type);
+ pr_debug("problem processing %d event, skipping it.\n",
+ event->header.type);
return -1;
}

@@ -650,7 +650,7 @@ parse_callchain_opt(const struct option *opt, const char *arg, int unset)
return -1;
setup:
if (callchain_register_param(&callchain_param) < 0) {
- fprintf(stderr, "Can't register callchain params\n");
+ pr_err("Can't register callchain params\n");
return -1;
}
return 0;
@@ -872,7 +872,7 @@ repeat:
}
if (report.mem_mode) {
if (sort__mode == SORT_MODE__BRANCH) {
- fprintf(stderr, "branch and mem mode incompatible\n");
+ pr_err("branch and mem mode incompatible\n");
goto error;
}
sort__mode = SORT_MODE__MEMORY;

Namhyung Kim

unread,
Dec 20, 2013, 12:20:01 AM12/20/13
to
From: Namhyung Kim <namhyu...@lge.com>

Press '/' key to input filter string like main hist browser does.

Requested-by: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/ui/browsers/log.c | 62 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/log.c b/tools/perf/ui/browsers/log.c
index 592e2774e919..84b708073441 100644
--- a/tools/perf/ui/browsers/log.c
+++ b/tools/perf/ui/browsers/log.c
@@ -25,7 +25,7 @@ static void ui_browser__file_write(struct ui_browser *browser,
char empty[] = " ";
FILE *fp = perf_log.fp;
bool current_entry = ui_browser__is_current_entry(browser, row);
- off_t *linemap = perf_log.linemap;
+ off_t *linemap = browser->entries;
unsigned int idx = *(unsigned int *)entry;
unsigned long offset = (unsigned long)browser->priv;

@@ -59,9 +59,60 @@ static unsigned int ui_browser__file_refresh(struct ui_browser *browser)
return row;
}

+static void log_menu__filter(struct ui_browser *menu, char *filter)
+{
+ char buf[1024];
+ off_t *linemap = NULL;
+ u32 lines = 0;
+ u32 alloc = 0;
+ u32 i;
+
+ if (*filter == '\0') {
+ linemap = perf_log.linemap;
+ lines = perf_log.lines;
+ goto out;
+ }
+
+ if (menu->entries != perf_log.linemap)
+ free(menu->entries);
+
+ for (i = 0; i < perf_log.lines; i++) {
+ fseek(perf_log.fp, perf_log.linemap[i], SEEK_SET);
+ if (fgets(buf, sizeof(buf), perf_log.fp) == NULL)
+ goto error;
+
+ if (strstr(buf, filter) == NULL)
+ continue;
+
+ if (lines == alloc) {
+ off_t *map;
+
+ map = realloc(linemap, (alloc + 128) * sizeof(*linemap));
+ if (map == NULL)
+ goto error;
+
+ linemap = map;
+ alloc += 128;
+ }
+
+ linemap[lines++] = perf_log.linemap[i];
+ }
+out:
+ menu->entries = linemap;
+ menu->nr_entries = lines;
+
+ menu->top_idx = 0;
+ menu->index = 0;
+ return;
+
+error:
+ free(linemap);
+}
+
static int log_menu__run(struct ui_browser *menu)
{
int key;
+ char buf[64];
unsigned long offset;

if (ui_browser__show(menu, "Log messages", "Press 'q' to exit") < 0)
@@ -82,6 +133,14 @@ static int log_menu__run(struct ui_browser *menu)
offset -= 10;
menu->priv = (void *)offset;
continue;
+ case '/':
+ if (ui_browser__input_window("Symbol to show",
+ "Please enter the name of symbol you want to see",
+ buf, "ENTER: OK, ESC: Cancel",
+ 0) == K_ENTER) {
+ log_menu__filter(menu, buf);
+ }
+ continue;
case K_ESC:
case 'q':
case CTRL('c'):
@@ -104,6 +163,7 @@ int tui__log_window(void)
.refresh = ui_browser__file_refresh,
.seek = ui_browser__file_seek,
.write = ui_browser__file_write,
+ .entries = perf_log.linemap,
.nr_entries = perf_log.lines,

Namhyung Kim

unread,
Dec 20, 2013, 12:20:02 AM12/20/13
to
From: Namhyung Kim <namhyu...@lge.com>

Move those print functions under "if (use_browser == 0)" so that they
cannot interfere TUI output. Maybe they can handle other UIs later.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/builtin-report.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index dcaab5e0bfa7..1a120da6b2cd 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -482,15 +482,17 @@ static int __cmd_report(struct perf_report *rep)
desc);
}

- if (verbose > 3)
- perf_session__fprintf(session, stdout);
+ if (use_browser == 0) {
+ if (verbose > 3)
+ perf_session__fprintf(session, stdout);

- if (verbose > 2)
- perf_session__fprintf_dsos(session, stdout);
+ if (verbose > 2)
+ perf_session__fprintf_dsos(session, stdout);

- if (dump_trace) {
- perf_session__fprintf_nr_events(session, stdout);
- return 0;
+ if (dump_trace) {
+ perf_session__fprintf_nr_events(session, stdout);
+ return 0;
+ }
}

nr_samples = 0;

Namhyung Kim

unread,
Dec 20, 2013, 12:20:02 AM12/20/13
to
From: Namhyung Kim <namhyu...@lge.com>

Implement a simple, full-screen header window which shows session
header (metadata) information. Press 'i' key to display the header
window.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/Makefile.perf | 1 +
tools/perf/ui/browser.h | 2 +
tools/perf/ui/browsers/header.c | 116 ++++++++++++++++++++++++++++++++++++++++
tools/perf/ui/browsers/hists.c | 6 +++
4 files changed, 125 insertions(+)
create mode 100644 tools/perf/ui/browsers/header.c

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 30aabace33a0..4d046e97d0e7 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -491,6 +491,7 @@ ifndef NO_SLANG
LIB_OBJS += $(OUTPUT)ui/browsers/map.o
LIB_OBJS += $(OUTPUT)ui/browsers/scripts.o
LIB_OBJS += $(OUTPUT)ui/browsers/log.o
+ LIB_OBJS += $(OUTPUT)ui/browsers/header.o
LIB_OBJS += $(OUTPUT)ui/tui/setup.o
LIB_OBJS += $(OUTPUT)ui/tui/util.o
LIB_OBJS += $(OUTPUT)ui/tui/helpline.o
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index fff46942a8c7..b77d8728b3f0 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -60,6 +60,8 @@ bool ui_browser__dialog_yesno(struct ui_browser *browser, const char *text);
int ui_browser__input_window(const char *title, const char *text, char *input,
const char *exit_msg, int delay_sec);
int tui__log_window(void);
+struct perf_session_env;
+int tui__header_window(struct perf_session_env *env);

void ui_browser__argv_seek(struct ui_browser *browser, off_t offset, int whence);
unsigned int ui_browser__argv_refresh(struct ui_browser *browser);
diff --git a/tools/perf/ui/browsers/header.c b/tools/perf/ui/browsers/header.c
new file mode 100644
index 000000000000..b5f0961bf793
--- /dev/null
+++ b/tools/perf/ui/browsers/header.c
@@ -0,0 +1,116 @@
+#include "util/cache.h"
+#include "util/debug.h"
+#include "ui/browser.h"
+#include "ui/ui.h"
+#include "ui/util.h"
+#include "ui/libslang.h"
+#include "util/header.h"
+#include "util/session.h"
+
+static void ui_browser__argv_write(struct ui_browser *browser,
+ void *entry, int row)
+{
+ char **arg = entry;
+ char *str = *arg;
+ char empty[] = " ";
+ bool current_entry = ui_browser__is_current_entry(browser, row);
+ unsigned long offset = (unsigned long)browser->priv;
+
+ if (offset >= strlen(str))
+ str = empty;
+ else
+ str = str + offset;
+
+ ui_browser__set_color(browser, current_entry ? HE_COLORSET_SELECTED :
+ HE_COLORSET_NORMAL);
+
+ slsmg_write_nstring(str, browser->width);
+}
+
+static int list_menu__run(struct ui_browser *menu)
+{
+ int key;
+ unsigned long offset;
+
+ if (ui_browser__show(menu, "Header information", "Press 'q' to exit") < 0)
+ return -1;
+
+ while (1) {
+ key = ui_browser__run(menu, 0);
+
+ switch (key) {
+ case K_RIGHT:
+ offset = (unsigned long)menu->priv;
+ offset += 10;
+ menu->priv = (void *)offset;
+ continue;
+ case K_LEFT:
+ offset = (unsigned long)menu->priv;
+ if (offset >= 10)
+ offset -= 10;
+ menu->priv = (void *)offset;
+ continue;
+ case K_ESC:
+ case 'q':
+ case CTRL('c'):
+ key = -1;
+ break;
+ default:
+ continue;
+ }
+
+ break;
+ }
+
+ ui_browser__hide(menu);
+ return key;
+}
+
+ goto out;
+
+ argv[0] = pos = ptr;
+ for (i = 1; (pos = strchr(pos, '\n')) != NULL; i++) {
+ *pos++ = '\0';
+ argv[i] = pos;
+ }
+
+ BUG_ON(i != argc + 1);
+
+ ui__list_menu(argc, argv);
+
+out:
+ free(argv);
+ free(ptr);
+ return 0;
+}
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 8cc6f5eaa2cf..c25859cae32d 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1487,6 +1487,11 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
case 'l':
tui__log_window();
continue;
+ case 'i':
+ /* env->arch is NULL for live-mode (i.e. perf top) */
+ if (env->arch)
+ tui__header_window(env);
+ continue;
case K_F1:
case 'h':
case '?':
@@ -1510,6 +1515,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
"P Print histograms to perf.hist.N\n"
"V Verbose (DSO names in callchains, etc)\n"
"l Show log messages\n"
+ "i Show header information\n"
"/ Filter symbol by name");
continue;
case K_ENTER:

Namhyung Kim

unread,
Dec 20, 2013, 12:20:02 AM12/20/13
to
From: Namhyung Kim <namhyu...@lge.com>

Add new functions to save error messages in a temp file. It'll be
used by some UI front-ends to see the messages.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/Makefile.perf | 1 +
tools/perf/perf.c | 3 ++
tools/perf/util/debug.h | 15 +++++++
tools/perf/util/log.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 124 insertions(+)
create mode 100644 tools/perf/util/log.c

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 97a2145e4cc6..ec8184014a76 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
new file mode 100644
index 000000000000..4a34bed2af44
--- /dev/null
+++ b/tools/perf/util/log.c
@@ -0,0 +1,105 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include "util/debug.h"
+
+#define LINEMAP_GROW 128
+
+struct perf_log perf_log = {
+ .seen_newline = true,
+};
+
+int perf_log_init(void)
+{
+ FILE *fp;
+ char name[] = "/tmp/perf-tui-log-XXXXXX";
+ int fd = mkstemp(name);
+
+ if (fd < 0)
+ return -1;
+
+ fp = fdopen(fd, "r+");
+ if (fp == NULL) {
+ close(fd);
+ return -1;
+ }
+
+ return -1;
+
+ log->nr_alloc = newsize;
+ log->linemap = newmap;
+ return 0;
+}
+
+static int __add_to_linemap(struct perf_log *log, off_t index)
+{
+ if (log->lines == log->nr_alloc)
+ if (grow_linemap(log) < 0)
+ return -1;
+

Namhyung Kim

unread,
Dec 20, 2013, 12:20:02 AM12/20/13
to
From: Namhyung Kim <namhyu...@lge.com>

Implement a simple, full-screen log window which shows error messages
saved so far. Press 'l' (lower-case 'L') key to display the log
window. It'll be used usually with -v option.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/perf/Makefile.perf | 1 +
tools/perf/ui/browser.h | 1 +
tools/perf/ui/browsers/hists.c | 4 ++
tools/perf/ui/browsers/log.c | 111 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 117 insertions(+)
create mode 100644 tools/perf/ui/browsers/log.c

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index ec8184014a76..30aabace33a0 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -490,6 +490,7 @@ ifndef NO_SLANG
LIB_OBJS += $(OUTPUT)ui/browsers/hists.o
LIB_OBJS += $(OUTPUT)ui/browsers/map.o
LIB_OBJS += $(OUTPUT)ui/browsers/scripts.o
+ LIB_OBJS += $(OUTPUT)ui/browsers/log.o
LIB_OBJS += $(OUTPUT)ui/tui/setup.o
LIB_OBJS += $(OUTPUT)ui/tui/util.o
LIB_OBJS += $(OUTPUT)ui/tui/helpline.o
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index 7d45d2f53601..fff46942a8c7 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -59,6 +59,7 @@ int ui_browser__help_window(struct ui_browser *browser, const char *text);
bool ui_browser__dialog_yesno(struct ui_browser *browser, const char *text);
int ui_browser__input_window(const char *title, const char *text, char *input,
const char *exit_msg, int delay_sec);
+int tui__log_window(void);

void ui_browser__argv_seek(struct ui_browser *browser, off_t offset, int whence);
unsigned int ui_browser__argv_refresh(struct ui_browser *browser);
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index a440e03cd8c2..8cc6f5eaa2cf 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1484,6 +1484,9 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
if (is_report_browser(hbt))
goto do_data_switch;
continue;
+ case 'l':
+ tui__log_window();
+ continue;
case K_F1:
case 'h':
case '?':
@@ -1506,6 +1509,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
"s Switch to another data file in PWD ('perf report' only)\n"
"P Print histograms to perf.hist.N\n"
"V Verbose (DSO names in callchains, etc)\n"
+ "l Show log messages\n"
"/ Filter symbol by name");
continue;
case K_ENTER:
diff --git a/tools/perf/ui/browsers/log.c b/tools/perf/ui/browsers/log.c
new file mode 100644
index 000000000000..592e2774e919
--- /dev/null
+++ b/tools/perf/ui/browsers/log.c
@@ -0,0 +1,111 @@
+#include <stdio.h>
+
+#include "perf.h"
+#include "util/util.h"
+#include "util/cache.h"
+#include "util/debug.h"
+#include "ui/ui.h"
+#include "ui/util.h"
+#include "ui/browser.h"
+#include "ui/libslang.h"
+#include "ui/keysyms.h"
+#include "ui/tui/tui.h"
+
+static void ui_browser__file_seek(struct ui_browser *browser __maybe_unused,
+ off_t offset __maybe_unused,
+ int whence __maybe_unused)
+{
+ /* do nothing */
+}
+
+static void ui_browser__file_write(struct ui_browser *browser,
+ void *entry, int row)
+{
+ char buf[1024];
+ char empty[] = " ";
+ FILE *fp = perf_log.fp;
+ bool current_entry = ui_browser__is_current_entry(browser, row);
+ off_t *linemap = perf_log.linemap;
+ unsigned int idx = *(unsigned int *)entry;
+ unsigned long offset = (unsigned long)browser->priv;
+
+ fseek(fp, linemap[idx], SEEK_SET);
+ if (fgets(buf, sizeof(buf), fp) == NULL)
+ return;
+
+ ui_browser__set_color(browser, current_entry ? HE_COLORSET_SELECTED :
+ HE_COLORSET_NORMAL);
+
+ if (offset < strlen(buf))
+ slsmg_write_nstring(&buf[offset], browser->width);
+ else
+ slsmg_write_nstring(empty, browser->width);
+}
+
+static unsigned int ui_browser__file_refresh(struct ui_browser *browser)
+{
+ unsigned int row = 0;
+ unsigned int idx = browser->top_idx;
+
+ while (idx < browser->nr_entries) {
+ ui_browser__gotorc(browser, row, 0);
+ browser->write(browser, &idx, row);
+ if (++row == browser->height)
+ break;
+
+ ++idx;
+ }
+
+ return row;
+}
+
+static int log_menu__run(struct ui_browser *menu)
+{
+ int key;
+ unsigned long offset;
+
+ if (ui_browser__show(menu, "Log messages", "Press 'q' to exit") < 0)
+ return -1;
+
+int tui__log_window(void)
+{
+ struct ui_browser log_menu = {
+ .refresh = ui_browser__file_refresh,
+ .seek = ui_browser__file_seek,
+ .write = ui_browser__file_write,
+ .nr_entries = perf_log.lines,
+ };
+
+ return log_menu__run(&log_menu);

Namhyung Kim

unread,
Dec 20, 2013, 12:20:02 AM12/20/13
to
Hello,

I was playing with TUI code and added two new windows. One for
showing log messages and another for showing header information.
(Maybe they can be implemented on the GTK code too someday.)

* changes from v1)
- fix segfault on perf top (Ingo)
- split print function handling patch (Arnaldo)
- add filtering support on log window (Jiri, Ingo)


I put the patches on 'perf/tui-v2' branch in my tree:

git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Any feedbacks are more than welcome, thanks
Namhyung


Namhyung Kim (7):
perf report: Use pr_*() functions if possible
perf report: Print session information only if --stdio is given
perf tools: Introduce struct perf_log
perf tools: Save message when pr_*() was called
perf ui/tui: Implement log window
perf ui/tui: Implement header window
perf ui/tui: Filter messages in log window

tools/perf/Makefile.perf | 3 +
tools/perf/builtin-report.c | 24 +++---
tools/perf/perf.c | 3 +
tools/perf/ui/browser.h | 3 +
tools/perf/ui/browsers/header.c | 116 +++++++++++++++++++++++++++
tools/perf/ui/browsers/hists.c | 10 +++
tools/perf/ui/browsers/log.c | 171 ++++++++++++++++++++++++++++++++++++++++
tools/perf/util/debug.c | 6 ++
tools/perf/util/debug.h | 15 ++++
tools/perf/util/log.c | 105 ++++++++++++++++++++++++
10 files changed, 445 insertions(+), 11 deletions(-)
create mode 100644 tools/perf/ui/browsers/header.c
create mode 100644 tools/perf/ui/browsers/log.c
create mode 100644 tools/perf/util/log.c

Namhyung Kim

unread,
Dec 20, 2013, 12:30:02 AM12/20/13
to
From: Namhyung Kim <namhyu...@lge.com>

Press '/' key to input filter string like main hist browser does.

Requested-by: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
Fixed some memory leaks. I also updated my perf/tui-v2 branch.

tools/perf/ui/browsers/log.c | 70 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 68 insertions(+), 2 deletions(-)

diff --git a/tools/perf/ui/browsers/log.c b/tools/perf/ui/browsers/log.c
index 592e2774e919..9ff1bcb1c972 100644
@@ -104,8 +163,15 @@ int tui__log_window(void)
.refresh = ui_browser__file_refresh,
.seek = ui_browser__file_seek,
.write = ui_browser__file_write,
+ .entries = perf_log.linemap,
.nr_entries = perf_log.lines,
};
+ int key;
+
+ key = log_menu__run(&log_menu);

- return log_menu__run(&log_menu);
+ if (log_menu.entries != perf_log.linemap)
+ free(log_menu.entries);
+
+ return key;

Ingo Molnar

unread,
Dec 20, 2013, 3:20:02 AM12/20/13
to

* Namhyung Kim <namh...@kernel.org> wrote:

> Hello,
>
> I was playing with TUI code and added two new windows. One for
> showing log messages and another for showing header information.
> (Maybe they can be implemented on the GTK code too someday.)
>
> * changes from v1)
> - fix segfault on perf top (Ingo)
> - split print function handling patch (Arnaldo)
> - add filtering support on log window (Jiri, Ingo)
>
>
> I put the patches on 'perf/tui-v2' branch in my tree:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> Any feedbacks are more than welcome, thanks
> Namhyung

Looks pretty good now!

I found four small inconsistencies:

- in 'perf top' the '?' help text states that there's an 'i' key, but
that key does nothing.

- filtering support would be useful in the 'log' window as well :-)

- in both 'perf top' and 'perf report' neither the 'i' nor the 'l'
window shows any help window, so one has to guess that '/' does the
filtering.

- the hotkeys in the help window used to be ordered alphabetically -
they aren't anymore.

While testing 'perf top' I also found three new features which would
be very nice to have, in case you are interested in implementing them:

- it would be nice to have a hotkey to start/stop data collection on
demand, and another hotkey to reset the data. SysProf has this
feature, and it's a convenient workflow to have a separate 'data
collection' period (possibly done without any screen refresh, so
that data collection does not disturb the measured workload), and
a quiet 'look at all the data that is not being changed' period.
Especially with fast changing workloads the latter can be useful.

- it would be nice if 'perf report' had a 'view raw trace' window as
well, with filtering. That would be roughly equivalent to the 'perf
report -D' output [but one line per trace entry, i.e. no hex dump
shown by default], all available within the TUI. With filtering
that would be a pretty good way to look at various details.

- it might also be useful if it was possible to save a perf.data from
a 'perf top' session - and to start a 'perf top' session from a
specific perf.data [and with data collection disabled]. I.e. allow
intermediate modes between 'perf top', 'perf report' and 'perf
record' profiling workflows, all in a single TUI.

Thanks,

Ingo

Jiri Olsa

unread,
Dec 20, 2013, 12:40:02 PM12/20/13
to
On Fri, Dec 20, 2013 at 02:11:11PM +0900, Namhyung Kim wrote:
> Hello,
>
> I was playing with TUI code and added two new windows. One for
> showing log messages and another for showing header information.
> (Maybe they can be implemented on the GTK code too someday.)
>
> * changes from v1)
> - fix segfault on perf top (Ingo)
> - split print function handling patch (Arnaldo)
> - add filtering support on log window (Jiri, Ingo)
>
>
> I put the patches on 'perf/tui-v2' branch in my tree:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> Any feedbacks are more than welcome, thanks
> Namhyung

got segfault in perf top:
(pressed 'l' and several searches)

0x14cde0 __memmove_ssse3_back: period++ [addr: 0x14db58, 0xd78, evidx=0] => 10
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffeaf4a700 (LWP 8368)]
____rb_erase_color (augment_rotate=<optimized out>, root=<optimized out>, parent=0x157bd70)
at ../../lib/rbtree.c:229
229 rb_set_parent_color(tmp1, parent, RB_BLACK);
Missing separate debuginfos, use: debuginfo-install audit-libs-2.3.2-1.fc19.x86_64 elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64 libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.3-1.fc19.x86_64 numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64 python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64 xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64
(gdb) bt
#0 ____rb_erase_color (augment_rotate=<optimized out>, root=<optimized out>, parent=0x157bd70)
at ../../lib/rbtree.c:229
#1 rb_erase (node=node@entry=0x29bb500, root=root@entry=0x946a00) at ../../lib/rbtree.c:397
#2 0x000000000049e0f4 in hists__collapse_resort (hists=0x9469f8, prog=prog@entry=0x0) at util/hist.c:567
#3 0x0000000000432135 in perf_top__sort_new_samples (arg=0x7fffffffb270) at builtin-top.c:553
#4 0x00000000004cb64d in hist_browser__run (hbt=0x7fffeaf49ea0, ev_name=0x946c60 "cycles",
browser=<optimized out>) at ui/browsers/hists.c:340
#5 perf_evsel__hists_browse (evsel=evsel@entry=0x946950, nr_events=nr_events@entry=1,
helpline=helpline@entry=0x57dc78 "For a higher level overview, try: perf top --sort comm,dso",
ev_name=0x946c60 "cycles", left_exits=left_exits@entry=false, hbt=hbt@entry=0x7fffeaf49ea0,
min_pcnt=min_pcnt@entry=0, env=env@entry=0x947000) at ui/browsers/hists.c:1430
#6 0x00000000004cd425 in perf_evlist__tui_browse_hists (evlist=0x8e6840,
help=help@entry=0x57dc78 "For a higher level overview, try: perf top --sort comm,dso",
hbt=hbt@entry=0x7fffeaf49ea0, min_pcnt=0, env=0x947000) at ui/browsers/hists.c:1959
#7 0x0000000000433928 in display_thread_tui (arg=0x7fffffffb270) at builtin-top.c:581
#8 0x00000031c5407c53 in start_thread (arg=0x7fffeaf4a700) at pthread_create.c:308
#9 0x00000031c48f5dbd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113
(gdb)

haven't checked deeply yet

jirka

Namhyung Kim

unread,
Dec 23, 2013, 12:20:02 AM12/23/13
to
Hi Ingo,

On Fri, 20 Dec 2013 09:13:57 +0100, Ingo Molnar wrote:
> * Namhyung Kim <namh...@kernel.org> wrote:
> Looks pretty good now!
>
> I found four small inconsistencies:
>
> - in 'perf top' the '?' help text states that there's an 'i' key, but
> that key does nothing.

Yes, I can split the help text for perf top and perf report as some keys
only work for perf report already.

>
> - filtering support would be useful in the 'log' window as well :-)

It's for the 'log' window. Did you mean the 'header' window? :)

>
> - in both 'perf top' and 'perf report' neither the 'i' nor the 'l'
> window shows any help window, so one has to guess that '/' does the
> filtering.

Okay, I'll add the help.

>
> - the hotkeys in the help window used to be ordered alphabetically -
> they aren't anymore.

Will be fixed with the split.

>
> While testing 'perf top' I also found three new features which would
> be very nice to have, in case you are interested in implementing them:

Hmm.. looks like features for the long-term plan. Not sure I can work
on it soon.

>
> - it would be nice to have a hotkey to start/stop data collection on
> demand, and another hotkey to reset the data. SysProf has this
> feature, and it's a convenient workflow to have a separate 'data
> collection' period (possibly done without any screen refresh, so
> that data collection does not disturb the measured workload), and
> a quiet 'look at all the data that is not being changed' period.
> Especially with fast changing workloads the latter can be useful.

I'm not sure I understood correctly. So do you want start/stop the
'record' part or 'report' part with hotkeys?

>
> - it would be nice if 'perf report' had a 'view raw trace' window as
> well, with filtering. That would be roughly equivalent to the 'perf
> report -D' output [but one line per trace entry, i.e. no hex dump
> shown by default], all available within the TUI. With filtering
> that would be a pretty good way to look at various details.

Do you want something like 'perf script'? :)

>
> - it might also be useful if it was possible to save a perf.data from
> a 'perf top' session - and to start a 'perf top' session from a
> specific perf.data [and with data collection disabled]. I.e. allow
> intermediate modes between 'perf top', 'perf report' and 'perf
> record' profiling workflows, all in a single TUI.

Someday.. :)


Thanks,
Namhyung

Namhyung Kim

unread,
Dec 23, 2013, 12:30:02 AM12/23/13
to
Hi Jiri,

On Fri, 20 Dec 2013 18:33:51 +0100, Jiri Olsa wrote:
> On Fri, Dec 20, 2013 at 02:11:11PM +0900, Namhyung Kim wrote:
>> Hello,
>>
>> I was playing with TUI code and added two new windows. One for
>> showing log messages and another for showing header information.
>> (Maybe they can be implemented on the GTK code too someday.)
>>
>> * changes from v1)
>> - fix segfault on perf top (Ingo)
>> - split print function handling patch (Arnaldo)
>> - add filtering support on log window (Jiri, Ingo)
>>
>>
>> I put the patches on 'perf/tui-v2' branch in my tree:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>>
>> Any feedbacks are more than welcome, thanks
>> Namhyung
>
> got segfault in perf top:
> (pressed 'l' and several searches)

Hmm.. so did you this crash in the log window? How many logs in the
window - more than 100 or so? Could you reproduce it regularly?
Looks like not directly related to the log window.. maybe it already
corrupted some memory regions.

One thing I can think of is the perf_log.linemap was reallocated but the
log window accessed an old linemap.

Thanks,
Namhyung

Jiri Olsa

unread,
Dec 23, 2013, 6:30:02 AM12/23/13
to
On Mon, Dec 23, 2013 at 02:23:37PM +0900, Namhyung Kim wrote:
> Hi Jiri,
>
> On Fri, 20 Dec 2013 18:33:51 +0100, Jiri Olsa wrote:
> > On Fri, Dec 20, 2013 at 02:11:11PM +0900, Namhyung Kim wrote:
> >> Hello,
> >>
> >> I was playing with TUI code and added two new windows. One for
> >> showing log messages and another for showing header information.
> >> (Maybe they can be implemented on the GTK code too someday.)
> >>
> >> * changes from v1)
> >> - fix segfault on perf top (Ingo)
> >> - split print function handling patch (Arnaldo)
> >> - add filtering support on log window (Jiri, Ingo)
> >>
> >>
> >> I put the patches on 'perf/tui-v2' branch in my tree:
> >>
> >> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> >>
> >> Any feedbacks are more than welcome, thanks
> >> Namhyung
> >
> > got segfault in perf top:
> > (pressed 'l' and several searches)
>
> Hmm.. so did you this crash in the log window? How many logs in the
> window - more than 100 or so? Could you reproduce it regularly?

I'm on your perf/tui-v3

and reproducing via '$ sudo ./perf top -vvv'

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffeb142700 (LWP 21873)]
__GI___libc_free (mem=0x7fffd4488010) at malloc.c:2896
2896 if (chunk_is_mmapped(p)) /* release mmapped memory. */
(gdb) bt
#0 __GI___libc_free (mem=0x7fffd4488010) at malloc.c:2896
#1 0x00000000004cd1f2 in log_menu__filter (filter=0x7fffeb13eac0 "ia32_sysenter_target", menu=0x7fffeb13ea50) at ui/browsers/log.c:106
#2 log_menu__run (menu=0x7fffeb13ea50) at ui/browsers/log.c:157
#3 tui__log_window () at ui/browsers/log.c:187
#4 0x00000000004ca955 in perf_evsel__hists_browse (evsel=evsel@entry=0x984ad0, nr_events=nr_events@entry=1,
helpline=helpline@entry=0x57ca08 "For a higher level overview, try: perf top --sort comm,dso", ev_name=0x984de0 "cycles",
left_exits=left_exits@entry=false, hbt=hbt@entry=0x7fffeb141ea0, min_pcnt=min_pcnt@entry=0, env=env@entry=0x985180)
at ui/browsers/hists.c:1520
#5 0x00000000004cc135 in perf_evlist__tui_browse_hists (evlist=0x8e4840,
help=help@entry=0x57ca08 "For a higher level overview, try: perf top --sort comm,dso", hbt=hbt@entry=0x7fffeb141ea0, min_pcnt=0,
env=0x985180) at ui/browsers/hists.c:1971
#6 0x0000000000433538 in display_thread_tui (arg=0x7fffffffb270) at builtin-top.c:581
#7 0x00000031c5407c53 in start_thread (arg=0x7fffeb142700) at pthread_create.c:308
#8 0x00000031c48f5dbd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113
(gdb)

doing 'l', couple of Pg(Down|Up)s and searchs '/'

jirka

Ingo Molnar

unread,
Dec 23, 2013, 8:10:01 AM12/23/13
to

* Namhyung Kim <namh...@kernel.org> wrote:

> > - it would be nice to have a hotkey to start/stop data collection on
> > demand, and another hotkey to reset the data. SysProf has this
> > feature, and it's a convenient workflow to have a separate 'data
> > collection' period (possibly done without any screen refresh, so
> > that data collection does not disturb the measured workload), and
> > a quiet 'look at all the data that is not being changed' period.
> > Especially with fast changing workloads the latter can be useful.
>
> I'm not sure I understood correctly. So do you want start/stop the
> 'record' part or 'report' part with hotkeys?

The 'record' part - a bit like SysProf.

> > - it would be nice if 'perf report' had a 'view raw trace' window as
> > well, with filtering. That would be roughly equivalent to the 'perf
> > report -D' output [but one line per trace entry, i.e. no hex dump
> > shown by default], all available within the TUI. With filtering
> > that would be a pretty good way to look at various details.
>
> Do you want something like 'perf script'? :)

Or perf trace, just in a TUI ;-)

Thanks,

Ingo

tip-bot for Namhyung Kim

unread,
Jan 12, 2014, 1:40:01 PM1/12/14
to
Commit-ID: 73db8f82619b7538d9b4badfe13f3ab2fef7d9b3
Gitweb: http://git.kernel.org/tip/73db8f82619b7538d9b4badfe13f3ab2fef7d9b3
Author: Namhyung Kim <namhyu...@lge.com>
AuthorDate: Thu, 19 Dec 2013 16:00:08 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 19 Dec 2013 11:38:42 -0300

perf tools: Get rid of a duplicate va_end() in error reporting routine

The va_end() in _eprintf() should be removed since the caller also
invokes va_end().

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Acked-by: Jiri Olsa <jo...@redhat.com>
Cc: David Ahern <dsa...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Link: http://lkml.kernel.org/r/1387436411-20160-4-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/util/debug.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 8640a91..299b555 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -25,7 +25,6 @@ static int _eprintf(int level, const char *fmt, va_list args)
ui_helpline__vshow(fmt, args);
else
ret = vfprintf(stderr, fmt, args);
- va_end(args);
}

return ret;

tip-bot for Namhyung Kim

unread,
Jan 12, 2014, 1:40:03 PM1/12/14
to
Commit-ID: a42101418072d3be357b534521be2849518611e6
Gitweb: http://git.kernel.org/tip/a42101418072d3be357b534521be2849518611e6
Author: Namhyung Kim <namhyu...@lge.com>
AuthorDate: Fri, 20 Dec 2013 14:11:12 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Fri, 20 Dec 2013 13:34:53 -0300

perf report: Use pr_*() functions where applicable

There're some places printing messages to stdout/err directly.

It should be converted to use proper error printing functions instead.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Cc: David Ahern <dsa...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Link: http://lkml.kernel.org/r/1387516278-17024-2-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/builtin-report.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index ec7399a..0c9ec3e 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -242,8 +242,8 @@ static int process_sample_event(struct perf_tool *tool,
int ret;

if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
- fprintf(stderr, "problem processing %d event, skipping it.\n",
- event->header.type);
+ pr_debug("problem processing %d event, skipping it.\n",
+ event->header.type);
return -1;
}

@@ -637,7 +637,7 @@ parse_callchain_opt(const struct option *opt, const char *arg, int unset)
return -1;
setup:
if (callchain_register_param(&callchain_param) < 0) {
- fprintf(stderr, "Can't register callchain params\n");
+ pr_err("Can't register callchain params\n");
return -1;
}
return 0;
@@ -859,7 +859,7 @@ repeat:
}
if (report.mem_mode) {
if (sort__mode == SORT_MODE__BRANCH) {
- fprintf(stderr, "branch and mem mode incompatible\n");
+ pr_err("branch and mem mode incompatible\n");
goto error;
}
sort__mode = SORT_MODE__MEMORY;
--

tip-bot for Namhyung Kim

unread,
Jan 12, 2014, 1:40:05 PM1/12/14
to
Commit-ID: 150e465ac99ed18fb9555c16e0def7ce01913a2a
Gitweb: http://git.kernel.org/tip/150e465ac99ed18fb9555c16e0def7ce01913a2a
Author: Namhyung Kim <namhyu...@lge.com>
AuthorDate: Fri, 20 Dec 2013 14:11:13 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Fri, 20 Dec 2013 13:36:41 -0300

perf report: Print session information only if --stdio is given

Move those print functions under "if (use_browser == 0)" so that they
don't interfere with TUI output.

Maybe they can handle other UIs later.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Cc: David Ahern <dsa...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Link: http://lkml.kernel.org/r/1387516278-17024-3-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/builtin-report.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 0c9ec3e..bf8dd2e 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -469,15 +469,17 @@ static int __cmd_report(struct report *rep)
desc);
}

- if (verbose > 3)
- perf_session__fprintf(session, stdout);
+ if (use_browser == 0) {
+ if (verbose > 3)
+ perf_session__fprintf(session, stdout);

- if (verbose > 2)
- perf_session__fprintf_dsos(session, stdout);
+ if (verbose > 2)
+ perf_session__fprintf_dsos(session, stdout);

- if (dump_trace) {
- perf_session__fprintf_nr_events(session, stdout);
- return 0;
+ if (dump_trace) {
+ perf_session__fprintf_nr_events(session, stdout);
+ return 0;
+ }
}

nr_samples = 0;
--
0 new messages