On Mon, Jun 07, 2021 at 11:45:00AM +0100, Daniel Thompson wrote:
> On Wed, Jun 02, 2021 at 03:12:31PM +0200, Peter Zijlstra wrote:
> > Change the type and name of task_struct::state. Drop the volatile and
> > shrink it to an 'unsigned int'. Rename it in order to find all uses
> > such that we can use READ_ONCE/WRITE_ONCE as appropriate.
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> > ---
> > ...
> > kernel/debug/kdb/kdb_support.c | 18 +++++++------
> > ...
> > --- a/kernel/debug/kdb/kdb_support.c
> > +++ b/kernel/debug/kdb/kdb_support.c
> > @@ -609,23 +609,25 @@ unsigned long kdb_task_state_string(cons
> > */
> > char kdb_task_state_char (const struct task_struct *p)
> > {
> > - int cpu;
> > - char state;
> > + unsigned int p_state;
> > unsigned long tmp;
> > + char state;
> > + int cpu;
> >
> > if (!p ||
> > copy_from_kernel_nofault(&tmp, (char *)p, sizeof(unsigned long)))
> > return 'E';
> >
> > cpu = kdb_process_cpu(p);
> > - state = (p->state == 0) ? 'R' :
> > - (p->state < 0) ? 'U' :
> > - (p->state & TASK_UNINTERRUPTIBLE) ? 'D' :
> > - (p->state & TASK_STOPPED) ? 'T' :
> > - (p->state & TASK_TRACED) ? 'C' :
> > + p_state = READ_ONCE(p->__state);
> > + state = (p_state == 0) ? 'R' :
> > + (p_state < 0) ? 'U' :
>
> Looks like the U here stands for Unreachable since this patch makes it
> more obvious that this clause is (and previously was) exactly that!
>
> Dropping the U state would be good since I guess this will show up as a
> "new" warning in some tools. However it was a preexisting problem so with
> or without this cleaned up:
> Acked-by: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
Thanks!
Note that there's a second instance of this exact code in
arch/powerpc/xmon/xmon.c, with the same 'U' issue.
I'll repost this soon, as it seems I've fixed all robot failout (fingers
crossed).
