LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] perf ui/tui: Show fatal error message only if exists
@ 2015-01-20  6:40 Namhyung Kim
  2015-01-20 12:49 ` Jiri Olsa
  2015-01-28 15:10 ` [tip:perf/core] " tip-bot for Namhyung Kim
  0 siblings, 2 replies; 5+ messages in thread
From: Namhyung Kim @ 2015-01-20  6:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML

When perf exits with some error it shows the error message with
ui__error() or ui__warning() and then calls ui__exit() during
exit_browser().

On TUI, it then shows a window titled "Fatal Error" to inform user a
last message which might be related with this condition.  However it
sometimes contains no message and just annoyes users.

The usual case for this is running perf top as normal user.  (And
/proc/sys/kernel/perf_event_paranoid being 1).

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

diff --git a/tools/perf/ui/tui/helpline.c b/tools/perf/ui/tui/helpline.c
index 1c8b9afd5d6e..88f5143a5981 100644
--- a/tools/perf/ui/tui/helpline.c
+++ b/tools/perf/ui/tui/helpline.c
@@ -9,6 +9,7 @@
 #include "../libslang.h"
 
 char ui_helpline__last_msg[1024];
+bool tui_helpline__set;
 
 static void tui_helpline__pop(void)
 {
@@ -35,6 +36,8 @@ static int tui_helpline__show(const char *format, va_list ap)
 			sizeof(ui_helpline__last_msg) - backlog, format, ap);
 	backlog += ret;
 
+	tui_helpline__set = true;
+
 	if (ui_helpline__last_msg[backlog - 1] == '\n') {
 		ui_helpline__puts(ui_helpline__last_msg);
 		SLsmg_refresh();
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index 3c38f25b1695..b77e1d771363 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -17,6 +17,7 @@
 static volatile int ui__need_resize;
 
 extern struct perf_error_ops perf_tui_eops;
+extern bool tui_helpline__set;
 
 extern void hist_browser__init_hpp(void);
 
@@ -159,7 +160,7 @@ int ui__init(void)
 
 void ui__exit(bool wait_for_ok)
 {
-	if (wait_for_ok)
+	if (wait_for_ok && tui_helpline__set)
 		ui__question_window("Fatal Error",
 				    ui_helpline__last_msg,
 				    "Press any key...", 0);
-- 
2.2.2


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

* Re: [PATCH] perf ui/tui: Show fatal error message only if exists
  2015-01-20  6:40 [PATCH] perf ui/tui: Show fatal error message only if exists Namhyung Kim
@ 2015-01-20 12:49 ` Jiri Olsa
  2015-01-22 15:08   ` Namhyung Kim
  2015-01-28 15:10 ` [tip:perf/core] " tip-bot for Namhyung Kim
  1 sibling, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2015-01-20 12:49 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, LKML

On Tue, Jan 20, 2015 at 03:40:50PM +0900, Namhyung Kim wrote:
> When perf exits with some error it shows the error message with
> ui__error() or ui__warning() and then calls ui__exit() during
> exit_browser().
> 
> On TUI, it then shows a window titled "Fatal Error" to inform user a
> last message which might be related with this condition.  However it
> sometimes contains no message and just annoyes users.
> 
> The usual case for this is running perf top as normal user.  (And
> /proc/sys/kernel/perf_event_paranoid being 1).

cool, I couldn't recreate the 'no message' box but I recall seeing it

> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/ui/tui/helpline.c | 3 +++
>  tools/perf/ui/tui/setup.c    | 3 ++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/ui/tui/helpline.c b/tools/perf/ui/tui/helpline.c
> index 1c8b9afd5d6e..88f5143a5981 100644
> --- a/tools/perf/ui/tui/helpline.c
> +++ b/tools/perf/ui/tui/helpline.c
> @@ -9,6 +9,7 @@
>  #include "../libslang.h"
>  
>  char ui_helpline__last_msg[1024];
> +bool tui_helpline__set;
>  
>  static void tui_helpline__pop(void)
>  {
> @@ -35,6 +36,8 @@ static int tui_helpline__show(const char *format, va_list ap)
>  			sizeof(ui_helpline__last_msg) - backlog, format, ap);
>  	backlog += ret;
>  
> +	tui_helpline__set = true;
> +
>  	if (ui_helpline__last_msg[backlog - 1] == '\n') {
>  		ui_helpline__puts(ui_helpline__last_msg);
>  		SLsmg_refresh();
> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> index 3c38f25b1695..b77e1d771363 100644
> --- a/tools/perf/ui/tui/setup.c
> +++ b/tools/perf/ui/tui/setup.c
> @@ -17,6 +17,7 @@
>  static volatile int ui__need_resize;
>  
>  extern struct perf_error_ops perf_tui_eops;
> +extern bool tui_helpline__set;
>  
>  extern void hist_browser__init_hpp(void);
>  
> @@ -159,7 +160,7 @@ int ui__init(void)
>  
>  void ui__exit(bool wait_for_ok)
>  {
> -	if (wait_for_ok)
> +	if (wait_for_ok && tui_helpline__set)
>  		ui__question_window("Fatal Error",
>  				    ui_helpline__last_msg,
>  				    "Press any key...", 0);

looks like we display ui_helpline__last_msg also in ui/browsers/annotate.c

do we want to have some common function that unsets tui_helpline__set
so we're sure we dont get the message twice?

jirka

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

* Re: [PATCH] perf ui/tui: Show fatal error message only if exists
  2015-01-20 12:49 ` Jiri Olsa
@ 2015-01-22 15:08   ` Namhyung Kim
  2015-01-22 15:23     ` Jiri Olsa
  0 siblings, 1 reply; 5+ messages in thread
From: Namhyung Kim @ 2015-01-22 15:08 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, LKML

On Tue, Jan 20, 2015 at 01:49:26PM +0100, Jiri Olsa wrote:
> On Tue, Jan 20, 2015 at 03:40:50PM +0900, Namhyung Kim wrote:
> > When perf exits with some error it shows the error message with
> > ui__error() or ui__warning() and then calls ui__exit() during
> > exit_browser().
> > 
> > On TUI, it then shows a window titled "Fatal Error" to inform user a
> > last message which might be related with this condition.  However it
> > sometimes contains no message and just annoyes users.
> > 
> > The usual case for this is running perf top as normal user.  (And
> > /proc/sys/kernel/perf_event_paranoid being 1).
> 
> cool, I couldn't recreate the 'no message' box but I recall seeing it
> 
> > 
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/ui/tui/helpline.c | 3 +++
> >  tools/perf/ui/tui/setup.c    | 3 ++-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/ui/tui/helpline.c b/tools/perf/ui/tui/helpline.c
> > index 1c8b9afd5d6e..88f5143a5981 100644
> > --- a/tools/perf/ui/tui/helpline.c
> > +++ b/tools/perf/ui/tui/helpline.c
> > @@ -9,6 +9,7 @@
> >  #include "../libslang.h"
> >  
> >  char ui_helpline__last_msg[1024];
> > +bool tui_helpline__set;
> >  
> >  static void tui_helpline__pop(void)
> >  {
> > @@ -35,6 +36,8 @@ static int tui_helpline__show(const char *format, va_list ap)
> >  			sizeof(ui_helpline__last_msg) - backlog, format, ap);
> >  	backlog += ret;
> >  
> > +	tui_helpline__set = true;
> > +
> >  	if (ui_helpline__last_msg[backlog - 1] == '\n') {
> >  		ui_helpline__puts(ui_helpline__last_msg);
> >  		SLsmg_refresh();
> > diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> > index 3c38f25b1695..b77e1d771363 100644
> > --- a/tools/perf/ui/tui/setup.c
> > +++ b/tools/perf/ui/tui/setup.c
> > @@ -17,6 +17,7 @@
> >  static volatile int ui__need_resize;
> >  
> >  extern struct perf_error_ops perf_tui_eops;
> > +extern bool tui_helpline__set;
> >  
> >  extern void hist_browser__init_hpp(void);
> >  
> > @@ -159,7 +160,7 @@ int ui__init(void)
> >  
> >  void ui__exit(bool wait_for_ok)
> >  {
> > -	if (wait_for_ok)
> > +	if (wait_for_ok && tui_helpline__set)
> >  		ui__question_window("Fatal Error",
> >  				    ui_helpline__last_msg,
> >  				    "Press any key...", 0);
> 
> looks like we display ui_helpline__last_msg also in ui/browsers/annotate.c

Right, I also saw it.

> 
> do we want to have some common function that unsets tui_helpline__set
> so we're sure we dont get the message twice?

I doubt that it's really needed..  The annotation code does not
propagate an error so the wait_for_ok arg in ui__exit() will be always
false.  In addition, I think it's better to use ui__error() or
ui__warning() directly at the point when something bad happened and we
need to inform user.

Thanks,
Namhyung

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

* Re: [PATCH] perf ui/tui: Show fatal error message only if exists
  2015-01-22 15:08   ` Namhyung Kim
@ 2015-01-22 15:23     ` Jiri Olsa
  0 siblings, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2015-01-22 15:23 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, LKML

On Fri, Jan 23, 2015 at 12:08:34AM +0900, Namhyung Kim wrote:

SNIP

> > >  				    "Press any key...", 0);
> > 
> > looks like we display ui_helpline__last_msg also in ui/browsers/annotate.c
> 
> Right, I also saw it.
> 
> > 
> > do we want to have some common function that unsets tui_helpline__set
> > so we're sure we dont get the message twice?
> 
> I doubt that it's really needed..  The annotation code does not
> propagate an error so the wait_for_ok arg in ui__exit() will be always
> false.  In addition, I think it's better to use ui__error() or
> ui__warning() directly at the point when something bad happened and we
> need to inform user.

ok..

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka


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

* [tip:perf/core] perf ui/tui: Show fatal error message only if exists
  2015-01-20  6:40 [PATCH] perf ui/tui: Show fatal error message only if exists Namhyung Kim
  2015-01-20 12:49 ` Jiri Olsa
@ 2015-01-28 15:10 ` tip-bot for Namhyung Kim
  1 sibling, 0 replies; 5+ messages in thread
From: tip-bot for Namhyung Kim @ 2015-01-28 15:10 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, a.p.zijlstra, acme, tglx, namhyung, mingo, linux-kernel, jolsa

Commit-ID:  4397bd2f90459d550deca7f6ba32c12e382d8b57
Gitweb:     http://git.kernel.org/tip/4397bd2f90459d550deca7f6ba32c12e382d8b57
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Tue, 20 Jan 2015 15:40:50 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 22 Jan 2015 17:05:10 -0300

perf ui/tui: Show fatal error message only if exists

When perf exits with some error it shows the error message with
ui__error() or ui__warning() and then calls ui__exit() during
exit_browser().

On TUI, it then shows a window titled "Fatal Error" to inform user a
last message which might be related with this condition.  However it
sometimes contains no message and just annoyes users.

The usual case for this is running perf top as normal user.  (And
/proc/sys/kernel/perf_event_paranoid being 1).

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1421736050-5283-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/tui/helpline.c | 3 +++
 tools/perf/ui/tui/setup.c    | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/tui/helpline.c b/tools/perf/ui/tui/helpline.c
index 1c8b9af..88f5143 100644
--- a/tools/perf/ui/tui/helpline.c
+++ b/tools/perf/ui/tui/helpline.c
@@ -9,6 +9,7 @@
 #include "../libslang.h"
 
 char ui_helpline__last_msg[1024];
+bool tui_helpline__set;
 
 static void tui_helpline__pop(void)
 {
@@ -35,6 +36,8 @@ static int tui_helpline__show(const char *format, va_list ap)
 			sizeof(ui_helpline__last_msg) - backlog, format, ap);
 	backlog += ret;
 
+	tui_helpline__set = true;
+
 	if (ui_helpline__last_msg[backlog - 1] == '\n') {
 		ui_helpline__puts(ui_helpline__last_msg);
 		SLsmg_refresh();
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index 3c38f25..b77e1d7 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -17,6 +17,7 @@
 static volatile int ui__need_resize;
 
 extern struct perf_error_ops perf_tui_eops;
+extern bool tui_helpline__set;
 
 extern void hist_browser__init_hpp(void);
 
@@ -159,7 +160,7 @@ out:
 
 void ui__exit(bool wait_for_ok)
 {
-	if (wait_for_ok)
+	if (wait_for_ok && tui_helpline__set)
 		ui__question_window("Fatal Error",
 				    ui_helpline__last_msg,
 				    "Press any key...", 0);

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

end of thread, other threads:[~2015-01-28 21:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-20  6:40 [PATCH] perf ui/tui: Show fatal error message only if exists Namhyung Kim
2015-01-20 12:49 ` Jiri Olsa
2015-01-22 15:08   ` Namhyung Kim
2015-01-22 15:23     ` Jiri Olsa
2015-01-28 15:10 ` [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).