All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf tools: Make Ctrl-C stop processing on TUI
@ 2015-05-28 23:40 Namhyung Kim
  2015-05-29  6:45 ` Ingo Molnar
  0 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2015-05-28 23:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern

It was inconvenient that perf cannot be quit with SIGINT during
processing samples on TUI especially for large data files.

This was because the first argument of SLang_init_tty(), abort_char,
being 0.  The manual says it's the ascii value of the control
character that will be used to generate the interrupt signal [1].
Passing -1 means to use the default value (Ctrl-C).

[1] http://jedsoft.org/slang/doc/html/cslang-6.html#ss6.1

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/tui/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index b77e1d771363..60d1f29b4b50 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -129,7 +129,7 @@ int ui__init(void)
 	err = SLsmg_init_smg();
 	if (err < 0)
 		goto out;
-	err = SLang_init_tty(0, 0, 0);
+	err = SLang_init_tty(-1, 0, 0);
 	if (err < 0)
 		goto out;
 
-- 
2.4.1


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

* Re: [PATCH] perf tools: Make Ctrl-C stop processing on TUI
  2015-05-28 23:40 [PATCH] perf tools: Make Ctrl-C stop processing on TUI Namhyung Kim
@ 2015-05-29  6:45 ` Ingo Molnar
  2015-05-29 12:53   ` [PATCH v2] " Namhyung Kim
  0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2015-05-29  6:45 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Jiri Olsa, LKML, David Ahern


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

> It was inconvenient that perf cannot be quit with SIGINT during
> processing samples on TUI especially for large data files.
> 
> This was because the first argument of SLang_init_tty(), abort_char,
> being 0.  The manual says it's the ascii value of the control
> character that will be used to generate the interrupt signal [1].
> Passing -1 means to use the default value (Ctrl-C).
> 
> [1] http://jedsoft.org/slang/doc/html/cslang-6.html#ss6.1
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/ui/tui/setup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> index b77e1d771363..60d1f29b4b50 100644
> --- a/tools/perf/ui/tui/setup.c
> +++ b/tools/perf/ui/tui/setup.c
> @@ -129,7 +129,7 @@ int ui__init(void)
>  	err = SLsmg_init_smg();
>  	if (err < 0)
>  		goto out;
> -	err = SLang_init_tty(0, 0, 0);
> +	err = SLang_init_tty(-1, 0, 0);
>  	if (err < 0)
>  		goto out;

Will this change the current Ctrl-C behavior in other places in any way?

Right now Ctrl-C behavior is pretty sensible: it essentially means instant abort 
of whatever was done, i.e. stepping back a level in 'perf top', exit the app if 
it's done at the highest level. I hope that's preserved! :-)

Thanks,

	Ingo

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

* [PATCH v2] perf tools: Make Ctrl-C stop processing on TUI
  2015-05-29  6:45 ` Ingo Molnar
@ 2015-05-29 12:53   ` Namhyung Kim
  2015-05-29 13:24     ` Arnaldo Carvalho de Melo
  2015-05-29 18:39     ` [tip:perf/core] " tip-bot for Namhyung Kim
  0 siblings, 2 replies; 6+ messages in thread
From: Namhyung Kim @ 2015-05-29 12:53 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern

It was inconvenient that perf cannot be quit with SIGINT during
processing samples on TUI especially for large data files.

This was because the first argument of SLang_init_tty(), abort_char,
being 0.  The manual says it's the ascii value of the control
character that will be used to generate the interrupt signal [1].
Passing -1 means to use the default value (Ctrl-C).

However, after processing samples, Ctrl-C was used to in other cases as
well - like stepping back from annotate.  So recover the original
behavior after processing.

[1] http://jedsoft.org/slang/doc/html/cslang-6.html#ss6.1

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/annotate.c | 4 ++++
 tools/perf/ui/browsers/hists.c    | 4 ++++
 tools/perf/ui/tui/setup.c         | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index e5250eb2dd57..acb0e23b138e 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -838,6 +838,10 @@ int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
 int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel,
 			     struct hist_browser_timer *hbt)
 {
+	/* reset abort key so that it can get Ctrl-C as a key */
+	SLang_reset_tty();
+	SLang_init_tty(0, 0, 0);
+
 	return map_symbol__tui_annotate(&he->ms, evsel, hbt);
 }
 
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index f981cb8f0158..e64893f2fd7f 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1741,6 +1741,10 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 	if (browser == NULL)
 		return -1;
 
+	/* reset abort key so that it can get Ctrl-C as a key */
+	SLang_reset_tty();
+	SLang_init_tty(0, 0, 0);
+
 	if (min_pcnt) {
 		browser->min_pcnt = min_pcnt;
 		hist_browser__update_nr_entries(browser);
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index b77e1d771363..60d1f29b4b50 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -129,7 +129,7 @@ int ui__init(void)
 	err = SLsmg_init_smg();
 	if (err < 0)
 		goto out;
-	err = SLang_init_tty(0, 0, 0);
+	err = SLang_init_tty(-1, 0, 0);
 	if (err < 0)
 		goto out;
 
-- 
2.4.0


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

* Re: [PATCH v2] perf tools: Make Ctrl-C stop processing on TUI
  2015-05-29 12:53   ` [PATCH v2] " Namhyung Kim
@ 2015-05-29 13:24     ` Arnaldo Carvalho de Melo
  2015-05-29 15:49       ` Arnaldo Carvalho de Melo
  2015-05-29 18:39     ` [tip:perf/core] " tip-bot for Namhyung Kim
  1 sibling, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-29 13:24 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern

Em Fri, May 29, 2015 at 09:53:44PM +0900, Namhyung Kim escreveu:
> It was inconvenient that perf cannot be quit with SIGINT during
> processing samples on TUI especially for large data files.
> 
> This was because the first argument of SLang_init_tty(), abort_char,
> being 0.  The manual says it's the ascii value of the control
> character that will be used to generate the interrupt signal [1].
> Passing -1 means to use the default value (Ctrl-C).
> 
> However, after processing samples, Ctrl-C was used to in other cases as
> well - like stepping back from annotate.  So recover the original
> behavior after processing.

Right, but the way this was implemented it is error prone, I think...

I.e. better would be to enclose whatever section of code we want to be
able to interrupt with control+C with something like:

  ui__control_c_terminates(true);

  process events, etc.

  ui__control_c_terminates(false);

Or some more suitable name, couldn't think right now of any that would
reuse something well established...

The way you implemented it, as soon as we implement a new browser, we
will have to recall this detail to have Control+C working as it always
worked on tools/perf, i.e. not terminating it.

- Arnaldo
 
> [1] http://jedsoft.org/slang/doc/html/cslang-6.html#ss6.1
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/ui/browsers/annotate.c | 4 ++++
>  tools/perf/ui/browsers/hists.c    | 4 ++++
>  tools/perf/ui/tui/setup.c         | 2 +-
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index e5250eb2dd57..acb0e23b138e 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -838,6 +838,10 @@ int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
>  int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel,
>  			     struct hist_browser_timer *hbt)
>  {
> +	/* reset abort key so that it can get Ctrl-C as a key */
> +	SLang_reset_tty();
> +	SLang_init_tty(0, 0, 0);
> +
>  	return map_symbol__tui_annotate(&he->ms, evsel, hbt);
>  }
>  
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index f981cb8f0158..e64893f2fd7f 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -1741,6 +1741,10 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
>  	if (browser == NULL)
>  		return -1;
>  
> +	/* reset abort key so that it can get Ctrl-C as a key */
> +	SLang_reset_tty();
> +	SLang_init_tty(0, 0, 0);
> +
>  	if (min_pcnt) {
>  		browser->min_pcnt = min_pcnt;
>  		hist_browser__update_nr_entries(browser);
> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> index b77e1d771363..60d1f29b4b50 100644
> --- a/tools/perf/ui/tui/setup.c
> +++ b/tools/perf/ui/tui/setup.c
> @@ -129,7 +129,7 @@ int ui__init(void)
>  	err = SLsmg_init_smg();
>  	if (err < 0)
>  		goto out;
> -	err = SLang_init_tty(0, 0, 0);
> +	err = SLang_init_tty(-1, 0, 0);
>  	if (err < 0)
>  		goto out;
>  
> -- 
> 2.4.0

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

* Re: [PATCH v2] perf tools: Make Ctrl-C stop processing on TUI
  2015-05-29 13:24     ` Arnaldo Carvalho de Melo
@ 2015-05-29 15:49       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-29 15:49 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern

Em Fri, May 29, 2015 at 10:24:37AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, May 29, 2015 at 09:53:44PM +0900, Namhyung Kim escreveu:
> > It was inconvenient that perf cannot be quit with SIGINT during
> > processing samples on TUI especially for large data files.
> > 
> > This was because the first argument of SLang_init_tty(), abort_char,
> > being 0.  The manual says it's the ascii value of the control
> > character that will be used to generate the interrupt signal [1].
> > Passing -1 means to use the default value (Ctrl-C).
> > 
> > However, after processing samples, Ctrl-C was used to in other cases as
> > well - like stepping back from annotate.  So recover the original
> > behavior after processing.
> 
> Right, but the way this was implemented it is error prone, I think...
> 
> I.e. better would be to enclose whatever section of code we want to be
> able to interrupt with control+C with something like:
> 
>   ui__control_c_terminates(true);
> 
>   process events, etc.
> 
>   ui__control_c_terminates(false);

Applied anyway, this can be done later...

- Arnaldo
 
> Or some more suitable name, couldn't think right now of any that would
> reuse something well established...
> 
> The way you implemented it, as soon as we implement a new browser, we
> will have to recall this detail to have Control+C working as it always
> worked on tools/perf, i.e. not terminating it.
> 
> - Arnaldo
>  
> > [1] http://jedsoft.org/slang/doc/html/cslang-6.html#ss6.1
> > 
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/ui/browsers/annotate.c | 4 ++++
> >  tools/perf/ui/browsers/hists.c    | 4 ++++
> >  tools/perf/ui/tui/setup.c         | 2 +-
> >  3 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index e5250eb2dd57..acb0e23b138e 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -838,6 +838,10 @@ int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
> >  int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel,
> >  			     struct hist_browser_timer *hbt)
> >  {
> > +	/* reset abort key so that it can get Ctrl-C as a key */
> > +	SLang_reset_tty();
> > +	SLang_init_tty(0, 0, 0);
> > +
> >  	return map_symbol__tui_annotate(&he->ms, evsel, hbt);
> >  }
> >  
> > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > index f981cb8f0158..e64893f2fd7f 100644
> > --- a/tools/perf/ui/browsers/hists.c
> > +++ b/tools/perf/ui/browsers/hists.c
> > @@ -1741,6 +1741,10 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
> >  	if (browser == NULL)
> >  		return -1;
> >  
> > +	/* reset abort key so that it can get Ctrl-C as a key */
> > +	SLang_reset_tty();
> > +	SLang_init_tty(0, 0, 0);
> > +
> >  	if (min_pcnt) {
> >  		browser->min_pcnt = min_pcnt;
> >  		hist_browser__update_nr_entries(browser);
> > diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> > index b77e1d771363..60d1f29b4b50 100644
> > --- a/tools/perf/ui/tui/setup.c
> > +++ b/tools/perf/ui/tui/setup.c
> > @@ -129,7 +129,7 @@ int ui__init(void)
> >  	err = SLsmg_init_smg();
> >  	if (err < 0)
> >  		goto out;
> > -	err = SLang_init_tty(0, 0, 0);
> > +	err = SLang_init_tty(-1, 0, 0);
> >  	if (err < 0)
> >  		goto out;
> >  
> > -- 
> > 2.4.0

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

* [tip:perf/core] perf tools: Make Ctrl-C stop processing on TUI
  2015-05-29 12:53   ` [PATCH v2] " Namhyung Kim
  2015-05-29 13:24     ` Arnaldo Carvalho de Melo
@ 2015-05-29 18:39     ` tip-bot for Namhyung Kim
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Namhyung Kim @ 2015-05-29 18:39 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, dsahern, mingo, acme, tglx, a.p.zijlstra, jolsa,
	linux-kernel, namhyung

Commit-ID:  ed426915900db3c58c410b8b38f6ff0e46bf6c96
Gitweb:     http://git.kernel.org/tip/ed426915900db3c58c410b8b38f6ff0e46bf6c96
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 29 May 2015 21:53:44 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 29 May 2015 12:49:00 -0300

perf tools: Make Ctrl-C stop processing on TUI

It was inconvenient that perf cannot be quit with SIGINT during
processing samples on TUI especially for large data files.

This was because the first argument of SLang_init_tty(), abort_char,
being 0.  The manual says it's the ascii value of the control character
that will be used to generate the interrupt signal [1].  Passing -1
means to use the default value (Ctrl-C).

However, after processing samples, Ctrl-C was used to in other cases as
well - like stepping back from annotate.  So recover the original
behavior after processing.

[1] http://jedsoft.org/slang/doc/html/cslang-6.html#ss6.1

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
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/1432904024-13170-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/annotate.c | 4 ++++
 tools/perf/ui/browsers/hists.c    | 4 ++++
 tools/perf/ui/tui/setup.c         | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index e5250eb..acb0e23 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -838,6 +838,10 @@ int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
 int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel,
 			     struct hist_browser_timer *hbt)
 {
+	/* reset abort key so that it can get Ctrl-C as a key */
+	SLang_reset_tty();
+	SLang_init_tty(0, 0, 0);
+
 	return map_symbol__tui_annotate(&he->ms, evsel, hbt);
 }
 
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index f981cb8..e64893f 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1741,6 +1741,10 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 	if (browser == NULL)
 		return -1;
 
+	/* reset abort key so that it can get Ctrl-C as a key */
+	SLang_reset_tty();
+	SLang_init_tty(0, 0, 0);
+
 	if (min_pcnt) {
 		browser->min_pcnt = min_pcnt;
 		hist_browser__update_nr_entries(browser);
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index b77e1d7..60d1f29 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -129,7 +129,7 @@ int ui__init(void)
 	err = SLsmg_init_smg();
 	if (err < 0)
 		goto out;
-	err = SLang_init_tty(0, 0, 0);
+	err = SLang_init_tty(-1, 0, 0);
 	if (err < 0)
 		goto out;
 

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

end of thread, other threads:[~2015-05-29 18:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-28 23:40 [PATCH] perf tools: Make Ctrl-C stop processing on TUI Namhyung Kim
2015-05-29  6:45 ` Ingo Molnar
2015-05-29 12:53   ` [PATCH v2] " Namhyung Kim
2015-05-29 13:24     ` Arnaldo Carvalho de Melo
2015-05-29 15:49       ` Arnaldo Carvalho de Melo
2015-05-29 18:39     ` [tip:perf/core] " tip-bot for Namhyung Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.