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

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 <lijiang(a)redhat.com&gt; > --- > 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