Message-ID: <YdiSSGLinhOVqS8a@cello>
Date:   Fri, 7 Jan 2022 22:19:36 +0300
From:   Arseny Maslennikov <ar@...msu.ru>
To:     Theodore Ts'o <tytso@....edu>
Cc:     "Eric W. Biederman" <ebiederm@...ssion.com>,
        Walt Drummond <walt@...mmond.us>, aacraid@...rosemi.com,
        viro@...iv.linux.org.uk, anna.schumaker@...app.com, arnd@...db.de,
        bsegall@...gle.com, bp@...en8.de, chuck.lever@...cle.com,
        bristot@...hat.com, dave.hansen@...ux.intel.com,
        dwmw2@...radead.org, dietmar.eggemann@....com, dinguyen@...nel.org,
        geert@...ux-m68k.org, gregkh@...uxfoundation.org, hpa@...or.com,
        idryomov@...il.com, mingo@...hat.com, yzaikin@...gle.com,
        ink@...assic.park.msu.ru, jejb@...ux.ibm.com, jmorris@...ei.org,
        bfields@...ldses.org, jlayton@...nel.org, jirislaby@...nel.org,
        john.johansen@...onical.com, juri.lelli@...hat.com,
        keescook@...omium.org, mcgrof@...nel.org,
        martin.petersen@...cle.com, mattst88@...il.com, mgorman@...e.de,
        oleg@...hat.com, pbonzini@...hat.com, peterz@...radead.org,
        rth@...ddle.net, richard@....at, serge@...lyn.com,
        rostedt@...dmis.org, tglx@...utronix.de,
        trond.myklebust@...merspace.com, vincent.guittot@...aro.org,
        x86@...nel.org, linux-kernel@...r.kernel.org,
        ceph-devel@...r.kernel.org, kvm@...r.kernel.org,
        linux-alpha@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-m68k@...r.kernel.org,
        linux-mtd@...ts.infradead.org, linux-nfs@...r.kernel.org,
        linux-scsi@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [RFC PATCH 0/8] signals: Support more than 64 signals

I generally agree with Ted's suggestion that we could merge the
easy-to-design part — the VSTATUS+kerninfo — first and deal with the
SIGINFO part later. The only concern I have here is that the "later"
part might never practically arrive... :)

Still, some notes on the SIGINFO/userspace-status
part:

On Tue, Jan 04, 2022 at 03:52:28PM -0500, Theodore Ts'o wrote:
> On Tue, Jan 04, 2022 at 12:00:34PM -0600, Eric W. Biederman wrote:
> > I dug through the previous conversations and there is a little debate
> > about what makes sense for SIGPWR to do by default.  Alan Cox remembered
> > SIGPWR was sent when the power was restored, so ignoring SIGPWR by
> > default made sense.  Ted Tso pointed out a different scenario where it
> > was reasonable for SIGPWR to be a terminating signal.
> > 
> > So far no one has actually found any applications that will regress if
> > SIGPWR becomes ignored by default.

Some folks from linux-api@ claimed otherwise, but unfortunately didn't elaborate.

> > Furthermore on linux SIGPWR is only
> > defined to be sent to init, and init ignores all signals by default so
> > in practice SIGPWR is ignored by the only process that receives it
> > currently.
> 
> As it turns out, systemd does *not* ignore SIGPWR.  Instead, it will
> initiate the sigpwr target.  From the systemd.special man page:
> 
>        sigpwr.target
>            A special target that is started when systemd receives the
>            SIGPWR process signal, which is normally sent by the kernel
>            or UPS daemons when power fails.

Not sure what you had in mind; in case you're suggesting that systemd has
to drop the sigpwr.target semantics — it doesn't.
We don't need to ask systemd to drop sigpwr.target semantics.

To introduce SIGINFO == SIGPWR to the kernel, the only "breaking" change
we have to do is to change the default disposition for SIGPWR, i. e. the
behaviour if the signal is set to SIG_DFL. If a process (including PID
1) installs its own signal handler for SIGPWR to do something when PWR
is received (or blocks the signal and handles it via signalfd
notifications), then the default disposition does not matter at all, as
Eric notes further in this thread.

From a quick glance at systemd code, pid1's main() function calls
manager_new() calls manager_setup_signals(); this function, in turn,
blocks a set of signals, including PWR, and sets up a signalfd(2) on
that set. No changes have to be made in systemd, no need to remove the
sigpwr.target semantics.

The target activation does not send SIGPWR to anyone, it results in
systemd services being started and possibly stopped; the exact
consequences are out of scope for systemd.

There could be another concern: a VSTATUS keypress could result in
SIGINFO == SIGPWR being sent to pid1. In a correct implementation this
will not ever happen, because a sane PID 1 does not have (and never
acquires) a controlling terminal.

> And child processes of systemd are not ignoring SIGPWR.  Instead, they
> are getting terminated.
> 
> <tytso@...c>
> 41% /bin/sleep 50 &
> [1] 180671
> <tytso@...c>
> 42% kill -PWR 180671
> [1]+  Power failure           /bin/sleep 50

All the possible surprises with the SIGINFO == SIGPWR approach we might
get stem from here, not from the sigpwr.target.

> > Where I saw the last conversation falter was in making a persuasive
> > case of why SIGINFO was interesting to add.  Given a world of ssh
> > connections I expect a persuasive case can be made.  Especially if there
> > are a handful of utilities where it is already implemented that just
> > need to be built with SIGINFO defined.
> 
> One thing that's perhaps worth disentangling is the value of
> supporting VSTATUS --- which is a control character much like VINTR
> (^C) or VQUIT (control backslash) which is set via the c_cc[] array in
> termios structure.  Quoting from the termios man page:
> 
>        VSTATUS
>               (not in POSIX; not supported under Linux; status
>               request: 024, DC4, Ctrl-T).  Status character (STATUS).
>               Display status information at terminal, including state
>               of foreground process and amount of CPU time it has
>               consumed.  Also sends a SIGINFO signal (not supported on
>               Linux) to the foreground process group.
> 
> The basic idea is that when you type C-t, you can find out information
> about the currently running process.  This is a feature that
> originally comes from TOPS-10's TENEX operating system, and it is
> supported today on FreeBSD and Mac OS.  For example, it might display
> something like this:
> 
> load: 2.39  cmd: ping 5374 running 0.00u 0.00s
> 
> The reason why SIGINFO is sent to the foreground process group is that
> it gives the process an opportunity print application specific
> information about currently running process.  For example, maybe the C
> compiler could print something like "parsing 2042 of 5000 header
> files", or some such.  :-)
> 
> There are people who wish that Linux supported Control-T / VSTATUS,
> for example, just last week, on TUHS, the Unix greybeards list, there
> were two such heartfelt wishes for Control-T support from two such
> greybeards:
> 
>     "It's my biggest annoyance with Linux that it doesn't [support
>     control-t]
>     - https://minnie.tuhs.org/pipermail/tuhs/2021-December/024849.html
> 
>     "I personally can't stand using Linux, even casually for a very
>      short sys-admin task, because of this missing feature"
>     - https://minnie.tuhs.org/pipermail/tuhs/2021-December/024898.html
> 
> I claim, though, that we could implement VSTATUS without implenting
> the SIGINFO part of the feature.  Previous few applications *ever*
> implemented SIGINFO signal handlers so they could give status
> information, it's the hard one, since we don't have any spare signals
> left.  If we were to repurpose some lesser used signal, whether it be
> SIGPWR, SIGLOST, or SIGSTKFLT, the danger is that there might be some
> userspace program (such as a UPS monitoring program which wants to
> trigger power fail handling, or a userspace NFSv4 process that wants
> to signal that it was unable to recover a file's file lock after a
> server reboot), and if we try to take over the signal assignment, it's
> possible that we might get surprised.  Furthermore, all of the
> possibly unused signals that we might try to reclaim terminate the
> process by default, and SIGINFO *has* to have a default signal
> handling action of Ignore, since otherwise typing Control-T will end
> up killing the current foreground application.
> 
> Personally, I don't care all that much about VSTATUS support --- I
> used it when I was in university, but honestly, I've never missed it.
> But if there is someone who wants to try to implement VSTATUS, and
> make some Unix greybeards happy, and maybe even switch from FreeBSD to
> Linux as a result, go wild.  I'm not convinced, though, that adding
> the SIGINFO part of the support is worth the effort.
> 
> Not only do almost no programs implement SIGINFO support, a lot of CPU

To be fair, many programs are a lot younger than 4.3BSD, and with the
current ubiquity of Linux without VSTATUS, it's kind of a chicken-egg
problem. :)

> bound programs where this might be actually useful, end up running a
> large number of processes in parallel.  Take the "parsing 2042 of 5000
> header files" example I gave above.  Consider what would happen if gcc
> implemented support for SIGINFO, but the user was running a "make -j
> 16" and typed Control-T.   The result would be chaos!
> 
> So if you really miss Control-T, and it's the only thing holding back
> a few FreeBSD users from Linux, I don't see the problem with
> implementing that part of the feature.  Why not just do the easy part
> of the feature which is perhaps 5% of the work, and might provide 99%
> of the benefit (at least for those people who care).
> 
> > Without seeing the persuasive case for more signals I have to say that
> > adding more signals to the kernel sounds like a bad idea.
> 
> Concur, 100%.
> 
> 						- Ted

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists