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 <[email protected]> ... > 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
