linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2)
@ 2013-12-20  5:11 Namhyung Kim
  2013-12-20  5:11 ` [PATCH 1/7] perf report: Use pr_*() functions if possible Namhyung Kim
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Namhyung Kim @ 2013-12-20  5:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
	Jiri Olsa, David Ahern

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

-- 
1.7.11.7


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

* [PATCH 1/7] perf report: Use pr_*() functions if possible
  2013-12-20  5:11 [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Namhyung Kim
@ 2013-12-20  5:11 ` Namhyung Kim
  2014-01-12 18:36   ` [tip:perf/core] perf report: Use pr_*() functions where applicable tip-bot for Namhyung Kim
  2013-12-20  5:11 ` [PATCH 2/7] perf report: Print session information only if --stdio is given Namhyung Kim
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Namhyung Kim @ 2013-12-20  5:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
	Jiri Olsa, David Ahern

From: Namhyung Kim <namhyung.kim@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 <namhyung@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;
-- 
1.7.11.7


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

* [PATCH 2/7] perf report: Print session information only if --stdio is given
  2013-12-20  5:11 [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Namhyung Kim
  2013-12-20  5:11 ` [PATCH 1/7] perf report: Use pr_*() functions if possible Namhyung Kim
@ 2013-12-20  5:11 ` Namhyung Kim
  2014-01-12 18:36   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2013-12-20  5:11 ` [PATCH 3/7] perf tools: Introduce struct perf_log Namhyung Kim
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Namhyung Kim @ 2013-12-20  5:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
	Jiri Olsa, David Ahern

From: Namhyung Kim <namhyung.kim@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 <namhyung@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;
-- 
1.7.11.7


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

* [PATCH 3/7] perf tools: Introduce struct perf_log
  2013-12-20  5:11 [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Namhyung Kim
  2013-12-20  5:11 ` [PATCH 1/7] perf report: Use pr_*() functions if possible Namhyung Kim
  2013-12-20  5:11 ` [PATCH 2/7] perf report: Print session information only if --stdio is given Namhyung Kim
@ 2013-12-20  5:11 ` Namhyung Kim
  2013-12-20  5:11 ` [PATCH 4/7] perf tools: Save message when pr_*() was called Namhyung Kim
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Namhyung Kim @ 2013-12-20  5:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
	Jiri Olsa, David Ahern

From: Namhyung Kim <namhyung.kim@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 <namhyung@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);
+}
-- 
1.7.11.7


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

* [PATCH 4/7] perf tools: Save message when pr_*() was called
  2013-12-20  5:11 [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Namhyung Kim
                   ` (2 preceding siblings ...)
  2013-12-20  5:11 ` [PATCH 3/7] perf tools: Introduce struct perf_log Namhyung Kim
@ 2013-12-20  5:11 ` Namhyung Kim
  2013-12-20  5:11 ` [PATCH 5/7] perf ui/tui: Implement log window Namhyung Kim
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Namhyung Kim @ 2013-12-20  5:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
	Jiri Olsa, David Ahern

From: Namhyung Kim <namhyung.kim@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 <namhyung@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


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

* [PATCH 5/7] perf ui/tui: Implement log window
  2013-12-20  5:11 [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Namhyung Kim
                   ` (3 preceding siblings ...)
  2013-12-20  5:11 ` [PATCH 4/7] perf tools: Save message when pr_*() was called Namhyung Kim
@ 2013-12-20  5:11 ` Namhyung Kim
  2013-12-20  5:11 ` [PATCH 6/7] perf ui/tui: Implement header window Namhyung Kim
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Namhyung Kim @ 2013-12-20  5:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
	Jiri Olsa, David Ahern

From: Namhyung Kim <namhyung.kim@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 <namhyung@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;
+
+	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;
+}
+
+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);
+}
-- 
1.7.11.7


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

* [PATCH 6/7] perf ui/tui: Implement header window
  2013-12-20  5:11 [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Namhyung Kim
                   ` (4 preceding siblings ...)
  2013-12-20  5:11 ` [PATCH 5/7] perf ui/tui: Implement log window Namhyung Kim
@ 2013-12-20  5:11 ` Namhyung Kim
  2013-12-20  5:11 ` [PATCH 7/7] perf ui/tui: Filter messages in log window Namhyung Kim
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Namhyung Kim @ 2013-12-20  5:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
	Jiri Olsa, David Ahern

From: Namhyung Kim <namhyung.kim@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 <namhyung@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;
+}
+
+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..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:
-- 
1.7.11.7


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

* [PATCH 7/7] perf ui/tui: Filter messages in log window
  2013-12-20  5:11 [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Namhyung Kim
                   ` (5 preceding siblings ...)
  2013-12-20  5:11 ` [PATCH 6/7] perf ui/tui: Implement header window Namhyung Kim
@ 2013-12-20  5:11 ` Namhyung Kim
  2013-12-20  5:21   ` [PATCH v2.1 " Namhyung Kim
  2013-12-20  8:13 ` [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Ingo Molnar
  2013-12-20 17:33 ` Jiri Olsa
  8 siblings, 1 reply; 17+ messages in thread
From: Namhyung Kim @ 2013-12-20  5:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
	Jiri Olsa, David Ahern

From: Namhyung Kim <namhyung.kim@lge.com>

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

Requested-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@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,
 	};
 
-- 
1.7.11.7


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

* [PATCH v2.1 7/7] perf ui/tui: Filter messages in log window
  2013-12-20  5:11 ` [PATCH 7/7] perf ui/tui: Filter messages in log window Namhyung Kim
@ 2013-12-20  5:21   ` Namhyung Kim
  0 siblings, 0 replies; 17+ messages in thread
From: Namhyung Kim @ 2013-12-20  5:21 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
	Jiri Olsa, David Ahern

From: Namhyung Kim <namhyung.kim@lge.com>

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

Requested-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@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
--- 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;
+	}
+
+	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:
+	if (menu->entries != perf_log.linemap)
+		free(menu->entries);
+
+	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,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;
 }
-- 
1.7.11.7


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

* Re: [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2)
  2013-12-20  5:11 [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Namhyung Kim
                   ` (6 preceding siblings ...)
  2013-12-20  5:11 ` [PATCH 7/7] perf ui/tui: Filter messages in log window Namhyung Kim
@ 2013-12-20  8:13 ` Ingo Molnar
  2013-12-23  5:13   ` Namhyung Kim
  2013-12-20 17:33 ` Jiri Olsa
  8 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2013-12-20  8:13 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
	Namhyung Kim, LKML, Jiri Olsa, David Ahern


* Namhyung Kim <namhyung@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

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

* Re: [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2)
  2013-12-20  5:11 [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Namhyung Kim
                   ` (7 preceding siblings ...)
  2013-12-20  8:13 ` [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Ingo Molnar
@ 2013-12-20 17:33 ` Jiri Olsa
  2013-12-23  5:23   ` Namhyung Kim
  8 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2013-12-20 17:33 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
	Ingo Molnar, Namhyung Kim, LKML, David Ahern

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

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

* Re: [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2)
  2013-12-20  8:13 ` [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Ingo Molnar
@ 2013-12-23  5:13   ` Namhyung Kim
  2013-12-23 13:06     ` Ingo Molnar
  0 siblings, 1 reply; 17+ messages in thread
From: Namhyung Kim @ 2013-12-23  5:13 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
	Namhyung Kim, LKML, Jiri Olsa, David Ahern

Hi Ingo,

On Fri, 20 Dec 2013 09:13:57 +0100, Ingo Molnar wrote:
> * Namhyung Kim <namhyung@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

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

* Re: [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2)
  2013-12-20 17:33 ` Jiri Olsa
@ 2013-12-23  5:23   ` Namhyung Kim
  2013-12-23 11:26     ` Jiri Olsa
  0 siblings, 1 reply; 17+ messages in thread
From: Namhyung Kim @ 2013-12-23  5:23 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
	Ingo Molnar, Namhyung Kim, LKML, David Ahern

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?

>
> 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) 

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

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

* Re: [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2)
  2013-12-23  5:23   ` Namhyung Kim
@ 2013-12-23 11:26     ` Jiri Olsa
  0 siblings, 0 replies; 17+ messages in thread
From: Jiri Olsa @ 2013-12-23 11:26 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
	Ingo Molnar, Namhyung Kim, LKML, David Ahern

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

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

* Re: [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2)
  2013-12-23  5:13   ` Namhyung Kim
@ 2013-12-23 13:06     ` Ingo Molnar
  0 siblings, 0 replies; 17+ messages in thread
From: Ingo Molnar @ 2013-12-23 13:06 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
	Namhyung Kim, LKML, Jiri Olsa, David Ahern


* Namhyung Kim <namhyung@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

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

* [tip:perf/core] perf report: Use pr_*() functions where applicable
  2013-12-20  5:11 ` [PATCH 1/7] perf report: Use pr_*() functions if possible Namhyung Kim
@ 2014-01-12 18:36   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Namhyung Kim @ 2014-01-12 18:36 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra,
	namhyung.kim, namhyung, jolsa, dsahern, tglx

Commit-ID:  a42101418072d3be357b534521be2849518611e6
Gitweb:     http://git.kernel.org/tip/a42101418072d3be357b534521be2849518611e6
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Fri, 20 Dec 2013 14:11:12 +0900
Committer:  Arnaldo Carvalho de Melo <acme@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 <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1387516278-17024-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@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;

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

* [tip:perf/core] perf report: Print session information only if --stdio is given
  2013-12-20  5:11 ` [PATCH 2/7] perf report: Print session information only if --stdio is given Namhyung Kim
@ 2014-01-12 18:36   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Namhyung Kim @ 2014-01-12 18:36 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra,
	namhyung.kim, namhyung, jolsa, dsahern, tglx

Commit-ID:  150e465ac99ed18fb9555c16e0def7ce01913a2a
Gitweb:     http://git.kernel.org/tip/150e465ac99ed18fb9555c16e0def7ce01913a2a
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Fri, 20 Dec 2013 14:11:13 +0900
Committer:  Arnaldo Carvalho de Melo <acme@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 <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1387516278-17024-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@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;

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

end of thread, other threads:[~2014-01-12 18:36 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-20  5:11 [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Namhyung Kim
2013-12-20  5:11 ` [PATCH 1/7] perf report: Use pr_*() functions if possible Namhyung Kim
2014-01-12 18:36   ` [tip:perf/core] perf report: Use pr_*() functions where applicable tip-bot for Namhyung Kim
2013-12-20  5:11 ` [PATCH 2/7] perf report: Print session information only if --stdio is given Namhyung Kim
2014-01-12 18:36   ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-12-20  5:11 ` [PATCH 3/7] perf tools: Introduce struct perf_log Namhyung Kim
2013-12-20  5:11 ` [PATCH 4/7] perf tools: Save message when pr_*() was called Namhyung Kim
2013-12-20  5:11 ` [PATCH 5/7] perf ui/tui: Implement log window Namhyung Kim
2013-12-20  5:11 ` [PATCH 6/7] perf ui/tui: Implement header window Namhyung Kim
2013-12-20  5:11 ` [PATCH 7/7] perf ui/tui: Filter messages in log window Namhyung Kim
2013-12-20  5:21   ` [PATCH v2.1 " Namhyung Kim
2013-12-20  8:13 ` [PATCHSET 0/7] perf tools: A couple of TUI improvements (v2) Ingo Molnar
2013-12-23  5:13   ` Namhyung Kim
2013-12-23 13:06     ` Ingo Molnar
2013-12-20 17:33 ` Jiri Olsa
2013-12-23  5:23   ` Namhyung Kim
2013-12-23 11:26     ` Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).