[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <37FFCAF0-B0BA-48BD-B688-B1C5E7A10A1A@didiglobal.com>
Date: Tue, 17 Jan 2023 08:33:56 +0000
From: 张元瀚 Tio Zhang <tiozhang@...iglobal.com>
To: Chen Yu <yu.c.chen@...el.com>
CC: Peter Zijlstra <peterz@...radead.org>,
Vincent Guittot <vincent.guittot@...aro.org>,
Ingo Molnar <mingo@...hat.com>,
Juri Lelli <juri.lelli@...hat.com>,
Petr Mladek <pmladek@...e.com>,
LKML <linux-kernel@...r.kernel.org>,
Yuanhan Zhang <zyhtheonly@...il.com>,
"zwp10758@...il.com" <zwp10758@...il.com>,
"zyhtheonly@...h.net" <zyhtheonly@...h.net>
Subject: Re: [PATCH] sched: print parent comm in sched_show_task()
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@...el.com <mailto:yu.c.chen@...el.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@...iglobal.com <mailto:tiozhang@...iglobal.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
Powered by blists - more mailing lists