On 2023/08/02 17:18, 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"
>    ...
> 
> 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 specified identifier is equal to the actual task
> state identifier, so that it can filter out the unspecified 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 <liji...@redhat.com>
> ---
>   defs.h |  2 +-
>   task.c | 52 +++++++++++++++++++---------------------------------
>   2 files changed, 20 insertions(+), 34 deletions(-)
> 
> diff --git a/defs.h b/defs.h
> index 358f365585cf..5ee60f1eb3a5 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -1203,7 +1203,7 @@ struct foreach_data {
>               char *pattern;
>               regex_t regex;
>       } regex_info[MAX_REGEX_ARGS];
> -     ulong state;
> +     const char *state;
>       char *reference;
>       int keys;
>       int pids;
> diff --git a/task.c b/task.c
> index b9076da35565..20a9ce3aa40b 100644
> --- a/task.c
> +++ b/task.c
> @@ -6636,39 +6636,42 @@ cmd_foreach(void)
>                   STREQ(args[optind], "NE") ||
>                   STREQ(args[optind], "SW")) {
>   
> +                     ulong state = TASK_STATE_UNINITIALIZED;
> +
>                       if (fd->flags & FOREACH_STATE)
>                               error(FATAL, "only one task state allowed\n");
>   
>                       if (STREQ(args[optind], "RU"))
> -                             fd->state = _RUNNING_;
> +                             state = _RUNNING_;
>                       else if (STREQ(args[optind], "IN"))
> -                             fd->state = _INTERRUPTIBLE_;
> +                             state = _INTERRUPTIBLE_;
>                       else if (STREQ(args[optind], "UN"))
> -                             fd->state = _UNINTERRUPTIBLE_;
> +                             state = _UNINTERRUPTIBLE_;
>                       else if (STREQ(args[optind], "ST"))
> -                             fd->state = _STOPPED_;
> +                             state = _STOPPED_;
>                       else if (STREQ(args[optind], "TR"))
> -                             fd->state = _TRACING_STOPPED_;
> +                             state = _TRACING_STOPPED_;
>                       else if (STREQ(args[optind], "ZO"))
> -                             fd->state = _ZOMBIE_;
> +                             state = _ZOMBIE_;
>                       else if (STREQ(args[optind], "DE"))
> -                             fd->state = _DEAD_;
> +                             state = _DEAD_;
>                       else if (STREQ(args[optind], "SW"))
> -                             fd->state = _SWAPPING_;
> +                             state = _SWAPPING_;
>                       else if (STREQ(args[optind], "PA"))
> -                             fd->state = _PARKED_;
> +                             state = _PARKED_;
>                       else if (STREQ(args[optind], "WA"))
> -                             fd->state = _WAKING_;
> +                             state = _WAKING_;
>                       else if (STREQ(args[optind], "ID"))
> -                             fd->state = _UNINTERRUPTIBLE_|_NOLOAD_;
> +                             state = _UNINTERRUPTIBLE_|_NOLOAD_;
>                       else if (STREQ(args[optind], "NE"))
> -                             fd->state = _NEW_;
> +                             state = _NEW_;
>   
> -                     if (fd->state == TASK_STATE_UNINITIALIZED)
> +                     if (state == TASK_STATE_UNINITIALIZED)
>                               error(FATAL,
>                                   "invalid task state for this kernel: %s\n",
>                                       args[optind]);
>   
> +                     fd->state = args[optind];
>                       fd->flags |= FOREACH_STATE;
>   
>                       optind++;
> @@ -7039,26 +7042,9 @@ foreach(struct foreach_data *fd)
>               if ((fd->flags & FOREACH_KERNEL) && !is_kernel_thread(tc->task))
>                       continue;
>   
> -             if (fd->flags & FOREACH_STATE) {
> -                     if (fd->state == _RUNNING_) {
> -                             if (task_state(tc->task) != _RUNNING_)
> -                                     continue;
> -                     } else if (fd->state & _UNINTERRUPTIBLE_) {
> -                             if (!(task_state(tc->task) & _UNINTERRUPTIBLE_))
> -                                     continue;
> -
> -                             if (valid_task_state(_NOLOAD_)) {
> -                                     if (fd->state & _NOLOAD_) {
> -                                             if (!(task_state(tc->task) & 
> _NOLOAD_))
> -                                                     continue;
> -                                     } else {
> -                                             if ((task_state(tc->task) & 
> _NOLOAD_))
> -                                                     continue;
> -                                     }
> -                             }
> -                     } else if (!(task_state(tc->task) & fd->state))
> -                             continue;
> -             }
> +             if ((fd->flags & FOREACH_STATE) &&
> +                     (!STRNEQ(task_state_string(tc->task, buf, 0), 
> fd->state)))
> +                     continue;

Thank you for the update.

sorry for kind of nitpicking, why is this STRNEQ()?

Thanks,
Kazu
--
Crash-utility mailing list
Crash-utility@redhat.com
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

Reply via email to