Messages in this thread | | | From | 张元瀚 Tio Zhang <> | Subject | Re: [PATCH] sched: print parent comm in sched_show_task() | Date | Tue, 17 Jan 2023 08:33:56 +0000 |
| |
Hi Chen, Thanks for your reply! I implement this according to Petr's suggestion here:
> A solution would be to move the parent value to another line. > It would even better solve the situation when the task > is not alive and we could not get information about the parent: > > if (pid_alive(p)) { > struct parent = rcu_dereference(p->real_parent); > > pr_info("parent:%-15.15s ppid:%-6d\n", > parent->comm, task_pid_nr(parent)); > }
It seems do break the original format, but I guess printing 0 as ppid when the task is not alive would also confuse people sometimes.
For example, when people (and also most system monitor software) see ppid, they read the value in /proc/PID/status. According to task_tgid_nr_ns(), when the task is in a container with its parent outside the namespace, we will also see that ppid is 0 inside the container. And in our sched_show_task() here, we are calling task_pid_nr(), so the inconsistency maybe would confuse people under this scenario.
So maybe this new line style would be a better choice? Or we just keep the original format and move the parent's info (and should we print the parent's pid again here) to a new line.
Thanks, Tio
在 2023/1/16 下午5:26,“Chen Yu”<yu.c.chen@intel.com <mailto:yu.c.chen@intel.com>> 写入:
Hi Tio, On 2023-01-13 at 18:54:13 +0800, Tio Zhang wrote: > Knowing who the parent is might be useful for debugging. > For example, we can sometimes resolve kernel hung tasks by stopping > the person who begins those hung tasks. > With the parent's name printed in sched_show_task(), > it might be helpful to let people know which "service" should be operated. > Also, we move the parent info to a following new line. > It would better solve the situation when the task > is not alive and we could not get information about the parent. > Maybe generate the patch via git format-patch -v2 because it is a second one. Also Cced corresponding sched maintainers as they will make the decision. > Signed-off-by: Tio Zhang <tiozhang@didiglobal.com <mailto:tiozhang@didiglobal.com>> > --- > kernel/sched/core.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index cb2aa2b54c7a..5be3f476ee5b 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -8853,7 +8853,6 @@ SYSCALL_DEFINE2(sched_rr_get_interval_time32, pid_t, pid, > void sched_show_task(struct task_struct *p) > { > unsigned long free = 0; > - int ppid; > > if (!try_get_task_stack(p)) > return; > @@ -8865,14 +8864,16 @@ void sched_show_task(struct task_struct *p) > #ifdef CONFIG_DEBUG_STACK_USAGE > free = stack_not_used(p); > #endif > - ppid = 0; > + pr_cont(" stack:%-5lu pid:%-5d flags:0x%08lx\n", > + free, task_pid_nr(p), read_task_thread_flags(p)); > + > rcu_read_lock(); > - if (pid_alive(p)) > - ppid = task_pid_nr(rcu_dereference(p->real_parent)); > + if (pid_alive(p)) { > + struct task_struct *parent = rcu_dereference(p->real_parent); > + > + pr_info("parent:%-15.15s ppid:%-6d", parent->comm, task_pid_nr(parent)); This might change the original scenario: if !pid_alive(p), the sched_show_task() will print ppid = 0(init task?).
thanks, Chenyu
| |