* [PATCH] perf top: Move toggling event logic into hists browser
@ 2015-06-21 3:41 Namhyung Kim
2015-06-22 14:19 ` Arnaldo Carvalho de Melo
2015-06-25 7:55 ` [tip:perf/core] " tip-bot for Namhyung Kim
0 siblings, 2 replies; 3+ messages in thread
From: Namhyung Kim @ 2015-06-21 3:41 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern
Current 'f' key action to enable/disable events won't work if there're
more than one event since perf_evsel_menu__run() doesn't return the key.
So move it to the hists browser loop so that it can be processed as like
other key action, and it's more natural to handle it there IMHO.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-top.c | 24 +++---------------------
tools/perf/ui/browsers/hists.c | 19 +++++++++++++++++--
2 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 619a8696fda7..ecf319728f25 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -586,27 +586,9 @@ static void *display_thread_tui(void *arg)
hists->uid_filter_str = top->record_opts.target.uid_str;
}
- while (true) {
- int key = perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
- top->min_percent,
- &top->session->header.env);
-
- if (key != 'f')
- break;
-
- perf_evlist__toggle_enable(top->evlist);
- /*
- * No need to refresh, resort/decay histogram entries
- * if we are not collecting samples:
- */
- if (top->evlist->enabled) {
- hbt.refresh = top->delay_secs;
- help = "Press 'f' to disable the events or 'h' to see other hotkeys";
- } else {
- help = "Press 'f' again to re-enable the events";
- hbt.refresh = 0;
- }
- }
+ perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
+ top->min_percent,
+ &top->session->header.env);
done = 1;
return NULL;
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index c42adb600091..7629bef2fd79 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1902,8 +1902,23 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
case CTRL('c'):
goto out_free_stack;
case 'f':
- if (!is_report_browser(hbt))
- goto out_free_stack;
+ if (!is_report_browser(hbt)) {
+ struct perf_top *top = hbt->arg;
+
+ perf_evlist__toggle_enable(top->evlist);
+ /*
+ * No need to refresh, resort/decay histogram
+ * entries if we are not collecting samples:
+ */
+ if (top->evlist->enabled) {
+ helpline = "Press 'f' to disable the events or 'h' to see other hotkeys";
+ hbt->refresh = delay_secs;
+ } else {
+ helpline = "Press 'f' again to re-enable the events";
+ hbt->refresh = 0;
+ }
+ continue;
+ }
/* Fall thru */
default:
helpline = "Press '?' for help on key bindings";
--
2.4.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] perf top: Move toggling event logic into hists browser
2015-06-21 3:41 [PATCH] perf top: Move toggling event logic into hists browser Namhyung Kim
@ 2015-06-22 14:19 ` Arnaldo Carvalho de Melo
2015-06-25 7:55 ` [tip:perf/core] " tip-bot for Namhyung Kim
1 sibling, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-22 14:19 UTC (permalink / raw)
To: Namhyung Kim; +Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern
Em Sun, Jun 21, 2015 at 12:41:16PM +0900, Namhyung Kim escreveu:
> Current 'f' key action to enable/disable events won't work if there're
> more than one event since perf_evsel_menu__run() doesn't return the key.
> So move it to the hists browser loop so that it can be processed as like
> other key action, and it's more natural to handle it there IMHO.
Yeah, at first I did it to try to remove references to whatever tool is
using the hists browser, i.e. it should not know it is being called from
something called 'top' or 'report' or something altogether different :-\
But yeah, your patch fixes the issue when we have multiple events, I am
applying, but what sounds better is to make those is_foo_tool_browser()
layering violations be something more abstract like hb->dynamic_mode,
avoid accesses to evlist in perf_evsel__ prefixed functions and have the
menu routine handle the 'f' hotkey.
Thanks!
- Arnaldo
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-top.c | 24 +++---------------------
> tools/perf/ui/browsers/hists.c | 19 +++++++++++++++++--
> 2 files changed, 20 insertions(+), 23 deletions(-)
>
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 619a8696fda7..ecf319728f25 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -586,27 +586,9 @@ static void *display_thread_tui(void *arg)
> hists->uid_filter_str = top->record_opts.target.uid_str;
> }
>
> - while (true) {
> - int key = perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
> - top->min_percent,
> - &top->session->header.env);
> -
> - if (key != 'f')
> - break;
> -
> - perf_evlist__toggle_enable(top->evlist);
> - /*
> - * No need to refresh, resort/decay histogram entries
> - * if we are not collecting samples:
> - */
> - if (top->evlist->enabled) {
> - hbt.refresh = top->delay_secs;
> - help = "Press 'f' to disable the events or 'h' to see other hotkeys";
> - } else {
> - help = "Press 'f' again to re-enable the events";
> - hbt.refresh = 0;
> - }
> - }
> + perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
> + top->min_percent,
> + &top->session->header.env);
>
> done = 1;
> return NULL;
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index c42adb600091..7629bef2fd79 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -1902,8 +1902,23 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
> case CTRL('c'):
> goto out_free_stack;
> case 'f':
> - if (!is_report_browser(hbt))
> - goto out_free_stack;
> + if (!is_report_browser(hbt)) {
> + struct perf_top *top = hbt->arg;
> +
> + perf_evlist__toggle_enable(top->evlist);
> + /*
> + * No need to refresh, resort/decay histogram
> + * entries if we are not collecting samples:
> + */
> + if (top->evlist->enabled) {
> + helpline = "Press 'f' to disable the events or 'h' to see other hotkeys";
> + hbt->refresh = delay_secs;
> + } else {
> + helpline = "Press 'f' again to re-enable the events";
> + hbt->refresh = 0;
> + }
> + continue;
> + }
> /* Fall thru */
> default:
> helpline = "Press '?' for help on key bindings";
> --
> 2.4.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 3+ messages in thread
* [tip:perf/core] perf top: Move toggling event logic into hists browser
2015-06-21 3:41 [PATCH] perf top: Move toggling event logic into hists browser Namhyung Kim
2015-06-22 14:19 ` Arnaldo Carvalho de Melo
@ 2015-06-25 7:55 ` tip-bot for Namhyung Kim
1 sibling, 0 replies; 3+ messages in thread
From: tip-bot for Namhyung Kim @ 2015-06-25 7:55 UTC (permalink / raw)
To: linux-tip-commits
Cc: tglx, a.p.zijlstra, dsahern, hpa, acme, jolsa, namhyung,
linux-kernel, mingo
Commit-ID: 13d1e536b14ec2d404319a25e681a3287ca084ad
Gitweb: http://git.kernel.org/tip/13d1e536b14ec2d404319a25e681a3287ca084ad
Author: Namhyung Kim <namhyung@kernel.org>
AuthorDate: Sun, 21 Jun 2015 12:41:16 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 22 Jun 2015 11:21:11 -0300
perf top: Move toggling event logic into hists browser
Current 'f' key action to enable/disable events won't work if there're
more than one event since perf_evsel_menu__run() doesn't return the key.
So move it to the hists browser loop so that it can be processed as like
other key action, and it's more natural to handle it there IMHO.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1434858076-6533-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-top.c | 24 +++---------------------
tools/perf/ui/browsers/hists.c | 19 +++++++++++++++++--
2 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 619a869..ecf3197 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -586,27 +586,9 @@ static void *display_thread_tui(void *arg)
hists->uid_filter_str = top->record_opts.target.uid_str;
}
- while (true) {
- int key = perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
- top->min_percent,
- &top->session->header.env);
-
- if (key != 'f')
- break;
-
- perf_evlist__toggle_enable(top->evlist);
- /*
- * No need to refresh, resort/decay histogram entries
- * if we are not collecting samples:
- */
- if (top->evlist->enabled) {
- hbt.refresh = top->delay_secs;
- help = "Press 'f' to disable the events or 'h' to see other hotkeys";
- } else {
- help = "Press 'f' again to re-enable the events";
- hbt.refresh = 0;
- }
- }
+ perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
+ top->min_percent,
+ &top->session->header.env);
done = 1;
return NULL;
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index c42adb6..7629bef 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1902,8 +1902,23 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
case CTRL('c'):
goto out_free_stack;
case 'f':
- if (!is_report_browser(hbt))
- goto out_free_stack;
+ if (!is_report_browser(hbt)) {
+ struct perf_top *top = hbt->arg;
+
+ perf_evlist__toggle_enable(top->evlist);
+ /*
+ * No need to refresh, resort/decay histogram
+ * entries if we are not collecting samples:
+ */
+ if (top->evlist->enabled) {
+ helpline = "Press 'f' to disable the events or 'h' to see other hotkeys";
+ hbt->refresh = delay_secs;
+ } else {
+ helpline = "Press 'f' again to re-enable the events";
+ hbt->refresh = 0;
+ }
+ continue;
+ }
/* Fall thru */
default:
helpline = "Press '?' for help on key bindings";
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-06-25 7:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-21 3:41 [PATCH] perf top: Move toggling event logic into hists browser Namhyung Kim
2015-06-22 14:19 ` Arnaldo Carvalho de Melo
2015-06-25 7:55 ` [tip:perf/core] " tip-bot for Namhyung Kim
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).