Re: [PATCH v9 3/3] procfs: treat parked tasks as sleeping for task state

Andrew Morton
2015-04-29T22:27:02Z

On Fri, 17 Apr 2015 14:37:18 -0400 Chris Metcalf <cmetcalf@ezchip.com> wrote:

> Allowing watchdog threads to be parked means that we now have the
> opportunity of actually seeing persistent parked threads in the output
> of /proc's stat and status files. The existing code reported such

"/proc's stat" is ambiguous (/proc/stat?). We can remove all doubt by
using full pathnames: /proc/<pid>/stat.

> threads as "Running", which is kind-of true if you think of the case
> where we park them as part of taking cpus offline. But if we allow
> parking them indefinitely, "Running" is pretty misleading, so we report
> them as "Sleeping" instead.
>
> We could simply report them with a new string, "Parked", but it feels
> like it's a bit risky for userspace to see unexpected new values.
> The scheduler does report parked tasks with a "P" in debugging output
> from sched_show_task() or dump_cpu_task(), but that's a different API.
>
> This change seemed slightly cleaner than updating the task_state_array
> to have additional rows. TASK_DEAD should be subsumed by the exit_state
> bits; TASK_WAKEKILL is just a modifier; and TASK_WAKING can very
> reasonably be reported as "Running" (as it is now). Only TASK_PARKED
> shows up with unreasonable output here.

Documentation/filesystems/proc.txt documents /proc/<pid>/status. It
documents "State" explicitly.

> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -126,6 +126,10 @@ static inline const char *get_task_state(struct task_struct *tsk)
> {
> unsigned int state = (tsk->state | tsk->exit_state) & TASK_REPORT;
>
> + /* Treat parked tasks as sleeping. */
> + if (tsk->state == TASK_PARKED)
> + state = TASK_INTERRUPTIBLE;

The comment describes something which is utterly obvious. What it
doesn't describe (and should) is *why* we do this.