linux-kernel - Re: [PATCH] freezer,sched: Report TASK_FROZEN tasks as TASK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231023121546.2d6acb62@gandalf.local.home>
Date:   Mon, 23 Oct 2023 12:15:46 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Liangliang Li <liliangliang@...o.com>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
        Daniel Bristot de Oliveira <bristot@...hat.com>,
        Valentin Schneider <vschneid@...hat.com>,
        linux-kernel@...r.kernel.org, opensource.kernel@...o.com
Subject: Re: [PATCH] freezer,sched: Report TASK_FROZEN tasks as
 TASK_UNINTERRUPTIBLE

On Mon, 23 Oct 2023 21:57:35 +0800
Liangliang Li <liliangliang@...o.com> wrote:

> TASK_FROZEN is not in TASK_REPORT, thus a frozen task will appear as
> state == 0, IOW TASK_RUNNING.
> 
> Fix this by make TASK_FROZEN appear as TASK_UNINTERRUPTIBLE, thus we
> dont need to imply a new state to userspace tools.
> 

I'm not so sure I agree with this. If anything, it should be reported as
something other than RUNNING or TASK_UNINTERRUPTIBLE, (yes a new state).

The reason I state that, is that tasks may not be the one that sets its
state to FROZEN. A TASK_UNINTERRUPTIBLE is a task that purposely took
itself off the runqueue and is waiting for a response. A FROZEN task did
not necessarily take itself off the run queue, as something else did. It's
more like 'T' than 'D'.

I don't find 'D' any more useful that 'R' for a frozen task. In fact, I
find it even more deceiving.

If you see a task in 'D' that's not running, you will likely look to see
what its blocked on. If it's not blocked on anything, that will be very
confusing. If you see 'R' not running, you can then at least go look to see
why it's not running.

-- Steve

> Signed-off-by: Liangliang Li <liliangliang@...o.com>
> ---
>  include/linux/sched.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 292c31697248..0cfe2ba4fc6f 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1690,6 +1690,10 @@ static inline unsigned int __task_state_index(unsigned int tsk_state,
>  	if (tsk_state & TASK_RTLOCK_WAIT)
>  		state = TASK_UNINTERRUPTIBLE;
>  
> +	/* Frozen tasks should appear as "D (disk sleep)' in userspace. */
> +	if (tsk_state & TASK_FROZEN)
> +		state = TASK_UNINTERRUPTIBLE;
> +
>  	return fls(state);
>  }
>