All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf tui: don't ignore job control
@ 2022-12-26 19:35 Ahelenia Ziemiańska
  2022-12-26 19:50 ` [PATCH 2/1] perf tools: don't include signature in version strings Ahelenia Ziemiańska
  2022-12-27 20:24 ` [PATCH] perf tui: don't ignore job control Namhyung Kim
  0 siblings, 2 replies; 11+ messages in thread
From: Ahelenia Ziemiańska @ 2022-12-26 19:35 UTC (permalink / raw)
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	yaowenbin, linux-perf-users, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3831 bytes --]

In its infinite wisdom, by default, SLang sets susp undef,
and this can only be un-done by calling SLtty_set_suspend_state(true).
After every SLang_init_tty().

Additionally, no provisions are made for maintaining the teletype
attributes across suspend/continue (outside of curses emulation mode(?!),
which provides full support, naturally), so we need to save and restore
the flags ourselves. We need to also re-draw the screen, and raising
SIGWINCH, shockingly, Just Works.

The correct solution would be to Not Use SLang, but as a stop-gap,
this makes TUI perf report usable.

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 tools/perf/ui/browsers/annotate.c |  1 +
 tools/perf/ui/browsers/hists.c    |  2 ++
 tools/perf/ui/browsers/scripts.c  |  1 +
 tools/perf/ui/tui/setup.c         | 19 +++++++++++++++++++
 4 files changed, 23 insertions(+)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index c03fa76c02ff..6a4ffbf66c7f 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -942,6 +942,7 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
 	/* reset abort key so that it can get Ctrl-C as a key */
 	SLang_reset_tty();
 	SLang_init_tty(0, 0, 0);
+	SLtty_set_suspend_state(true);
 
 	return map_symbol__tui_annotate(&he->ms, evsel, hbt, opts);
 }
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index b72ee6822222..2479e6d42e7c 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -3010,6 +3010,7 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
 	/* reset abort key so that it can get Ctrl-C as a key */
 	SLang_reset_tty();
 	SLang_init_tty(0, 0, 0);
+	SLtty_set_suspend_state(true);
 
 	if (min_pcnt)
 		browser->min_pcnt = min_pcnt;
@@ -3682,6 +3683,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
 	/* reset abort key so that it can get Ctrl-C as a key */
 	SLang_reset_tty();
 	SLang_init_tty(0, 0, 0);
+	SLtty_set_suspend_state(true);
 
 	memset(&action, 0, sizeof(action));
 
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index 47d2c7a8cbe1..50d45054ed6c 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -166,6 +166,7 @@ void run_script(char *cmd)
 	printf("\033[c\033[H\033[J");
 	fflush(stdout);
 	SLang_init_tty(0, 0, 0);
+	SLtty_set_suspend_state(true);
 	SLsmg_refresh();
 }
 
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index a3b8c397c24d..4211a161458a 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -2,6 +2,7 @@
 #include <signal.h>
 #include <stdbool.h>
 #include <stdlib.h>
+#include <termios.h>
 #include <unistd.h>
 #include <linux/kernel.h>
 #ifdef HAVE_BACKTRACE_SUPPORT
@@ -122,6 +123,21 @@ static void ui__signal(int sig)
 	exit(0);
 }
 
+static void ui__sigcont(int sig)
+{
+	static struct termios tty;
+
+	if (sig == SIGTSTP) {
+		while (tcgetattr(SLang_TT_Read_FD, &tty) == -1 && errno == EINTR)
+			;
+		raise(SIGSTOP);
+	} else {
+		while (tcsetattr(SLang_TT_Read_FD, TCSADRAIN, &tty) == -1 && errno == EINTR)
+			;
+		raise(SIGWINCH);
+	}
+}
+
 int ui__init(void)
 {
 	int err;
@@ -136,6 +152,7 @@ int ui__init(void)
 	err = SLang_init_tty(-1, 0, 0);
 	if (err < 0)
 		goto out;
+	SLtty_set_suspend_state(true);
 
 	err = SLkp_init();
 	if (err < 0) {
@@ -150,6 +167,8 @@ int ui__init(void)
 	signal(SIGINT, ui__signal);
 	signal(SIGQUIT, ui__signal);
 	signal(SIGTERM, ui__signal);
+	signal(SIGTSTP, ui__sigcont);
+	signal(SIGCONT, ui__sigcont);
 
 	perf_error__register(&perf_tui_eops);
 
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* [PATCH 2/1] perf tools: don't include signature in version strings
  2022-12-26 19:35 [PATCH] perf tui: don't ignore job control Ahelenia Ziemiańska
@ 2022-12-26 19:50 ` Ahelenia Ziemiańska
  2022-12-27 20:29   ` Namhyung Kim
  2022-12-27 20:24 ` [PATCH] perf tui: don't ignore job control Namhyung Kim
  1 sibling, 1 reply; 11+ messages in thread
From: Ahelenia Ziemiańska @ 2022-12-26 19:50 UTC (permalink / raw)
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	linux-perf-users, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1527 bytes --]

This explodes the build if HEAD is signed, since the generated version
is gpg: Signature made Mon 26 Dec 2022 20:34:48 CET, then a few more
lines, then the SHA.

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 tools/perf/Documentation/Makefile | 2 +-
 tools/perf/util/PERF-VERSION-GEN  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/Makefile b/tools/perf/Documentation/Makefile
index 6e7b88917ca0..ba5d942e4c6a 100644
--- a/tools/perf/Documentation/Makefile
+++ b/tools/perf/Documentation/Makefile
@@ -267,7 +267,7 @@ $(OUTPUT)%.xml : %.txt
 	$(ASCIIDOC) -b docbook -d manpage \
 		$(ASCIIDOC_EXTRA) -aperf_version=$(PERF_VERSION) \
 		-aperf_date=$(shell git log -1 --pretty="format:%cd" \
-				--date=short $<) \
+				--date=short --no-show-signature $<) \
 		-o $@+ $< && \
 	mv $@+ $@
 
diff --git a/tools/perf/util/PERF-VERSION-GEN b/tools/perf/util/PERF-VERSION-GEN
index 3cc42821d9b3..d7dc7c28508c 100755
--- a/tools/perf/util/PERF-VERSION-GEN
+++ b/tools/perf/util/PERF-VERSION-GEN
@@ -19,7 +19,7 @@ TAG=
 if test -d ../../.git -o -f ../../.git
 then
 	TAG=$(MAKEFLAGS= make -sC ../.. kernelversion)
-	CID=$(git log -1 --abbrev=12 --pretty=format:"%h" 2>/dev/null) && CID="-g$CID"
+	CID=$(git log -1 --abbrev=12 --pretty=format:"%h" --no-show-signature 2>/dev/null) && CID="-g$CID"
 elif test -f ../../PERF-VERSION-FILE
 then
 	TAG=$(cut -d' ' -f3 ../../PERF-VERSION-FILE | sed -e 's/\"//g')
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH] perf tui: don't ignore job control
  2022-12-26 19:35 [PATCH] perf tui: don't ignore job control Ahelenia Ziemiańska
  2022-12-26 19:50 ` [PATCH 2/1] perf tools: don't include signature in version strings Ahelenia Ziemiańska
@ 2022-12-27 20:24 ` Namhyung Kim
  2022-12-27 20:57   ` [PATCH v2 1/2] " Ahelenia Ziemiańska
  2022-12-27 20:58   ` [PATCH v2 2/2] perf tools: don't include signature in version strings Ahelenia Ziemiańska
  1 sibling, 2 replies; 11+ messages in thread
From: Namhyung Kim @ 2022-12-27 20:24 UTC (permalink / raw)
  To: Ahelenia Ziemiańska
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, yaowenbin,
	linux-perf-users, linux-kernel

Hello,

On Mon, Dec 26, 2022 at 11:35 AM Ahelenia Ziemiańska
<nabijaczleweli@nabijaczleweli.xyz> wrote:
>
> In its infinite wisdom, by default, SLang sets susp undef,
> and this can only be un-done by calling SLtty_set_suspend_state(true).
> After every SLang_init_tty().
>
> Additionally, no provisions are made for maintaining the teletype
> attributes across suspend/continue (outside of curses emulation mode(?!),
> which provides full support, naturally), so we need to save and restore
> the flags ourselves. We need to also re-draw the screen, and raising
> SIGWINCH, shockingly, Just Works.
>
> The correct solution would be to Not Use SLang, but as a stop-gap,
> this makes TUI perf report usable.
>
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

Cool, this makes Ctrl-Z and then 'fg' work.  But it comes with the
color for a selected line depending on the timing.  Maybe we can
reset the color before going to the background?

Thanks,
Namhyung


> ---
>  tools/perf/ui/browsers/annotate.c |  1 +
>  tools/perf/ui/browsers/hists.c    |  2 ++
>  tools/perf/ui/browsers/scripts.c  |  1 +
>  tools/perf/ui/tui/setup.c         | 19 +++++++++++++++++++
>  4 files changed, 23 insertions(+)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index c03fa76c02ff..6a4ffbf66c7f 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -942,6 +942,7 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
>         /* reset abort key so that it can get Ctrl-C as a key */
>         SLang_reset_tty();
>         SLang_init_tty(0, 0, 0);
> +       SLtty_set_suspend_state(true);
>
>         return map_symbol__tui_annotate(&he->ms, evsel, hbt, opts);
>  }
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index b72ee6822222..2479e6d42e7c 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -3010,6 +3010,7 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>         /* reset abort key so that it can get Ctrl-C as a key */
>         SLang_reset_tty();
>         SLang_init_tty(0, 0, 0);
> +       SLtty_set_suspend_state(true);
>
>         if (min_pcnt)
>                 browser->min_pcnt = min_pcnt;
> @@ -3682,6 +3683,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
>         /* reset abort key so that it can get Ctrl-C as a key */
>         SLang_reset_tty();
>         SLang_init_tty(0, 0, 0);
> +       SLtty_set_suspend_state(true);
>
>         memset(&action, 0, sizeof(action));
>
> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> index 47d2c7a8cbe1..50d45054ed6c 100644
> --- a/tools/perf/ui/browsers/scripts.c
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -166,6 +166,7 @@ void run_script(char *cmd)
>         printf("\033[c\033[H\033[J");
>         fflush(stdout);
>         SLang_init_tty(0, 0, 0);
> +       SLtty_set_suspend_state(true);
>         SLsmg_refresh();
>  }
>
> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> index a3b8c397c24d..4211a161458a 100644
> --- a/tools/perf/ui/tui/setup.c
> +++ b/tools/perf/ui/tui/setup.c
> @@ -2,6 +2,7 @@
>  #include <signal.h>
>  #include <stdbool.h>
>  #include <stdlib.h>
> +#include <termios.h>
>  #include <unistd.h>
>  #include <linux/kernel.h>
>  #ifdef HAVE_BACKTRACE_SUPPORT
> @@ -122,6 +123,21 @@ static void ui__signal(int sig)
>         exit(0);
>  }
>
> +static void ui__sigcont(int sig)
> +{
> +       static struct termios tty;
> +
> +       if (sig == SIGTSTP) {
> +               while (tcgetattr(SLang_TT_Read_FD, &tty) == -1 && errno == EINTR)
> +                       ;
> +               raise(SIGSTOP);
> +       } else {
> +               while (tcsetattr(SLang_TT_Read_FD, TCSADRAIN, &tty) == -1 && errno == EINTR)
> +                       ;
> +               raise(SIGWINCH);
> +       }
> +}
> +
>  int ui__init(void)
>  {
>         int err;
> @@ -136,6 +152,7 @@ int ui__init(void)
>         err = SLang_init_tty(-1, 0, 0);
>         if (err < 0)
>                 goto out;
> +       SLtty_set_suspend_state(true);
>
>         err = SLkp_init();
>         if (err < 0) {
> @@ -150,6 +167,8 @@ int ui__init(void)
>         signal(SIGINT, ui__signal);
>         signal(SIGQUIT, ui__signal);
>         signal(SIGTERM, ui__signal);
> +       signal(SIGTSTP, ui__sigcont);
> +       signal(SIGCONT, ui__sigcont);
>
>         perf_error__register(&perf_tui_eops);
>
> --
> 2.30.2

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

* Re: [PATCH 2/1] perf tools: don't include signature in version strings
  2022-12-26 19:50 ` [PATCH 2/1] perf tools: don't include signature in version strings Ahelenia Ziemiańska
@ 2022-12-27 20:29   ` Namhyung Kim
  0 siblings, 0 replies; 11+ messages in thread
From: Namhyung Kim @ 2022-12-27 20:29 UTC (permalink / raw)
  To: Ahelenia Ziemiańska
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, linux-perf-users,
	linux-kernel

On Mon, Dec 26, 2022 at 11:50 AM Ahelenia Ziemiańska
<nabijaczleweli@nabijaczleweli.xyz> wrote:
>
> This explodes the build if HEAD is signed, since the generated version
> is gpg: Signature made Mon 26 Dec 2022 20:34:48 CET, then a few more
> lines, then the SHA.
>
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  tools/perf/Documentation/Makefile | 2 +-
>  tools/perf/util/PERF-VERSION-GEN  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Documentation/Makefile b/tools/perf/Documentation/Makefile
> index 6e7b88917ca0..ba5d942e4c6a 100644
> --- a/tools/perf/Documentation/Makefile
> +++ b/tools/perf/Documentation/Makefile
> @@ -267,7 +267,7 @@ $(OUTPUT)%.xml : %.txt
>         $(ASCIIDOC) -b docbook -d manpage \
>                 $(ASCIIDOC_EXTRA) -aperf_version=$(PERF_VERSION) \
>                 -aperf_date=$(shell git log -1 --pretty="format:%cd" \
> -                               --date=short $<) \
> +                               --date=short --no-show-signature $<) \
>                 -o $@+ $< && \
>         mv $@+ $@
>
> diff --git a/tools/perf/util/PERF-VERSION-GEN b/tools/perf/util/PERF-VERSION-GEN
> index 3cc42821d9b3..d7dc7c28508c 100755
> --- a/tools/perf/util/PERF-VERSION-GEN
> +++ b/tools/perf/util/PERF-VERSION-GEN
> @@ -19,7 +19,7 @@ TAG=
>  if test -d ../../.git -o -f ../../.git
>  then
>         TAG=$(MAKEFLAGS= make -sC ../.. kernelversion)
> -       CID=$(git log -1 --abbrev=12 --pretty=format:"%h" 2>/dev/null) && CID="-g$CID"
> +       CID=$(git log -1 --abbrev=12 --pretty=format:"%h" --no-show-signature 2>/dev/null) && CID="-g$CID"
>  elif test -f ../../PERF-VERSION-FILE
>  then
>         TAG=$(cut -d' ' -f3 ../../PERF-VERSION-FILE | sed -e 's/\"//g')
> --
> 2.30.2

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

* [PATCH v2 1/2] perf tui: don't ignore job control
  2022-12-27 20:24 ` [PATCH] perf tui: don't ignore job control Namhyung Kim
@ 2022-12-27 20:57   ` Ahelenia Ziemiańska
  2022-12-27 21:21     ` Namhyung Kim
                       ` (2 more replies)
  2022-12-27 20:58   ` [PATCH v2 2/2] perf tools: don't include signature in version strings Ahelenia Ziemiańska
  1 sibling, 3 replies; 11+ messages in thread
From: Ahelenia Ziemiańska @ 2022-12-27 20:57 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	yaowenbin, linux-perf-users, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4397 bytes --]

In its infinite wisdom, by default, SLang sets susp undef,
and this can only be un-done by calling SLtty_set_suspend_state(true).
After every SLang_init_tty().

Additionally, no provisions are made for maintaining the teletype
attributes across suspend/continue (outside of curses emulation mode(?!),
which provides full support, naturally), so we need to save and restore
the flags ourselves, as well as reset the text colours when going under.
We need to also re-draw the screen, and raising SIGWINCH, shockingly,
Just Works.

The correct solution would be to Not Use SLang, but as a stop-gap,
this makes TUI perf report usable.

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
Yeah, somehow I didn't hit that in my tests, but it was relatively easy
to trigger when I tried. A full partial write retry loop is an overkill
here, I think, and few other places in perf actually try to resubmit
partial writes, so.

 tools/perf/ui/browsers/annotate.c |  1 +
 tools/perf/ui/browsers/hists.c    |  2 ++
 tools/perf/ui/browsers/scripts.c  |  1 +
 tools/perf/ui/tui/setup.c         | 22 ++++++++++++++++++++++
 4 files changed, 26 insertions(+)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index c03fa76c02ff..6a4ffbf66c7f 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -942,6 +942,7 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
 	/* reset abort key so that it can get Ctrl-C as a key */
 	SLang_reset_tty();
 	SLang_init_tty(0, 0, 0);
+	SLtty_set_suspend_state(true);
 
 	return map_symbol__tui_annotate(&he->ms, evsel, hbt, opts);
 }
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index b72ee6822222..2479e6d42e7c 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -3010,6 +3010,7 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
 	/* reset abort key so that it can get Ctrl-C as a key */
 	SLang_reset_tty();
 	SLang_init_tty(0, 0, 0);
+	SLtty_set_suspend_state(true);
 
 	if (min_pcnt)
 		browser->min_pcnt = min_pcnt;
@@ -3682,6 +3683,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
 	/* reset abort key so that it can get Ctrl-C as a key */
 	SLang_reset_tty();
 	SLang_init_tty(0, 0, 0);
+	SLtty_set_suspend_state(true);
 
 	memset(&action, 0, sizeof(action));
 
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index 47d2c7a8cbe1..50d45054ed6c 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -166,6 +166,7 @@ void run_script(char *cmd)
 	printf("\033[c\033[H\033[J");
 	fflush(stdout);
 	SLang_init_tty(0, 0, 0);
+	SLtty_set_suspend_state(true);
 	SLsmg_refresh();
 }
 
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index a3b8c397c24d..09524ba04975 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -2,12 +2,14 @@
 #include <signal.h>
 #include <stdbool.h>
 #include <stdlib.h>
+#include <termios.h>
 #include <unistd.h>
 #include <linux/kernel.h>
 #ifdef HAVE_BACKTRACE_SUPPORT
 #include <execinfo.h>
 #endif
 
+#include "../../util/color.h"
 #include "../../util/debug.h"
 #include "../../perf.h"
 #include "../browser.h"
@@ -122,6 +124,23 @@ static void ui__signal(int sig)
 	exit(0);
 }
 
+static void ui__sigcont(int sig)
+{
+	static struct termios tty;
+
+	if (sig == SIGTSTP) {
+		while (tcgetattr(SLang_TT_Read_FD, &tty) == -1 && errno == EINTR)
+			;
+		while (write(SLang_TT_Read_FD, PERF_COLOR_RESET, sizeof(PERF_COLOR_RESET) - 1) == -1 && errno == EINTR)
+			;
+		raise(SIGSTOP);
+	} else {
+		while (tcsetattr(SLang_TT_Read_FD, TCSADRAIN, &tty) == -1 && errno == EINTR)
+			;
+		raise(SIGWINCH);
+	}
+}
+
 int ui__init(void)
 {
 	int err;
@@ -136,6 +155,7 @@ int ui__init(void)
 	err = SLang_init_tty(-1, 0, 0);
 	if (err < 0)
 		goto out;
+	SLtty_set_suspend_state(true);
 
 	err = SLkp_init();
 	if (err < 0) {
@@ -150,6 +170,8 @@ int ui__init(void)
 	signal(SIGINT, ui__signal);
 	signal(SIGQUIT, ui__signal);
 	signal(SIGTERM, ui__signal);
+	signal(SIGTSTP, ui__sigcont);
+	signal(SIGCONT, ui__sigcont);
 
 	perf_error__register(&perf_tui_eops);
 
-- 
2.30.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* [PATCH v2 2/2] perf tools: don't include signature in version strings
  2022-12-27 20:24 ` [PATCH] perf tui: don't ignore job control Namhyung Kim
  2022-12-27 20:57   ` [PATCH v2 1/2] " Ahelenia Ziemiańska
@ 2022-12-27 20:58   ` Ahelenia Ziemiańska
  2023-01-02 15:34     ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 11+ messages in thread
From: Ahelenia Ziemiańska @ 2022-12-27 20:58 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	linux-perf-users, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1573 bytes --]

This explodes the build if HEAD is signed, since the generated version
is gpg: Signature made Mon 26 Dec 2022 20:34:48 CET, then a few more
lines, then the SHA.

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/Makefile | 2 +-
 tools/perf/util/PERF-VERSION-GEN  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/Makefile b/tools/perf/Documentation/Makefile
index 6e7b88917ca0..ba5d942e4c6a 100644
--- a/tools/perf/Documentation/Makefile
+++ b/tools/perf/Documentation/Makefile
@@ -267,7 +267,7 @@ $(OUTPUT)%.xml : %.txt
 	$(ASCIIDOC) -b docbook -d manpage \
 		$(ASCIIDOC_EXTRA) -aperf_version=$(PERF_VERSION) \
 		-aperf_date=$(shell git log -1 --pretty="format:%cd" \
-				--date=short $<) \
+				--date=short --no-show-signature $<) \
 		-o $@+ $< && \
 	mv $@+ $@
 
diff --git a/tools/perf/util/PERF-VERSION-GEN b/tools/perf/util/PERF-VERSION-GEN
index 3cc42821d9b3..d7dc7c28508c 100755
--- a/tools/perf/util/PERF-VERSION-GEN
+++ b/tools/perf/util/PERF-VERSION-GEN
@@ -19,7 +19,7 @@ TAG=
 if test -d ../../.git -o -f ../../.git
 then
 	TAG=$(MAKEFLAGS= make -sC ../.. kernelversion)
-	CID=$(git log -1 --abbrev=12 --pretty=format:"%h" 2>/dev/null) && CID="-g$CID"
+	CID=$(git log -1 --abbrev=12 --pretty=format:"%h" --no-show-signature 2>/dev/null) && CID="-g$CID"
 elif test -f ../../PERF-VERSION-FILE
 then
 	TAG=$(cut -d' ' -f3 ../../PERF-VERSION-FILE | sed -e 's/\"//g')
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 1/2] perf tui: don't ignore job control
  2022-12-27 20:57   ` [PATCH v2 1/2] " Ahelenia Ziemiańska
@ 2022-12-27 21:21     ` Namhyung Kim
  2023-01-02 15:35     ` Arnaldo Carvalho de Melo
  2024-01-04 21:23     ` Arnaldo Carvalho de Melo
  2 siblings, 0 replies; 11+ messages in thread
From: Namhyung Kim @ 2022-12-27 21:21 UTC (permalink / raw)
  To: Ahelenia Ziemiańska
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, linux-perf-users,
	linux-kernel

On Tue, Dec 27, 2022 at 12:57 PM Ahelenia Ziemiańska
<nabijaczleweli@nabijaczleweli.xyz> wrote:
>
> In its infinite wisdom, by default, SLang sets susp undef,
> and this can only be un-done by calling SLtty_set_suspend_state(true).
> After every SLang_init_tty().
>
> Additionally, no provisions are made for maintaining the teletype
> attributes across suspend/continue (outside of curses emulation mode(?!),
> which provides full support, naturally), so we need to save and restore
> the flags ourselves, as well as reset the text colours when going under.
> We need to also re-draw the screen, and raising SIGWINCH, shockingly,
> Just Works.
>
> The correct solution would be to Not Use SLang, but as a stop-gap,
> this makes TUI perf report usable.
>
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

Tested-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
> Yeah, somehow I didn't hit that in my tests, but it was relatively easy
> to trigger when I tried. A full partial write retry loop is an overkill
> here, I think, and few other places in perf actually try to resubmit
> partial writes, so.
>
>  tools/perf/ui/browsers/annotate.c |  1 +
>  tools/perf/ui/browsers/hists.c    |  2 ++
>  tools/perf/ui/browsers/scripts.c  |  1 +
>  tools/perf/ui/tui/setup.c         | 22 ++++++++++++++++++++++
>  4 files changed, 26 insertions(+)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index c03fa76c02ff..6a4ffbf66c7f 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -942,6 +942,7 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
>         /* reset abort key so that it can get Ctrl-C as a key */
>         SLang_reset_tty();
>         SLang_init_tty(0, 0, 0);
> +       SLtty_set_suspend_state(true);
>
>         return map_symbol__tui_annotate(&he->ms, evsel, hbt, opts);
>  }
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index b72ee6822222..2479e6d42e7c 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -3010,6 +3010,7 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>         /* reset abort key so that it can get Ctrl-C as a key */
>         SLang_reset_tty();
>         SLang_init_tty(0, 0, 0);
> +       SLtty_set_suspend_state(true);
>
>         if (min_pcnt)
>                 browser->min_pcnt = min_pcnt;
> @@ -3682,6 +3683,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
>         /* reset abort key so that it can get Ctrl-C as a key */
>         SLang_reset_tty();
>         SLang_init_tty(0, 0, 0);
> +       SLtty_set_suspend_state(true);
>
>         memset(&action, 0, sizeof(action));
>
> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> index 47d2c7a8cbe1..50d45054ed6c 100644
> --- a/tools/perf/ui/browsers/scripts.c
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -166,6 +166,7 @@ void run_script(char *cmd)
>         printf("\033[c\033[H\033[J");
>         fflush(stdout);
>         SLang_init_tty(0, 0, 0);
> +       SLtty_set_suspend_state(true);
>         SLsmg_refresh();
>  }
>
> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> index a3b8c397c24d..09524ba04975 100644
> --- a/tools/perf/ui/tui/setup.c
> +++ b/tools/perf/ui/tui/setup.c
> @@ -2,12 +2,14 @@
>  #include <signal.h>
>  #include <stdbool.h>
>  #include <stdlib.h>
> +#include <termios.h>
>  #include <unistd.h>
>  #include <linux/kernel.h>
>  #ifdef HAVE_BACKTRACE_SUPPORT
>  #include <execinfo.h>
>  #endif
>
> +#include "../../util/color.h"
>  #include "../../util/debug.h"
>  #include "../../perf.h"
>  #include "../browser.h"
> @@ -122,6 +124,23 @@ static void ui__signal(int sig)
>         exit(0);
>  }
>
> +static void ui__sigcont(int sig)
> +{
> +       static struct termios tty;
> +
> +       if (sig == SIGTSTP) {
> +               while (tcgetattr(SLang_TT_Read_FD, &tty) == -1 && errno == EINTR)
> +                       ;
> +               while (write(SLang_TT_Read_FD, PERF_COLOR_RESET, sizeof(PERF_COLOR_RESET) - 1) == -1 && errno == EINTR)
> +                       ;
> +               raise(SIGSTOP);
> +       } else {
> +               while (tcsetattr(SLang_TT_Read_FD, TCSADRAIN, &tty) == -1 && errno == EINTR)
> +                       ;
> +               raise(SIGWINCH);
> +       }
> +}
> +
>  int ui__init(void)
>  {
>         int err;
> @@ -136,6 +155,7 @@ int ui__init(void)
>         err = SLang_init_tty(-1, 0, 0);
>         if (err < 0)
>                 goto out;
> +       SLtty_set_suspend_state(true);
>
>         err = SLkp_init();
>         if (err < 0) {
> @@ -150,6 +170,8 @@ int ui__init(void)
>         signal(SIGINT, ui__signal);
>         signal(SIGQUIT, ui__signal);
>         signal(SIGTERM, ui__signal);
> +       signal(SIGTSTP, ui__sigcont);
> +       signal(SIGCONT, ui__sigcont);
>
>         perf_error__register(&perf_tui_eops);
>
> --
> 2.30.2
>

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

* Re: [PATCH v2 2/2] perf tools: don't include signature in version strings
  2022-12-27 20:58   ` [PATCH v2 2/2] perf tools: don't include signature in version strings Ahelenia Ziemiańska
@ 2023-01-02 15:34     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-01-02 15:34 UTC (permalink / raw)
  To: Ahelenia Ziemiańska
  Cc: Namhyung Kim, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, linux-perf-users, linux-kernel

Em Tue, Dec 27, 2022 at 09:58:00PM +0100, Ahelenia Ziemiańska escreveu:
> This explodes the build if HEAD is signed, since the generated version
> is gpg: Signature made Mon 26 Dec 2022 20:34:48 CET, then a few more
> lines, then the SHA.
> 
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied to perf/urgent.

- Arnaldo

> ---
>  tools/perf/Documentation/Makefile | 2 +-
>  tools/perf/util/PERF-VERSION-GEN  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/Documentation/Makefile b/tools/perf/Documentation/Makefile
> index 6e7b88917ca0..ba5d942e4c6a 100644
> --- a/tools/perf/Documentation/Makefile
> +++ b/tools/perf/Documentation/Makefile
> @@ -267,7 +267,7 @@ $(OUTPUT)%.xml : %.txt
>  	$(ASCIIDOC) -b docbook -d manpage \
>  		$(ASCIIDOC_EXTRA) -aperf_version=$(PERF_VERSION) \
>  		-aperf_date=$(shell git log -1 --pretty="format:%cd" \
> -				--date=short $<) \
> +				--date=short --no-show-signature $<) \
>  		-o $@+ $< && \
>  	mv $@+ $@
>  
> diff --git a/tools/perf/util/PERF-VERSION-GEN b/tools/perf/util/PERF-VERSION-GEN
> index 3cc42821d9b3..d7dc7c28508c 100755
> --- a/tools/perf/util/PERF-VERSION-GEN
> +++ b/tools/perf/util/PERF-VERSION-GEN
> @@ -19,7 +19,7 @@ TAG=
>  if test -d ../../.git -o -f ../../.git
>  then
>  	TAG=$(MAKEFLAGS= make -sC ../.. kernelversion)
> -	CID=$(git log -1 --abbrev=12 --pretty=format:"%h" 2>/dev/null) && CID="-g$CID"
> +	CID=$(git log -1 --abbrev=12 --pretty=format:"%h" --no-show-signature 2>/dev/null) && CID="-g$CID"
>  elif test -f ../../PERF-VERSION-FILE
>  then
>  	TAG=$(cut -d' ' -f3 ../../PERF-VERSION-FILE | sed -e 's/\"//g')
> -- 
> 2.30.2



-- 

- Arnaldo

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

* Re: [PATCH v2 1/2] perf tui: don't ignore job control
  2022-12-27 20:57   ` [PATCH v2 1/2] " Ahelenia Ziemiańska
  2022-12-27 21:21     ` Namhyung Kim
@ 2023-01-02 15:35     ` Arnaldo Carvalho de Melo
  2024-01-04 21:23     ` Arnaldo Carvalho de Melo
  2 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-01-02 15:35 UTC (permalink / raw)
  To: Ahelenia Ziemiańska
  Cc: Namhyung Kim, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, yaowenbin, linux-perf-users,
	linux-kernel

Em Tue, Dec 27, 2022 at 09:57:40PM +0100, Ahelenia Ziemiańska escreveu:
> In its infinite wisdom, by default, SLang sets susp undef,
> and this can only be un-done by calling SLtty_set_suspend_state(true).
> After every SLang_init_tty().
> 
> Additionally, no provisions are made for maintaining the teletype
> attributes across suspend/continue (outside of curses emulation mode(?!),
> which provides full support, naturally), so we need to save and restore
> the flags ourselves, as well as reset the text colours when going under.
> We need to also re-draw the screen, and raising SIGWINCH, shockingly,
> Just Works.
> 
> The correct solution would be to Not Use SLang, but as a stop-gap,
> this makes TUI perf report usable.
> 
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> ---
> Yeah, somehow I didn't hit that in my tests, but it was relatively easy
> to trigger when I tried. A full partial write retry loop is an overkill
> here, I think, and few other places in perf actually try to resubmit
> partial writes, so.

We've been with this problem since forever, so I'll apply it to
perf/core, i.e. for the next merge window, ok?

- Arnaldo
 
>  tools/perf/ui/browsers/annotate.c |  1 +
>  tools/perf/ui/browsers/hists.c    |  2 ++
>  tools/perf/ui/browsers/scripts.c  |  1 +
>  tools/perf/ui/tui/setup.c         | 22 ++++++++++++++++++++++
>  4 files changed, 26 insertions(+)
> 
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index c03fa76c02ff..6a4ffbf66c7f 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -942,6 +942,7 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
>  	/* reset abort key so that it can get Ctrl-C as a key */
>  	SLang_reset_tty();
>  	SLang_init_tty(0, 0, 0);
> +	SLtty_set_suspend_state(true);
>  
>  	return map_symbol__tui_annotate(&he->ms, evsel, hbt, opts);
>  }
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index b72ee6822222..2479e6d42e7c 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -3010,6 +3010,7 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>  	/* reset abort key so that it can get Ctrl-C as a key */
>  	SLang_reset_tty();
>  	SLang_init_tty(0, 0, 0);
> +	SLtty_set_suspend_state(true);
>  
>  	if (min_pcnt)
>  		browser->min_pcnt = min_pcnt;
> @@ -3682,6 +3683,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
>  	/* reset abort key so that it can get Ctrl-C as a key */
>  	SLang_reset_tty();
>  	SLang_init_tty(0, 0, 0);
> +	SLtty_set_suspend_state(true);
>  
>  	memset(&action, 0, sizeof(action));
>  
> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> index 47d2c7a8cbe1..50d45054ed6c 100644
> --- a/tools/perf/ui/browsers/scripts.c
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -166,6 +166,7 @@ void run_script(char *cmd)
>  	printf("\033[c\033[H\033[J");
>  	fflush(stdout);
>  	SLang_init_tty(0, 0, 0);
> +	SLtty_set_suspend_state(true);
>  	SLsmg_refresh();
>  }
>  
> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> index a3b8c397c24d..09524ba04975 100644
> --- a/tools/perf/ui/tui/setup.c
> +++ b/tools/perf/ui/tui/setup.c
> @@ -2,12 +2,14 @@
>  #include <signal.h>
>  #include <stdbool.h>
>  #include <stdlib.h>
> +#include <termios.h>
>  #include <unistd.h>
>  #include <linux/kernel.h>
>  #ifdef HAVE_BACKTRACE_SUPPORT
>  #include <execinfo.h>
>  #endif
>  
> +#include "../../util/color.h"
>  #include "../../util/debug.h"
>  #include "../../perf.h"
>  #include "../browser.h"
> @@ -122,6 +124,23 @@ static void ui__signal(int sig)
>  	exit(0);
>  }
>  
> +static void ui__sigcont(int sig)
> +{
> +	static struct termios tty;
> +
> +	if (sig == SIGTSTP) {
> +		while (tcgetattr(SLang_TT_Read_FD, &tty) == -1 && errno == EINTR)
> +			;
> +		while (write(SLang_TT_Read_FD, PERF_COLOR_RESET, sizeof(PERF_COLOR_RESET) - 1) == -1 && errno == EINTR)
> +			;
> +		raise(SIGSTOP);
> +	} else {
> +		while (tcsetattr(SLang_TT_Read_FD, TCSADRAIN, &tty) == -1 && errno == EINTR)
> +			;
> +		raise(SIGWINCH);
> +	}
> +}
> +
>  int ui__init(void)
>  {
>  	int err;
> @@ -136,6 +155,7 @@ int ui__init(void)
>  	err = SLang_init_tty(-1, 0, 0);
>  	if (err < 0)
>  		goto out;
> +	SLtty_set_suspend_state(true);
>  
>  	err = SLkp_init();
>  	if (err < 0) {
> @@ -150,6 +170,8 @@ int ui__init(void)
>  	signal(SIGINT, ui__signal);
>  	signal(SIGQUIT, ui__signal);
>  	signal(SIGTERM, ui__signal);
> +	signal(SIGTSTP, ui__sigcont);
> +	signal(SIGCONT, ui__sigcont);
>  
>  	perf_error__register(&perf_tui_eops);
>  
> -- 
> 2.30.2
> 



-- 

- Arnaldo

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

* Re: [PATCH v2 1/2] perf tui: don't ignore job control
  2022-12-27 20:57   ` [PATCH v2 1/2] " Ahelenia Ziemiańska
  2022-12-27 21:21     ` Namhyung Kim
  2023-01-02 15:35     ` Arnaldo Carvalho de Melo
@ 2024-01-04 21:23     ` Arnaldo Carvalho de Melo
  2024-01-10 16:11       ` Ian Rogers
  2 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-01-04 21:23 UTC (permalink / raw)
  To: Ahelenia Ziemiańska
  Cc: Namhyung Kim, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, yaowenbin, linux-perf-users,
	linux-kernel

Em Tue, Dec 27, 2022 at 09:57:40PM +0100, Ahelenia Ziemiańska escreveu:
> In its infinite wisdom, by default, SLang sets susp undef,
> and this can only be un-done by calling SLtty_set_suspend_state(true).
> After every SLang_init_tty().
> 
> Additionally, no provisions are made for maintaining the teletype
> attributes across suspend/continue (outside of curses emulation mode(?!),
> which provides full support, naturally), so we need to save and restore
> the flags ourselves, as well as reset the text colours when going under.
> We need to also re-draw the screen, and raising SIGWINCH, shockingly,
> Just Works.
> 
> The correct solution would be to Not Use SLang, but as a stop-gap,
> this makes TUI perf report usable.

This was in the compost pile for a long time, but I got to it, indeed
'perf report' works, the refresh after returning to it, etc.

'perf top' needs some care, but we can do it later.

About not using slang, perf started with newt, I moved to slang, there
are people suggesting we move to some more modern thing, maybe
notcurses?

- Arnaldo
 
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> ---
> Yeah, somehow I didn't hit that in my tests, but it was relatively easy
> to trigger when I tried. A full partial write retry loop is an overkill
> here, I think, and few other places in perf actually try to resubmit
> partial writes, so.
> 
>  tools/perf/ui/browsers/annotate.c |  1 +
>  tools/perf/ui/browsers/hists.c    |  2 ++
>  tools/perf/ui/browsers/scripts.c  |  1 +
>  tools/perf/ui/tui/setup.c         | 22 ++++++++++++++++++++++
>  4 files changed, 26 insertions(+)
> 
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index c03fa76c02ff..6a4ffbf66c7f 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -942,6 +942,7 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
>  	/* reset abort key so that it can get Ctrl-C as a key */
>  	SLang_reset_tty();
>  	SLang_init_tty(0, 0, 0);
> +	SLtty_set_suspend_state(true);
>  
>  	return map_symbol__tui_annotate(&he->ms, evsel, hbt, opts);
>  }
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index b72ee6822222..2479e6d42e7c 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -3010,6 +3010,7 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>  	/* reset abort key so that it can get Ctrl-C as a key */
>  	SLang_reset_tty();
>  	SLang_init_tty(0, 0, 0);
> +	SLtty_set_suspend_state(true);
>  
>  	if (min_pcnt)
>  		browser->min_pcnt = min_pcnt;
> @@ -3682,6 +3683,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
>  	/* reset abort key so that it can get Ctrl-C as a key */
>  	SLang_reset_tty();
>  	SLang_init_tty(0, 0, 0);
> +	SLtty_set_suspend_state(true);
>  
>  	memset(&action, 0, sizeof(action));
>  
> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> index 47d2c7a8cbe1..50d45054ed6c 100644
> --- a/tools/perf/ui/browsers/scripts.c
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -166,6 +166,7 @@ void run_script(char *cmd)
>  	printf("\033[c\033[H\033[J");
>  	fflush(stdout);
>  	SLang_init_tty(0, 0, 0);
> +	SLtty_set_suspend_state(true);
>  	SLsmg_refresh();
>  }
>  
> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> index a3b8c397c24d..09524ba04975 100644
> --- a/tools/perf/ui/tui/setup.c
> +++ b/tools/perf/ui/tui/setup.c
> @@ -2,12 +2,14 @@
>  #include <signal.h>
>  #include <stdbool.h>
>  #include <stdlib.h>
> +#include <termios.h>
>  #include <unistd.h>
>  #include <linux/kernel.h>
>  #ifdef HAVE_BACKTRACE_SUPPORT
>  #include <execinfo.h>
>  #endif
>  
> +#include "../../util/color.h"
>  #include "../../util/debug.h"
>  #include "../../perf.h"
>  #include "../browser.h"
> @@ -122,6 +124,23 @@ static void ui__signal(int sig)
>  	exit(0);
>  }
>  
> +static void ui__sigcont(int sig)
> +{
> +	static struct termios tty;
> +
> +	if (sig == SIGTSTP) {
> +		while (tcgetattr(SLang_TT_Read_FD, &tty) == -1 && errno == EINTR)
> +			;
> +		while (write(SLang_TT_Read_FD, PERF_COLOR_RESET, sizeof(PERF_COLOR_RESET) - 1) == -1 && errno == EINTR)
> +			;
> +		raise(SIGSTOP);
> +	} else {
> +		while (tcsetattr(SLang_TT_Read_FD, TCSADRAIN, &tty) == -1 && errno == EINTR)
> +			;
> +		raise(SIGWINCH);
> +	}
> +}
> +
>  int ui__init(void)
>  {
>  	int err;
> @@ -136,6 +155,7 @@ int ui__init(void)
>  	err = SLang_init_tty(-1, 0, 0);
>  	if (err < 0)
>  		goto out;
> +	SLtty_set_suspend_state(true);
>  
>  	err = SLkp_init();
>  	if (err < 0) {
> @@ -150,6 +170,8 @@ int ui__init(void)
>  	signal(SIGINT, ui__signal);
>  	signal(SIGQUIT, ui__signal);
>  	signal(SIGTERM, ui__signal);
> +	signal(SIGTSTP, ui__sigcont);
> +	signal(SIGCONT, ui__sigcont);
>  
>  	perf_error__register(&perf_tui_eops);
>  
> -- 
> 2.30.2
> 



-- 

- Arnaldo

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

* Re: [PATCH v2 1/2] perf tui: don't ignore job control
  2024-01-04 21:23     ` Arnaldo Carvalho de Melo
@ 2024-01-10 16:11       ` Ian Rogers
  0 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2024-01-10 16:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ahelenia Ziemiańska, Namhyung Kim, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	yaowenbin, linux-perf-users, linux-kernel

On Thu, Jan 4, 2024 at 1:23 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Tue, Dec 27, 2022 at 09:57:40PM +0100, Ahelenia Ziemiańska escreveu:
> > In its infinite wisdom, by default, SLang sets susp undef,
> > and this can only be un-done by calling SLtty_set_suspend_state(true).
> > After every SLang_init_tty().
> >
> > Additionally, no provisions are made for maintaining the teletype
> > attributes across suspend/continue (outside of curses emulation mode(?!),
> > which provides full support, naturally), so we need to save and restore
> > the flags ourselves, as well as reset the text colours when going under.
> > We need to also re-draw the screen, and raising SIGWINCH, shockingly,
> > Just Works.
> >
> > The correct solution would be to Not Use SLang, but as a stop-gap,
> > this makes TUI perf report usable.
>
> This was in the compost pile for a long time, but I got to it, indeed
> 'perf report' works, the refresh after returning to it, etc.
>
> 'perf top' needs some care, but we can do it later.
>
> About not using slang, perf started with newt, I moved to slang, there
> are people suggesting we move to some more modern thing, maybe
> notcurses?

Fwiw, my feeling is that doing UI stuff in C is a particular pain.
Maybe there will be a move to allow C++ in the kernel and so perf
could follow [1]. For C++ the most maintained/best text UI looks like
ftxui [2], but what I think looks best is a python library called
textual [3] that already has support for things like flamegraphs [4]
as well as having a HTML/JavaScript output/interaction option [5]. So
my feeling is that if we're rebuilding the UI we should be moving more
to python+textual than a C/C++ library that gets a fraction of the
care.

Thanks,
Ian

[1] https://www.phoronix.com/news/CPP-Linux-Kernel-2024-Discuss
[2] https://github.com/ArthurSonzogni/FTXUI
[3] https://github.com/Textualize/textual
[4] https://github.com/Textualize/textual/discussions/3390
[5] https://github.com/Textualize/textual-web

> - Arnaldo
>
> > Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> > ---
> > Yeah, somehow I didn't hit that in my tests, but it was relatively easy
> > to trigger when I tried. A full partial write retry loop is an overkill
> > here, I think, and few other places in perf actually try to resubmit
> > partial writes, so.
> >
> >  tools/perf/ui/browsers/annotate.c |  1 +
> >  tools/perf/ui/browsers/hists.c    |  2 ++
> >  tools/perf/ui/browsers/scripts.c  |  1 +
> >  tools/perf/ui/tui/setup.c         | 22 ++++++++++++++++++++++
> >  4 files changed, 26 insertions(+)
> >
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index c03fa76c02ff..6a4ffbf66c7f 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -942,6 +942,7 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
> >       /* reset abort key so that it can get Ctrl-C as a key */
> >       SLang_reset_tty();
> >       SLang_init_tty(0, 0, 0);
> > +     SLtty_set_suspend_state(true);
> >
> >       return map_symbol__tui_annotate(&he->ms, evsel, hbt, opts);
> >  }
> > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > index b72ee6822222..2479e6d42e7c 100644
> > --- a/tools/perf/ui/browsers/hists.c
> > +++ b/tools/perf/ui/browsers/hists.c
> > @@ -3010,6 +3010,7 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
> >       /* reset abort key so that it can get Ctrl-C as a key */
> >       SLang_reset_tty();
> >       SLang_init_tty(0, 0, 0);
> > +     SLtty_set_suspend_state(true);
> >
> >       if (min_pcnt)
> >               browser->min_pcnt = min_pcnt;
> > @@ -3682,6 +3683,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
> >       /* reset abort key so that it can get Ctrl-C as a key */
> >       SLang_reset_tty();
> >       SLang_init_tty(0, 0, 0);
> > +     SLtty_set_suspend_state(true);
> >
> >       memset(&action, 0, sizeof(action));
> >
> > diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> > index 47d2c7a8cbe1..50d45054ed6c 100644
> > --- a/tools/perf/ui/browsers/scripts.c
> > +++ b/tools/perf/ui/browsers/scripts.c
> > @@ -166,6 +166,7 @@ void run_script(char *cmd)
> >       printf("\033[c\033[H\033[J");
> >       fflush(stdout);
> >       SLang_init_tty(0, 0, 0);
> > +     SLtty_set_suspend_state(true);
> >       SLsmg_refresh();
> >  }
> >
> > diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> > index a3b8c397c24d..09524ba04975 100644
> > --- a/tools/perf/ui/tui/setup.c
> > +++ b/tools/perf/ui/tui/setup.c
> > @@ -2,12 +2,14 @@
> >  #include <signal.h>
> >  #include <stdbool.h>
> >  #include <stdlib.h>
> > +#include <termios.h>
> >  #include <unistd.h>
> >  #include <linux/kernel.h>
> >  #ifdef HAVE_BACKTRACE_SUPPORT
> >  #include <execinfo.h>
> >  #endif
> >
> > +#include "../../util/color.h"
> >  #include "../../util/debug.h"
> >  #include "../../perf.h"
> >  #include "../browser.h"
> > @@ -122,6 +124,23 @@ static void ui__signal(int sig)
> >       exit(0);
> >  }
> >
> > +static void ui__sigcont(int sig)
> > +{
> > +     static struct termios tty;
> > +
> > +     if (sig == SIGTSTP) {
> > +             while (tcgetattr(SLang_TT_Read_FD, &tty) == -1 && errno == EINTR)
> > +                     ;
> > +             while (write(SLang_TT_Read_FD, PERF_COLOR_RESET, sizeof(PERF_COLOR_RESET) - 1) == -1 && errno == EINTR)
> > +                     ;
> > +             raise(SIGSTOP);
> > +     } else {
> > +             while (tcsetattr(SLang_TT_Read_FD, TCSADRAIN, &tty) == -1 && errno == EINTR)
> > +                     ;
> > +             raise(SIGWINCH);
> > +     }
> > +}
> > +
> >  int ui__init(void)
> >  {
> >       int err;
> > @@ -136,6 +155,7 @@ int ui__init(void)
> >       err = SLang_init_tty(-1, 0, 0);
> >       if (err < 0)
> >               goto out;
> > +     SLtty_set_suspend_state(true);
> >
> >       err = SLkp_init();
> >       if (err < 0) {
> > @@ -150,6 +170,8 @@ int ui__init(void)
> >       signal(SIGINT, ui__signal);
> >       signal(SIGQUIT, ui__signal);
> >       signal(SIGTERM, ui__signal);
> > +     signal(SIGTSTP, ui__sigcont);
> > +     signal(SIGCONT, ui__sigcont);
> >
> >       perf_error__register(&perf_tui_eops);
> >
> > --
> > 2.30.2
> >
>
>
>
> --
>
> - Arnaldo
>

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

end of thread, other threads:[~2024-01-10 16:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-26 19:35 [PATCH] perf tui: don't ignore job control Ahelenia Ziemiańska
2022-12-26 19:50 ` [PATCH 2/1] perf tools: don't include signature in version strings Ahelenia Ziemiańska
2022-12-27 20:29   ` Namhyung Kim
2022-12-27 20:24 ` [PATCH] perf tui: don't ignore job control Namhyung Kim
2022-12-27 20:57   ` [PATCH v2 1/2] " Ahelenia Ziemiańska
2022-12-27 21:21     ` Namhyung Kim
2023-01-02 15:35     ` Arnaldo Carvalho de Melo
2024-01-04 21:23     ` Arnaldo Carvalho de Melo
2024-01-10 16:11       ` Ian Rogers
2022-12-27 20:58   ` [PATCH v2 2/2] perf tools: don't include signature in version strings Ahelenia Ziemiańska
2023-01-02 15:34     ` Arnaldo Carvalho de Melo

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.