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

[Petr Mladek]

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
> >>
> >