[PATCH v2] sched: print parent comm in sched_show_task()

[PATCH v2] sched: print parent comm in sched_show_task()

[Tio Zhang]

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 while keeping the
original line the same.
And we would print "parent:unknown         ppid:<NULL>"
when the task is not alive.

Signed-off-by: Tio Zhang <tiozhang@didiglobal.com>
---
 kernel/sched/core.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index cb2aa2b54c7a..5690a5700f9e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8854,6 +8854,7 @@ void sched_show_task(struct task_struct *p)
 {
 	unsigned long free = 0;
 	int ppid;
+	char *pcomm = NULL;
 
 	if (!try_get_task_stack(p))
 		return;
@@ -8867,13 +8868,22 @@ void sched_show_task(struct task_struct *p)
 #endif
 	ppid = 0;
 	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);
+
+		ppid = task_pid_nr(parent);
+		pcomm = parent->comm;
+	}
 	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));
 
+	if (!ppid)
+		pr_info("parent:unknown         ppid:<NULL>\n");
+	else
+		pr_info("parent:%-15.15s ppid:%-6d\n", pcomm, ppid);
+
 	print_worker_info(KERN_INFO, p);
 	print_stop_info(KERN_INFO, p);
 	show_stack(p, NULL, KERN_INFO);
-- 
2.17.1

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

[Petr Mladek]

On Thu 2023-01-19 19:06:42, 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 while keeping the
> original line the same.
> And we would print "parent:unknown         ppid:<NULL>"
> when the task is not alive.
> 
> Signed-off-by: Tio Zhang <tiozhang@didiglobal.com>
> ---
>  kernel/sched/core.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index cb2aa2b54c7a..5690a5700f9e 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8854,6 +8854,7 @@ void sched_show_task(struct task_struct *p)
>  {
>  	unsigned long free = 0;
>  	int ppid;
> +	char *pcomm = NULL;
>  
>  	if (!try_get_task_stack(p))
>  		return;
> @@ -8867,13 +8868,22 @@ void sched_show_task(struct task_struct *p)
>  #endif
>  	ppid = 0;
>  	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);
> +
> +		ppid = task_pid_nr(parent);
> +		pcomm = parent->comm;
> +	}
>  	rcu_read_unlock();
>  	pr_cont(" stack:%-5lu pid:%-5d ppid:%-6d flags:0x%08lx\n",
>  		free, task_pid_nr(p), ppid,

This prints "ppid:0" when pid_alive() returns false.

>  		read_task_thread_flags(p));
>  
> +	if (!ppid)
> +		pr_info("parent:unknown         ppid:<NULL>\n");

This prints "ppid:<NULL>". The inconsistency is weird.

Also there is no need to print ppid twice. I would remove it
from the " stack:..." line as it was done in the previous version
of the patch.

> +	else

> +		pr_info("parent:%-15.15s ppid:%-6d\n", pcomm, ppid);

"pcomm" must not be accessed after rcu_read_unlock(). The parent might
disappear in the meantime.

I suggest to got back to the previous version and just add printing
"parent:unknown         ppid:<NULL>\n" when pid_alive(p) fails.

Best Regards,
Petr

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

[Tio Zhang]

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.

Signed-off-by: Tio Zhang <tiozhang@didiglobal.com>
---
 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

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

[张元瀚 Tio Zhang]

Hi Chen and sched maintainers,

Hope you are doing well!
Please help review this version and let me know if there are any following suggestions.

Thanks,  

在 2023/1/31 下午4:10，“张元瀚 Tio Zhang”<tiozhang@didiglobal.com <mailto:tiozhang@didiglobal.com>> 写入:


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.


Signed-off-by: Tio Zhang <tiozhang@didiglobal.com <mailto:tiozhang@didiglobal.com>>
---
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

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

[Chen Yu]

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@didiglobal.com>
>
Looks ok to me,
Tested-by: Chen Yu <yu.c.chen@intel.com> 

thanks,
Chenyu

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@linux-foundation.org>.
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@didiglobal.com>
> ---
>  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@gmail.com> 于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@didiglobal.com>
> > ---
> >  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@intel.com> 于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@didiglobal.com>
> >> >
> >> Looks ok to me,
> >> Tested-by: Chen Yu <yu.c.chen@intel.com>
> >>
> >> thanks,
> >> Chenyu
> >>
> >

[PATCH v3] sched: print parent comm in sched_show_task()

[Tio Zhang]

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@didiglobal.com>
---
 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

Re: [PATCH v3] sched: print parent comm in sched_show_task()

[Petr Mladek]

On Tue 2023-03-28 11:44:59, 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 be better to solve the situation when the task
> is not alive and we could not get information about the parent.
> 
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -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));

There is a missing new line delimiter "\n".

> +	} 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);

Otherwise, it looks good. With the added new line delimiter:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

[PATCH v3 2/2] sched: print parent comm in sched_show_task()

[Tio Zhang]

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@didiglobal.com>
---
 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\n", 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