Re: [Crash-utility] [PATCH v2] ps: Add support to "ps -l|-m" to properly display process list
-----Original Message-----
> > > crash> ps -m | head
> > > [ 0 00:59:36.582] [RU] PID: 4023608 TASK: ffff916f7c6b1840 CPU: 15
COMMAND: "makedumpfile"
> > > ^^^^^^^^^(1)
> > > [ 0 00:59:37.831] [ID] PID: 413 TASK: ffff916f772d3080 CPU: 15
COMMAND: "kworker/15:1H"
> > > [ 0 00:59:39.765] [IN] PID: 3929504 TASK: ffff916f5f0748c0 CPU: 15
COMMAND: "respawn_actlog"
> > > [ 0 00:59:40.650] [IN] PID: 1974 TASK: ffff91647dc53080 CPU: 15
COMMAND: "CPU 2/KVM"
> > > [ 0 00:59:41.925] [IN] PID: 1297 TASK: ffff916f63c46100 CPU: 15
COMMAND: "NetworkManager"
> > > [ 0 00:59:42.944] [ID] PID: 3763057 TASK: ffff9160c4519840 CPU: 15
COMMAND: "kworker/15:0"
> > > [ 0 00:59:42.944] [IN] PID: 101 TASK: ffff916040c91840 CPU: 15
COMMAND: "migration/15"
> > > [ 0 00:59:43.078] [IN] PID: 100 TASK: ffff916040c5b080 CPU: 15
COMMAND: "watchdog/15"
> > > [ 0 00:59:47.533] [IN] PID: 1292 TASK: ffff916f63c43080 CPU: 15
COMMAND: "lsmd"
> > > [ 0 00:59:49.089] [IN] PID: 113105 TASK: ffff9160412248c0 CPU: 15
COMMAND: "kvm-nx-lpage-re"
> > >
^^^^^^^(2)
> > > (1) large difference from zero
> > > (2) large differences among CPUs (probably due to the differences of irq
time)
> > >
> > > (1) might be solved with rq->clock_task, but (2) will be misleading and
confusing.
> > > So currently I'm thinking that the "ps -l|-m" options should
not use the se.exec_start.
> > > What do you think?
> > >
> >
> > Agreed. (As you indicated) Since rq->clock cannot hold accurate
> > timestamp we can rely on,
> > "ps -l|-m" options should not access the se.exec_start in case of
kernel image
> > without CONFIG_SCHEDSTATS or CONFIG_SCHED_INFO.
> >
> > Thanks for your code review.
> > If there is something to improve, let me propose a patch again.
>
> Sure.
>
> I think that the se.exec_start itself may be somewhat useful, especially for
> dumpfiles without CONFIG_SCHED{STATS,_INFO}, but not suitable for the current
> "ps -l|-m" behavior. There may be another way to make good use of it.
>
Now that you consider se.exec_start to be useful indicator,
how about adding seperate option to ps(ex: "ps -e") which display all
processes
sorted with most recently-run process based on se.exec_start?
Do you mean that it displays all processes together in a list sorted with
(rq->clock_task - se.exec_start)?
If so, personally I would not recommend it, because rq->clock{,_task} may not
be updated when a CPU stalls. I've seen such a vmcore occasionally, which
"runq -T" shows a big time lag on a CPU. In such a case, if they are sorted
with the above, the result will be misleading and hard to realize, I think.
(So it makes sense to me that the "ps -m" also sorts processes with the
value of last_arrival, not with (rq->clock - last_arrival).)
But adding a new option may be a possible way to use the se.exec_start.
If we do so, I would suggest to display processes for each CPU to avoid
misleading.
For example, you can see the -C with -l option display processes for
each CPU (with CONFIG_SCHEDSTATS or CONFIG_SCHED_INFO):
crash> ps -l -C 0-1
CPU: 0
[23604910745100388] [IN] PID: 1332212 TASK: ffff916f76718000 CPU: 0 COMMAND:
"kvm-pit/1332189"
[23604909788870007] [IN] PID: 1950 TASK: ffff916eabc06100 CPU: 0 COMMAND:
"qemu-kvm"
[23604909299613494] [IN] PID: 2347388 TASK: ffff916d539e6100 CPU: 0 COMMAND:
"CPU 3/KVM"
...
CPU: 1
[23604910550325113] [IN] PID: 1744 TASK: ffff916f6e250000 CPU: 1 COMMAND:
"tuned"
...
Like the above, the new -e option displays processes for each CPU
without -C, in other words, it cannot display all processes in a list:
crash> ps -e
CPU: 0
[23597255887600040] [IN] PID: 1332212 TASK: ffff916f76718000 CPU: 0 COMMAND:
"kvm-pit/1332189"
[23597254932332895] [IN] PID: 1950 TASK: ffff916eabc06100 CPU: 0 COMMAND:
"qemu-kvm"
[23597254443638756] [IN] PID: 2347388 TASK: ffff916d539e6100 CPU: 0 COMMAND:
"CPU 3/KVM"
...
CPU: 1
[23599915368410083] [IN] PID: 1744 TASK: ffff916f6e250000 CPU: 1 COMMAND:
"tuned"
...
This is a possible way I think of. Do you think this is useful?
Thanks,
Kazu
This does not affect "ps -l|-m" output at all.
If you agree with this idea, let me send another patch soon.
Anyway thanks for feedback and great code review.
BR,
Austin Kim
> Thanks,
> Kazu
>