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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
---
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
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”<[email protected] <mailto:[email protected]>> 写入:
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 <[email protected] <mailto:[email protected]>>
---
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
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 <[email protected]>
>
Looks ok to me,
Tested-by: Chen Yu <[email protected]>
thanks,
Chenyu