Message ID | 0354dcae23a8713f75f4fed609e0caec3c6e3cd5.1672174189.git.nabijaczleweli@nabijaczleweli.xyz |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4e01:0:0:0:0:0 with SMTP id p1csp1585037wrt; Tue, 27 Dec 2022 13:11:40 -0800 (PST) X-Google-Smtp-Source: AMrXdXvzNrK0D/KPrXQuguomnk7LS9Wf/wpoXFzctwE3Kz7vJrqJv/LNGybTucilck+wpQlD7kwO X-Received: by 2002:a17:907:7d8f:b0:7c1:f63:8f66 with SMTP id oz15-20020a1709077d8f00b007c10f638f66mr29944312ejc.51.1672175499797; Tue, 27 Dec 2022 13:11:39 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1672175499; cv=none; d=google.com; s=arc-20160816; b=Tu4T15uEcquH1JwaSLC+JZFUoslDxtMWbKVtpFoxZGQj2reUk60xtwyN4aAXYt6Wa7 j/Lq+cTbiNbl02XeGJSZ7r+U5fS/Scxj2yq0OpqWAElmH6I3KhcQfRZuc6QGtfleTo3Y ELApMSLBqLwqPsKZtH9xWoProbtjVpZuowotErlg1FEMwJ/DxD/eFu2znpkuUfPzjCEL Cwnz1YvddAeOwy/IADzPaXoazleYnFM7NVyUry997ubPZX0yWZ7vBtvJTt6FPlSjuxBB sIwm0CvDpqJN9BYHUTgVGm+tKOBjA1rjMGc1gBNHvp7BIb68JbtqvXueSGoANNxYjy5k pwIw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:user-agent:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :dkim-signature; bh=oxhBi6QVZCCKVOpq6H+nY/NSwVWWmFbH2mfjrnkGe3g=; b=XNDqlOUfCY8bHT60qxmVwfPMn8FHhDVgRQNyOqYxUTBZQA90/AEE8NCcnIncHhd5XN TQLGOfR1vqln9qAb4XKadHEbGc8jXvsfZTx9FDWEmd8ziZVxB2iu/C1DvitWUorK0ZnB OA0Xu0xCE7u8WdVMq9Xhr9BIG+IkLvfSYMHAhzqO+4pWHNSdBWTEFar5GDMbLFp09BR9 ZLWfK+ueqD6h7eFpBPSs+kcj9cXPe8pwcVaLZCrr80L4YZNPA3qr9qSNPcCxvqgr7h64 UtFFOlwQF2nvt4l6AA/bDsZ5n1dkv8o0OBTz261Wexletur6JOVj+9l2X+uR47LOduoZ evKA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@nabijaczleweli.xyz header.s=202211 header.b="B/ibEgq9"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nabijaczleweli.xyz Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id q9-20020a056402518900b0046b9c536fa7si12967091edd.286.2022.12.27.13.11.15; Tue, 27 Dec 2022 13:11:39 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@nabijaczleweli.xyz header.s=202211 header.b="B/ibEgq9"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nabijaczleweli.xyz Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229495AbiL0VIZ (ORCPT <rfc822;eddaouddi.ayoub@gmail.com> + 99 others); Tue, 27 Dec 2022 16:08:25 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46186 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232036AbiL0VH5 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 27 Dec 2022 16:07:57 -0500 Received: from tarta.nabijaczleweli.xyz (unknown [139.28.40.42]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 00613DAB; Tue, 27 Dec 2022 12:57:45 -0800 (PST) Received: from tarta.nabijaczleweli.xyz (unknown [192.168.1.250]) by tarta.nabijaczleweli.xyz (Postfix) with ESMTPSA id C6624E2A; Tue, 27 Dec 2022 21:57:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=nabijaczleweli.xyz; s=202211; t=1672174661; bh=u+4jI8sJ77aO6ldqx4NYEzP0FaHDnE/y5VlgR4hoBjc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=B/ibEgq982ZTO2/K+QueqWfH8IEBEYrQGQVoaF3jwenqJwLwUmhgVFj2k2vfVWB46 NbJ6l2rrli/dL2OEo1vwoXCzhHL+SqNLvfrKxMYWaUH/vcxW5dWT8qw1D16Rx9/VUA SwRHz4WCtSXL6zbncBxty7xHcXlNTvjGjJ1pH8NjB3IxtNW8ssv89KvPSfzVI8Ejzp THkAMi0dGKJuyD9J2aYt51GxabcUCwoxwonn9sdv6nf36GDYUj02p/LB9WwMuhl3VQ CviQEZY4ujyyOWsFSB2dABMVvIZDFesAMVB5q5DmTOMvtI4t5oXEoAJf36Oi/BVNC5 /3quRgYJ5T/Xw== Date: Tue, 27 Dec 2022 21:57:40 +0100 From: Ahelenia =?utf-8?q?Ziemia=C5=84ska?= <nabijaczleweli@nabijaczleweli.xyz> To: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>, yaowenbin <yaowenbin1@huawei.com>, linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2 1/2] perf tui: don't ignore job control Message-ID: <0354dcae23a8713f75f4fed609e0caec3c6e3cd5.1672174189.git.nabijaczleweli@nabijaczleweli.xyz> References: <CAM9d7ciuh1H2Mfx5ToYGT1fOm8E3jrQhkzg304JKDd7BhT=h5g@mail.gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="zypchkvuvqgxav6j" Content-Disposition: inline In-Reply-To: <CAM9d7ciuh1H2Mfx5ToYGT1fOm8E3jrQhkzg304JKDd7BhT=h5g@mail.gmail.com> User-Agent: NeoMutt/20220429 X-Spam-Status: No, score=-0.6 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FROM_SUSPICIOUS_NTLD, PDS_RDNS_DYNAMIC_FP,RDNS_DYNAMIC,SPF_HELO_PASS,SPF_PASS, T_PDS_OTHER_BAD_TLD autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1753309911322368615?= X-GMAIL-MSGID: =?utf-8?q?1753403096738702126?= |
Series |
[v2,1/2] perf tui: don't ignore job control
|
|
Commit Message
Ahelenia Ziemiańska
Dec. 27, 2022, 8:57 p.m. UTC
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(+)
Comments
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 >
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 >
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 >
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 >
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);