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 <[email protected]>.
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 <[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
>
> Yuanhan Zhang <[email protected]> 于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 <[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
> >
> > Chen Yu <[email protected]> 于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 <[email protected]>
> >> >
> >> Looks ok to me,
> >> Tested-by: Chen Yu <[email protected]>
> >>
> >> thanks,
> >> Chenyu
> >>
> >
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 <[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 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 <[email protected]>
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 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 <[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\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