[PATCH] powerpc/xmon: fix task state output

[PATCH] powerpc/xmon: fix task state output

[Denis Kirjanov]

p_state is unsigned since the commit 2f064a59a11f

The patch also uses TASK_RUNNING instead of null.

Fixes: 2f064a59a11f ("sched: Change task_struct::state")
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
 arch/powerpc/xmon/xmon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index dd8241c009e5..8b28ff9d98d1 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3264,8 +3264,7 @@ static void show_task(struct task_struct *volatile tsk)
 	 * appropriate for calling from xmon. This could be moved
 	 * to a common, generic, routine used by both.
 	 */
-	state = (p_state == 0) ? 'R' :
-		(p_state < 0) ? 'U' :
+	state = (p_state == TASK_RUNNING) ? 'R' :
 		(p_state & TASK_UNINTERRUPTIBLE) ? 'D' :
 		(p_state & TASK_STOPPED) ? 'T' :
 		(p_state & TASK_TRACED) ? 'C' :
-- 
2.16.4

Re: [PATCH] powerpc/xmon: fix task state output

[Michael Ellerman]

Denis Kirjanov <kda@linux-powerpc.org> writes:
> p_state is unsigned since the commit 2f064a59a11f
>
> The patch also uses TASK_RUNNING instead of null.
>
> Fixes: 2f064a59a11f ("sched: Change task_struct::state")
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> ---
>  arch/powerpc/xmon/xmon.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index dd8241c009e5..8b28ff9d98d1 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -3264,8 +3264,7 @@ static void show_task(struct task_struct *volatile tsk)
>  	 * appropriate for calling from xmon. This could be moved
>  	 * to a common, generic, routine used by both.
>  	 */
> -	state = (p_state == 0) ? 'R' :
> -		(p_state < 0) ? 'U' :

I guess 'U' meant 'unknown'? I always thought it meant uninterruptible,
but obviously that is 'D'.

> +	state = (p_state == TASK_RUNNING) ? 'R' :
>  		(p_state & TASK_UNINTERRUPTIBLE) ? 'D' :
>  		(p_state & TASK_STOPPED) ? 'T' :
>  		(p_state & TASK_TRACED) ? 'C' :

I think a better cleanup would be to use task_is_running(),
task_is_traced(), task_is_stopped(). That way we're insulated somewhat
from any future changes.

That would add additional READ_ONCE()s of the state, but I don't think
we care, the task should not be running if the system is in xmon.

cheers

Re: [PATCH] powerpc/xmon: fix task state output

[Denis Kirjanov]

On 10/27/21, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Denis Kirjanov <kda@linux-powerpc.org> writes:
>> p_state is unsigned since the commit 2f064a59a11f
>>
>> The patch also uses TASK_RUNNING instead of null.
>>
>> Fixes: 2f064a59a11f ("sched: Change task_struct::state")
>> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>> ---
>>  arch/powerpc/xmon/xmon.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> index dd8241c009e5..8b28ff9d98d1 100644
>> --- a/arch/powerpc/xmon/xmon.c
>> +++ b/arch/powerpc/xmon/xmon.c
>> @@ -3264,8 +3264,7 @@ static void show_task(struct task_struct *volatile
>> tsk)
>>  	 * appropriate for calling from xmon. This could be moved
>>  	 * to a common, generic, routine used by both.
>>  	 */
>> -	state = (p_state == 0) ? 'R' :
>> -		(p_state < 0) ? 'U' :
>
> I guess 'U' meant 'unknown'? I always thought it meant uninterruptible,
> but obviously that is 'D'.

Right.

>
>> +	state = (p_state == TASK_RUNNING) ? 'R' :
>>  		(p_state & TASK_UNINTERRUPTIBLE) ? 'D' :
>>  		(p_state & TASK_STOPPED) ? 'T' :
>>  		(p_state & TASK_TRACED) ? 'C' :
>
> I think a better cleanup would be to use task_is_running(),
> task_is_traced(), task_is_stopped(). That way we're insulated somewhat
> from any future changes.

In this case we end up with some states wrapped with a macro but
others are still explicitly checked using p_state variable.


>
> That would add additional READ_ONCE()s of the state, but I don't think
> we care, the task should not be running if the system is in xmon.
>
> cheers
>

Re: [PATCH] powerpc/xmon: fix task state output

[Michael Ellerman]

On Tue, 26 Oct 2021 16:31:08 +0300, Denis Kirjanov wrote:
> p_state is unsigned since the commit 2f064a59a11f
> 
> The patch also uses TASK_RUNNING instead of null.
> 
> 

Applied to powerpc/next.

[1/1] powerpc/xmon: fix task state output
      https://git.kernel.org/powerpc/c/b1f896ce3542eb2eede5949ee2e481526fae1108

cheers