* [PATCH 1/8] perf ui/tui: Protect windows by ui__lock
2013-12-26 5:37 [PATCHSET 0/8] perf tools: A couple of TUI improvements (v3) Namhyung Kim
@ 2013-12-26 5:37 ` Namhyung Kim
2014-01-12 18:39 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-12-26 5:37 ` [PATCH 2/8] perf ui/tui: Split help message for perf top and report Namhyung Kim
` (6 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2013-12-26 5:37 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>
Sometimes perf top TUI breaks display with concurrent help/input
window and pr_* messages since they're not protected by ui__lock.
You can check it by pressing (and not releasing) 'h' key on a
"perf top -vvv" TUI session.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/tui/util.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/tools/perf/ui/tui/util.c b/tools/perf/ui/tui/util.c
index 092902e30cee..bf890f72fe80 100644
--- a/tools/perf/ui/tui/util.c
+++ b/tools/perf/ui/tui/util.c
@@ -92,6 +92,8 @@ int ui_browser__input_window(const char *title, const char *text, char *input,
t = sep + 1;
}
+ pthread_mutex_lock(&ui__lock);
+
max_len += 2;
nr_lines += 8;
y = SLtt_Screen_Rows / 2 - nr_lines / 2;
@@ -120,13 +122,19 @@ int ui_browser__input_window(const char *title, const char *text, char *input,
SLsmg_write_nstring((char *)exit_msg, max_len);
SLsmg_refresh();
+ pthread_mutex_unlock(&ui__lock);
+
x += 2;
len = 0;
key = ui__getch(delay_secs);
while (key != K_TIMER && key != K_ENTER && key != K_ESC) {
+ pthread_mutex_lock(&ui__lock);
+
if (key == K_BKSPC) {
- if (len == 0)
+ if (len == 0) {
+ pthread_mutex_unlock(&ui__lock);
goto next_key;
+ }
SLsmg_gotorc(y, x + --len);
SLsmg_write_char(' ');
} else {
@@ -136,6 +144,8 @@ int ui_browser__input_window(const char *title, const char *text, char *input,
}
SLsmg_refresh();
+ pthread_mutex_unlock(&ui__lock);
+
/* XXX more graceful overflow handling needed */
if (len == sizeof(buf) - 1) {
ui_helpline__push("maximum size of symbol name reached!");
@@ -174,6 +184,8 @@ int ui__question_window(const char *title, const char *text,
t = sep + 1;
}
+ pthread_mutex_lock(&ui__lock);
+
max_len += 2;
nr_lines += 4;
y = SLtt_Screen_Rows / 2 - nr_lines / 2,
@@ -195,6 +207,9 @@ int ui__question_window(const char *title, const char *text,
SLsmg_gotorc(y + nr_lines - 1, x);
SLsmg_write_nstring((char *)exit_msg, max_len);
SLsmg_refresh();
+
+ pthread_mutex_unlock(&ui__lock);
+
return ui__getch(delay_secs);
}
@@ -215,9 +230,7 @@ static int __ui__warning(const char *title, const char *format, va_list args)
if (vasprintf(&s, format, args) > 0) {
int key;
- pthread_mutex_lock(&ui__lock);
key = ui__question_window(title, s, "Press any key...", 0);
- pthread_mutex_unlock(&ui__lock);
free(s);
return key;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread* [tip:perf/core] perf ui/tui: Protect windows by ui__lock
2013-12-26 5:37 ` [PATCH 1/8] perf ui/tui: Protect windows by ui__lock Namhyung Kim
@ 2014-01-12 18:39 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2014-01-12 18:39 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: 5c743cf573e6974befe917ed4a36d42b39ef1ce0
Gitweb: http://git.kernel.org/tip/5c743cf573e6974befe917ed4a36d42b39ef1ce0
Author: Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 26 Dec 2013 14:37:57 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 26 Dec 2013 11:22:01 -0300
perf ui/tui: Protect windows by ui__lock
Sometimes perf top TUI breaks display with concurrent help/input window
and pr_* messages since they're not protected by ui__lock.
You can check it by pressing (and not releasing) 'h' key on a "perf top
-vvv" TUI session.
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/1388036284-32342-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/tui/util.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/tools/perf/ui/tui/util.c b/tools/perf/ui/tui/util.c
index 092902e..bf890f7 100644
--- a/tools/perf/ui/tui/util.c
+++ b/tools/perf/ui/tui/util.c
@@ -92,6 +92,8 @@ int ui_browser__input_window(const char *title, const char *text, char *input,
t = sep + 1;
}
+ pthread_mutex_lock(&ui__lock);
+
max_len += 2;
nr_lines += 8;
y = SLtt_Screen_Rows / 2 - nr_lines / 2;
@@ -120,13 +122,19 @@ int ui_browser__input_window(const char *title, const char *text, char *input,
SLsmg_write_nstring((char *)exit_msg, max_len);
SLsmg_refresh();
+ pthread_mutex_unlock(&ui__lock);
+
x += 2;
len = 0;
key = ui__getch(delay_secs);
while (key != K_TIMER && key != K_ENTER && key != K_ESC) {
+ pthread_mutex_lock(&ui__lock);
+
if (key == K_BKSPC) {
- if (len == 0)
+ if (len == 0) {
+ pthread_mutex_unlock(&ui__lock);
goto next_key;
+ }
SLsmg_gotorc(y, x + --len);
SLsmg_write_char(' ');
} else {
@@ -136,6 +144,8 @@ int ui_browser__input_window(const char *title, const char *text, char *input,
}
SLsmg_refresh();
+ pthread_mutex_unlock(&ui__lock);
+
/* XXX more graceful overflow handling needed */
if (len == sizeof(buf) - 1) {
ui_helpline__push("maximum size of symbol name reached!");
@@ -174,6 +184,8 @@ int ui__question_window(const char *title, const char *text,
t = sep + 1;
}
+ pthread_mutex_lock(&ui__lock);
+
max_len += 2;
nr_lines += 4;
y = SLtt_Screen_Rows / 2 - nr_lines / 2,
@@ -195,6 +207,9 @@ int ui__question_window(const char *title, const char *text,
SLsmg_gotorc(y + nr_lines - 1, x);
SLsmg_write_nstring((char *)exit_msg, max_len);
SLsmg_refresh();
+
+ pthread_mutex_unlock(&ui__lock);
+
return ui__getch(delay_secs);
}
@@ -215,9 +230,7 @@ static int __ui__warning(const char *title, const char *format, va_list args)
if (vasprintf(&s, format, args) > 0) {
int key;
- pthread_mutex_lock(&ui__lock);
key = ui__question_window(title, s, "Press any key...", 0);
- pthread_mutex_unlock(&ui__lock);
free(s);
return key;
}
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/8] perf ui/tui: Split help message for perf top and report
2013-12-26 5:37 [PATCHSET 0/8] perf tools: A couple of TUI improvements (v3) Namhyung Kim
2013-12-26 5:37 ` [PATCH 1/8] perf ui/tui: Protect windows by ui__lock Namhyung Kim
@ 2013-12-26 5:37 ` Namhyung Kim
2013-12-26 14:05 ` Arnaldo Carvalho de Melo
2014-01-12 18:39 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-12-26 5:37 ` [PATCH 3/8] perf ui/tui: Implement header window Namhyung Kim
` (5 subsequent siblings)
7 siblings, 2 replies; 18+ messages in thread
From: Namhyung Kim @ 2013-12-26 5:37 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>
Some hotkeys don't work for perf top so split help messages for them.
It'll be helpful to a future modification. Also sort the message by
alphabetical order of the hotkey.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/hists.c | 49 ++++++++++++++++++++++++++----------------
1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index a440e03cd8c2..d43ec79ea4e3 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1400,6 +1400,35 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
char script_opt[64];
int delay_secs = hbt ? hbt->refresh : 0;
+#define HIST_BROWSER_HELP_COMMON \
+ "h/?/F1 Show this window\n" \
+ "UP/DOWN/PGUP\n" \
+ "PGDN/SPACE Navigate\n" \
+ "q/ESC/CTRL+C Exit browser\n\n" \
+ "For multiple event sessions:\n\n" \
+ "TAB/UNTAB Switch events\n\n" \
+ "For symbolic views (--sort has sym):\n\n" \
+ "-> Zoom into DSO/Threads & Annotate current symbol\n" \
+ "<- Zoom out\n" \
+ "a Annotate current symbol\n" \
+ "C Collapse all callchains\n" \
+ "d Zoom into current DSO\n" \
+ "E Expand all callchains\n" \
+
+ /* help messages are sorted by lexical order of the hotkey */
+ const char report_help[] = HIST_BROWSER_HELP_COMMON
+ "P Print histograms to perf.hist.N\n"
+ "r Run available scripts\n"
+ "s Switch to another data file in PWD\n"
+ "t Zoom into current Thread\n"
+ "V Verbose (DSO names in callchains, etc)\n"
+ "/ Filter symbol by name";
+ const char top_help[] = HIST_BROWSER_HELP_COMMON
+ "P Print histograms to perf.hist.N\n"
+ "t Zoom into current Thread\n"
+ "V Verbose (DSO names in callchains, etc)\n"
+ "/ Filter symbol by name";
+
if (browser == NULL)
return -1;
@@ -1488,25 +1517,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
case 'h':
case '?':
ui_browser__help_window(&browser->b,
- "h/?/F1 Show this window\n"
- "UP/DOWN/PGUP\n"
- "PGDN/SPACE Navigate\n"
- "q/ESC/CTRL+C Exit browser\n\n"
- "For multiple event sessions:\n\n"
- "TAB/UNTAB Switch events\n\n"
- "For symbolic views (--sort has sym):\n\n"
- "-> Zoom into DSO/Threads & Annotate current symbol\n"
- "<- Zoom out\n"
- "a Annotate current symbol\n"
- "C Collapse all callchains\n"
- "E Expand all callchains\n"
- "d Zoom into current DSO\n"
- "t Zoom into current Thread\n"
- "r Run available scripts('perf report' only)\n"
- "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"
- "/ Filter symbol by name");
+ is_report_browser(hbt) ? report_help : top_help);
continue;
case K_ENTER:
case K_RIGHT:
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 2/8] perf ui/tui: Split help message for perf top and report
2013-12-26 5:37 ` [PATCH 2/8] perf ui/tui: Split help message for perf top and report Namhyung Kim
@ 2013-12-26 14:05 ` Arnaldo Carvalho de Melo
2013-12-26 14:12 ` Arnaldo Carvalho de Melo
2014-01-12 18:39 ` [tip:perf/core] " tip-bot for Namhyung Kim
1 sibling, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-12-26 14:05 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa, David Ahern
Em Thu, Dec 26, 2013 at 02:37:58PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyung.kim@lge.com>
>
> Some hotkeys don't work for perf top so split help messages for them.
> It'll be helpful to a future modification. Also sort the message by
> alphabetical order of the hotkey.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/ui/browsers/hists.c | 49 ++++++++++++++++++++++++++----------------
> 1 file changed, 30 insertions(+), 19 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index a440e03cd8c2..d43ec79ea4e3 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -1400,6 +1400,35 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
> char script_opt[64];
> int delay_secs = hbt ? hbt->refresh : 0;
>
> +#define HIST_BROWSER_HELP_COMMON \
> + "h/?/F1 Show this window\n" \
> + "UP/DOWN/PGUP\n" \
> + "PGDN/SPACE Navigate\n" \
> + "q/ESC/CTRL+C Exit browser\n\n" \
> + "For multiple event sessions:\n\n" \
> + "TAB/UNTAB Switch events\n\n" \
> + "For symbolic views (--sort has sym):\n\n" \
> + "-> Zoom into DSO/Threads & Annotate current symbol\n" \
> + "<- Zoom out\n" \
> + "a Annotate current symbol\n" \
> + "C Collapse all callchains\n" \
> + "d Zoom into current DSO\n" \
> + "E Expand all callchains\n" \
> +
> + /* help messages are sorted by lexical order of the hotkey */
> + const char report_help[] = HIST_BROWSER_HELP_COMMON
> + "P Print histograms to perf.hist.N\n"
> + "r Run available scripts\n"
> + "s Switch to another data file in PWD\n"
> + "t Zoom into current Thread\n"
> + "V Verbose (DSO names in callchains, etc)\n"
> + "/ Filter symbol by name";
> + const char top_help[] = HIST_BROWSER_HELP_COMMON
> + "P Print histograms to perf.hist.N\n"
> + "t Zoom into current Thread\n"
> + "V Verbose (DSO names in callchains, etc)\n"
> + "/ Filter symbol by name";
> +
> if (browser == NULL)
return -1;
This wastes space, why not have the HIST_BROWSER_HELP_COMMON as a
const char common_help[] = ...
and then use:
ui_browser_helo(..., "%s%s", common_help,
is_report_browser(hbt) ? report_help : top_help);
?
- Arnaldo
>
> @@ -1488,25 +1517,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
> case 'h':
> case '?':
> ui_browser__help_window(&browser->b,
> - "h/?/F1 Show this window\n"
> - "UP/DOWN/PGUP\n"
> - "PGDN/SPACE Navigate\n"
> - "q/ESC/CTRL+C Exit browser\n\n"
> - "For multiple event sessions:\n\n"
> - "TAB/UNTAB Switch events\n\n"
> - "For symbolic views (--sort has sym):\n\n"
> - "-> Zoom into DSO/Threads & Annotate current symbol\n"
> - "<- Zoom out\n"
> - "a Annotate current symbol\n"
> - "C Collapse all callchains\n"
> - "E Expand all callchains\n"
> - "d Zoom into current DSO\n"
> - "t Zoom into current Thread\n"
> - "r Run available scripts('perf report' only)\n"
> - "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"
> - "/ Filter symbol by name");
> + is_report_browser(hbt) ? report_help : top_help);
> continue;
> case K_ENTER:
> case K_RIGHT:
> --
> 1.7.11.7
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/8] perf ui/tui: Split help message for perf top and report
2013-12-26 14:05 ` Arnaldo Carvalho de Melo
@ 2013-12-26 14:12 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-12-26 14:12 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa, David Ahern
Em Thu, Dec 26, 2013 at 11:05:21AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Dec 26, 2013 at 02:37:58PM +0900, Namhyung Kim escreveu:
> > Some hotkeys don't work for perf top so split help messages for them.
> > It'll be helpful to a future modification. Also sort the message by
> > alphabetical order of the hotkey.
> > +++ b/tools/perf/ui/browsers/hists.c
> > @@ -1400,6 +1400,35 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
> > char script_opt[64];
> > int delay_secs = hbt ? hbt->refresh : 0;
> >
> > +#define HIST_BROWSER_HELP_COMMON \
> > + "h/?/F1 Show this window\n" \
> > + "UP/DOWN/PGUP\n" \
> > + "PGDN/SPACE Navigate\n" \
> > + "q/ESC/CTRL+C Exit browser\n\n" \
> > + "For multiple event sessions:\n\n" \
> > + "TAB/UNTAB Switch events\n\n" \
> > + "For symbolic views (--sort has sym):\n\n" \
> > + "-> Zoom into DSO/Threads & Annotate current symbol\n" \
> > + "<- Zoom out\n" \
> > + "a Annotate current symbol\n" \
> > + "C Collapse all callchains\n" \
> > + "d Zoom into current DSO\n" \
> > + "E Expand all callchains\n" \
> > +
> > + /* help messages are sorted by lexical order of the hotkey */
> > + const char report_help[] = HIST_BROWSER_HELP_COMMON
> > + "P Print histograms to perf.hist.N\n"
> > + "r Run available scripts\n"
> > + "s Switch to another data file in PWD\n"
> > + "t Zoom into current Thread\n"
> > + "V Verbose (DSO names in callchains, etc)\n"
> > + "/ Filter symbol by name";
> > + const char top_help[] = HIST_BROWSER_HELP_COMMON
> > + "P Print histograms to perf.hist.N\n"
> > + "t Zoom into current Thread\n"
> > + "V Verbose (DSO names in callchains, etc)\n"
> > + "/ Filter symbol by name";
> > +
> > if (browser == NULL)
> return -1;
>
> This wastes space, why not have the HIST_BROWSER_HELP_COMMON as a
>
> const char common_help[] = ...
>
> and then use:
> ui_browser__help(..., "%s%s", common_help,
> is_report_browser(hbt) ? report_help : top_help);
>
> ?
Because ui__help_window doesn't support va_arg, ok, applying this one
and later we can remove this space wastage when we make it support
variadic arguments.
- Arnaldo
^ permalink raw reply [flat|nested] 18+ messages in thread
* [tip:perf/core] perf ui/tui: Split help message for perf top and report
2013-12-26 5:37 ` [PATCH 2/8] perf ui/tui: Split help message for perf top and report Namhyung Kim
2013-12-26 14:05 ` Arnaldo Carvalho de Melo
@ 2014-01-12 18:39 ` tip-bot for Namhyung Kim
1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2014-01-12 18:39 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: e8e684a58b9bddde3fdb1a65cd26eb7a3e1e746e
Gitweb: http://git.kernel.org/tip/e8e684a58b9bddde3fdb1a65cd26eb7a3e1e746e
Author: Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 26 Dec 2013 14:37:58 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 26 Dec 2013 11:22:02 -0300
perf ui/tui: Split help message for perf top and report
Some hotkeys don't work for perf top so split help messages for them.
It'll be helpful to a future modification. Also sort the message by
alphabetical order of the hotkey.
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/1388036284-32342-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/hists.c | 49 ++++++++++++++++++++++++++----------------
1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index a440e03..d43ec79 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1400,6 +1400,35 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
char script_opt[64];
int delay_secs = hbt ? hbt->refresh : 0;
+#define HIST_BROWSER_HELP_COMMON \
+ "h/?/F1 Show this window\n" \
+ "UP/DOWN/PGUP\n" \
+ "PGDN/SPACE Navigate\n" \
+ "q/ESC/CTRL+C Exit browser\n\n" \
+ "For multiple event sessions:\n\n" \
+ "TAB/UNTAB Switch events\n\n" \
+ "For symbolic views (--sort has sym):\n\n" \
+ "-> Zoom into DSO/Threads & Annotate current symbol\n" \
+ "<- Zoom out\n" \
+ "a Annotate current symbol\n" \
+ "C Collapse all callchains\n" \
+ "d Zoom into current DSO\n" \
+ "E Expand all callchains\n" \
+
+ /* help messages are sorted by lexical order of the hotkey */
+ const char report_help[] = HIST_BROWSER_HELP_COMMON
+ "P Print histograms to perf.hist.N\n"
+ "r Run available scripts\n"
+ "s Switch to another data file in PWD\n"
+ "t Zoom into current Thread\n"
+ "V Verbose (DSO names in callchains, etc)\n"
+ "/ Filter symbol by name";
+ const char top_help[] = HIST_BROWSER_HELP_COMMON
+ "P Print histograms to perf.hist.N\n"
+ "t Zoom into current Thread\n"
+ "V Verbose (DSO names in callchains, etc)\n"
+ "/ Filter symbol by name";
+
if (browser == NULL)
return -1;
@@ -1488,25 +1517,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
case 'h':
case '?':
ui_browser__help_window(&browser->b,
- "h/?/F1 Show this window\n"
- "UP/DOWN/PGUP\n"
- "PGDN/SPACE Navigate\n"
- "q/ESC/CTRL+C Exit browser\n\n"
- "For multiple event sessions:\n\n"
- "TAB/UNTAB Switch events\n\n"
- "For symbolic views (--sort has sym):\n\n"
- "-> Zoom into DSO/Threads & Annotate current symbol\n"
- "<- Zoom out\n"
- "a Annotate current symbol\n"
- "C Collapse all callchains\n"
- "E Expand all callchains\n"
- "d Zoom into current DSO\n"
- "t Zoom into current Thread\n"
- "r Run available scripts('perf report' only)\n"
- "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"
- "/ Filter symbol by name");
+ is_report_browser(hbt) ? report_help : top_help);
continue;
case K_ENTER:
case K_RIGHT:
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/8] perf ui/tui: Implement header window
2013-12-26 5:37 [PATCHSET 0/8] perf tools: A couple of TUI improvements (v3) Namhyung Kim
2013-12-26 5:37 ` [PATCH 1/8] perf ui/tui: Protect windows by ui__lock Namhyung Kim
2013-12-26 5:37 ` [PATCH 2/8] perf ui/tui: Split help message for perf top and report Namhyung Kim
@ 2013-12-26 5:37 ` Namhyung Kim
2014-01-12 18:39 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-12-26 5:38 ` [PATCH 4/8] perf tools: Introduce struct perf_log Namhyung Kim
` (4 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2013-12-26 5:37 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 +
| 127 ++++++++++++++++++++++++++++++++++++++++
tools/perf/ui/browsers/hists.c | 6 ++
4 files changed, 136 insertions(+)
create mode 100644 tools/perf/ui/browsers/header.c
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 97a2145e4cc6..3638b0bd20dc 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -489,6 +489,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/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 7d45d2f53601..118cca29dd26 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -59,6 +59,8 @@ 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);
+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);
--git a/tools/perf/ui/browsers/header.c b/tools/perf/ui/browsers/header.c
new file mode 100644
index 000000000000..89c16b988618
--- /dev/null
+++ b/tools/perf/ui/browsers/header.c
@@ -0,0 +1,127 @@
+#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;
+ const char help[] =
+ "h/?/F1 Show this window\n"
+ "UP/DOWN/PGUP\n"
+ "PGDN/SPACE\n"
+ "LEFT/RIGHT Navigate\n"
+ "q/ESC/CTRL+C Exit browser";
+
+ 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_F1:
+ case 'h':
+ case '?':
+ ui_browser__help_window(menu, help);
+ 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 d43ec79ea4e3..0d9dd99507ee 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1417,6 +1417,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
/* help messages are sorted by lexical order of the hotkey */
const char report_help[] = HIST_BROWSER_HELP_COMMON
+ "i Show header information\n"
"P Print histograms to perf.hist.N\n"
"r Run available scripts\n"
"s Switch to another data file in PWD\n"
@@ -1513,6 +1514,11 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
if (is_report_browser(hbt))
goto do_data_switch;
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 '?':
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread* [tip:perf/core] perf ui/tui: Implement header window
2013-12-26 5:37 ` [PATCH 3/8] perf ui/tui: Implement header window Namhyung Kim
@ 2014-01-12 18:39 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2014-01-12 18:39 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: 6dd601354f14b5cd7a0a4103811e52ccec22ac53
Gitweb: http://git.kernel.org/tip/6dd601354f14b5cd7a0a4103811e52ccec22ac53
Author: Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 26 Dec 2013 14:37:59 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 26 Dec 2013 11:22:02 -0300
perf ui/tui: Implement header window
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>
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/1388036284-32342-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/Makefile.perf | 1 +
tools/perf/ui/browser.h | 2 +
| 127 ++++++++++++++++++++++++++++++++++++++++
tools/perf/ui/browsers/hists.c | 6 ++
4 files changed, 136 insertions(+)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 97a2145..3638b0b 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -489,6 +489,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/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 7d45d2f..118cca2 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -59,6 +59,8 @@ 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);
+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);
--git a/tools/perf/ui/browsers/header.c b/tools/perf/ui/browsers/header.c
new file mode 100644
index 0000000..89c16b9
--- /dev/null
+++ b/tools/perf/ui/browsers/header.c
@@ -0,0 +1,127 @@
+#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;
+ const char help[] =
+ "h/?/F1 Show this window\n"
+ "UP/DOWN/PGUP\n"
+ "PGDN/SPACE\n"
+ "LEFT/RIGHT Navigate\n"
+ "q/ESC/CTRL+C Exit browser";
+
+ 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_F1:
+ case 'h':
+ case '?':
+ ui_browser__help_window(menu, help);
+ 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 d43ec79..0d9dd99 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1417,6 +1417,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
/* help messages are sorted by lexical order of the hotkey */
const char report_help[] = HIST_BROWSER_HELP_COMMON
+ "i Show header information\n"
"P Print histograms to perf.hist.N\n"
"r Run available scripts\n"
"s Switch to another data file in PWD\n"
@@ -1513,6 +1514,11 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
if (is_report_browser(hbt))
goto do_data_switch;
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 '?':
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/8] perf tools: Introduce struct perf_log
2013-12-26 5:37 [PATCHSET 0/8] perf tools: A couple of TUI improvements (v3) Namhyung Kim
` (2 preceding siblings ...)
2013-12-26 5:37 ` [PATCH 3/8] perf ui/tui: Implement header window Namhyung Kim
@ 2013-12-26 5:38 ` Namhyung Kim
2013-12-26 14:50 ` Arnaldo Carvalho de Melo
2013-12-26 14:51 ` David Ahern
2013-12-26 5:38 ` [PATCH 5/8] perf tools: Save message when pr_*() was called Namhyung Kim
` (3 subsequent siblings)
7 siblings, 2 replies; 18+ messages in thread
From: Namhyung Kim @ 2013-12-26 5:38 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 3638b0bd20dc..49c5c998009e 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..3838d49f82de
--- /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-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 idx)
+{
+ if (log->lines == log->nr_alloc)
+ if (grow_linemap(log) < 0)
+ return -1;
+
+ log->linemap[log->lines++] = idx;
+ 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] 18+ messages in thread* Re: [PATCH 4/8] perf tools: Introduce struct perf_log
2013-12-26 5:38 ` [PATCH 4/8] perf tools: Introduce struct perf_log Namhyung Kim
@ 2013-12-26 14:50 ` Arnaldo Carvalho de Melo
2014-01-03 8:23 ` Namhyung Kim
2013-12-26 14:51 ` David Ahern
1 sibling, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-12-26 14:50 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa, David Ahern
Em Thu, Dec 26, 2013 at 02:38:00PM +0900, Namhyung Kim escreveu:
> 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 3638b0bd20dc..49c5c998009e 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);
The convention in tools/perf/ has been to use class__method, i.e. in the
above case we would have:
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);
But I have some questions about the implementation, will we go on
allocating memory for each and every line?
Can't we just come out with a simple ui_file_browser class that would
then be usable for any file, including this one?
The ui_file_browser__seek() method would have to go on reading lines and
seeking newlines, with the ui_file_browser__seek(browser, 0, SEEK_SET)
would map directly to fseek(log_fp, 0, SEEK_SET), etc.
It should handle "live" files, like the one we're feeding log lines,
etc.
The way you implemented it will grow memory consumption without limits, no?
- Arnaldo
> #endif /* __PERF_DEBUG_H */
> diff --git a/tools/perf/util/log.c b/tools/perf/util/log.c
> new file mode 100644
> index 000000000000..3838d49f82de
> --- /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-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 idx)
> +{
> + if (log->lines == log->nr_alloc)
> + if (grow_linemap(log) < 0)
> + return -1;
> +
> + log->linemap[log->lines++] = idx;
> + 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 [flat|nested] 18+ messages in thread* Re: [PATCH 4/8] perf tools: Introduce struct perf_log
2013-12-26 14:50 ` Arnaldo Carvalho de Melo
@ 2014-01-03 8:23 ` Namhyung Kim
0 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2014-01-03 8:23 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa, David Ahern
Hi Arnaldo,
On Thu, 26 Dec 2013 11:50:51 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Dec 26, 2013 at 02:38:00PM +0900, Namhyung Kim escreveu:
>> 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.
[SNIP]
>> +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);
>
> The convention in tools/perf/ has been to use class__method, i.e. in the
> above case we would have:
>
> 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);
Okay. (I wasn't follow the convention since the functions do not pass
the perf_log as an argument, but I agree it's better to follow it.)
Will change.
>
>
> But I have some questions about the implementation, will we go on
> allocating memory for each and every line?
Yes, it needs an offset for each line in order to find starting point.
>
> Can't we just come out with a simple ui_file_browser class that would
> then be usable for any file, including this one?
Yes we can do it if need be. Do you think of another use case?
>
> The ui_file_browser__seek() method would have to go on reading lines and
> seeking newlines, with the ui_file_browser__seek(browser, 0, SEEK_SET)
> would map directly to fseek(log_fp, 0, SEEK_SET), etc.
It supposed to. But in this case, the ->seek() method is called in
ui_browser__run() which is not protected by ui__lock. So it's possible
that new log message alters file position if it's called after ->seek()
method was executed.
If it's guaranteed that there's no concurrent access to the file, we can
move fseek() to the ->seek() method IMHO.
>
> It should handle "live" files, like the one we're feeding log lines,
> etc.
>
> The way you implemented it will grow memory consumption without
> limits, no?
Right, it'll consume 8 bytes for each line of the file. What's the
reasonable limitation?
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/8] perf tools: Introduce struct perf_log
2013-12-26 5:38 ` [PATCH 4/8] perf tools: Introduce struct perf_log Namhyung Kim
2013-12-26 14:50 ` Arnaldo Carvalho de Melo
@ 2013-12-26 14:51 ` David Ahern
2014-01-03 8:49 ` Namhyung Kim
1 sibling, 1 reply; 18+ messages in thread
From: David Ahern @ 2013-12-26 14:51 UTC (permalink / raw)
To: Namhyung Kim, Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa
On 12/26/13, 12:38 AM, Namhyung Kim wrote:
> diff --git a/tools/perf/util/log.c b/tools/perf/util/log.c
> new file mode 100644
> index 000000000000..3838d49f82de
> --- /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)
Why return int if the rc is not checked? Failure here is not going to
stop the perf command right?
> +{
> + FILE *fp;
> + char name[] = "/tmp/perf-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;
Add 'unlink(name);' here to ensure the file is removed regardless of how
perf terminates.
> +
> + 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;
> +}
What's the point of linemap?
> +
> +static int __add_to_linemap(struct perf_log *log, off_t idx)
> +{
> + if (log->lines == log->nr_alloc)
> + if (grow_linemap(log) < 0)
> + return -1;
> +
> + log->linemap[log->lines++] = idx;
> + 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;
Don't assume every user of libperf calls perf_log_init() or that the
file was actually created. i.e., add 'if (fp == NULL) return;'
> + off_t offset = ftello(fp);
> +
> + add_to_linemap(&perf_log, msg, offset);
> +
> + fwrite(msg, 1, strlen(msg), fp);
And if write fails?
> +}
> +
> +void perf_log_addv(const char *fmt, va_list ap)
> +{
> + char buf[4096];
Add as an optimization add the fp != NULL check here too. Don't need to
do the vsnprintf only to drop it.
> +
> + vsnprintf(buf, sizeof(buf), fmt, ap);
> + perf_log_add(buf);
> +}
>
What limits the size of the file - other than the obvious out of space
in /tmp? Allow the file to grow without bounds in case a user wants the
messages seems dangerous.
What about using a circular buffer instead?
David
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 4/8] perf tools: Introduce struct perf_log
2013-12-26 14:51 ` David Ahern
@ 2014-01-03 8:49 ` Namhyung Kim
0 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2014-01-03 8:49 UTC (permalink / raw)
To: David Ahern
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Ingo Molnar, Namhyung Kim, LKML, Jiri Olsa
Hi David,
On Thu, 26 Dec 2013 09:51:25 -0500, David Ahern wrote:
> On 12/26/13, 12:38 AM, Namhyung Kim wrote:
>> diff --git a/tools/perf/util/log.c b/tools/perf/util/log.c
>> new file mode 100644
>> index 000000000000..3838d49f82de
>> --- /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)
>
> Why return int if the rc is not checked? Failure here is not going to
> stop the perf command right?
Right. I'll add a debug print if it's failed.
>
>> +{
>> + FILE *fp;
>> + char name[] = "/tmp/perf-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;
>
> Add 'unlink(name);' here to ensure the file is removed regardless of
> how perf terminates.
Ah, okay. I thought the mkstemp() unlinked the file after open()
returns. It seems this is what tmpfile(3) does. I'll switch to
tmpfile() then.
>
>> +
>> + 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;
>> +}
>
> What's the point of linemap?
To save an offset of each line. We need to keep it in order to move to
an arbitraty line in the browser.
>
>> +
>> +static int __add_to_linemap(struct perf_log *log, off_t idx)
>> +{
>> + if (log->lines == log->nr_alloc)
>> + if (grow_linemap(log) < 0)
>> + return -1;
>> +
>> + log->linemap[log->lines++] = idx;
>> + 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;
>
> Don't assume every user of libperf calls perf_log_init() or that the
> file was actually created. i.e., add 'if (fp == NULL) return;'
Okay.
>
>
>> + off_t offset = ftello(fp);
>> +
>> + add_to_linemap(&perf_log, msg, offset);
>> +
>> + fwrite(msg, 1, strlen(msg), fp);
>
> And if write fails?
Hmm.. it's a problem. We might go back to original linemap position for
a failure case. I'll save the offset and line number and restore them.
>
>> +}
>> +
>> +void perf_log_addv(const char *fmt, va_list ap)
>> +{
>> + char buf[4096];
>
> Add as an optimization add the fp != NULL check here too. Don't need
> to do the vsnprintf only to drop it.
Okay.
>
>> +
>> + vsnprintf(buf, sizeof(buf), fmt, ap);
>> + perf_log_add(buf);
>> +}
>>
>
> What limits the size of the file - other than the obvious out of space
> in /tmp? Allow the file to grow without bounds in case a user wants
> the messages seems dangerous.
Hmm. I don't have an idea what the reasonable size. It's a temp file
anyway and usually it contains not much data - well, 'perf top -vvv' is
different and might need some surgery.
>
> What about using a circular buffer instead?
Instead of a file? Yes, it's possible but I think it might not be a
good choice for the browser since it could alter the index during the
circulation so that the browser can confuse to update the entries.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 5/8] perf tools: Save message when pr_*() was called
2013-12-26 5:37 [PATCHSET 0/8] perf tools: A couple of TUI improvements (v3) Namhyung Kim
` (3 preceding siblings ...)
2013-12-26 5:38 ` [PATCH 4/8] perf tools: Introduce struct perf_log Namhyung Kim
@ 2013-12-26 5:38 ` Namhyung Kim
2013-12-26 5:38 ` [PATCH 6/8] perf ui/tui: Implement log window Namhyung Kim
` (2 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2013-12-26 5:38 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] 18+ messages in thread* [PATCH 6/8] perf ui/tui: Implement log window
2013-12-26 5:37 [PATCHSET 0/8] perf tools: A couple of TUI improvements (v3) Namhyung Kim
` (4 preceding siblings ...)
2013-12-26 5:38 ` [PATCH 5/8] perf tools: Save message when pr_*() was called Namhyung Kim
@ 2013-12-26 5:38 ` Namhyung Kim
2013-12-26 5:38 ` [PATCH 7/8] perf ui/tui: Filter messages in " Namhyung Kim
2013-12-26 5:38 ` [PATCH 8/8] perf ui/tui: Remember last log line for filtering Namhyung Kim
7 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2013-12-26 5:38 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.
Protect updating log by ui__lock to prevent the linemap reallocation
during log window refresh (it led to a segfault) and add
linemap_changed member to perf_log in order to detect the change.
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 | 5 ++
tools/perf/ui/browsers/log.c | 132 +++++++++++++++++++++++++++++++++++++++++
tools/perf/util/debug.h | 1 +
tools/perf/util/log.c | 8 ++-
6 files changed, 147 insertions(+), 1 deletion(-)
create mode 100644 tools/perf/ui/browsers/log.c
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 49c5c998009e..7cce69b9911a 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/header.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 118cca29dd26..1f676c8ec4f3 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -61,6 +61,7 @@ int ui_browser__input_window(const char *title, const char *text, char *input,
const char *exit_msg, int delay_sec);
struct perf_session_env;
int tui__header_window(struct perf_session_env *env);
+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 0d9dd99507ee..828b0113a213 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1418,6 +1418,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
/* help messages are sorted by lexical order of the hotkey */
const char report_help[] = HIST_BROWSER_HELP_COMMON
"i Show header information\n"
+ "l Show log messages\n"
"P Print histograms to perf.hist.N\n"
"r Run available scripts\n"
"s Switch to another data file in PWD\n"
@@ -1425,6 +1426,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
"V Verbose (DSO names in callchains, etc)\n"
"/ Filter symbol by name";
const char top_help[] = HIST_BROWSER_HELP_COMMON
+ "l Show log messages\n"
"P Print histograms to perf.hist.N\n"
"t Zoom into current Thread\n"
"V Verbose (DSO names in callchains, etc)\n"
@@ -1519,6 +1521,9 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
if (env->arch)
tui__header_window(env);
continue;
+ case 'l':
+ tui__log_window();
+ continue;
case K_F1:
case 'h':
case '?':
diff --git a/tools/perf/ui/browsers/log.c b/tools/perf/ui/browsers/log.c
new file mode 100644
index 000000000000..c64aeb8f9aac
--- /dev/null
+++ b/tools/perf/ui/browsers/log.c
@@ -0,0 +1,132 @@
+#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"
+
+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;
+ fpos_t pos;
+
+ fgetpos(perf_log.fp, &pos);
+
+ if (perf_log.linemap_changed) {
+ /* update log window with new linemap */
+ browser->entries = perf_log.linemap;
+ browser->nr_entries = perf_log.lines;
+ perf_log.linemap_changed = false;
+ }
+
+ while (idx < browser->nr_entries) {
+ ui_browser__gotorc(browser, row, 0);
+ browser->write(browser, &idx, row);
+ if (++row == browser->height)
+ break;
+
+ ++idx;
+ }
+
+ fsetpos(perf_log.fp, &pos);
+ return row;
+}
+
+static int log_menu__run(struct ui_browser *menu)
+{
+ int key;
+ unsigned long offset;
+ const char help[] =
+ "h/?/F1 Show this window\n"
+ "UP/DOWN/PGUP\n"
+ "PGDN/SPACE\n"
+ "LEFT/RIGHT Navigate\n"
+ "q/ESC/CTRL+C Exit browser";
+
+ 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_F1:
+ case 'h':
+ case '?':
+ ui_browser__help_window(menu, help);
+ 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);
+}
diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
index ea160abc2ae0..bc3affed682e 100644
--- a/tools/perf/util/debug.h
+++ b/tools/perf/util/debug.h
@@ -25,6 +25,7 @@ struct perf_log {
u32 lines;
u32 nr_alloc;
bool seen_newline;
+ bool linemap_changed;
};
extern struct perf_log perf_log;
diff --git a/tools/perf/util/log.c b/tools/perf/util/log.c
index 3838d49f82de..aee71da25a24 100644
--- a/tools/perf/util/log.c
+++ b/tools/perf/util/log.c
@@ -89,11 +89,17 @@ static void add_to_linemap(struct perf_log *log, const char *msg, off_t base)
void perf_log_add(const char *msg)
{
FILE *fp = perf_log.fp;
- off_t offset = ftello(fp);
+ off_t offset;
+
+ pthread_mutex_lock(&ui__lock);
+ offset = ftello(fp);
add_to_linemap(&perf_log, msg, offset);
fwrite(msg, 1, strlen(msg), fp);
+
+ perf_log.linemap_changed = true;
+ pthread_mutex_unlock(&ui__lock);
}
void perf_log_addv(const char *fmt, va_list ap)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 7/8] perf ui/tui: Filter messages in log window
2013-12-26 5:37 [PATCHSET 0/8] perf tools: A couple of TUI improvements (v3) Namhyung Kim
` (5 preceding siblings ...)
2013-12-26 5:38 ` [PATCH 6/8] perf ui/tui: Implement log window Namhyung Kim
@ 2013-12-26 5:38 ` Namhyung Kim
2013-12-26 5:38 ` [PATCH 8/8] perf ui/tui: Remember last log line for filtering Namhyung Kim
7 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2013-12-26 5:38 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 | 117 +++++++++++++++++++++++++++++++++++++------
1 file changed, 103 insertions(+), 14 deletions(-)
diff --git a/tools/perf/ui/browsers/log.c b/tools/perf/ui/browsers/log.c
index c64aeb8f9aac..62343e0578bb 100644
--- a/tools/perf/ui/browsers/log.c
+++ b/tools/perf/ui/browsers/log.c
@@ -10,6 +10,14 @@
#include "ui/libslang.h"
#include "ui/keysyms.h"
+struct log_browser_data {
+ unsigned long offset;
+ u32 alloc;
+ char filter[64];
+};
+
+static void __log_menu__filter(struct ui_browser *menu, char *filter);
+
static void ui_browser__file_seek(struct ui_browser *browser __maybe_unused,
off_t offset __maybe_unused,
int whence __maybe_unused)
@@ -23,10 +31,11 @@ static void ui_browser__file_write(struct ui_browser *browser,
char buf[1024];
char empty[] = " ";
FILE *fp = perf_log.fp;
+ struct log_browser_data *lbd = browser->priv;
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;
+ unsigned long offset = lbd->offset;
fseek(fp, linemap[idx], SEEK_SET);
if (fgets(buf, sizeof(buf), fp) == NULL)
@@ -45,14 +54,14 @@ static unsigned int ui_browser__file_refresh(struct ui_browser *browser)
{
unsigned int row = 0;
unsigned int idx = browser->top_idx;
+ struct log_browser_data *lbd = browser->priv;
fpos_t pos;
fgetpos(perf_log.fp, &pos);
if (perf_log.linemap_changed) {
/* update log window with new linemap */
- browser->entries = perf_log.linemap;
- browser->nr_entries = perf_log.lines;
+ __log_menu__filter(browser, lbd->filter);
perf_log.linemap_changed = false;
}
@@ -69,16 +78,81 @@ 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];
+ struct log_browser_data *lbd = menu->priv;
+ 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(*map));
+ if (map == NULL)
+ goto error;
+
+ linemap = map;
+ alloc += 128;
+ }
+
+ linemap[lines++] = perf_log.linemap[i];
+ }
+
+out:
+ if (lbd->alloc) {
+ BUG_ON(menu->entries == perf_log.linemap);
+ free(menu->entries);
+ }
+ lbd->alloc = alloc;
+
+ menu->entries = linemap;
+ ui_browser__update_nr_entries(menu, lines);
+ return;
+
+error:
+ free(linemap);
+}
+
+static void log_menu__filter(struct ui_browser *menu, char *filter)
+{
+ fpos_t pos;
+
+ pthread_mutex_lock(&ui__lock);
+ fgetpos(perf_log.fp, &pos);
+ __log_menu__filter(menu, filter);
+ fsetpos(perf_log.fp, &pos);
+ perf_log.linemap_changed = false;
+ pthread_mutex_unlock(&ui__lock);
+}
+
static int log_menu__run(struct ui_browser *menu)
{
int key;
- unsigned long offset;
+ struct log_browser_data *lbd = menu->priv;
const char help[] =
"h/?/F1 Show this window\n"
"UP/DOWN/PGUP\n"
"PGDN/SPACE\n"
"LEFT/RIGHT Navigate\n"
- "q/ESC/CTRL+C Exit browser";
+ "q/ESC/CTRL+C Exit browser\n\n"
+ "/ Filter log message";
if (ui_browser__show(menu, "Log messages", "Press 'q' to exit") < 0)
return -1;
@@ -88,21 +162,25 @@ static int log_menu__run(struct ui_browser *menu)
switch (key) {
case K_RIGHT:
- offset = (unsigned long)menu->priv;
- offset += 10;
- menu->priv = (void *)offset;
+ lbd->offset += 10;
continue;
case K_LEFT:
- offset = (unsigned long)menu->priv;
- if (offset >= 10)
- offset -= 10;
- menu->priv = (void *)offset;
+ if (lbd->offset >= 10)
+ lbd->offset -= 10;
continue;
case K_F1:
case 'h':
case '?':
ui_browser__help_window(menu, help);
continue;
+ case '/':
+ if (ui_browser__input_window("Symbol to filter",
+ "Please enter the name of symbol you want to see",
+ lbd->filter, "ENTER: OK, ESC: Cancel",
+ 0) == K_ENTER) {
+ log_menu__filter(menu, lbd->filter);
+ }
+ continue;
case K_ESC:
case 'q':
case CTRL('c'):
@@ -121,12 +199,23 @@ static int log_menu__run(struct ui_browser *menu)
int tui__log_window(void)
{
+ struct log_browser_data lbd = {
+ .filter = "",
+ };
struct ui_browser log_menu = {
.refresh = ui_browser__file_refresh,
.seek = ui_browser__file_seek,
.write = ui_browser__file_write,
+ .entries = perf_log.linemap,
.nr_entries = perf_log.lines,
+ .priv = &lbd,
};
+ int key;
+
+ key = log_menu__run(&log_menu);
- return log_menu__run(&log_menu);
+ if (lbd.alloc)
+ free(log_menu.entries);
+
+ return key;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 8/8] perf ui/tui: Remember last log line for filtering
2013-12-26 5:37 [PATCHSET 0/8] perf tools: A couple of TUI improvements (v3) Namhyung Kim
` (6 preceding siblings ...)
2013-12-26 5:38 ` [PATCH 7/8] perf ui/tui: Filter messages in " Namhyung Kim
@ 2013-12-26 5:38 ` Namhyung Kim
7 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2013-12-26 5:38 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>
When updating log window with filter string, no need to search all
logs everytime. Just save the last offset and update from there.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/log.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/tools/perf/ui/browsers/log.c b/tools/perf/ui/browsers/log.c
index 62343e0578bb..6491c353eef7 100644
--- a/tools/perf/ui/browsers/log.c
+++ b/tools/perf/ui/browsers/log.c
@@ -13,10 +13,11 @@
struct log_browser_data {
unsigned long offset;
u32 alloc;
+ u32 last_log_line;
char filter[64];
};
-static void __log_menu__filter(struct ui_browser *menu, char *filter);
+static void __log_menu__filter(struct ui_browser *menu, char *filter, bool update);
static void ui_browser__file_seek(struct ui_browser *browser __maybe_unused,
off_t offset __maybe_unused,
@@ -61,7 +62,7 @@ static unsigned int ui_browser__file_refresh(struct ui_browser *browser)
if (perf_log.linemap_changed) {
/* update log window with new linemap */
- __log_menu__filter(browser, lbd->filter);
+ __log_menu__filter(browser, lbd->filter, true);
perf_log.linemap_changed = false;
}
@@ -78,14 +79,15 @@ static unsigned int ui_browser__file_refresh(struct ui_browser *browser)
return row;
}
-static void __log_menu__filter(struct ui_browser *menu, char *filter)
+static void __log_menu__filter(struct ui_browser *menu, char *filter, bool update)
{
char buf[1024];
struct log_browser_data *lbd = menu->priv;
off_t *linemap = NULL;
u32 lines = 0;
u32 alloc = 0;
- u32 i;
+ u32 last = 0;
+ u32 i = 0;
if (*filter == '\0') {
linemap = perf_log.linemap;
@@ -93,7 +95,14 @@ static void __log_menu__filter(struct ui_browser *menu, char *filter)
goto out;
}
- for (i = 0; i < perf_log.lines; i++) {
+ if (update) {
+ linemap = menu->entries;
+ lines = menu->nr_entries;
+ alloc = lbd->alloc;
+ last = lbd->last_log_line;
+ }
+
+ for (i = last; 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;
@@ -120,7 +129,12 @@ out:
BUG_ON(menu->entries == perf_log.linemap);
free(menu->entries);
}
+
lbd->alloc = alloc;
+ lbd->last_log_line = i;
+
+ if (!update)
+ ui_browser__reset_index(menu);
menu->entries = linemap;
ui_browser__update_nr_entries(menu, lines);
@@ -136,7 +150,7 @@ static void log_menu__filter(struct ui_browser *menu, char *filter)
pthread_mutex_lock(&ui__lock);
fgetpos(perf_log.fp, &pos);
- __log_menu__filter(menu, filter);
+ __log_menu__filter(menu, filter, false);
fsetpos(perf_log.fp, &pos);
perf_log.linemap_changed = false;
pthread_mutex_unlock(&ui__lock);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread