| Message ID | 20211026133108.7113-1-kda@linux-powerpc.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | powerpc/xmon: fix task state output | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
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
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 >
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
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' :
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(-)