| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fUJK5FZAse3qVvAbVaNQ=qSqUsYmgxaj3eMRruv9ufEUA@mail.gmail.com>
Date: Wed, 10 Jan 2024 08:11:19 -0800
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Ahelenia Ziemiańska <nabijaczleweli@...ijaczleweli.xyz>,
Namhyung Kim <namhyung@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
yaowenbin <yaowenbin1@...wei.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] perf tui: don't ignore job control
On Thu, Jan 4, 2024 at 1:23 PM Arnaldo Carvalho de Melo <acme@...nel.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@...ijaczleweli.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
>
Powered by blists - more mailing lists