Re: [RESEND][PATCH v2 2/2] sched: print parent comm in sched_show_task()

From: Petr Mladek
Date: Mon Mar 27 2023 - 07:43:44 EST


Hi,

please, send the patch as a plain text generated by git format-patch.
Ideally send it using git send-email.

Also please use: [PATCH v3] instead of [PATCH v2 2/2].

This [RESEND] was a multi-part mail with html. It is hard
to proceed by the tools like "b4". And it was even ignored by
https://lore.kernel.org/ so that it is not acrived.

The whole thread is messy. I suggest to send sent it as a new
mail (In-Reply-To).

Also, please, add Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>.
He might take this kind of patch.

Otherwise, the patch looks OK.

Best Regards,
Petr




On Thu 2023-03-23 19:01:03, Yuanhan 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 be better to solve the situation when the task
> is not alive and we could not get information about the parent.
>
> Signed-off-by: Tio Zhang <tiozhang@xxxxxxxxxxxxxx>
> ---
> kernel/sched/core.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index cb2aa2b54c7a..d8fd35684d6c 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,19 @@ 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));
> + } else {
> + pr_info("parent:unknown ppid:<NULL>\n");
> + }
> rcu_read_unlock();
> - pr_cont(" stack:%-5lu pid:%-5d ppid:%-6d flags:0x%08lx\n",
> - free, task_pid_nr(p), ppid,
> - read_task_thread_flags(p));
>
> print_worker_info(KERN_INFO, p);
> print_stop_info(KERN_INFO, p);
> --
> 2.17.1
>
> Yuanhan Zhang <zyhtheonly@xxxxxxxxx> 于2023年2月16日周四 16:19写道:
>
> > 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 be better to solve the situation when the task
> > is not alive and we could not get information about the parent.
> >
> > Signed-off-by: Tio Zhang <tiozhang@xxxxxxxxxxxxxx>
> > ---
> > kernel/sched/core.c | 18 +++++++++++-------
> > 1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index cb2aa2b54c7a..d8fd35684d6c 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,19 @@ 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));
> > + } else {
> > + pr_info("parent:unknown ppid:<NULL>\n");
> > + }
> > rcu_read_unlock();
> > - pr_cont(" stack:%-5lu pid:%-5d ppid:%-6d flags:0x%08lx\n",
> > - free, task_pid_nr(p), ppid,
> > - read_task_thread_flags(p));
> >
> > print_worker_info(KERN_INFO, p);
> > print_stop_info(KERN_INFO, p);
> > --
> > 2.17.1
> >
> > Chen Yu <yu.c.chen@xxxxxxxxx> 于2023年2月10日周五 00:22写道:
> >
> >> On 2023-01-31 at 16:10:26 +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
> >> s/would better/would be better/
> >> > is not alive and we could not get information about the parent.
> >> >
> >> > Signed-off-by: Tio Zhang <tiozhang@xxxxxxxxxxxxxx>
> >> >
> >> Looks ok to me,
> >> Tested-by: Chen Yu <yu.c.chen@xxxxxxxxx>
> >>
> >> thanks,
> >> Chenyu
> >>
> >