freezer, sched: report the frozen task stat as 'D'

[PATCH] freezer, sched: report the frozen task stat as 'D'

Posted by Chen Ridong 1 week, 5 days ago

From: Chen Ridong <chenridong@huawei.com>

Before the commit f5d39b020809 ("freezer,sched: Rewrite core freezer
logic"), the frozen task stat was reported as 'D' in cgroup v1.
However, after rewriting core freezer logic, the frozen task stat is
reported as 'R'. This is confusing, especially when a task with stat of
'S' is frozen.

This can be reproduced as bellow step:
cd /sys/fs/cgroup/freezer/
mkdir test
sleep 1000 &
[1] 739         // task whose stat is 'S'
echo 739 > test/cgroup.procs
echo FROZEN > test/freezer.state
ps -aux | grep 739
root     739  0.1  0.0   8376  1812 pts/0    R    10:56   0:00 sleep 1000

As shown above, a task whose stat is 'S' was changed to 'R' when it was
frozen. To solve this issue, simply maintain the same reported state as
before the rewrite.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 include/linux/sched.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index f0f23efdb245..878acce2f8d6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1630,8 +1630,9 @@ static inline unsigned int __task_state_index(unsigned int tsk_state,
 	 * We're lying here, but rather than expose a completely new task state
 	 * to userspace, we can make this appear as if the task has gone through
 	 * a regular rt_mutex_lock() call.
+	 * Report the frozen task uninterruptible.
 	 */
-	if (tsk_state & TASK_RTLOCK_WAIT)
+	if (tsk_state & TASK_RTLOCK_WAIT || tsk_state & TASK_FROZEN)
 		state = TASK_UNINTERRUPTIBLE;
 
 	return fls(state);
-- 
2.34.1

Re: [PATCH] freezer, sched: report the frozen task stat as 'D'

Posted by Valentin Schneider 1 week, 3 days ago

On 11/11/24 13:54, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> Before the commit f5d39b020809 ("freezer,sched: Rewrite core freezer
> logic"), the frozen task stat was reported as 'D' in cgroup v1.
> However, after rewriting core freezer logic, the frozen task stat is
> reported as 'R'. This is confusing, especially when a task with stat of
> 'S' is frozen.
>
> This can be reproduced as bellow step:
> cd /sys/fs/cgroup/freezer/
> mkdir test
> sleep 1000 &
> [1] 739         // task whose stat is 'S'
> echo 739 > test/cgroup.procs
> echo FROZEN > test/freezer.state
> ps -aux | grep 739
> root     739  0.1  0.0   8376  1812 pts/0    R    10:56   0:00 sleep 1000
>
> As shown above, a task whose stat is 'S' was changed to 'R' when it was
> frozen. To solve this issue, simply maintain the same reported state as
> before the rewrite.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>  include/linux/sched.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index f0f23efdb245..878acce2f8d6 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1630,8 +1630,9 @@ static inline unsigned int __task_state_index(unsigned int tsk_state,
>  	 * We're lying here, but rather than expose a completely new task state
>  	 * to userspace, we can make this appear as if the task has gone through
>  	 * a regular rt_mutex_lock() call.
> +	 * Report the frozen task uninterruptible.
>  	 */
> -	if (tsk_state & TASK_RTLOCK_WAIT)
> +	if (tsk_state & TASK_RTLOCK_WAIT || tsk_state & TASK_FROZEN)

Hmph, for RTLOCK this was good enough since !PREEMPT_RT really would be
TASK_UNINTERRUPTIBLE. 

For the freezer AFAICT there's more variation, following your example:

  crash> task -R __state,saved_state 195
  PID: 195      TASK: ffff888004abab80  CPU: 1    COMMAND: "sh"
    __state = 32768    -> 0x8000 aka TASK_FROZEN
    saved_state = 8193 -> 0x2001 aka TASK_INTERRUPTIBLE | TASK_FREEZABLE

so we ought to read ->saved_state, but as pointed out in [1] this is
racy...

[1]: https://lore.kernel.org/lkml/20220120162520.570782-3-valentin.schneider@arm.com/

>  		state = TASK_UNINTERRUPTIBLE;
>  
>  	return fls(state);
> -- 
> 2.34.1

Re: [PATCH] freezer, sched: report the frozen task stat as 'D'

Posted by Chen Ridong 1 week, 2 days ago


On 2024/11/13 22:34, Valentin Schneider wrote:
> On 11/11/24 13:54, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Before the commit f5d39b020809 ("freezer,sched: Rewrite core freezer
>> logic"), the frozen task stat was reported as 'D' in cgroup v1.
>> However, after rewriting core freezer logic, the frozen task stat is
>> reported as 'R'. This is confusing, especially when a task with stat of
>> 'S' is frozen.
>>
>> This can be reproduced as bellow step:
>> cd /sys/fs/cgroup/freezer/
>> mkdir test
>> sleep 1000 &
>> [1] 739         // task whose stat is 'S'
>> echo 739 > test/cgroup.procs
>> echo FROZEN > test/freezer.state
>> ps -aux | grep 739
>> root     739  0.1  0.0   8376  1812 pts/0    R    10:56   0:00 sleep 1000
>>
>> As shown above, a task whose stat is 'S' was changed to 'R' when it was
>> frozen. To solve this issue, simply maintain the same reported state as
>> before the rewrite.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>  include/linux/sched.h | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index f0f23efdb245..878acce2f8d6 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -1630,8 +1630,9 @@ static inline unsigned int __task_state_index(unsigned int tsk_state,
>>  	 * We're lying here, but rather than expose a completely new task state
>>  	 * to userspace, we can make this appear as if the task has gone through
>>  	 * a regular rt_mutex_lock() call.
>> +	 * Report the frozen task uninterruptible.
>>  	 */
>> -	if (tsk_state & TASK_RTLOCK_WAIT)
>> +	if (tsk_state & TASK_RTLOCK_WAIT || tsk_state & TASK_FROZEN)
> 
> Hmph, for RTLOCK this was good enough since !PREEMPT_RT really would be
> TASK_UNINTERRUPTIBLE. 
> 
> For the freezer AFAICT there's more variation, following your example:
> 

Thank you for your reply.
I tried to freeze a task whose stat is 'R', the saved_state is variable.

crash> task -R __state,saved_state 821
PID: 821    TASK: ffff888101df8000  CPU: 6   COMMAND: "test.sh"
  __state = 32768,
  saved_state = 0,

However, if a task is frozen, it will be better to show same stat in
case of confusion. In cgroup v2, it is shown as 'S'. In cgroup v1, the
prior stat shown was 'D'. Therefore, it might be a good choice to
maintain the same reported state as before the rewrite.

Or does anyone have a better idea?

Best regards,
Ridong

>   crash> task -R __state,saved_state 195
>   PID: 195      TASK: ffff888004abab80  CPU: 1    COMMAND: "sh"
>     __state = 32768    -> 0x8000 aka TASK_FROZEN
>     saved_state = 8193 -> 0x2001 aka TASK_INTERRUPTIBLE | TASK_FREEZABLE
> 
> so we ought to read ->saved_state, but as pointed out in [1] this is
> racy...
> 
> [1]: https://lore.kernel.org/lkml/20220120162520.570782-3-valentin.schneider@arm.com/
> 
>>  		state = TASK_UNINTERRUPTIBLE;
>>  
>>  	return fls(state);
>> -- 
>> 2.34.1
> 
>