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) <pet...@infradead.org>
> ---
>  ...
>  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.thomp...@linaro.org>


Daniel.

> +             (p_state & TASK_UNINTERRUPTIBLE) ? 'D' :
> +             (p_state & TASK_STOPPED) ? 'T' :
> +             (p_state & TASK_TRACED) ? 'C' :
>               (p->exit_state & EXIT_ZOMBIE) ? 'Z' :
>               (p->exit_state & EXIT_DEAD) ? 'E' :
> -             (p->state & TASK_INTERRUPTIBLE) ? 'S' : '?';
> +             (p_state & TASK_INTERRUPTIBLE) ? 'S' : '?';
>       if (is_idle_task(p)) {
>               /* Idle task.  Is it really idle, apart from the kdb
>                * interrupt? */


_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Reply via email to