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

[Petr Mladek]

On Tue 2023-01-17 08:33:56, 张元瀚 Tio Zhang wrote:
> Hi Chen,
> Thanks for your reply! I implement this according to Petr's suggestion here:
> 
> > A solution would be to move the parent value to another line.
> > It would even better solve the situation when the task
> > is not alive and we could not get information about the parent:
> >
> >        if (pid_alive(p)) {
> >                struct parent = rcu_dereference(p->real_parent);
> >
> >                pr_info("parent:%-15.15s ppid:%-6d\n",
> >                        parent->comm, task_pid_nr(parent));
> >        }
> 
> It seems do break the original format, but I guess printing 0 as ppid when the task is not alive
> would also confuse people sometimes.

Well, a task with pid 0 does not exist so it is not that bad.
But I agree that we could do better.

> For example, when people (and also most system monitor software) see ppid, they read the value in /proc/PID/status. According to task_tgid_nr_ns(), when the task is in a container with its parent outside the
> namespace, we will also see that ppid is 0 inside the container. And in our sched_show_task() here, we are calling task_pid_nr(), so the inconsistency maybe would confuse people under this scenario.
> 
> So maybe this new line style would be a better choice? Or we just keep the original format and
> move the parent's info (and should we print the parent's pid again here) to a new line.   

What about printing something like:

		pr_info("parent:unknown\n");

or

		pr_info("parent:unknown         ppid:<NULL>;

or

		pr_info("parent:???\n");

or

		pr_info("parent:unknown (task is exiting)\n");


I slightly prefer the 2nd variant. The <NULL> string makes it rather
clear that the information is not accessible. And pid_alive() actually
does:

	return p->thread_pid != NULL;

Best Regards,
Petr