[Crash-utility] [PATCH] Fix the "foreach DE" task identifier displays incorrect state tasks.

[lijiang]

Thank you for the comments, Kazu.

On Mon, Jul 31, 2023 at 10:08 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab at nec.com>
wrote:

> On 2023/07/28 20:15, Lianbo Jiang wrote:
> > Currently, the "foreach DE ps -m" command may display "DE" as well as
> > "ZO" state tasks as below:
> >
> >    crash> foreach DE ps -m
> >    ...
> >    [0 00:00:00.040] [ZO]  PID: 11458    TASK: ffff91c75680d280  CPU: 7
>   COMMAND: "ora_w01o_p01mci"
> >    [0 00:00:00.044] [ZO]  PID: 49118    TASK: ffff91c7bf3e8000  CPU: 19
>  COMMAND: "oracle_49118_p0"
> >    [0 00:00:00.050] [ZO]  PID: 28748    TASK: ffff91a7cbde3180  CPU: 2
>   COMMAND: "ora_imr0_p01sci"
> >    [0 00:00:00.050] [DE]  PID: 28405    TASK: ffff91a7c8eb0000  CPU: 27
>  COMMAND: "ora_vktm_p01sci"
> >    [0 00:00:00.051] [ZO]  PID: 31716    TASK: ffff91a7f7192100  CPU: 6
>   COMMAND: "ora_p001_p01sci"
> >    ...
>
> Could you elaborate on why this happens?
>
>
When the "DE" identifier is specified in the foreach command, it needs to
skip the non-"DE" tasks. Currently, crash doesn't deal with this case, only
handles some cases, such as "_RUNNING_", "_UNINTERRUPTIBLE_", "_NOLOAD_",
etc. So most tasks will be displayed directly.

I'm concerned about the difference of the conditions between printing
> the "DE" state in task_state_string() and printing a "DE" task in the
> foreach function in your patch:
>
> char *
> task_state_string(ulong task, char *buf, int verbose)
> {
> ...
>          if ((state & _DEAD_) && !set) {
>                  sprintf(buf, "DE");
>                  valid++;
>                  set++;
>          }
>
> On the other hand, the following means that "state == _DEAD_" is the
> "DE" state, not "state & _DEAD".  So I'm wondering if "foreach DE"
> command might drop some "DE" tasks with your patch?
>
>
The condition "state & _DEAD" can not filter out the tasks with a state
value of 0x50(0x10 + 0x40). It indicates that the "ZO" state tasks will be
always displayed, when the "DE" identifier is specified.

I just did the debug on it, let me share an example:

crash> foreach DE ps -m
...
foreach: state: 0x60, task_state: 0x50
[0 00:00:00.040] [ZO]  PID: 11458    TASK: ffff91c75680d280  CPU: 7
 COMMAND: "ora_w01o_p01mci"
foreach: state: 0x60, task_state: 0x50
[0 00:00:00.044] [ZO]  PID: 49118    TASK: ffff91c7bf3e8000  CPU: 19
COMMAND: "oracle_49118_p0"
foreach: state: 0x60, task_state: 0x50
[0 00:00:00.050] [ZO]  PID: 28748    TASK: ffff91a7cbde3180  CPU: 2
 COMMAND: "ora_imr0_p01sci"
foreach: state: 0x60, task_state: 0x60
[0 00:00:00.050] [DE]  PID: 28405    TASK: ffff91a7c8eb0000  CPU: 27
COMMAND: "ora_vktm_p01sci"
foreach: state: 0x60, task_state: 0x50
[0 00:00:00.051] [ZO]  PID: 31716    TASK: ffff91a7f7192100  CPU: 6
 COMMAND: "ora_p001_p01sci"
...

The DEAD state value is 0x60(0x20 + 0x40), the actual task state value is
0x50(0x10 + 0x40), and the ZOMBIE state value is 0x10, regardless of the
"task_state(task) & _DEAD_" or "task_state(task) & _ZOMBIE_", the result is
always true. So the condition "state & _DEAD" can not handle this case.

And also the actual task has a "DE" state value of 0x60.
...
foreach: state: 0x60, task_state: 0x60
[0 00:00:00.050] [DE]  PID: 28405    TASK: ffff91a7c8eb0000  CPU: 27
COMMAND: "ora_vktm_p01sci"
...

crash> help -t
...
           RUNNING:   0 (0x0)
     INTERRUPTIBLE:   1 (0x1)
   UNINTERRUPTIBLE:   2 (0x2)
           STOPPED:   4 (0x4)
   TRACING_STOPPED:   8 (0x8)
            ZOMBIE:  16 (0x10)
              DEAD:  32 and 64 (0x20 and 0x40)
          WAKEKILL: 128 (0x80)
            WAKING: 256 (0x100)
            PARKED: 512 (0x200)
crash>

Thanks.
Lianbo

 > +                            if (task_state(tc->task) != _DEAD_)
>  > +                                    continue;
>
> Thanks,
> Kazu
>
> >
> > That is not expected behavior, the "foreach" command needs to handle
> > such cases. Let's add a check to determine if the task state identifier
> > is specified and the task state identifier is equal to the "DE", so that
> > it can filter out the non-"DE" state tasks.
> >
> > With the patch:
> >    crash> foreach DE ps -m
> >    [0 00:00:00.050] [DE]  PID: 28405    TASK: ffff91a7c8eb0000  CPU: 27
>  COMMAND: "ora_vktm_p01sci"
> >    crash>
> >
> > Signed-off-by: Lianbo Jiang <lijiang at redhat.com>
> > ---
> >   task.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/task.c b/task.c
> > index b9076da35565..4f40c396b195 100644
> > --- a/task.c
> > +++ b/task.c
> > @@ -7043,6 +7043,9 @@ foreach(struct foreach_data *fd)
> >                       if (fd->state == _RUNNING_) {
> >                               if (task_state(tc->task) != _RUNNING_)
> >                                       continue;
> > +                     } else if (fd->state == _DEAD_) {
> > +                             if (task_state(tc->task) != _DEAD_)
> > +                                     continue;
> >                       } else if (fd->state & _UNINTERRUPTIBLE_) {
> >                               if (!(task_state(tc->task) &
> _UNINTERRUPTIBLE_))
> >                                       continue;
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20230731/a1a4fc5d/attachment-0001.htm>