linux-kernel - Re: [PATCH] sched: print parent comm in sched_show

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Y8fSyVvnmoNnxiiD@alley>
Date:   Wed, 18 Jan 2023 12:06:49 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     张元瀚 Tio Zhang <tiozhang@...iglobal.com>
Cc:     Chen Yu <yu.c.chen@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Ingo Molnar <mingo@...hat.com>,
        Juri Lelli <juri.lelli@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Yuanhan Zhang <zyhtheonly@...il.com>,
        "zwp10758@...il.com" <zwp10758@...il.com>,
        "zyhtheonly@...h.net" <zyhtheonly@...h.net>
Subject: Re: [PATCH] sched: print parent comm in sched_show_task()

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