* [PATCH v3 1/7] signal.h: Define SIGINFO on all architectures
2020-04-30 6:42 [PATCH v3 0/7] TTY Keyboard Status Request Arseny Maslennikov
@ 2020-04-30 6:42 ` Arseny Maslennikov
2020-04-30 6:42 ` [PATCH v3 2/7] tty: termios: Reserve space for VSTATUS in .c_cc Arseny Maslennikov
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Arseny Maslennikov @ 2020-04-30 6:42 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
Cc: Arseny Maslennikov, Rob Landley, Eric W. Biederman, Pavel Machek,
linux-api, Vladimir D. Seleznev
This complementary patch defines SIGINFO as a synonym for SIGPWR
on every architecture supported by the kernel.
SIGPWR looks like a nice candidate for this role, because it is
defined on every supported arch; it is currently only used to inform
PID 1 of power failures, and daemons that care about low-level
events do not tend to have a controlling terminal.
However, on sparcs SIGPWR is a synonym for SIGLOST, a signal unique
to that architecture, with a narrow set of intended uses that do not
combine well with interactively requesting status.
SIGLOST is not used by any kernel code at the moment.
SIGINFO is not defined as a real-time signal for the following reasons:
* SIGRTMIN and SIGRTMAX can not be changed without breaking userspace
* the lack of queuing is irrelevant for the main use-case, since the
consumption rate of information by a terminal user and the
practical tty keypress processing rates are much slower than SIGINFO
processing by the line discipline.
Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
arch/arm/include/uapi/asm/signal.h | 1 +
arch/h8300/include/uapi/asm/signal.h | 1 +
arch/ia64/include/uapi/asm/signal.h | 1 +
arch/m68k/include/uapi/asm/signal.h | 1 +
arch/mips/include/uapi/asm/signal.h | 1 +
arch/parisc/include/uapi/asm/signal.h | 1 +
arch/powerpc/include/uapi/asm/signal.h | 1 +
arch/s390/include/uapi/asm/signal.h | 1 +
arch/sparc/include/uapi/asm/signal.h | 2 ++
arch/x86/include/uapi/asm/signal.h | 1 +
arch/xtensa/include/uapi/asm/signal.h | 1 +
include/uapi/asm-generic/signal.h | 1 +
12 files changed, 13 insertions(+)
diff --git a/arch/arm/include/uapi/asm/signal.h b/arch/arm/include/uapi/asm/signal.h
index 9b4185ba4..b80b53a17 100644
--- a/arch/arm/include/uapi/asm/signal.h
+++ b/arch/arm/include/uapi/asm/signal.h
@@ -50,6 +50,7 @@ typedef unsigned long sigset_t;
#define SIGLOST 29
*/
#define SIGPWR 30
+#define SIGINFO SIGPWR
#define SIGSYS 31
#define SIGUNUSED 31
diff --git a/arch/h8300/include/uapi/asm/signal.h b/arch/h8300/include/uapi/asm/signal.h
index e15521037..7a2b783af 100644
--- a/arch/h8300/include/uapi/asm/signal.h
+++ b/arch/h8300/include/uapi/asm/signal.h
@@ -50,6 +50,7 @@ typedef unsigned long sigset_t;
#define SIGLOST 29
*/
#define SIGPWR 30
+#define SIGINFO SIGPWR
#define SIGSYS 31
#define SIGUNUSED 31
diff --git a/arch/ia64/include/uapi/asm/signal.h b/arch/ia64/include/uapi/asm/signal.h
index aa98ff1b9..b4c98cb17 100644
--- a/arch/ia64/include/uapi/asm/signal.h
+++ b/arch/ia64/include/uapi/asm/signal.h
@@ -45,6 +45,7 @@
#define SIGLOST 29
*/
#define SIGPWR 30
+#define SIGINFO SIGPWR
#define SIGSYS 31
/* signal 31 is no longer "unused", but the SIGUNUSED macro remains for backwards compatibility */
#define SIGUNUSED 31
diff --git a/arch/m68k/include/uapi/asm/signal.h b/arch/m68k/include/uapi/asm/signal.h
index 915cc755a..a0b4e4108 100644
--- a/arch/m68k/include/uapi/asm/signal.h
+++ b/arch/m68k/include/uapi/asm/signal.h
@@ -50,6 +50,7 @@ typedef unsigned long sigset_t;
#define SIGLOST 29
*/
#define SIGPWR 30
+#define SIGINFO SIGPWR
#define SIGSYS 31
#define SIGUNUSED 31
diff --git a/arch/mips/include/uapi/asm/signal.h b/arch/mips/include/uapi/asm/signal.h
index 53104b10a..975a6f0d3 100644
--- a/arch/mips/include/uapi/asm/signal.h
+++ b/arch/mips/include/uapi/asm/signal.h
@@ -43,6 +43,7 @@ typedef unsigned long old_sigset_t; /* at least 32 bits */
#define SIGCHLD 18 /* Child status has changed (POSIX). */
#define SIGCLD SIGCHLD /* Same as SIGCHLD (System V). */
#define SIGPWR 19 /* Power failure restart (System V). */
+#define SIGINFO SIGPWR /* Keyboard status request (4.2 BSD). */
#define SIGWINCH 20 /* Window size change (4.3 BSD, Sun). */
#define SIGURG 21 /* Urgent condition on socket (4.2 BSD). */
#define SIGIO 22 /* I/O now possible (4.2 BSD). */
diff --git a/arch/parisc/include/uapi/asm/signal.h b/arch/parisc/include/uapi/asm/signal.h
index d38563a39..fe2e00d59 100644
--- a/arch/parisc/include/uapi/asm/signal.h
+++ b/arch/parisc/include/uapi/asm/signal.h
@@ -22,6 +22,7 @@
#define SIGUSR2 17
#define SIGCHLD 18
#define SIGPWR 19
+#define SIGINFO SIGPWR
#define SIGVTALRM 20
#define SIGPROF 21
#define SIGIO 22
diff --git a/arch/powerpc/include/uapi/asm/signal.h b/arch/powerpc/include/uapi/asm/signal.h
index 85b0a7aa4..e7f388590 100644
--- a/arch/powerpc/include/uapi/asm/signal.h
+++ b/arch/powerpc/include/uapi/asm/signal.h
@@ -53,6 +53,7 @@ typedef struct {
#define SIGLOST 29
*/
#define SIGPWR 30
+#define SIGINFO SIGPWR
#define SIGSYS 31
#define SIGUNUSED 31
diff --git a/arch/s390/include/uapi/asm/signal.h b/arch/s390/include/uapi/asm/signal.h
index 9a14a611e..12ee62987 100644
--- a/arch/s390/include/uapi/asm/signal.h
+++ b/arch/s390/include/uapi/asm/signal.h
@@ -58,6 +58,7 @@ typedef unsigned long sigset_t;
#define SIGLOST 29
*/
#define SIGPWR 30
+#define SIGINFO SIGPWR
#define SIGSYS 31
#define SIGUNUSED 31
diff --git a/arch/sparc/include/uapi/asm/signal.h b/arch/sparc/include/uapi/asm/signal.h
index ff9505923..b65516319 100644
--- a/arch/sparc/include/uapi/asm/signal.h
+++ b/arch/sparc/include/uapi/asm/signal.h
@@ -71,6 +71,8 @@
#define SIGWINCH 28
#define SIGLOST 29
#define SIGPWR SIGLOST
+/* XXX: is it OK for SIGINFO to collide with LOST? */
+#define SIGINFO SIGPWR
#define SIGUSR1 30
#define SIGUSR2 31
diff --git a/arch/x86/include/uapi/asm/signal.h b/arch/x86/include/uapi/asm/signal.h
index e5745d593..1539bb288 100644
--- a/arch/x86/include/uapi/asm/signal.h
+++ b/arch/x86/include/uapi/asm/signal.h
@@ -55,6 +55,7 @@ typedef unsigned long sigset_t;
#define SIGLOST 29
*/
#define SIGPWR 30
+#define SIGINFO SIGPWR
#define SIGSYS 31
#define SIGUNUSED 31
diff --git a/arch/xtensa/include/uapi/asm/signal.h b/arch/xtensa/include/uapi/asm/signal.h
index 005dec5bf..d64423430 100644
--- a/arch/xtensa/include/uapi/asm/signal.h
+++ b/arch/xtensa/include/uapi/asm/signal.h
@@ -65,6 +65,7 @@ typedef struct {
#define SIGPOLL SIGIO
/* #define SIGLOST 29 */
#define SIGPWR 30
+#define SIGINFO SIGPWR
#define SIGSYS 31
#define SIGUNUSED 31
diff --git a/include/uapi/asm-generic/signal.h b/include/uapi/asm-generic/signal.h
index 5c716a952..9f9a1db0d 100644
--- a/include/uapi/asm-generic/signal.h
+++ b/include/uapi/asm-generic/signal.h
@@ -43,6 +43,7 @@
#define SIGLOST 29
*/
#define SIGPWR 30
+#define SIGINFO SIGPWR
#define SIGSYS 31
#define SIGUNUSED 31
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v3 2/7] tty: termios: Reserve space for VSTATUS in .c_cc
2020-04-30 6:42 [PATCH v3 0/7] TTY Keyboard Status Request Arseny Maslennikov
2020-04-30 6:42 ` [PATCH v3 1/7] signal.h: Define SIGINFO on all architectures Arseny Maslennikov
@ 2020-04-30 6:42 ` Arseny Maslennikov
2020-04-30 6:42 ` [PATCH v3 3/7] n_tty: Send SIGINFO to fg pgrp on status request character Arseny Maslennikov
` (4 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Arseny Maslennikov @ 2020-04-30 6:42 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
Cc: Arseny Maslennikov, Rob Landley, Eric W. Biederman, Pavel Machek,
linux-api, Vladimir D. Seleznev
There was no architecture we had to increase NCCS on,
so the size of struct termios did not change.
Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
arch/alpha/include/asm/termios.h | 4 ++--
arch/alpha/include/uapi/asm/termbits.h | 1 +
arch/ia64/include/asm/termios.h | 4 ++--
arch/ia64/include/uapi/asm/termbits.h | 1 +
arch/mips/include/asm/termios.h | 4 ++--
arch/mips/include/uapi/asm/termbits.h | 1 +
arch/parisc/include/asm/termios.h | 4 ++--
arch/parisc/include/uapi/asm/termbits.h | 1 +
arch/powerpc/include/asm/termios.h | 4 ++--
arch/powerpc/include/uapi/asm/termbits.h | 1 +
arch/s390/include/asm/termios.h | 4 ++--
arch/sparc/include/asm/termios.h | 4 ++--
arch/sparc/include/uapi/asm/termbits.h | 1 +
arch/xtensa/include/uapi/asm/termbits.h | 1 +
include/asm-generic/termios.h | 4 ++--
include/uapi/asm-generic/termbits.h | 1 +
16 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/arch/alpha/include/asm/termios.h b/arch/alpha/include/asm/termios.h
index b7c77bb1b..0bf32d1af 100644
--- a/arch/alpha/include/asm/termios.h
+++ b/arch/alpha/include/asm/termios.h
@@ -8,9 +8,9 @@
werase=^W kill=^U reprint=^R sxtc=\0
intr=^C quit=^\ susp=^Z <OSF/1 VDSUSP>
start=^Q stop=^S lnext=^V discard=^U
- vmin=\1 vtime=\0
+ vmin=\1 vtime=\0 status=^T
*/
-#define INIT_C_CC "\004\000\000\177\027\025\022\000\003\034\032\000\021\023\026\025\001\000"
+#define INIT_C_CC "\004\000\000\177\027\025\022\000\003\034\032\000\021\023\026\025\001\000\024"
/*
* Translate a "termio" structure into a "termios". Ugh.
diff --git a/arch/alpha/include/uapi/asm/termbits.h b/arch/alpha/include/uapi/asm/termbits.h
index 4575ba34a..bb895d467 100644
--- a/arch/alpha/include/uapi/asm/termbits.h
+++ b/arch/alpha/include/uapi/asm/termbits.h
@@ -70,6 +70,7 @@ struct ktermios {
#define VDISCARD 15
#define VMIN 16
#define VTIME 17
+#define VSTATUS 18
/* c_iflag bits */
#define IGNBRK 0000001
diff --git a/arch/ia64/include/asm/termios.h b/arch/ia64/include/asm/termios.h
index 589c02644..b262e010b 100644
--- a/arch/ia64/include/asm/termios.h
+++ b/arch/ia64/include/asm/termios.h
@@ -15,9 +15,9 @@
eof=^D vtime=\0 vmin=\1 sxtc=\0
start=^Q stop=^S susp=^Z eol=\0
reprint=^R discard=^U werase=^W lnext=^V
- eol2=\0
+ eol2=\0 status=^T
*/
-#define INIT_C_CC "\003\034\177\025\004\0\1\0\021\023\032\0\022\017\027\026\0"
+#define INIT_C_CC "\003\034\177\025\004\0\1\0\021\023\032\0\022\017\027\026\0\024"
/*
* Translate a "termio" structure into a "termios". Ugh.
diff --git a/arch/ia64/include/uapi/asm/termbits.h b/arch/ia64/include/uapi/asm/termbits.h
index 000a1a297..86898e4c5 100644
--- a/arch/ia64/include/uapi/asm/termbits.h
+++ b/arch/ia64/include/uapi/asm/termbits.h
@@ -67,6 +67,7 @@ struct ktermios {
#define VWERASE 14
#define VLNEXT 15
#define VEOL2 16
+#define VSTATUS 17
/* c_iflag bits */
#define IGNBRK 0000001
diff --git a/arch/mips/include/asm/termios.h b/arch/mips/include/asm/termios.h
index bc29eeacc..9086e4f64 100644
--- a/arch/mips/include/asm/termios.h
+++ b/arch/mips/include/asm/termios.h
@@ -17,9 +17,9 @@
* vmin=\1 vtime=\0 eol2=\0 swtc=\0
* start=^Q stop=^S susp=^Z vdsusp=
* reprint=^R discard=^U werase=^W lnext=^V
- * eof=^D eol=\0
+ * eof=^D eol=\0 status=^T
*/
-#define INIT_C_CC "\003\034\177\025\1\0\0\0\021\023\032\0\022\017\027\026\004\0"
+#define INIT_C_CC "\003\034\177\025\1\0\0\0\021\023\032\0\022\017\027\026\004\0\024"
#include <linux/string.h>
diff --git a/arch/mips/include/uapi/asm/termbits.h b/arch/mips/include/uapi/asm/termbits.h
index dfeffba72..3dd60924f 100644
--- a/arch/mips/include/uapi/asm/termbits.h
+++ b/arch/mips/include/uapi/asm/termbits.h
@@ -78,6 +78,7 @@ struct ktermios {
#define VLNEXT 15 /* Literal-next character [IEXTEN]. */
#define VEOF 16 /* End-of-file character [ICANON]. */
#define VEOL 17 /* End-of-line character [ICANON]. */
+#define VSTATUS 18 /* Status request character [ISIG]. */
/* c_iflag bits */
#define IGNBRK 0000001 /* Ignore break condition. */
diff --git a/arch/parisc/include/asm/termios.h b/arch/parisc/include/asm/termios.h
index cded9dc90..37828b15a 100644
--- a/arch/parisc/include/asm/termios.h
+++ b/arch/parisc/include/asm/termios.h
@@ -9,9 +9,9 @@
eof=^D vtime=\0 vmin=\1 sxtc=\0
start=^Q stop=^S susp=^Z eol=\0
reprint=^R discard=^U werase=^W lnext=^V
- eol2=\0
+ eol2=\0 status=^T
*/
-#define INIT_C_CC "\003\034\177\025\004\0\1\0\021\023\032\0\022\017\027\026\0"
+#define INIT_C_CC "\003\034\177\025\004\0\1\0\021\023\032\0\022\017\027\026\0\024"
/*
* Translate a "termio" structure into a "termios". Ugh.
diff --git a/arch/parisc/include/uapi/asm/termbits.h b/arch/parisc/include/uapi/asm/termbits.h
index 40e920f8d..ecca3b7d0 100644
--- a/arch/parisc/include/uapi/asm/termbits.h
+++ b/arch/parisc/include/uapi/asm/termbits.h
@@ -58,6 +58,7 @@ struct ktermios {
#define VWERASE 14
#define VLNEXT 15
#define VEOL2 16
+#define VSTATUS 17
/* c_iflag bits */
diff --git a/arch/powerpc/include/asm/termios.h b/arch/powerpc/include/asm/termios.h
index 205de8f8a..263707fc3 100644
--- a/arch/powerpc/include/asm/termios.h
+++ b/arch/powerpc/include/asm/termios.h
@@ -10,8 +10,8 @@
#include <uapi/asm/termios.h>
-/* ^C ^\ del ^U ^D 1 0 0 0 0 ^W ^R ^Z ^Q ^S ^V ^U */
-#define INIT_C_CC "\003\034\177\025\004\001\000\000\000\000\027\022\032\021\023\026\025"
+/* ^C ^\ del ^U ^D 1 0 0 0 0 ^W ^R ^Z ^Q ^S ^V ^U ^T */
+#define INIT_C_CC "\003\034\177\025\004\001\000\000\000\000\027\022\032\021\023\026\025\024"
#include <asm-generic/termios-base.h>
diff --git a/arch/powerpc/include/uapi/asm/termbits.h b/arch/powerpc/include/uapi/asm/termbits.h
index ed18bc61f..c85e98d75 100644
--- a/arch/powerpc/include/uapi/asm/termbits.h
+++ b/arch/powerpc/include/uapi/asm/termbits.h
@@ -62,6 +62,7 @@ struct ktermios {
#define VSTOP 14
#define VLNEXT 15
#define VDISCARD 16
+#define VSTATUS 17
/* c_iflag bits */
#define IGNBRK 0000001
diff --git a/arch/s390/include/asm/termios.h b/arch/s390/include/asm/termios.h
index 46fa3020b..c84e49ed7 100644
--- a/arch/s390/include/asm/termios.h
+++ b/arch/s390/include/asm/termios.h
@@ -14,9 +14,9 @@
eof=^D vtime=\0 vmin=\1 sxtc=\0
start=^Q stop=^S susp=^Z eol=\0
reprint=^R discard=^U werase=^W lnext=^V
- eol2=\0
+ eol2=\0 status=^T
*/
-#define INIT_C_CC "\003\034\177\025\004\0\1\0\021\023\032\0\022\017\027\026\0"
+#define INIT_C_CC "\003\034\177\025\004\0\1\0\021\023\032\0\022\017\027\026\0\024"
#define user_termios_to_kernel_termios(k, u) copy_from_user(k, u, sizeof(struct termios2))
#define kernel_termios_to_user_termios(u, k) copy_to_user(u, k, sizeof(struct termios2))
diff --git a/arch/sparc/include/asm/termios.h b/arch/sparc/include/asm/termios.h
index 4a558efdf..249c95cf2 100644
--- a/arch/sparc/include/asm/termios.h
+++ b/arch/sparc/include/asm/termios.h
@@ -20,9 +20,9 @@
eof=^D eol=\0 eol2=\0 sxtc=\0
start=^Q stop=^S susp=^Z dsusp=^Y
reprint=^R discard=^U werase=^W lnext=^V
- vmin=\1 vtime=\0
+ vmin=\1 vtime=\0 status=^T
*/
-#define INIT_C_CC "\003\034\177\025\004\000\000\000\021\023\032\031\022\025\027\026\001"
+#define INIT_C_CC "\003\034\177\025\004\000\000\000\021\023\032\031\022\025\027\026\001\000\024"
/*
* Translate a "termio" structure into a "termios". Ugh.
diff --git a/arch/sparc/include/uapi/asm/termbits.h b/arch/sparc/include/uapi/asm/termbits.h
index ce5ad5d0f..a1638c9cd 100644
--- a/arch/sparc/include/uapi/asm/termbits.h
+++ b/arch/sparc/include/uapi/asm/termbits.h
@@ -80,6 +80,7 @@ struct ktermios {
#define VDISCARD 13
#define VWERASE 14
#define VLNEXT 15
+#define VSTATUS 16
/* Kernel keeps vmin/vtime separated, user apps assume vmin/vtime is
* shared with eof/eol
diff --git a/arch/xtensa/include/uapi/asm/termbits.h b/arch/xtensa/include/uapi/asm/termbits.h
index d4206a7c5..77969cb27 100644
--- a/arch/xtensa/include/uapi/asm/termbits.h
+++ b/arch/xtensa/include/uapi/asm/termbits.h
@@ -72,6 +72,7 @@ struct ktermios {
#define VWERASE 14
#define VLNEXT 15
#define VEOL2 16
+#define VSTATUS 17
/* c_iflag bits */
diff --git a/include/asm-generic/termios.h b/include/asm-generic/termios.h
index b1398d0d4..06fbfb87c 100644
--- a/include/asm-generic/termios.h
+++ b/include/asm-generic/termios.h
@@ -10,9 +10,9 @@
eof=^D vtime=\0 vmin=\1 sxtc=\0
start=^Q stop=^S susp=^Z eol=\0
reprint=^R discard=^U werase=^W lnext=^V
- eol2=\0
+ eol2=\0 status=^T
*/
-#define INIT_C_CC "\003\034\177\025\004\0\1\0\021\023\032\0\022\017\027\026\0"
+#define INIT_C_CC "\003\034\177\025\004\0\1\0\021\023\032\0\022\017\027\026\0\024"
/*
* Translate a "termio" structure into a "termios". Ugh.
diff --git a/include/uapi/asm-generic/termbits.h b/include/uapi/asm-generic/termbits.h
index 2fbaf9ae8..11bb142ba 100644
--- a/include/uapi/asm-generic/termbits.h
+++ b/include/uapi/asm-generic/termbits.h
@@ -58,6 +58,7 @@ struct ktermios {
#define VWERASE 14
#define VLNEXT 15
#define VEOL2 16
+#define VSTATUS 17
/* c_iflag bits */
#define IGNBRK 0000001
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v3 3/7] n_tty: Send SIGINFO to fg pgrp on status request character
2020-04-30 6:42 [PATCH v3 0/7] TTY Keyboard Status Request Arseny Maslennikov
2020-04-30 6:42 ` [PATCH v3 1/7] signal.h: Define SIGINFO on all architectures Arseny Maslennikov
2020-04-30 6:42 ` [PATCH v3 2/7] tty: termios: Reserve space for VSTATUS in .c_cc Arseny Maslennikov
@ 2020-04-30 6:42 ` Arseny Maslennikov
2020-04-30 6:42 ` [PATCH v3 4/7] linux/signal.h: Ignore SIGINFO by default in new tasks Arseny Maslennikov
` (3 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Arseny Maslennikov @ 2020-04-30 6:42 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
Cc: Arseny Maslennikov, Rob Landley, Eric W. Biederman, Pavel Machek,
linux-api, Vladimir D. Seleznev
No kerninfo line is printed yet.
No existing implementation of this on any Unix-like system echoes
the status character; no existing implementation discards or flushes
pending input on VSTATUS receipt. Thus we do neither.
There are existing popular TUI applications (e. g. mutt) that only
turn off icanon and not iexten, but still do not expect any special
treatment of the status request character — so we require all three:
isig, icanon and iexten to trigger this.
Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
drivers/tty/n_tty.c | 15 +++++++++++++--
include/linux/tty.h | 1 +
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 1794d84e7..10d6b60a5 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -79,6 +79,10 @@
#define ECHO_BLOCK 256
#define ECHO_DISCARD_WATERMARK N_TTY_BUF_SIZE - (ECHO_BLOCK + 32)
+#define SIG_FLUSHING_MASK ( \
+ rt_sigmask(SIGINT) | rt_sigmask(SIGQUIT) | \
+ rt_sigmask(SIGTSTP) )
+#define SIG_FLUSHING(sig) ((1 << sig) & SIG_FLUSHING_MASK)
#undef N_TTY_TRACE
#ifdef N_TTY_TRACE
@@ -1122,7 +1126,7 @@ static void isig(int sig, struct tty_struct *tty)
{
struct n_tty_data *ldata = tty->disc_data;
- if (L_NOFLSH(tty)) {
+ if (L_NOFLSH(tty) || (!SIG_FLUSHING(sig))) {
/* signal only */
__isig(sig, tty);
@@ -1244,7 +1248,8 @@ n_tty_receive_signal_char(struct tty_struct *tty, int signal, unsigned char c)
if (I_IXON(tty))
start_tty(tty);
if (L_ECHO(tty)) {
- echo_char(c, tty);
+ if (c != STATUS_CHAR(tty))
+ echo_char(c, tty);
commit_echoes(tty);
} else
process_echoes(tty);
@@ -1294,6 +1299,9 @@ n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
} else if (c == SUSP_CHAR(tty)) {
n_tty_receive_signal_char(tty, SIGTSTP, c);
return 0;
+ } else if (c == STATUS_CHAR(tty)) {
+ n_tty_receive_signal_char(tty, SIGINFO, c);
+ return 0;
}
}
@@ -1848,6 +1856,9 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
set_bit(INTR_CHAR(tty), ldata->char_map);
set_bit(QUIT_CHAR(tty), ldata->char_map);
set_bit(SUSP_CHAR(tty), ldata->char_map);
+ if (L_ICANON(tty) && L_IEXTEN(tty)) {
+ set_bit(STATUS_CHAR(tty), ldata->char_map);
+ }
}
clear_bit(__DISABLED_CHAR, ldata->char_map);
ldata->raw = 0;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index bd5fe0e90..8411fd18d 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -120,6 +120,7 @@ struct tty_bufhead {
#define WERASE_CHAR(tty) ((tty)->termios.c_cc[VWERASE])
#define LNEXT_CHAR(tty) ((tty)->termios.c_cc[VLNEXT])
#define EOL2_CHAR(tty) ((tty)->termios.c_cc[VEOL2])
+#define STATUS_CHAR(tty) ((tty)->termios.c_cc[VSTATUS])
#define _I_FLAG(tty, f) ((tty)->termios.c_iflag & (f))
#define _O_FLAG(tty, f) ((tty)->termios.c_oflag & (f))
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v3 4/7] linux/signal.h: Ignore SIGINFO by default in new tasks
2020-04-30 6:42 [PATCH v3 0/7] TTY Keyboard Status Request Arseny Maslennikov
` (2 preceding siblings ...)
2020-04-30 6:42 ` [PATCH v3 3/7] n_tty: Send SIGINFO to fg pgrp on status request character Arseny Maslennikov
@ 2020-04-30 6:42 ` Arseny Maslennikov
2020-04-30 6:53 ` Jiri Slaby
2020-04-30 6:42 ` [PATCH v3 5/7] tty: Add NOKERNINFO lflag to termios Arseny Maslennikov
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Arseny Maslennikov @ 2020-04-30 6:42 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
Cc: Arseny Maslennikov, Rob Landley, Eric W. Biederman, Pavel Machek,
linux-api, Vladimir D. Seleznev
This matches the behaviour of other Unix-like systems that have SIGINFO
and causes less harm to processes that do not install handlers for this
signal, making the keyboard status character non-fatal for them.
This is implemented with the assumption that SIGINFO is defined
to be equivalent to SIGPWR; still, there is no reason for PWR to
result in termination of the signal recipient anyway — it does not
indicate there is a fatal problem with the recipient's execution
context (like e.g. FPE/ILL do), and we have TERM/KILL for explicit
termination requests.
To put it another way:
The only scenario where system behaviour actually changes is when the
signal recipient has default disposition for SIGPWR. If a process
chose to interpret a SIGPWR as an incentive to cleanly terminate, it
would supply its own handler — and this commit does not affect processes
with non-default handlers.
Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
include/linux/signal.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 05bacd2ab..dc31da8fc 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -369,7 +369,7 @@ extern bool unhandled_signal(struct task_struct *tsk, int sig);
* | SIGSYS/SIGUNUSED | coredump |
* | SIGSTKFLT | terminate |
* | SIGWINCH | ignore |
- * | SIGPWR | terminate |
+ * | SIGPWR | ignore |
* | SIGRTMIN-SIGRTMAX | terminate |
* +--------------------+------------------+
* | non-POSIX signal | default action |
@@ -420,7 +420,8 @@ extern bool unhandled_signal(struct task_struct *tsk, int sig);
#define SIG_KERNEL_IGNORE_MASK (\
rt_sigmask(SIGCONT) | rt_sigmask(SIGCHLD) | \
- rt_sigmask(SIGWINCH) | rt_sigmask(SIGURG) )
+ rt_sigmask(SIGWINCH) | rt_sigmask(SIGURG) | \
+ rt_sigmask(SIGINFO) )
#define SIG_SPECIFIC_SICODES_MASK (\
rt_sigmask(SIGILL) | rt_sigmask(SIGFPE) | \
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 4/7] linux/signal.h: Ignore SIGINFO by default in new tasks
2020-04-30 6:42 ` [PATCH v3 4/7] linux/signal.h: Ignore SIGINFO by default in new tasks Arseny Maslennikov
@ 2020-04-30 6:53 ` Jiri Slaby
2020-04-30 7:14 ` Christian Brauner
0 siblings, 1 reply; 17+ messages in thread
From: Jiri Slaby @ 2020-04-30 6:53 UTC (permalink / raw)
To: Arseny Maslennikov, Greg Kroah-Hartman, linux-serial, linux-kernel
Cc: Rob Landley, Eric W. Biederman, Pavel Machek, linux-api,
Vladimir D. Seleznev
On 30. 04. 20, 8:42, Arseny Maslennikov wrote:
> This matches the behaviour of other Unix-like systems that have SIGINFO
> and causes less harm to processes that do not install handlers for this
> signal, making the keyboard status character non-fatal for them.
>
> This is implemented with the assumption that SIGINFO is defined
> to be equivalent to SIGPWR; still, there is no reason for PWR to
> result in termination of the signal recipient anyway — it does not
> indicate there is a fatal problem with the recipient's execution
> context (like e.g. FPE/ILL do), and we have TERM/KILL for explicit
> termination requests.
>
> To put it another way:
> The only scenario where system behaviour actually changes is when the
> signal recipient has default disposition for SIGPWR. If a process
> chose to interpret a SIGPWR as an incentive to cleanly terminate, it
> would supply its own handler — and this commit does not affect processes
> with non-default handlers.
>
> Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> ---
> include/linux/signal.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/signal.h b/include/linux/signal.h
> index 05bacd2ab..dc31da8fc 100644
> --- a/include/linux/signal.h
> +++ b/include/linux/signal.h
> @@ -369,7 +369,7 @@ extern bool unhandled_signal(struct task_struct *tsk, int sig);
> * | SIGSYS/SIGUNUSED | coredump |
> * | SIGSTKFLT | terminate |
> * | SIGWINCH | ignore |
> - * | SIGPWR | terminate |
> + * | SIGPWR | ignore |
You need to update signal.7 too:
https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man7/signal.7#n285
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 4/7] linux/signal.h: Ignore SIGINFO by default in new tasks
2020-04-30 6:53 ` Jiri Slaby
@ 2020-04-30 7:14 ` Christian Brauner
2020-04-30 7:19 ` Greg Kroah-Hartman
2020-04-30 7:37 ` Aleksa Sarai
0 siblings, 2 replies; 17+ messages in thread
From: Christian Brauner @ 2020-04-30 7:14 UTC (permalink / raw)
To: Jiri Slaby
Cc: Arseny Maslennikov, Greg Kroah-Hartman, linux-serial,
linux-kernel, Rob Landley, Eric W. Biederman, Pavel Machek,
linux-api, Vladimir D. Seleznev
On Thu, Apr 30, 2020 at 08:53:56AM +0200, Jiri Slaby wrote:
> On 30. 04. 20, 8:42, Arseny Maslennikov wrote:
> > This matches the behaviour of other Unix-like systems that have SIGINFO
> > and causes less harm to processes that do not install handlers for this
> > signal, making the keyboard status character non-fatal for them.
> >
> > This is implemented with the assumption that SIGINFO is defined
> > to be equivalent to SIGPWR; still, there is no reason for PWR to
> > result in termination of the signal recipient anyway — it does not
> > indicate there is a fatal problem with the recipient's execution
> > context (like e.g. FPE/ILL do), and we have TERM/KILL for explicit
> > termination requests.
> >
> > To put it another way:
> > The only scenario where system behaviour actually changes is when the
> > signal recipient has default disposition for SIGPWR. If a process
> > chose to interpret a SIGPWR as an incentive to cleanly terminate, it
> > would supply its own handler — and this commit does not affect processes
> > with non-default handlers.
> >
> > Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> > ---
> > include/linux/signal.h | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/signal.h b/include/linux/signal.h
> > index 05bacd2ab..dc31da8fc 100644
> > --- a/include/linux/signal.h
> > +++ b/include/linux/signal.h
> > @@ -369,7 +369,7 @@ extern bool unhandled_signal(struct task_struct *tsk, int sig);
> > * | SIGSYS/SIGUNUSED | coredump |
> > * | SIGSTKFLT | terminate |
> > * | SIGWINCH | ignore |
> > - * | SIGPWR | terminate |
> > + * | SIGPWR | ignore |
>
> You need to update signal.7 too:
> https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man7/signal.7#n285
(I fail this whole thread via b4 and it appears that a bunch of messages
are missing on lore. Might just be delay though.)
How this is this not going to break userspace?
Just for a start, SIGPWR (for better or worse) was used for a long time
by some sandboxing/container runtimes to shutdown a process and still
is.
Christian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 4/7] linux/signal.h: Ignore SIGINFO by default in new tasks
2020-04-30 7:14 ` Christian Brauner
@ 2020-04-30 7:19 ` Greg Kroah-Hartman
2020-04-30 7:37 ` Aleksa Sarai
1 sibling, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2020-04-30 7:19 UTC (permalink / raw)
To: Christian Brauner
Cc: Jiri Slaby, Arseny Maslennikov, linux-serial, linux-kernel,
Rob Landley, Eric W. Biederman, Pavel Machek, linux-api,
Vladimir D. Seleznev
On Thu, Apr 30, 2020 at 09:14:37AM +0200, Christian Brauner wrote:
> On Thu, Apr 30, 2020 at 08:53:56AM +0200, Jiri Slaby wrote:
> > On 30. 04. 20, 8:42, Arseny Maslennikov wrote:
> > > This matches the behaviour of other Unix-like systems that have SIGINFO
> > > and causes less harm to processes that do not install handlers for this
> > > signal, making the keyboard status character non-fatal for them.
> > >
> > > This is implemented with the assumption that SIGINFO is defined
> > > to be equivalent to SIGPWR; still, there is no reason for PWR to
> > > result in termination of the signal recipient anyway — it does not
> > > indicate there is a fatal problem with the recipient's execution
> > > context (like e.g. FPE/ILL do), and we have TERM/KILL for explicit
> > > termination requests.
> > >
> > > To put it another way:
> > > The only scenario where system behaviour actually changes is when the
> > > signal recipient has default disposition for SIGPWR. If a process
> > > chose to interpret a SIGPWR as an incentive to cleanly terminate, it
> > > would supply its own handler — and this commit does not affect processes
> > > with non-default handlers.
> > >
> > > Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> > > ---
> > > include/linux/signal.h | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/linux/signal.h b/include/linux/signal.h
> > > index 05bacd2ab..dc31da8fc 100644
> > > --- a/include/linux/signal.h
> > > +++ b/include/linux/signal.h
> > > @@ -369,7 +369,7 @@ extern bool unhandled_signal(struct task_struct *tsk, int sig);
> > > * | SIGSYS/SIGUNUSED | coredump |
> > > * | SIGSTKFLT | terminate |
> > > * | SIGWINCH | ignore |
> > > - * | SIGPWR | terminate |
> > > + * | SIGPWR | ignore |
> >
> > You need to update signal.7 too:
> > https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man7/signal.7#n285
>
> (I fail this whole thread via b4 and it appears that a bunch of messages
> are missing on lore. Might just be delay though.)
>
> How this is this not going to break userspace?
That's my main hesitation for taking this patchset.
> Just for a start, SIGPWR (for better or worse) was used for a long time
> by some sandboxing/container runtimes to shutdown a process and still
> is.
That's a good reason to not do this :(
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 4/7] linux/signal.h: Ignore SIGINFO by default in new tasks
2020-04-30 7:14 ` Christian Brauner
2020-04-30 7:19 ` Greg Kroah-Hartman
@ 2020-04-30 7:37 ` Aleksa Sarai
2020-04-30 8:00 ` Christian Brauner
1 sibling, 1 reply; 17+ messages in thread
From: Aleksa Sarai @ 2020-04-30 7:37 UTC (permalink / raw)
To: Christian Brauner
Cc: Jiri Slaby, Arseny Maslennikov, Greg Kroah-Hartman, linux-serial,
linux-kernel, Rob Landley, Eric W. Biederman, Pavel Machek,
linux-api, Vladimir D. Seleznev
[-- Attachment #1: Type: text/plain, Size: 3025 bytes --]
On 2020-04-30, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> On Thu, Apr 30, 2020 at 08:53:56AM +0200, Jiri Slaby wrote:
> > On 30. 04. 20, 8:42, Arseny Maslennikov wrote:
> > > This matches the behaviour of other Unix-like systems that have SIGINFO
> > > and causes less harm to processes that do not install handlers for this
> > > signal, making the keyboard status character non-fatal for them.
> > >
> > > This is implemented with the assumption that SIGINFO is defined
> > > to be equivalent to SIGPWR; still, there is no reason for PWR to
> > > result in termination of the signal recipient anyway — it does not
> > > indicate there is a fatal problem with the recipient's execution
> > > context (like e.g. FPE/ILL do), and we have TERM/KILL for explicit
> > > termination requests.
> > >
> > > To put it another way:
> > > The only scenario where system behaviour actually changes is when the
> > > signal recipient has default disposition for SIGPWR. If a process
> > > chose to interpret a SIGPWR as an incentive to cleanly terminate, it
> > > would supply its own handler — and this commit does not affect processes
> > > with non-default handlers.
> > >
> > > Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> > > ---
> > > include/linux/signal.h | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/linux/signal.h b/include/linux/signal.h
> > > index 05bacd2ab..dc31da8fc 100644
> > > --- a/include/linux/signal.h
> > > +++ b/include/linux/signal.h
> > > @@ -369,7 +369,7 @@ extern bool unhandled_signal(struct task_struct *tsk, int sig);
> > > * | SIGSYS/SIGUNUSED | coredump |
> > > * | SIGSTKFLT | terminate |
> > > * | SIGWINCH | ignore |
> > > - * | SIGPWR | terminate |
> > > + * | SIGPWR | ignore |
> >
> > You need to update signal.7 too:
> > https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man7/signal.7#n285
>
> (I fail this whole thread via b4 and it appears that a bunch of messages
> are missing on lore. Might just be delay though.)
>
> How this is this not going to break userspace? Just for a start,
> SIGPWR (for better or worse) was used for a long time by some
> sandboxing/container runtimes to shutdown a process and still is.
To play Devil's advocate -- pid1 has also always had a default-ignore
signal mask (which included SIGPWR), so any pid1 that obeyed SIGPWR
already had a non-default signal mask (and thus wouldn't be affected by
this patch).
But I do agree that this seems like a strange change to make (SIGPWR
seems like a signal you don't want to ignore by default). Unfortunately
the fact that it appears to always be equal to SIGINFO means that while
SIGINFO (to me at least) seems like it should be a no-op, the necessary
SIGPWR change makes it harder to justify IMHO.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 4/7] linux/signal.h: Ignore SIGINFO by default in new tasks
2020-04-30 7:37 ` Aleksa Sarai
@ 2020-04-30 8:00 ` Christian Brauner
0 siblings, 0 replies; 17+ messages in thread
From: Christian Brauner @ 2020-04-30 8:00 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Jiri Slaby, Arseny Maslennikov, Greg Kroah-Hartman, linux-serial,
linux-kernel, Rob Landley, Eric W. Biederman, Pavel Machek,
linux-api, Vladimir D. Seleznev
On Thu, Apr 30, 2020 at 05:37:28PM +1000, Aleksa Sarai wrote:
> On 2020-04-30, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> > On Thu, Apr 30, 2020 at 08:53:56AM +0200, Jiri Slaby wrote:
> > > On 30. 04. 20, 8:42, Arseny Maslennikov wrote:
> > > > This matches the behaviour of other Unix-like systems that have SIGINFO
> > > > and causes less harm to processes that do not install handlers for this
> > > > signal, making the keyboard status character non-fatal for them.
> > > >
> > > > This is implemented with the assumption that SIGINFO is defined
> > > > to be equivalent to SIGPWR; still, there is no reason for PWR to
> > > > result in termination of the signal recipient anyway — it does not
> > > > indicate there is a fatal problem with the recipient's execution
> > > > context (like e.g. FPE/ILL do), and we have TERM/KILL for explicit
> > > > termination requests.
> > > >
> > > > To put it another way:
> > > > The only scenario where system behaviour actually changes is when the
> > > > signal recipient has default disposition for SIGPWR. If a process
> > > > chose to interpret a SIGPWR as an incentive to cleanly terminate, it
> > > > would supply its own handler — and this commit does not affect processes
> > > > with non-default handlers.
> > > >
> > > > Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> > > > ---
> > > > include/linux/signal.h | 5 +++--
> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/include/linux/signal.h b/include/linux/signal.h
> > > > index 05bacd2ab..dc31da8fc 100644
> > > > --- a/include/linux/signal.h
> > > > +++ b/include/linux/signal.h
> > > > @@ -369,7 +369,7 @@ extern bool unhandled_signal(struct task_struct *tsk, int sig);
> > > > * | SIGSYS/SIGUNUSED | coredump |
> > > > * | SIGSTKFLT | terminate |
> > > > * | SIGWINCH | ignore |
> > > > - * | SIGPWR | terminate |
> > > > + * | SIGPWR | ignore |
> > >
> > > You need to update signal.7 too:
> > > https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man7/signal.7#n285
> >
> > (I fail this whole thread via b4 and it appears that a bunch of messages
> > are missing on lore. Might just be delay though.)
> >
> > How this is this not going to break userspace? Just for a start,
> > SIGPWR (for better or worse) was used for a long time by some
> > sandboxing/container runtimes to shutdown a process and still is.
>
> To play Devil's advocate -- pid1 has also always had a default-ignore
> signal mask (which included SIGPWR), so any pid1 that obeyed SIGPWR
> already had a non-default signal mask (and thus wouldn't be affected by
> this patch).
Sure, my point wasn't specifically about init systems but rather generic
processes. The reason that SIGPWR was originally used was because
older init systems - apart from systemd - could be shutdown with it
while other programs left it set to SIG_DFL and they would terminate
too. I'm not saying this is a great idea. But changing SIGPWR - if I
read this right - to go from "terminate" to "ignore" will mean that any
program that left SIGPWR unhandled/SIG_DFL will potentially see altered
behavior. Just looking at gvfsd and in debian.codesearch shows that they
explicitly set signal(SIGPWR, SIG_DFL) and I'm sure there are more.
(You also need to keep in mind that the default ignore mask applies to
signals sent from _within_ the pid namespace to pid 1. They'll be
delivered just fine from an ancestor pid namespace. Otherwise I'd be
interested to know how you've ever shutdown one of your containers. ;))
>
> But I do agree that this seems like a strange change to make (SIGPWR
> seems like a signal you don't want to ignore by default). Unfortunately
> the fact that it appears to always be equal to SIGINFO means that while
> SIGINFO (to me at least) seems like it should be a no-op, the necessary
> SIGPWR change makes it harder to justify IMHO.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 5/7] tty: Add NOKERNINFO lflag to termios
2020-04-30 6:42 [PATCH v3 0/7] TTY Keyboard Status Request Arseny Maslennikov
` (3 preceding siblings ...)
2020-04-30 6:42 ` [PATCH v3 4/7] linux/signal.h: Ignore SIGINFO by default in new tasks Arseny Maslennikov
@ 2020-04-30 6:42 ` Arseny Maslennikov
2020-04-30 6:55 ` Jiri Slaby
2020-04-30 6:43 ` [PATCH v3 6/7] n_tty: ->ops->write: Cut core logic out to a separate function Arseny Maslennikov
2020-04-30 6:43 ` [PATCH v3 7/7] n_tty: Provide an informational line on VSTATUS receipt Arseny Maslennikov
6 siblings, 1 reply; 17+ messages in thread
From: Arseny Maslennikov @ 2020-04-30 6:42 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
Cc: Arseny Maslennikov, Rob Landley, Eric W. Biederman, Pavel Machek,
linux-api, Vladimir D. Seleznev
The termios lflag is off by default.
Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
arch/alpha/include/uapi/asm/termbits.h | 1 +
arch/ia64/include/uapi/asm/termbits.h | 1 +
arch/mips/include/uapi/asm/termbits.h | 1 +
arch/parisc/include/uapi/asm/termbits.h | 1 +
arch/powerpc/include/uapi/asm/termbits.h | 1 +
arch/sparc/include/uapi/asm/termbits.h | 1 +
arch/xtensa/include/uapi/asm/termbits.h | 1 +
include/linux/tty.h | 1 +
include/uapi/asm-generic/termbits.h | 1 +
9 files changed, 9 insertions(+)
diff --git a/arch/alpha/include/uapi/asm/termbits.h b/arch/alpha/include/uapi/asm/termbits.h
index bb895d467..850dd8dc4 100644
--- a/arch/alpha/include/uapi/asm/termbits.h
+++ b/arch/alpha/include/uapi/asm/termbits.h
@@ -204,6 +204,7 @@ struct ktermios {
#define PENDIN 0x20000000
#define IEXTEN 0x00000400
#define EXTPROC 0x10000000
+#define NOKERNINFO 0x40000000
/* Values for the ACTION argument to `tcflow'. */
#define TCOOFF 0
diff --git a/arch/ia64/include/uapi/asm/termbits.h b/arch/ia64/include/uapi/asm/termbits.h
index 86898e4c5..f777b99bc 100644
--- a/arch/ia64/include/uapi/asm/termbits.h
+++ b/arch/ia64/include/uapi/asm/termbits.h
@@ -190,6 +190,7 @@ struct ktermios {
#define PENDIN 0040000
#define IEXTEN 0100000
#define EXTPROC 0200000
+#define NOKERNINFO 0400000
/* tcflow() and TCXONC use these */
#define TCOOFF 0
diff --git a/arch/mips/include/uapi/asm/termbits.h b/arch/mips/include/uapi/asm/termbits.h
index 3dd60924f..2755cfd14 100644
--- a/arch/mips/include/uapi/asm/termbits.h
+++ b/arch/mips/include/uapi/asm/termbits.h
@@ -206,6 +206,7 @@ struct ktermios {
#define TOSTOP 0100000 /* Send SIGTTOU for background output. */
#define ITOSTOP TOSTOP
#define EXTPROC 0200000 /* External processing on pty */
+#define NOKERNINFO 0400000 /* Disable kernel output from VSTATUS. */
/* ioctl (fd, TIOCSERGETLSR, &result) where result may be as below */
#define TIOCSER_TEMT 0x01 /* Transmitter physically empty */
diff --git a/arch/parisc/include/uapi/asm/termbits.h b/arch/parisc/include/uapi/asm/termbits.h
index ecca3b7d0..c7ba145ad 100644
--- a/arch/parisc/include/uapi/asm/termbits.h
+++ b/arch/parisc/include/uapi/asm/termbits.h
@@ -183,6 +183,7 @@ struct ktermios {
#define PENDIN 0040000
#define IEXTEN 0100000
#define EXTPROC 0200000
+#define NOKERNINFO 0400000
/* tcflow() and TCXONC use these */
#define TCOOFF 0
diff --git a/arch/powerpc/include/uapi/asm/termbits.h b/arch/powerpc/include/uapi/asm/termbits.h
index c85e98d75..79dbcc546 100644
--- a/arch/powerpc/include/uapi/asm/termbits.h
+++ b/arch/powerpc/include/uapi/asm/termbits.h
@@ -192,6 +192,7 @@ struct ktermios {
#define PENDIN 0x20000000
#define IEXTEN 0x00000400
#define EXTPROC 0x10000000
+#define NOKERNINFO 0x40000000
/* Values for the ACTION argument to `tcflow'. */
#define TCOOFF 0
diff --git a/arch/sparc/include/uapi/asm/termbits.h b/arch/sparc/include/uapi/asm/termbits.h
index a1638c9cd..108c039dd 100644
--- a/arch/sparc/include/uapi/asm/termbits.h
+++ b/arch/sparc/include/uapi/asm/termbits.h
@@ -225,6 +225,7 @@ struct ktermios {
#define PENDIN 0x00004000
#define IEXTEN 0x00008000
#define EXTPROC 0x00010000
+#define NOKERNINFO 0x00020000
/* modem lines */
#define TIOCM_LE 0x001
diff --git a/arch/xtensa/include/uapi/asm/termbits.h b/arch/xtensa/include/uapi/asm/termbits.h
index 77969cb27..6155e1c81 100644
--- a/arch/xtensa/include/uapi/asm/termbits.h
+++ b/arch/xtensa/include/uapi/asm/termbits.h
@@ -199,6 +199,7 @@ struct ktermios {
#define PENDIN 0040000
#define IEXTEN 0100000
#define EXTPROC 0200000
+#define NOKERNINFO 0400000
/* tcflow() and TCXONC use these */
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 8411fd18d..cafb9b7f7 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -186,6 +186,7 @@ struct tty_bufhead {
#define L_PENDIN(tty) _L_FLAG((tty), PENDIN)
#define L_IEXTEN(tty) _L_FLAG((tty), IEXTEN)
#define L_EXTPROC(tty) _L_FLAG((tty), EXTPROC)
+#define L_NOKERNINFO(tty) _L_FLAG((tty), NOKERNINFO)
struct device;
struct signal_struct;
diff --git a/include/uapi/asm-generic/termbits.h b/include/uapi/asm-generic/termbits.h
index 11bb142ba..6219803d6 100644
--- a/include/uapi/asm-generic/termbits.h
+++ b/include/uapi/asm-generic/termbits.h
@@ -181,6 +181,7 @@ struct ktermios {
#define PENDIN 0040000
#define IEXTEN 0100000
#define EXTPROC 0200000
+#define NOKERNINFO 0400000
/* tcflow() and TCXONC use these */
#define TCOOFF 0
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 5/7] tty: Add NOKERNINFO lflag to termios
2020-04-30 6:42 ` [PATCH v3 5/7] tty: Add NOKERNINFO lflag to termios Arseny Maslennikov
@ 2020-04-30 6:55 ` Jiri Slaby
2020-04-30 7:20 ` Arseny Maslennikov
0 siblings, 1 reply; 17+ messages in thread
From: Jiri Slaby @ 2020-04-30 6:55 UTC (permalink / raw)
To: Arseny Maslennikov, Greg Kroah-Hartman, linux-serial, linux-kernel
Cc: Rob Landley, Eric W. Biederman, Pavel Machek, linux-api,
Vladimir D. Seleznev
On 30. 04. 20, 8:42, Arseny Maslennikov wrote:
> The termios lflag is off by default.
This commit message is too poor. Describing the intended use would help.
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 5/7] tty: Add NOKERNINFO lflag to termios
2020-04-30 6:55 ` Jiri Slaby
@ 2020-04-30 7:20 ` Arseny Maslennikov
0 siblings, 0 replies; 17+ messages in thread
From: Arseny Maslennikov @ 2020-04-30 7:20 UTC (permalink / raw)
To: Jiri Slaby
Cc: Greg Kroah-Hartman, linux-serial, linux-kernel, Rob Landley,
Eric W. Biederman, Pavel Machek, linux-api, Vladimir D. Seleznev
[-- Attachment #1: Type: text/plain, Size: 517 bytes --]
On Thu, Apr 30, 2020 at 08:55:43AM +0200, Jiri Slaby wrote:
> On 30. 04. 20, 8:42, Arseny Maslennikov wrote:
> > The termios lflag is off by default.
>
> This commit message is too poor. Describing the intended use would help.
>
I described its use in the last patch of the series, where it actually
gains some use.
I agree, however, that the limited explanation in this commit does not
look helpful at all when looking at e.g. git blame output.
I'll resend a v4 with this commit message improved.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 6/7] n_tty: ->ops->write: Cut core logic out to a separate function
2020-04-30 6:42 [PATCH v3 0/7] TTY Keyboard Status Request Arseny Maslennikov
` (4 preceding siblings ...)
2020-04-30 6:42 ` [PATCH v3 5/7] tty: Add NOKERNINFO lflag to termios Arseny Maslennikov
@ 2020-04-30 6:43 ` Arseny Maslennikov
2020-04-30 6:43 ` [PATCH v3 7/7] n_tty: Provide an informational line on VSTATUS receipt Arseny Maslennikov
6 siblings, 0 replies; 17+ messages in thread
From: Arseny Maslennikov @ 2020-04-30 6:43 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
Cc: Arseny Maslennikov, Rob Landley, Eric W. Biederman, Pavel Machek,
linux-api, Vladimir D. Seleznev
To simplify internal re-use of the line discipline's write method,
we isolate the work it does to its own function.
Since in-kernel callers might not refer to the tty through a file,
the struct file* argument might make no sense, so we also stop
tty_io_nonblock() from dereferencing file too early, allowing
to pass NULL as the referring file here.
Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
drivers/tty/n_tty.c | 33 ++++++++++++++++++++++-----------
include/linux/tty.h | 2 +-
2 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 10d6b60a5..f72a3fd4b 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2309,22 +2309,15 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
* lock themselves)
*/
-static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
- const unsigned char *buf, size_t nr)
+static ssize_t do_n_tty_write(struct tty_struct *tty, struct file *file,
+ const unsigned char *buf, size_t nr)
{
const unsigned char *b = buf;
DEFINE_WAIT_FUNC(wait, woken_wake_function);
int c;
ssize_t retval = 0;
- /* Job control check -- must be done at start (POSIX.1 7.1.1.4). */
- if (L_TOSTOP(tty) && file->f_op->write != redirected_tty_write) {
- retval = tty_check_change(tty);
- if (retval)
- return retval;
- }
-
- down_read(&tty->termios_rwsem);
+ lockdep_assert_held_read(&tty->termios_rwsem);
/* Write out any echoed characters that are still pending */
process_echoes(tty);
@@ -2392,10 +2385,28 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
remove_wait_queue(&tty->write_wait, &wait);
if (nr && tty->fasync)
set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
- up_read(&tty->termios_rwsem);
return (b - buf) ? b - buf : retval;
}
+static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
+ const unsigned char *buf, size_t nr)
+{
+ ssize_t retval = 0;
+
+ /* Job control check -- must be done at start (POSIX.1 7.1.1.4). */
+ if (L_TOSTOP(tty) && file->f_op->write != redirected_tty_write) {
+ retval = tty_check_change(tty);
+ if (retval)
+ return retval;
+ }
+
+ down_read(&tty->termios_rwsem);
+ retval = do_n_tty_write(tty, file, buf, nr);
+ up_read(&tty->termios_rwsem);
+
+ return retval;
+}
+
/**
* n_tty_poll - poll method for N_TTY
* @tty: terminal device
diff --git a/include/linux/tty.h b/include/linux/tty.h
index cafb9b7f7..3499845ab 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -390,7 +390,7 @@ static inline void tty_set_flow_change(struct tty_struct *tty, int val)
static inline bool tty_io_nonblock(struct tty_struct *tty, struct file *file)
{
- return file->f_flags & O_NONBLOCK ||
+ return (file && file->f_flags & O_NONBLOCK) ||
test_bit(TTY_LDISC_CHANGING, &tty->flags);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v3 7/7] n_tty: Provide an informational line on VSTATUS receipt
2020-04-30 6:42 [PATCH v3 0/7] TTY Keyboard Status Request Arseny Maslennikov
` (5 preceding siblings ...)
2020-04-30 6:43 ` [PATCH v3 6/7] n_tty: ->ops->write: Cut core logic out to a separate function Arseny Maslennikov
@ 2020-04-30 6:43 ` Arseny Maslennikov
2020-04-30 7:29 ` Jiri Slaby
6 siblings, 1 reply; 17+ messages in thread
From: Arseny Maslennikov @ 2020-04-30 6:43 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
Cc: Arseny Maslennikov, Rob Landley, Eric W. Biederman, Pavel Machek,
linux-api, Vladimir D. Seleznev
If the three termios local flags isig, icanon, iexten are enabled
and the local flag nokerninfo is disabled for a tty governed
by the n_tty line discipline, then on receiving the keyboard status
character n_tty will generate a status message and write it out to
the tty before sending SIGINFO to the tty's foreground process group.
This kerninfo line contains information about the current system load
as well as some properties of "the most interesting" process in the
tty's current foreground process group, namely:
- its PID as seen inside its deepest PID namespace;
* the whole process group ought to be in a single PID namespace,
so this is actually deterministic
- its saved command name truncated to 16 bytes (task_struct::comm);
* at the time of writing TASK_COMM_LEN == 16
- its state and some related bits, procps-style;
- for S and D: its symbolic wait channel, if available; or a short
description for other process states instead;
- its user, system and real rusage time values;
- its resident set size (as well as the high watermark) in kilobytes.
The "most interesting" process is chosen as follows:
- runnables over everything
- uninterruptibles over everything else
- among 2 runnables pick the biggest utime + stime
- any unresolved ties are decided in favour of greatest PID.
While the kerninfo line is not very useful for debugging the kernel
itself, since we have much more powerful debugging tools, it still gives
the user behind the terminal some meaningful feedback to a VSTATUS that
works even if no processes respond.
Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
drivers/tty/Makefile | 3 +-
drivers/tty/n_tty.c | 22 +++
drivers/tty/n_tty_status.c | 338 +++++++++++++++++++++++++++++++++++++
include/linux/sched.h | 7 +
include/linux/tty.h | 3 +
5 files changed, 372 insertions(+), 1 deletion(-)
create mode 100644 drivers/tty/n_tty_status.c
diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
index 020b1cd92..8bb5efb40 100644
--- a/drivers/tty/Makefile
+++ b/drivers/tty/Makefile
@@ -1,7 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_TTY) += tty_io.o n_tty.o tty_ioctl.o tty_ldisc.o \
+obj-$(CONFIG_TTY) += tty_io.o tty_ioctl.o tty_ldisc.o \
tty_buffer.o tty_port.o tty_mutex.o \
tty_ldsem.o tty_baudrate.o tty_jobctrl.o \
+ n_tty.o n_tty_status.o \
n_null.o
obj-$(CONFIG_LEGACY_PTYS) += pty.o
obj-$(CONFIG_UNIX98_PTYS) += pty.o
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index f72a3fd4b..905cdd985 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -34,6 +34,7 @@
#include <linux/signal.h>
#include <linux/fcntl.h>
#include <linux/sched.h>
+#include <linux/kallsyms.h>
#include <linux/interrupt.h>
#include <linux/tty.h>
#include <linux/timer.h>
@@ -79,6 +80,8 @@
#define ECHO_BLOCK 256
#define ECHO_DISCARD_WATERMARK N_TTY_BUF_SIZE - (ECHO_BLOCK + 32)
+#define STATUS_LINE_LEN (2 * KSYM_NAME_LEN)
+
#define SIG_FLUSHING_MASK ( \
rt_sigmask(SIGINT) | rt_sigmask(SIGQUIT) | \
rt_sigmask(SIGTSTP) )
@@ -158,6 +161,8 @@ static inline unsigned char *echo_buf_addr(struct n_tty_data *ldata, size_t i)
return &ldata->echo_buf[i & (N_TTY_BUF_SIZE - 1)];
}
+static void n_tty_status_line(struct tty_struct *tty);
+
/* If we are not echoing the data, perhaps this is a secret so erase it */
static void zero_buffer(struct tty_struct *tty, u8 *buffer, int size)
{
@@ -1300,6 +1305,8 @@ n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
n_tty_receive_signal_char(tty, SIGTSTP, c);
return 0;
} else if (c == STATUS_CHAR(tty)) {
+ if (!L_NOKERNINFO(tty))
+ n_tty_status_line(tty);
n_tty_receive_signal_char(tty, SIGINFO, c);
return 0;
}
@@ -2489,6 +2496,21 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
}
}
+static void n_tty_status_line(struct tty_struct *tty)
+{
+ struct n_tty_data *ldata = tty->disc_data;
+ char *msg, *buf;
+ msg = buf = kzalloc(STATUS_LINE_LEN, GFP_KERNEL);
+ tty_sprint_status_line(tty, buf + 1, STATUS_LINE_LEN - 1);
+ /* The only current caller of this takes output_lock for us. */
+ if (ldata->column != 0)
+ *msg = '\n';
+ else
+ msg++;
+ do_n_tty_write(tty, NULL, msg, strlen(msg));
+ kfree(buf);
+}
+
static struct tty_ldisc_ops n_tty_ops = {
.magic = TTY_LDISC_MAGIC,
.name = "n_tty",
diff --git a/drivers/tty/n_tty_status.c b/drivers/tty/n_tty_status.c
new file mode 100644
index 000000000..d92261bbe
--- /dev/null
+++ b/drivers/tty/n_tty_status.c
@@ -0,0 +1,338 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/kallsyms.h>
+#include <linux/pid.h>
+#include <linux/sched.h>
+#include <linux/sched/signal.h>
+#include <linux/sched/loadavg.h>
+#include <linux/sched/cputime.h>
+#include <linux/sched/mm.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+
+#define BCOMPARE(lbool, rbool) ((lbool) << 1 | (rbool))
+#define BCOMPARE_NONE 0
+#define BCOMPARE_RIGHT 1
+#define BCOMPARE_LEFT 2
+#define BCOMPARE_BOTH (BCOMPARE_LEFT | BCOMPARE_RIGHT)
+
+/*
+ * Select the most interesting task of two.
+ *
+ * The implemented approach is simple for now:
+ * - pick runnable
+ * - if no runnables, pick uninterruptible
+ * - if tie between runnables, pick highest utime + stime
+ * - if a tie is not broken by the above, pick highest pid nr.
+ *
+ * For reference, here's the one used in FreeBSD:
+ * - pick runnables over anything
+ * - if both runnables, pick highest cpu utilization
+ * - if no runnables, pick shortest sleep time (the scheduler
+ * actually takes care to maintain this statistic)
+ * - other ties are decided in favour of youngest process.
+ */
+static struct task_struct *__better_proc_R(struct task_struct *a,
+ struct task_struct *b)
+{
+ unsigned long flags;
+ u64 atime, btime, tgutime, tgstime;
+ struct task_struct *ret;
+
+ if (!lock_task_sighand(a, &flags))
+ goto out_a_unlocked;
+ thread_group_cputime_adjusted(a, &tgutime, &tgstime);
+ atime = tgutime + tgstime;
+ unlock_task_sighand(a, &flags);
+
+ if (!lock_task_sighand(b, &flags))
+ goto out_b_unlocked;
+ thread_group_cputime_adjusted(b, &tgutime, &tgstime);
+ btime = tgutime + tgstime;
+ unlock_task_sighand(b, &flags);
+
+ ret = atime > btime ? a : b;
+
+ return ret;
+
+out_b_unlocked:
+out_a_unlocked:
+ return task_pid_vinr(a) > task_pid_vinr(b) ? a : b;
+}
+
+static struct task_struct *__better_proc(struct task_struct *a,
+ struct task_struct *b)
+{
+ if (!pid_alive(a))
+ return b;
+ if (!pid_alive(b))
+ return a;
+
+ switch (BCOMPARE(a->state == TASK_RUNNING,
+ b->state == TASK_RUNNING)) {
+ case BCOMPARE_LEFT:
+ return a;
+ case BCOMPARE_RIGHT:
+ return b;
+ case BCOMPARE_BOTH:
+ return __better_proc_R(a, b);
+ }
+
+ switch (BCOMPARE(a->state == TASK_UNINTERRUPTIBLE,
+ b->state == TASK_UNINTERRUPTIBLE)) {
+ case BCOMPARE_LEFT:
+ return a;
+ case BCOMPARE_RIGHT:
+ return b;
+ case BCOMPARE_BOTH:
+ break;
+ }
+
+ /* TODO: Perhaps we should check something else... */
+ return task_pid_vinr(a) > task_pid_vinr(b) ? a : b;
+}
+
+/* Weed out NULLs. */
+static inline struct task_struct *better_proc(struct task_struct *a,
+ struct task_struct *b) {
+ return a ? (b ? __better_proc(a, b) : a) : b;
+}
+
+static int scnprint_load(char *msgp, size_t size)
+{
+ unsigned long la[3];
+
+ get_avenrun(la, FIXED_1/200, 0);
+ return scnprintf(msgp, size, "load: %lu.%02lu; ",
+ LOAD_INT(la[0]), LOAD_FRAC(la[0]));
+}
+
+static int scnprint_task(char *msgp, size_t size, struct task_struct *task)
+{
+ char commname[TASK_COMM_LEN];
+
+ get_task_comm(commname, task);
+ return scnprintf(msgp, size, "%d/%s:", task_pid_vinr(task), commname);
+}
+
+static int scnprint_rusage(char *msgp, ssize_t size,
+ struct task_struct *task, struct mm_struct *mm)
+{
+ struct rusage ru;
+ struct timespec64 utime, stime;
+ struct timespec64 rtime;
+ u64 now;
+ int ret = 0;
+ int psz = 0;
+
+ getrusage(task, RUSAGE_BOTH, &ru);
+ now = ktime_get_ns();
+
+ utime.tv_sec = ru.ru_utime.tv_sec;
+ utime.tv_nsec = ru.ru_utime.tv_usec * NSEC_PER_USEC;
+ stime.tv_sec = ru.ru_stime.tv_sec;
+ stime.tv_nsec = ru.ru_stime.tv_usec * NSEC_PER_USEC;
+
+ rtime = ns_to_timespec64(now - task->start_time);
+
+ psz = scnprintf(msgp, size,
+ "%llu.%03lur %llu.%03luu %llu.%03lus",
+ rtime.tv_sec, rtime.tv_nsec / NSEC_PER_MSEC,
+ utime.tv_sec, utime.tv_nsec / NSEC_PER_MSEC,
+ stime.tv_sec, stime.tv_nsec / NSEC_PER_MSEC);
+ ret += psz;
+ msgp += psz;
+ size -= psz;
+
+ if (mm) {
+ psz = scnprintf(msgp, size,
+ " %luk/%luk",
+ get_mm_rss(mm) * PAGE_SIZE / 1024,
+ get_mm_hiwater_rss(mm) * PAGE_SIZE / 1024);
+ ret += psz;
+ }
+ return ret;
+}
+
+static int scnprint_state(char *msgp, ssize_t size,
+ struct task_struct *task, struct mm_struct *mm)
+{
+ char stat[8] = {0};
+ const char *state_descr = "";
+ struct task_struct *parent = NULL;
+ struct task_struct *real_parent = NULL;
+ unsigned long wchan = 0;
+ int psz = 0;
+ char symname[KSYM_NAME_LEN];
+
+ stat[psz++] = task_state_to_char(task);
+ if (task_nice(task) < 0)
+ stat[psz++] = '<';
+ else if (task_nice(task) > 0)
+ stat[psz++] = 'N';
+ if (mm && mm->locked_vm)
+ stat[psz++] = 'L';
+ if (get_nr_threads(task) > 1)
+ stat[psz++] = 'l';
+
+ switch (stat[0]) {
+ case 'R':
+ if (task_curr(task))
+ stat[psz++] = '!';
+ break;
+ case 'S':
+ case 'D':
+ wchan = get_wchan(task);
+ if (!wchan)
+ break;
+ if (!lookup_symbol_name(wchan, symname))
+ state_descr = symname;
+ else
+ /* get_wchan() returned something
+ * that looks like no kernel symbol.
+ */
+ state_descr = "*unknown*";
+ break;
+ case 'T':
+ state_descr = "stopped";
+ break;
+ case 't':
+ state_descr = "traced";
+ break;
+ case 'Z':
+ rcu_read_lock();
+ real_parent = rcu_dereference(task->real_parent);
+ parent = rcu_dereference(task->parent);
+ psz = sprintf(symname, "zombie; ppid=%d",
+ task_tgid_nr_ns(real_parent,
+ ns_of_pid(task_pid(task))));
+ if (parent != real_parent)
+ sprintf(symname + psz, " reaper=%d",
+ task_tgid_nr_ns(parent,
+ ns_of_pid(task_pid(task))));
+ rcu_read_unlock();
+ state_descr = symname;
+ break;
+ case 'I':
+ /* Can this even happen? */
+ state_descr = "idle";
+ break;
+ default:
+ state_descr = "unknown";
+ }
+
+ psz = scnprintf(msgp, size, "%s", stat);
+ msgp += psz;
+ size -= psz;
+ if (*state_descr)
+ psz += scnprintf(msgp, size, wchan ? " [%s]" : " (%s)", state_descr);
+
+ return psz;
+}
+
+/**
+ * tty_sprint_status_line - produce kerninfo line
+ * @tty: terminal device
+ * @msg: preallocated memory buffer
+ * @length: maximum line length
+ *
+ * Reports state of foreground process group in a null-terminated string
+ * located at @msg, @length bytes long. If @length is insufficient,
+ * the line gets truncated.
+ */
+void tty_sprint_status_line(struct tty_struct *t, char *msg, size_t length)
+{
+ struct task_struct *tsk = NULL, *mit = NULL;
+ struct mm_struct *mm;
+ struct pid *pgrp = NULL;
+ char *msgp = msg;
+ int psz = 0;
+
+ if (!length)
+ return;
+ length--; /* Make room for trailing '\n' */
+
+ psz = scnprint_load(msgp, length);
+ if (psz > 0) {
+ msgp += psz;
+ length -= psz;
+ }
+ if (!length)
+ goto finalize_message;
+
+ /* Not sure if session pid is protected by ctrl_lock
+ * or tasklist_lock...
+ */
+ pgrp = t->session;
+ if (pgrp == NULL) {
+ psz = scnprintf(msgp, length, "not a controlling tty");
+ if (psz > 0)
+ msgp += psz;
+ goto finalize_message;
+ }
+ pgrp = tty_get_pgrp(t);
+ if (pgrp == NULL) {
+ psz = scnprintf(msgp, length, "no foreground process group");
+ if (psz > 0)
+ msgp += psz;
+ goto finalize_message;
+ }
+
+ read_lock(&tasklist_lock);
+ do_each_pid_task(pgrp, PIDTYPE_PGID, tsk)
+ {
+ /* Select the most interesting task. */
+ if (tsk == better_proc(mit, tsk))
+ mit = tsk;
+ } while_each_pid_task(pgrp, PIDTYPE_PGID, tsk);
+ read_unlock(&tasklist_lock);
+
+ if (!pid_alive(mit))
+ goto finalize_message;
+
+ /* Gather intel on most interesting task. */
+ /* Can the mm of a foreground process turn out to be NULL?
+ * Definitely; for example, if it is a zombie.
+ */
+ mm = get_task_mm(mit);
+
+ psz = scnprint_task(msgp, length, mit);
+ if (psz > 0) {
+ msgp += psz;
+ length -= psz;
+ }
+ if (!length)
+ goto finalize_message;
+ *msgp++ = ' ';
+ length--;
+
+ psz = scnprint_state(msgp, length, mit, mm);
+ if (psz > 0) {
+ msgp += psz;
+ length -= psz;
+ }
+ if (!length)
+ goto finalize_message;
+ *msgp++ = ' ';
+ length--;
+
+ psz = scnprint_rusage(msgp, length, mit, mm);
+ if (psz > 0) {
+ msgp += psz;
+ length -= psz;
+ }
+ if (!length)
+ goto finalize_message;
+ *msgp++ = ' ';
+ length--;
+
+ if (!mm)
+ goto finalize_message;
+
+ mmput(mm);
+
+finalize_message:
+ *msgp++ = '\n';
+ if (pgrp)
+ put_pid(pgrp);
+}
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 4418f5cb8..195abd47d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1318,6 +1318,8 @@ static inline struct pid *task_pid(struct task_struct *task)
* task_xid_nr() : global id, i.e. the id seen from the init namespace;
* task_xid_vnr() : virtual id, i.e. the id seen from the pid namespace of
* current.
+ * task_xid_vinr() : virtual inner id, i.e. the id seen from the namespace of
+ * the task itself;
* task_xid_nr_ns() : id seen from the ns specified;
*
* see also pid_nr() etc in include/linux/pid.h
@@ -1339,6 +1341,11 @@ static inline pid_t task_pid_vnr(struct task_struct *tsk)
return __task_pid_nr_ns(tsk, PIDTYPE_PID, NULL);
}
+static inline pid_t task_pid_vinr(struct task_struct *tsk)
+{
+ return __task_pid_nr_ns(tsk, PIDTYPE_PID, ns_of_pid(task_pid(tsk)));
+}
+
static inline pid_t task_tgid_nr(struct task_struct *tsk)
{
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 3499845ab..2023addaf 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -727,6 +727,9 @@ extern void __init n_tty_init(void);
static inline void n_tty_init(void) { }
#endif
+/* n_tty_status.c */
+extern void tty_sprint_status_line(struct tty_struct *tty, char *msg, size_t size);
+
/* tty_audit.c */
#ifdef CONFIG_AUDIT
extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 7/7] n_tty: Provide an informational line on VSTATUS receipt
2020-04-30 6:43 ` [PATCH v3 7/7] n_tty: Provide an informational line on VSTATUS receipt Arseny Maslennikov
@ 2020-04-30 7:29 ` Jiri Slaby
2020-04-30 9:08 ` Arseny Maslennikov
0 siblings, 1 reply; 17+ messages in thread
From: Jiri Slaby @ 2020-04-30 7:29 UTC (permalink / raw)
To: Arseny Maslennikov, Greg Kroah-Hartman, linux-serial, linux-kernel
Cc: Rob Landley, Eric W. Biederman, Pavel Machek, linux-api,
Vladimir D. Seleznev, Ingo Molnar, Peter Zijlstra, Oleg Nesterov,
mm
General comments:
- care to CC scheduler and mm people?
- couldn't this share some code with fs/proc?
- I am not sure/convinced it is worth the hassle
On 30. 04. 20, 8:43, Arseny Maslennikov wrote:
> If the three termios local flags isig, icanon, iexten are enabled
> and the local flag nokerninfo is disabled for a tty governed
> by the n_tty line discipline, then on receiving the keyboard status
> character n_tty will generate a status message and write it out to
> the tty before sending SIGINFO to the tty's foreground process group.
>
> This kerninfo line contains information about the current system load
> as well as some properties of "the most interesting" process in the
> tty's current foreground process group, namely:
> - its PID as seen inside its deepest PID namespace;
> * the whole process group ought to be in a single PID namespace,
> so this is actually deterministic
> - its saved command name truncated to 16 bytes (task_struct::comm);
> * at the time of writing TASK_COMM_LEN == 16
> - its state and some related bits, procps-style;
> - for S and D: its symbolic wait channel, if available; or a short
> description for other process states instead;
> - its user, system and real rusage time values;
> - its resident set size (as well as the high watermark) in kilobytes.
>
> The "most interesting" process is chosen as follows:
> - runnables over everything
> - uninterruptibles over everything else
> - among 2 runnables pick the biggest utime + stime
> - any unresolved ties are decided in favour of greatest PID.
>
> While the kerninfo line is not very useful for debugging the kernel
> itself, since we have much more powerful debugging tools, it still gives
> the user behind the terminal some meaningful feedback to a VSTATUS that
> works even if no processes respond.
Care to append an example output to the commit message?
> Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
...
> index f72a3fd4b..905cdd985 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -79,6 +80,8 @@
> #define ECHO_BLOCK 256
> #define ECHO_DISCARD_WATERMARK N_TTY_BUF_SIZE - (ECHO_BLOCK + 32)
>
> +#define STATUS_LINE_LEN (2 * KSYM_NAME_LEN)
It's too magic constant.
> @@ -2489,6 +2496,21 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
> }
> }
>
> +static void n_tty_status_line(struct tty_struct *tty)
> +{
> + struct n_tty_data *ldata = tty->disc_data;
> + char *msg, *buf;
> + msg = buf = kzalloc(STATUS_LINE_LEN, GFP_KERNEL);
> + tty_sprint_status_line(tty, buf + 1, STATUS_LINE_LEN - 1);
> + /* The only current caller of this takes output_lock for us. */
So add a lockdep assertion?
> + if (ldata->column != 0)
> + *msg = '\n';
> + else
> + msg++;
> + do_n_tty_write(tty, NULL, msg, strlen(msg));
> + kfree(buf);
> +}
> +
> static struct tty_ldisc_ops n_tty_ops = {
> .magic = TTY_LDISC_MAGIC,
> .name = "n_tty",
> diff --git a/drivers/tty/n_tty_status.c b/drivers/tty/n_tty_status.c
> new file mode 100644
> index 000000000..d92261bbe
> --- /dev/null
> +++ b/drivers/tty/n_tty_status.c
> @@ -0,0 +1,338 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <linux/kallsyms.h>
> +#include <linux/pid.h>
> +#include <linux/sched.h>
> +#include <linux/sched/signal.h>
> +#include <linux/sched/loadavg.h>
> +#include <linux/sched/cputime.h>
> +#include <linux/sched/mm.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +
> +#define BCOMPARE(lbool, rbool) ((lbool) << 1 | (rbool))
> +#define BCOMPARE_NONE 0
> +#define BCOMPARE_RIGHT 1
> +#define BCOMPARE_LEFT 2
> +#define BCOMPARE_BOTH (BCOMPARE_LEFT | BCOMPARE_RIGHT)
> +
> +/*
> + * Select the most interesting task of two.
> + *
> + * The implemented approach is simple for now:
> + * - pick runnable
> + * - if no runnables, pick uninterruptible
> + * - if tie between runnables, pick highest utime + stime
> + * - if a tie is not broken by the above, pick highest pid nr.
> + *
> + * For reference, here's the one used in FreeBSD:
> + * - pick runnables over anything
> + * - if both runnables, pick highest cpu utilization
> + * - if no runnables, pick shortest sleep time (the scheduler
> + * actually takes care to maintain this statistic)
> + * - other ties are decided in favour of youngest process.
> + */
> +static struct task_struct *__better_proc_R(struct task_struct *a,
Why so weird name __better_proc_R?
> + struct task_struct *b)
> +{
> + unsigned long flags;
> + u64 atime, btime, tgutime, tgstime;
> + struct task_struct *ret;
> +
> + if (!lock_task_sighand(a, &flags))
> + goto out_a_unlocked;
> + thread_group_cputime_adjusted(a, &tgutime, &tgstime);
> + atime = tgutime + tgstime;
> + unlock_task_sighand(a, &flags);
> +
> + if (!lock_task_sighand(b, &flags))
> + goto out_b_unlocked;
> + thread_group_cputime_adjusted(b, &tgutime, &tgstime);
> + btime = tgutime + tgstime;
> + unlock_task_sighand(b, &flags);
> +
> + ret = atime > btime ? a : b;
> +
> + return ret;
> +
> +out_b_unlocked:
> +out_a_unlocked:
> + return task_pid_vinr(a) > task_pid_vinr(b) ? a : b;
> +}
> +
> +static struct task_struct *__better_proc(struct task_struct *a,
Again, why "__" in the name?
> + struct task_struct *b)
> +{
> + if (!pid_alive(a))
> + return b;
> + if (!pid_alive(b))
> + return a;
> +
> + switch (BCOMPARE(a->state == TASK_RUNNING,
> + b->state == TASK_RUNNING)) {
> + case BCOMPARE_LEFT:
> + return a;
> + case BCOMPARE_RIGHT:
> + return b;
> + case BCOMPARE_BOTH:
> + return __better_proc_R(a, b);
> + }
Doesn't this look saner and better:
if (a->state == TASK_RUNNING && b->state == TASK_RUNNING)
return __better_proc_R(a, b);
if (a->state == TASK_RUNNING)
return a;
if (b->state == TASK_RUNNING)
return b;
?
And one doesn't need to decrypt the defines.
> + switch (BCOMPARE(a->state == TASK_UNINTERRUPTIBLE,
> + b->state == TASK_UNINTERRUPTIBLE)) {
> + case BCOMPARE_LEFT:
> + return a;
> + case BCOMPARE_RIGHT:
> + return b;
> + case BCOMPARE_BOTH:
> + break;
dtto.
> + }
> +
> + /* TODO: Perhaps we should check something else... */
> + return task_pid_vinr(a) > task_pid_vinr(b) ? a : b;
> +}
> +
> +/* Weed out NULLs. */
> +static inline struct task_struct *better_proc(struct task_struct *a,
> + struct task_struct *b) {
> + return a ? (b ? __better_proc(a, b) : a) : b;
This urgently calls for ifs and not ternany operators.
Actually you don't need this helper at all. Check a and b in the same if
as the respective !pid_alive above.
> +}
> +
> +static int scnprint_load(char *msgp, size_t size)
> +{
> + unsigned long la[3];
> +
> + get_avenrun(la, FIXED_1/200, 0);
> + return scnprintf(msgp, size, "load: %lu.%02lu; ",
> + LOAD_INT(la[0]), LOAD_FRAC(la[0]));
> +}
> +
> +static int scnprint_task(char *msgp, size_t size, struct task_struct *task)
> +{
> + char commname[TASK_COMM_LEN];
> +
> + get_task_comm(commname, task);
> + return scnprintf(msgp, size, "%d/%s:", task_pid_vinr(task), commname);
> +}
> +
> +static int scnprint_rusage(char *msgp, ssize_t size,
> + struct task_struct *task, struct mm_struct *mm)
> +{
> + struct rusage ru;
> + struct timespec64 utime, stime;
> + struct timespec64 rtime;
> + u64 now;
> + int ret = 0;
> + int psz = 0;
> +
> + getrusage(task, RUSAGE_BOTH, &ru);
> + now = ktime_get_ns();
> +
> + utime.tv_sec = ru.ru_utime.tv_sec;
> + utime.tv_nsec = ru.ru_utime.tv_usec * NSEC_PER_USEC;
> + stime.tv_sec = ru.ru_stime.tv_sec;
> + stime.tv_nsec = ru.ru_stime.tv_usec * NSEC_PER_USEC;
> +
> + rtime = ns_to_timespec64(now - task->start_time);
> +
> + psz = scnprintf(msgp, size,
> + "%llu.%03lur %llu.%03luu %llu.%03lus",
> + rtime.tv_sec, rtime.tv_nsec / NSEC_PER_MSEC,
> + utime.tv_sec, utime.tv_nsec / NSEC_PER_MSEC,
> + stime.tv_sec, stime.tv_nsec / NSEC_PER_MSEC);
> + ret += psz;
> + msgp += psz;
> + size -= psz;
> +
> + if (mm) {
> + psz = scnprintf(msgp, size,
> + " %luk/%luk",
> + get_mm_rss(mm) * PAGE_SIZE / 1024,
> + get_mm_hiwater_rss(mm) * PAGE_SIZE / 1024);
> + ret += psz;
> + }
> + return ret;
> +}
> +
> +static int scnprint_state(char *msgp, ssize_t size,
> + struct task_struct *task, struct mm_struct *mm)
> +{
> + char stat[8] = {0};
> + const char *state_descr = "";
> + struct task_struct *parent = NULL;
> + struct task_struct *real_parent = NULL;
> + unsigned long wchan = 0;
> + int psz = 0;
> + char symname[KSYM_NAME_LEN];
> +
> + stat[psz++] = task_state_to_char(task);
> + if (task_nice(task) < 0)
> + stat[psz++] = '<';
> + else if (task_nice(task) > 0)
> + stat[psz++] = 'N';
> + if (mm && mm->locked_vm)
> + stat[psz++] = 'L';
> + if (get_nr_threads(task) > 1)
> + stat[psz++] = 'l';
> +
> + switch (stat[0]) {
> + case 'R':
> + if (task_curr(task))
> + stat[psz++] = '!';
> + break;
> + case 'S':
> + case 'D':
> + wchan = get_wchan(task);
> + if (!wchan)
> + break;
> + if (!lookup_symbol_name(wchan, symname))
> + state_descr = symname;
> + else
> + /* get_wchan() returned something
> + * that looks like no kernel symbol.
> + */
> + state_descr = "*unknown*";
> + break;
> + case 'T':
> + state_descr = "stopped";
> + break;
> + case 't':
> + state_descr = "traced";
> + break;
> + case 'Z':
> + rcu_read_lock();
> + real_parent = rcu_dereference(task->real_parent);
> + parent = rcu_dereference(task->parent);
> + psz = sprintf(symname, "zombie; ppid=%d",
> + task_tgid_nr_ns(real_parent,
> + ns_of_pid(task_pid(task))));
> + if (parent != real_parent)
> + sprintf(symname + psz, " reaper=%d",
> + task_tgid_nr_ns(parent,
> + ns_of_pid(task_pid(task))));
> + rcu_read_unlock();
> + state_descr = symname;
> + break;
> + case 'I':
> + /* Can this even happen? */
> + state_descr = "idle";
> + break;
> + default:
> + state_descr = "unknown";
> + }
> +
> + psz = scnprintf(msgp, size, "%s", stat);
> + msgp += psz;
> + size -= psz;
> + if (*state_descr)
> + psz += scnprintf(msgp, size, wchan ? " [%s]" : " (%s)", state_descr);
> +
> + return psz;
> +}
> +
> +/**
> + * tty_sprint_status_line - produce kerninfo line
> + * @tty: terminal device
> + * @msg: preallocated memory buffer
> + * @length: maximum line length
> + *
> + * Reports state of foreground process group in a null-terminated string
> + * located at @msg, @length bytes long. If @length is insufficient,
> + * the line gets truncated.
> + */
> +void tty_sprint_status_line(struct tty_struct *t, char *msg, size_t length)
> +{
> + struct task_struct *tsk = NULL, *mit = NULL;
> + struct mm_struct *mm;
> + struct pid *pgrp = NULL;
> + char *msgp = msg;
> + int psz = 0;
> +
> + if (!length)
> + return;
> + length--; /* Make room for trailing '\n' */
> +
> + psz = scnprint_load(msgp, length);
> + if (psz > 0) {
> + msgp += psz;
> + length -= psz;
> + }
> + if (!length)
> + goto finalize_message;
> +
> + /* Not sure if session pid is protected by ctrl_lock
> + * or tasklist_lock...
> + */
> + pgrp = t->session;
> + if (pgrp == NULL) {
> + psz = scnprintf(msgp, length, "not a controlling tty");
> + if (psz > 0)
> + msgp += psz;
> + goto finalize_message;
> + }
> + pgrp = tty_get_pgrp(t);
> + if (pgrp == NULL) {
> + psz = scnprintf(msgp, length, "no foreground process group");
> + if (psz > 0)
> + msgp += psz;
> + goto finalize_message;
> + }
> +
> + read_lock(&tasklist_lock);
> + do_each_pid_task(pgrp, PIDTYPE_PGID, tsk)
> + {
> + /* Select the most interesting task. */
> + if (tsk == better_proc(mit, tsk))
> + mit = tsk;
> + } while_each_pid_task(pgrp, PIDTYPE_PGID, tsk);
> + read_unlock(&tasklist_lock);
> +
> + if (!pid_alive(mit))
> + goto finalize_message;
> +
> + /* Gather intel on most interesting task. */
> + /* Can the mm of a foreground process turn out to be NULL?
> + * Definitely; for example, if it is a zombie.
> + */
> + mm = get_task_mm(mit);
> +
> + psz = scnprint_task(msgp, length, mit);
> + if (psz > 0) {
> + msgp += psz;
> + length -= psz;
> + }
> + if (!length)
> + goto finalize_message;
> + *msgp++ = ' ';
> + length--;
> +
> + psz = scnprint_state(msgp, length, mit, mm);
> + if (psz > 0) {
> + msgp += psz;
> + length -= psz;
> + }
> + if (!length)
> + goto finalize_message;
> + *msgp++ = ' ';
> + length--;
> +
> + psz = scnprint_rusage(msgp, length, mit, mm);
> + if (psz > 0) {
> + msgp += psz;
> + length -= psz;
> + }
> + if (!length)
> + goto finalize_message;
> + *msgp++ = ' ';
> + length--;
> +
> + if (!mm)
> + goto finalize_message;
> +
> + mmput(mm);
> +
> +finalize_message:
> + *msgp++ = '\n';
> + if (pgrp)
> + put_pid(pgrp);
> +}
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 4418f5cb8..195abd47d 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1318,6 +1318,8 @@ static inline struct pid *task_pid(struct task_struct *task)
> * task_xid_nr() : global id, i.e. the id seen from the init namespace;
> * task_xid_vnr() : virtual id, i.e. the id seen from the pid namespace of
> * current.
> + * task_xid_vinr() : virtual inner id, i.e. the id seen from the namespace of
> + * the task itself;
> * task_xid_nr_ns() : id seen from the ns specified;
> *
> * see also pid_nr() etc in include/linux/pid.h
> @@ -1339,6 +1341,11 @@ static inline pid_t task_pid_vnr(struct task_struct *tsk)
> return __task_pid_nr_ns(tsk, PIDTYPE_PID, NULL);
> }
>
> +static inline pid_t task_pid_vinr(struct task_struct *tsk)
> +{
> + return __task_pid_nr_ns(tsk, PIDTYPE_PID, ns_of_pid(task_pid(tsk)));
> +}
> +
This smells like it should be done in a separate patch.
> static inline pid_t task_tgid_nr(struct task_struct *tsk)
> {
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 3499845ab..2023addaf 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -727,6 +727,9 @@ extern void __init n_tty_init(void);
> static inline void n_tty_init(void) { }
> #endif
>
> +/* n_tty_status.c */
> +extern void tty_sprint_status_line(struct tty_struct *tty, char *msg, size_t size);
No need for extern.
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 7/7] n_tty: Provide an informational line on VSTATUS receipt
2020-04-30 7:29 ` Jiri Slaby
@ 2020-04-30 9:08 ` Arseny Maslennikov
0 siblings, 0 replies; 17+ messages in thread
From: Arseny Maslennikov @ 2020-04-30 9:08 UTC (permalink / raw)
To: Jiri Slaby
Cc: Greg Kroah-Hartman, linux-serial, linux-kernel, Rob Landley,
Eric W. Biederman, Pavel Machek, linux-api, Vladimir D. Seleznev,
Ingo Molnar, Peter Zijlstra, Oleg Nesterov, mm
[-- Attachment #1: Type: text/plain, Size: 21102 bytes --]
On Thu, Apr 30, 2020 at 09:29:28AM +0200, Jiri Slaby wrote:
> General comments:
> - care to CC scheduler and mm people?
I did in v1, but dropped those CCs in v2 and on, being afraid to
over-cc.
I'm sorry if I did the wrong thing.
> - couldn't this share some code with fs/proc?
We can't use the proc handlers directly, since they are often written to
output to the proc seqfile interface, and we can't use that.
This shares code with proc at a lower level by using the same
primitives to obtain info from.
> - I am not sure/convinced it is worth the hassle
>
> On 30. 04. 20, 8:43, Arseny Maslennikov wrote:
> > If the three termios local flags isig, icanon, iexten are enabled
> > and the local flag nokerninfo is disabled for a tty governed
> > by the n_tty line discipline, then on receiving the keyboard status
> > character n_tty will generate a status message and write it out to
> > the tty before sending SIGINFO to the tty's foreground process group.
> >
> > This kerninfo line contains information about the current system load
> > as well as some properties of "the most interesting" process in the
> > tty's current foreground process group, namely:
> > - its PID as seen inside its deepest PID namespace;
> > * the whole process group ought to be in a single PID namespace,
> > so this is actually deterministic
> > - its saved command name truncated to 16 bytes (task_struct::comm);
> > * at the time of writing TASK_COMM_LEN == 16
> > - its state and some related bits, procps-style;
> > - for S and D: its symbolic wait channel, if available; or a short
> > description for other process states instead;
> > - its user, system and real rusage time values;
> > - its resident set size (as well as the high watermark) in kilobytes.
> >
> > The "most interesting" process is chosen as follows:
> > - runnables over everything
> > - uninterruptibles over everything else
> > - among 2 runnables pick the biggest utime + stime
> > - any unresolved ties are decided in favour of greatest PID.
> >
> > While the kerninfo line is not very useful for debugging the kernel
> > itself, since we have much more powerful debugging tools, it still gives
> > the user behind the terminal some meaningful feedback to a VSTATUS that
> > works even if no processes respond.
>
> Care to append an example output to the commit message?
$ cat
load: 0.32; 1108279/cat: S 0.524r 0.000u 0.000s 700k/700k
load: 0.32; 1108279/cat: S 1.873r 0.000u 0.000s 700k/700k
$ sha256sum /dev/zero
load: 0.32; 1108298/sha256sum: R! 1.302r 1.274u 0.015s 700k/700k
load: 0.32; 1108298/sha256sum: R! 2.445r 2.378u 0.055s 700k/700k
load: 0.32; 1108298/sha256sum: R! 3.309r 3.226u 0.071s 700k/700k
load: 0.42; 1108298/sha256sum: R! 14.547r 14.340u 0.195s 700k/700k
^C
Here I pressed ^T 2 times before supplying EOF to cat and 4 times before
interrupting sha256sum.
An example with a patched sleep:
% sleep 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 0.92r 0.00u 0.00s 1544k/1544k
sleep: about 7 second(s) left out of the original 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 2.61r 0.00u 0.00s 1552k/1552k
sleep: about 5 second(s) left out of the original 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 3.44r 0.00u 0.00s 1552k/1552k
sleep: about 4 second(s) left out of the original 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 3.65r 0.00u 0.00s 1552k/1552k
sleep: about 4 second(s) left out of the original 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 4.30r 0.00u 0.00s 1552k/1552k
sleep: about 3 second(s) left out of the original 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 4.72r 0.00u 0.00s 1552k/1552k
sleep: about 3 second(s) left out of the original 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 5.44r 0.00u 0.00s 1552k/1552k
sleep: about 2 second(s) left out of the original 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 5.57r 0.00u 0.00s 1552k/1552k
sleep: about 2 second(s) left out of the original 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 6.02r 0.00u 0.00s 1552k/1552k
sleep: about 1 second(s) left out of the original 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 6.15r 0.00u 0.00s 1552k/1552k
sleep: about 1 second(s) left out of the original 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 6.23r 0.00u 0.00s 1552k/1552k
sleep: about 1 second(s) left out of the original 8
load: 0.04; 66573/sleep: S [hrtimer_nanosleep] 7.20r 0.00u 0.00s 1552k/1552k
sleep: about 0 second(s) left out of the original 8
Another short example:
$ cat <<EOX zombospawner.c
#include <unistd.h>
#include <stdio.h>
#include <error.h>
#include <errno.h>
int main(int argc, char* argv[]) {
char* prog;
int e;
if (argc <= 1) return -1;
fprintf(stderr, "[my pid is %d]\n", getpid());
e = fork();
if (e)
for (;;) sleep(4);
fprintf(stderr, "[and my pid is %d. Will i become a Z?]\n", getpid());
e = execv(argv[1], argv+1);
error(e, errno, "oops");
}
EOX
$ gcc zombospawner.c -o zombospawner
$ ./zombospawner xx
[my pid is 1109004]
[and my pid is 1109005. Will i become a Z?]
./zombospawner: oops: No such file or directory
load: 1.35; 1109005/zombospawner: Z (zombie; ppid=1109004) 3.443r 0.000u 0.000s
load: 1.35; 1109005/zombospawner: Z (zombie; ppid=1109004) 3.743r 0.000u 0.000s
load: 1.32; 1109005/zombospawner: Z (zombie; ppid=1109004) 5.175r 0.000u 0.000s
^C
But you're right, this should be in the commit message, since it makes
little sense to me to devote to this its own Documentation/ page.
>
> > Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> ...
> > index f72a3fd4b..905cdd985 100644
> > --- a/drivers/tty/n_tty.c
> > +++ b/drivers/tty/n_tty.c
> > @@ -79,6 +80,8 @@
> > #define ECHO_BLOCK 256
> > #define ECHO_DISCARD_WATERMARK N_TTY_BUF_SIZE - (ECHO_BLOCK + 32)
> >
> > +#define STATUS_LINE_LEN (2 * KSYM_NAME_LEN)
>
> It's too magic constant.
The line to be constructed can optionally contain a kernel symbol name,
which is not longer than KSYM_NAME_LEN, and the effective size of all
the other characters together is also less than KSYM_NAME_LEN — that
includes:
* "load: ", which is 6 bytes;
* the load average and the ';', which takes from 5 bytes if LA < 10 to
13 bytes for a billion CPU cores (huh!) and scales logarithmically;
* the decimal PID nr, which is not longer than 9 bytes;
* the comm, which is not longer than TASK_COMM_LEN;
* the ": S ", which is 4 bytes;
* the rusage values, each of those 3 is at least 7 bytes and scales
at most logarithmically with seconds elapsed.
6 + 13 + 9 + 16 (comm) + 4 + 51 is 99 < 128, and this assumes close-to
worst case for 10-char-long decimals for all the times and the PID.
Instead of a kernel symbol name, the middle "wchan" field can show
things like "zombie; ppid=123456 reaper=98765", which generally take
much less space than KSYM_NAME_LEN (most kernel symbol names that
processes sleep at also take less space).
So realistically we're not hitting the specified generous limit, which
is 256 bytes. Another reason for that number is, since the memory chunk
is short-lived, it makes little sense to kmalloc less.
If some time later we miraculously do hit the 256 char limit, the line
will just get truncated.
>
> > @@ -2489,6 +2496,21 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
> > }
> > }
> >
> > +static void n_tty_status_line(struct tty_struct *tty)
> > +{
> > + struct n_tty_data *ldata = tty->disc_data;
> > + char *msg, *buf;
> > + msg = buf = kzalloc(STATUS_LINE_LEN, GFP_KERNEL);
> > + tty_sprint_status_line(tty, buf + 1, STATUS_LINE_LEN - 1);
> > + /* The only current caller of this takes output_lock for us. */
>
> So add a lockdep assertion?
Yes, this is a good idea. I added an assertion in a similar situation in
6/7, but hesitated here.
>
> > + if (ldata->column != 0)
> > + *msg = '\n';
> > + else
> > + msg++;
> > + do_n_tty_write(tty, NULL, msg, strlen(msg));
> > + kfree(buf);
> > +}
> > +
> > static struct tty_ldisc_ops n_tty_ops = {
> > .magic = TTY_LDISC_MAGIC,
> > .name = "n_tty",
> > diff --git a/drivers/tty/n_tty_status.c b/drivers/tty/n_tty_status.c
> > new file mode 100644
> > index 000000000..d92261bbe
> > --- /dev/null
> > +++ b/drivers/tty/n_tty_status.c
> > @@ -0,0 +1,338 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +#include <linux/kallsyms.h>
> > +#include <linux/pid.h>
> > +#include <linux/sched.h>
> > +#include <linux/sched/signal.h>
> > +#include <linux/sched/loadavg.h>
> > +#include <linux/sched/cputime.h>
> > +#include <linux/sched/mm.h>
> > +#include <linux/slab.h>
> > +#include <linux/tty.h>
> > +
> > +#define BCOMPARE(lbool, rbool) ((lbool) << 1 | (rbool))
> > +#define BCOMPARE_NONE 0
> > +#define BCOMPARE_RIGHT 1
> > +#define BCOMPARE_LEFT 2
> > +#define BCOMPARE_BOTH (BCOMPARE_LEFT | BCOMPARE_RIGHT)
> > +
> > +/*
> > + * Select the most interesting task of two.
> > + *
> > + * The implemented approach is simple for now:
> > + * - pick runnable
> > + * - if no runnables, pick uninterruptible
> > + * - if tie between runnables, pick highest utime + stime
> > + * - if a tie is not broken by the above, pick highest pid nr.
> > + *
> > + * For reference, here's the one used in FreeBSD:
> > + * - pick runnables over anything
> > + * - if both runnables, pick highest cpu utilization
> > + * - if no runnables, pick shortest sleep time (the scheduler
> > + * actually takes care to maintain this statistic)
> > + * - other ties are decided in favour of youngest process.
> > + */
> > +static struct task_struct *__better_proc_R(struct task_struct *a,
>
> Why so weird name __better_proc_R?
>
Please see below.
> > + struct task_struct *b)
> > +{
> > + unsigned long flags;
> > + u64 atime, btime, tgutime, tgstime;
> > + struct task_struct *ret;
> > +
> > + if (!lock_task_sighand(a, &flags))
> > + goto out_a_unlocked;
> > + thread_group_cputime_adjusted(a, &tgutime, &tgstime);
> > + atime = tgutime + tgstime;
> > + unlock_task_sighand(a, &flags);
> > +
> > + if (!lock_task_sighand(b, &flags))
> > + goto out_b_unlocked;
> > + thread_group_cputime_adjusted(b, &tgutime, &tgstime);
> > + btime = tgutime + tgstime;
> > + unlock_task_sighand(b, &flags);
> > +
> > + ret = atime > btime ? a : b;
> > +
> > + return ret;
> > +
> > +out_b_unlocked:
> > +out_a_unlocked:
> > + return task_pid_vinr(a) > task_pid_vinr(b) ? a : b;
> > +}
> > +
> > +static struct task_struct *__better_proc(struct task_struct *a,
>
> Again, why "__" in the name?
Does the prefix __ have a very special meaning so it cannot be used
here, in a static symbol?
I'm sorry if it does, Documentation/process/coding-style.rst does not
warn against it.
I used it to generally mean "wrapped implementation detail".
>
> > + struct task_struct *b)
> > +{
> > + if (!pid_alive(a))
> > + return b;
> > + if (!pid_alive(b))
> > + return a;
> > +
> > + switch (BCOMPARE(a->state == TASK_RUNNING,
> > + b->state == TASK_RUNNING)) {
> > + case BCOMPARE_LEFT:
> > + return a;
> > + case BCOMPARE_RIGHT:
> > + return b;
> > + case BCOMPARE_BOTH:
> > + return __better_proc_R(a, b);
> > + }
>
> Doesn't this look saner and better:
>
> if (a->state == TASK_RUNNING && b->state == TASK_RUNNING)
> return __better_proc_R(a, b);
> if (a->state == TASK_RUNNING)
> return a;
> if (b->state == TASK_RUNNING)
> return b;
>
> ?
>
> And one doesn't need to decrypt the defines.
I felt like the version with ifs would read worse.
No problem changing this.
>
> > + switch (BCOMPARE(a->state == TASK_UNINTERRUPTIBLE,
> > + b->state == TASK_UNINTERRUPTIBLE)) {
> > + case BCOMPARE_LEFT:
> > + return a;
> > + case BCOMPARE_RIGHT:
> > + return b;
> > + case BCOMPARE_BOTH:
> > + break;
>
> dtto.
>
> > + }
> > +
> > + /* TODO: Perhaps we should check something else... */
> > + return task_pid_vinr(a) > task_pid_vinr(b) ? a : b;
> > +}
> > +
> > +/* Weed out NULLs. */
> > +static inline struct task_struct *better_proc(struct task_struct *a,
> > + struct task_struct *b) {
> > + return a ? (b ? __better_proc(a, b) : a) : b;
>
> This urgently calls for ifs and not ternany operators.
>
> Actually you don't need this helper at all. Check a and b in the same if
> as the respective !pid_alive above.
>
> > +}
> > +
> > +static int scnprint_load(char *msgp, size_t size)
> > +{
> > + unsigned long la[3];
> > +
> > + get_avenrun(la, FIXED_1/200, 0);
> > + return scnprintf(msgp, size, "load: %lu.%02lu; ",
> > + LOAD_INT(la[0]), LOAD_FRAC(la[0]));
> > +}
> > +
> > +static int scnprint_task(char *msgp, size_t size, struct task_struct *task)
> > +{
> > + char commname[TASK_COMM_LEN];
> > +
> > + get_task_comm(commname, task);
> > + return scnprintf(msgp, size, "%d/%s:", task_pid_vinr(task), commname);
> > +}
> > +
> > +static int scnprint_rusage(char *msgp, ssize_t size,
> > + struct task_struct *task, struct mm_struct *mm)
> > +{
> > + struct rusage ru;
> > + struct timespec64 utime, stime;
> > + struct timespec64 rtime;
> > + u64 now;
> > + int ret = 0;
> > + int psz = 0;
> > +
> > + getrusage(task, RUSAGE_BOTH, &ru);
> > + now = ktime_get_ns();
> > +
> > + utime.tv_sec = ru.ru_utime.tv_sec;
> > + utime.tv_nsec = ru.ru_utime.tv_usec * NSEC_PER_USEC;
> > + stime.tv_sec = ru.ru_stime.tv_sec;
> > + stime.tv_nsec = ru.ru_stime.tv_usec * NSEC_PER_USEC;
> > +
> > + rtime = ns_to_timespec64(now - task->start_time);
> > +
> > + psz = scnprintf(msgp, size,
> > + "%llu.%03lur %llu.%03luu %llu.%03lus",
> > + rtime.tv_sec, rtime.tv_nsec / NSEC_PER_MSEC,
> > + utime.tv_sec, utime.tv_nsec / NSEC_PER_MSEC,
> > + stime.tv_sec, stime.tv_nsec / NSEC_PER_MSEC);
> > + ret += psz;
> > + msgp += psz;
> > + size -= psz;
> > +
> > + if (mm) {
> > + psz = scnprintf(msgp, size,
> > + " %luk/%luk",
> > + get_mm_rss(mm) * PAGE_SIZE / 1024,
> > + get_mm_hiwater_rss(mm) * PAGE_SIZE / 1024);
> > + ret += psz;
> > + }
> > + return ret;
> > +}
> > +
> > +static int scnprint_state(char *msgp, ssize_t size,
> > + struct task_struct *task, struct mm_struct *mm)
> > +{
> > + char stat[8] = {0};
> > + const char *state_descr = "";
> > + struct task_struct *parent = NULL;
> > + struct task_struct *real_parent = NULL;
> > + unsigned long wchan = 0;
> > + int psz = 0;
> > + char symname[KSYM_NAME_LEN];
> > +
> > + stat[psz++] = task_state_to_char(task);
> > + if (task_nice(task) < 0)
> > + stat[psz++] = '<';
> > + else if (task_nice(task) > 0)
> > + stat[psz++] = 'N';
> > + if (mm && mm->locked_vm)
> > + stat[psz++] = 'L';
> > + if (get_nr_threads(task) > 1)
> > + stat[psz++] = 'l';
> > +
> > + switch (stat[0]) {
> > + case 'R':
> > + if (task_curr(task))
> > + stat[psz++] = '!';
> > + break;
> > + case 'S':
> > + case 'D':
> > + wchan = get_wchan(task);
> > + if (!wchan)
> > + break;
> > + if (!lookup_symbol_name(wchan, symname))
> > + state_descr = symname;
> > + else
> > + /* get_wchan() returned something
> > + * that looks like no kernel symbol.
> > + */
> > + state_descr = "*unknown*";
> > + break;
> > + case 'T':
> > + state_descr = "stopped";
> > + break;
> > + case 't':
> > + state_descr = "traced";
> > + break;
> > + case 'Z':
> > + rcu_read_lock();
> > + real_parent = rcu_dereference(task->real_parent);
> > + parent = rcu_dereference(task->parent);
> > + psz = sprintf(symname, "zombie; ppid=%d",
> > + task_tgid_nr_ns(real_parent,
> > + ns_of_pid(task_pid(task))));
> > + if (parent != real_parent)
> > + sprintf(symname + psz, " reaper=%d",
> > + task_tgid_nr_ns(parent,
> > + ns_of_pid(task_pid(task))));
> > + rcu_read_unlock();
> > + state_descr = symname;
> > + break;
> > + case 'I':
> > + /* Can this even happen? */
> > + state_descr = "idle";
> > + break;
> > + default:
> > + state_descr = "unknown";
> > + }
> > +
> > + psz = scnprintf(msgp, size, "%s", stat);
> > + msgp += psz;
> > + size -= psz;
> > + if (*state_descr)
> > + psz += scnprintf(msgp, size, wchan ? " [%s]" : " (%s)", state_descr);
> > +
> > + return psz;
> > +}
> > +
> > +/**
> > + * tty_sprint_status_line - produce kerninfo line
> > + * @tty: terminal device
> > + * @msg: preallocated memory buffer
> > + * @length: maximum line length
> > + *
> > + * Reports state of foreground process group in a null-terminated string
> > + * located at @msg, @length bytes long. If @length is insufficient,
> > + * the line gets truncated.
> > + */
> > +void tty_sprint_status_line(struct tty_struct *t, char *msg, size_t length)
> > +{
> > + struct task_struct *tsk = NULL, *mit = NULL;
> > + struct mm_struct *mm;
> > + struct pid *pgrp = NULL;
> > + char *msgp = msg;
> > + int psz = 0;
> > +
> > + if (!length)
> > + return;
> > + length--; /* Make room for trailing '\n' */
> > +
> > + psz = scnprint_load(msgp, length);
> > + if (psz > 0) {
> > + msgp += psz;
> > + length -= psz;
> > + }
> > + if (!length)
> > + goto finalize_message;
> > +
> > + /* Not sure if session pid is protected by ctrl_lock
> > + * or tasklist_lock...
> > + */
> > + pgrp = t->session;
> > + if (pgrp == NULL) {
> > + psz = scnprintf(msgp, length, "not a controlling tty");
> > + if (psz > 0)
> > + msgp += psz;
> > + goto finalize_message;
> > + }
> > + pgrp = tty_get_pgrp(t);
> > + if (pgrp == NULL) {
> > + psz = scnprintf(msgp, length, "no foreground process group");
> > + if (psz > 0)
> > + msgp += psz;
> > + goto finalize_message;
> > + }
> > +
> > + read_lock(&tasklist_lock);
> > + do_each_pid_task(pgrp, PIDTYPE_PGID, tsk)
> > + {
> > + /* Select the most interesting task. */
> > + if (tsk == better_proc(mit, tsk))
> > + mit = tsk;
> > + } while_each_pid_task(pgrp, PIDTYPE_PGID, tsk);
> > + read_unlock(&tasklist_lock);
> > +
> > + if (!pid_alive(mit))
> > + goto finalize_message;
> > +
> > + /* Gather intel on most interesting task. */
> > + /* Can the mm of a foreground process turn out to be NULL?
> > + * Definitely; for example, if it is a zombie.
> > + */
> > + mm = get_task_mm(mit);
> > +
> > + psz = scnprint_task(msgp, length, mit);
> > + if (psz > 0) {
> > + msgp += psz;
> > + length -= psz;
> > + }
> > + if (!length)
> > + goto finalize_message;
> > + *msgp++ = ' ';
> > + length--;
> > +
> > + psz = scnprint_state(msgp, length, mit, mm);
> > + if (psz > 0) {
> > + msgp += psz;
> > + length -= psz;
> > + }
> > + if (!length)
> > + goto finalize_message;
> > + *msgp++ = ' ';
> > + length--;
> > +
> > + psz = scnprint_rusage(msgp, length, mit, mm);
> > + if (psz > 0) {
> > + msgp += psz;
> > + length -= psz;
> > + }
> > + if (!length)
> > + goto finalize_message;
> > + *msgp++ = ' ';
> > + length--;
> > +
> > + if (!mm)
> > + goto finalize_message;
> > +
> > + mmput(mm);
> > +
> > +finalize_message:
> > + *msgp++ = '\n';
> > + if (pgrp)
> > + put_pid(pgrp);
> > +}
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 4418f5cb8..195abd47d 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -1318,6 +1318,8 @@ static inline struct pid *task_pid(struct task_struct *task)
> > * task_xid_nr() : global id, i.e. the id seen from the init namespace;
> > * task_xid_vnr() : virtual id, i.e. the id seen from the pid namespace of
> > * current.
> > + * task_xid_vinr() : virtual inner id, i.e. the id seen from the namespace of
> > + * the task itself;
> > * task_xid_nr_ns() : id seen from the ns specified;
> > *
> > * see also pid_nr() etc in include/linux/pid.h
> > @@ -1339,6 +1341,11 @@ static inline pid_t task_pid_vnr(struct task_struct *tsk)
> > return __task_pid_nr_ns(tsk, PIDTYPE_PID, NULL);
> > }
> >
> > +static inline pid_t task_pid_vinr(struct task_struct *tsk)
> > +{
> > + return __task_pid_nr_ns(tsk, PIDTYPE_PID, ns_of_pid(task_pid(tsk)));
> > +}
> > +
>
> This smells like it should be done in a separate patch.
OK, no problem.
This also solves the over-cc consideration mentioned above.
>
> > static inline pid_t task_tgid_nr(struct task_struct *tsk)
> > {
> > diff --git a/include/linux/tty.h b/include/linux/tty.h
> > index 3499845ab..2023addaf 100644
> > --- a/include/linux/tty.h
> > +++ b/include/linux/tty.h
> > @@ -727,6 +727,9 @@ extern void __init n_tty_init(void);
> > static inline void n_tty_init(void) { }
> > #endif
> >
> > +/* n_tty_status.c */
> > +extern void tty_sprint_status_line(struct tty_struct *tty, char *msg, size_t size);
>
> No need for extern.
Will fix, thanks.
>
> thanks,
> --
> js
> suse labs
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread