All of lore.kernel.org
 help / color / mirror / Atom feed
* /proc/$PID/sched does not take PID namespace into account
@ 2013-09-03 14:22 Emmanuel Deloget
  2013-09-09 11:01 ` Peter Zijlstra
  0 siblings, 1 reply; 5+ messages in thread
From: Emmanuel Deloget @ 2013-09-03 14:22 UTC (permalink / raw)
  To: [ML] linux-kernel; +Cc: Emmanuel Deloget, Ingo Molnar, Peter Zijlstra

[-- Attachment #1: Type: text/plain, Size: 2580 bytes --]

Hello,

(please CC me when answering this mail ; and sorry for my borken
English).

I noticed that when a process is executed in a PID namespace it can
still find his real PID by parsing /proc/$PID_IN_NS/sched.

(in this example I created a new PID namespace and ran /bin/bash at
PID 1)

  # uname -a
  Linux my-thinkpad 3.9-1-amd64 #1 SMP Debian 3.9.6-1 x86_64 GNU/Linux
  #
  # pidof bash
  1
  # head -n 1 /proc/1/sched
  bash (14957, #threads: 1)
  # cat /proc/1/stat
  1 (bash) S 0 1 0 34823 220 4202752 11359 (...)

In the root PID namespace

  # pidof bash
  (...) 14957 (...)
  # head -n 1 /proc/14957/sched
  bash (14957, #threads: 1)
  # cat /proc/14957/stat
  14957 (bash) S 14956 14957 23465 34823 14957 4202752 11386 (...)

The principle of least surprise tells me that if my PID is n in a
particular PID namespace then every stat or debug info should tell
me that my PID is n (i.e. I should not be able to get the real PID
of my program). This is a consistency issue: if I get different
information from different sources I may not be able to tell which
one is the right one.

The issue (if this is really an issue) lies in kernel/sched/debug.c,
function proc_sched_show_task(). The code says [1]:

    SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
                        get_nr_threads(p));

I understand that you're not supposed to use the content of a /proc
file that has been generated by a kernel source file named "debug.c"
in a production environment yet several distributions out there seems
to enable this by default (at least the Debian distribution I'm using
does it).

I see a few options:

* either it's a bug and it should be corrected (I'm not sure how to
  do it; the printed PID should reflect the current PID namespace
  and I don't how how to get this information).

* or it has been decided on purpose (i.e. it's not a bug);
  /proc/$PID/sched then offers a reliable way to get the real PID
  of any process, including processes that run in a child PID
  namespace.

* or it's a bug but it cannot be corrected (kernel ABI ; it has been
  here for a looooong time - possibly since the PID namespace
  integration).

(There might be other options here).

In the two last cases there is no need for a patch but I'd be happy
if someone explains the reasoning.

Best regards,

-- Emmanuel Deloget

[1]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/kernel/sched/debug.c?id=refs/tags/v3.11#n495
[2]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/kernel/sched/debug.c?id=refs/tags/v3.11#n118



[-- Attachment #2: emmanuel_deloget.vcf --]
[-- Type: text/x-vcard, Size: 280 bytes --]

begin:vcard
fn:Emmanuel Deloget
n:Deloget;Emmanuel
org:efixo / SFR;DATA
adr;quoted-printable:;;67 mont=C3=A9e de St Menet;MARSEILLE;;13011;FRANCE
email;internet:emmanuel.deloget@efixo.com
title:Team Leader
tel;work:04 88 15 50 77
url:www.sfr.fr
version:2.1
end:vcard


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: /proc/$PID/sched does not take PID namespace into account
  2013-09-03 14:22 /proc/$PID/sched does not take PID namespace into account Emmanuel Deloget
@ 2013-09-09 11:01 ` Peter Zijlstra
  2013-09-09 13:58   ` Emmanuel Deloget
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Peter Zijlstra @ 2013-09-09 11:01 UTC (permalink / raw)
  To: Emmanuel Deloget; +Cc: [ML] linux-kernel, Ingo Molnar

On Tue, Sep 03, 2013 at 04:22:33PM +0200, Emmanuel Deloget wrote:
> The issue (if this is really an issue) lies in kernel/sched/debug.c,
> function proc_sched_show_task(). The code says [1]:
> 
>     SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
>                         get_nr_threads(p));
> 

> I see a few options:
> 
> * either it's a bug and it should be corrected (I'm not sure how to
>   do it; the printed PID should reflect the current PID namespace
>   and I don't how how to get this information).

I suppose something like the below ought to work? -- completely
untested, please confirm.

---
Subject: sched, debug: Use PID namespaces

Emmanuel reported that /proc/sched_debug didn't report the right PIDs
when using namespaces, cure this.

Reported-by: Emmanuel Deloget <emmanuel.deloget@efixo.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
---
 kernel/sched/debug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index e076bdd..e30bf44 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -124,7 +124,7 @@ print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
 		SEQ_printf(m, " ");
 
 	SEQ_printf(m, "%15s %5d %9Ld.%06ld %9Ld %5d ",
-		p->comm, p->pid,
+		p->comm, task_pid(p),,
 		SPLIT_NS(p->se.vruntime),
 		(long long)(p->nvcsw + p->nivcsw),
 		p->prio);
@@ -289,7 +289,7 @@ do {									\
 	P(nr_load_updates);
 	P(nr_uninterruptible);
 	PN(next_balance);
-	P(curr->pid);
+	P(task_pid(curr));
 	PN(clock);
 	P(cpu_load[0]);
 	P(cpu_load[1]);
@@ -492,7 +492,7 @@ void proc_sched_show_task(struct task_struct *p, struct seq_file *m)
 {
 	unsigned long nr_switches;
 
-	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
+	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid(p),
 						get_nr_threads(p));
 	SEQ_printf(m,
 		"---------------------------------------------------------"

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: /proc/$PID/sched does not take PID namespace into account
  2013-09-09 11:01 ` Peter Zijlstra
@ 2013-09-09 13:58   ` Emmanuel Deloget
  2013-09-10  9:18   ` Peter Zijlstra
  2013-09-12 18:05   ` [tip:sched/core] sched/debug: Take " tip-bot for Peter Zijlstra
  2 siblings, 0 replies; 5+ messages in thread
From: Emmanuel Deloget @ 2013-09-09 13:58 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: [ML] linux-kernel, Ingo Molnar

[-- Attachment #1: Type: text/plain, Size: 2203 bytes --]

Hello,

On 09/09/2013 13:01, Peter Zijlstra wrote:
> On Tue, Sep 03, 2013 at 04:22:33PM +0200, Emmanuel Deloget wrote:
>> The issue (if this is really an issue) lies in kernel/sched/debug.c,
>> function proc_sched_show_task(). The code says [1]:
>>
>>     SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
>>                         get_nr_threads(p));
>>
>> I see a few options:
>>
>> * either it's a bug and it should be corrected (I'm not sure how to
>>   do it; the printed PID should reflect the current PID namespace
>>   and I don't how how to get this information).
> I suppose something like the below ought to work? -- completely
> untested, please confirm.

Thanks !

I'll test that as soon as possible and I'll report to you.

Best regards,

-- Emmanuel Deloget

>
> ---
> Subject: sched, debug: Use PID namespaces
>
> Emmanuel reported that /proc/sched_debug didn't report the right PIDs
> when using namespaces, cure this.
>
> Reported-by: Emmanuel Deloget <emmanuel.deloget@efixo.com>
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> ---
>  kernel/sched/debug.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index e076bdd..e30bf44 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -124,7 +124,7 @@ print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
>  		SEQ_printf(m, " ");
>  
>  	SEQ_printf(m, "%15s %5d %9Ld.%06ld %9Ld %5d ",
> -		p->comm, p->pid,
> +		p->comm, task_pid(p),,
>  		SPLIT_NS(p->se.vruntime),
>  		(long long)(p->nvcsw + p->nivcsw),
>  		p->prio);
> @@ -289,7 +289,7 @@ do {									\
>  	P(nr_load_updates);
>  	P(nr_uninterruptible);
>  	PN(next_balance);
> -	P(curr->pid);
> +	P(task_pid(curr));
>  	PN(clock);
>  	P(cpu_load[0]);
>  	P(cpu_load[1]);
> @@ -492,7 +492,7 @@ void proc_sched_show_task(struct task_struct *p, struct seq_file *m)
>  {
>  	unsigned long nr_switches;
>  
> -	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
> +	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid(p),
>  						get_nr_threads(p));
>  	SEQ_printf(m,
>  		"---------------------------------------------------------"
> .
>


-- 

[-- Attachment #2: emmanuel_deloget.vcf --]
[-- Type: text/x-vcard, Size: 268 bytes --]

begin:vcard
fn:Emmanuel Deloget
n:Deloget;Emmanuel
org:efixo / SFR;DATA
adr;quoted-printable:;;67 mont=C3=A9e de St Menet;MARSEILLE;;13011;FRANCE
email;internet:emmanuel.deloget@efixo.com
title:Team Leader
tel;work:04 88 15 50 77
url:www.sfr.fr
version:2.1
end:vcard


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: /proc/$PID/sched does not take PID namespace into account
  2013-09-09 11:01 ` Peter Zijlstra
  2013-09-09 13:58   ` Emmanuel Deloget
@ 2013-09-10  9:18   ` Peter Zijlstra
  2013-09-12 18:05   ` [tip:sched/core] sched/debug: Take " tip-bot for Peter Zijlstra
  2 siblings, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2013-09-10  9:18 UTC (permalink / raw)
  To: Emmanuel Deloget; +Cc: [ML] linux-kernel, Ingo Molnar

On Mon, Sep 09, 2013 at 01:01:41PM +0200, Peter Zijlstra wrote:
> On Tue, Sep 03, 2013 at 04:22:33PM +0200, Emmanuel Deloget wrote:
> > The issue (if this is really an issue) lies in kernel/sched/debug.c,
> > function proc_sched_show_task(). The code says [1]:
> > 
> >     SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
> >                         get_nr_threads(p));
> > 
> 
> > I see a few options:
> > 
> > * either it's a bug and it should be corrected (I'm not sure how to
> >   do it; the printed PID should reflect the current PID namespace
> >   and I don't how how to get this information).
> 
> I suppose something like the below ought to work? -- completely
> untested, please confirm.
> 
> ---
> Subject: sched, debug: Use PID namespaces
> 
> Emmanuel reported that /proc/sched_debug didn't report the right PIDs
> when using namespaces, cure this.
> 
> Reported-by: Emmanuel Deloget <emmanuel.deloget@efixo.com>
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> ---
>  kernel/sched/debug.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index e076bdd..e30bf44 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -124,7 +124,7 @@ print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
>  		SEQ_printf(m, " ");
>  
>  	SEQ_printf(m, "%15s %5d %9Ld.%06ld %9Ld %5d ",
> -		p->comm, p->pid,
> +		p->comm, task_pid(p),,
>  		SPLIT_NS(p->se.vruntime),
>  		(long long)(p->nvcsw + p->nivcsw),
>  		p->prio);
> @@ -289,7 +289,7 @@ do {									\
>  	P(nr_load_updates);
>  	P(nr_uninterruptible);
>  	PN(next_balance);
> -	P(curr->pid);
> +	P(task_pid(curr));
>  	PN(clock);
>  	P(cpu_load[0]);
>  	P(cpu_load[1]);
> @@ -492,7 +492,7 @@ void proc_sched_show_task(struct task_struct *p, struct seq_file *m)
>  {
>  	unsigned long nr_switches;
>  
> -	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
> +	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid(p),
>  						get_nr_threads(p));
>  	SEQ_printf(m,
>  		"---------------------------------------------------------"

Please do s/task_pid/task_pid_nr/g on this before trying. 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip:sched/core] sched/debug: Take PID namespace into account
  2013-09-09 11:01 ` Peter Zijlstra
  2013-09-09 13:58   ` Emmanuel Deloget
  2013-09-10  9:18   ` Peter Zijlstra
@ 2013-09-12 18:05   ` tip-bot for Peter Zijlstra
  2 siblings, 0 replies; 5+ messages in thread
From: tip-bot for Peter Zijlstra @ 2013-09-12 18:05 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, peterz, tglx, emmanuel.deloget

Commit-ID:  fc840914e9b07ab4685c195e1e54e58de4f84c03
Gitweb:     http://git.kernel.org/tip/fc840914e9b07ab4685c195e1e54e58de4f84c03
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Mon, 9 Sep 2013 13:01:41 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 12 Sep 2013 19:14:16 +0200

sched/debug: Take PID namespace into account

Emmanuel reported that /proc/sched_debug didn't report the right PIDs
when using namespaces, cure this.

Reported-by: Emmanuel Deloget <emmanuel.deloget@efixo.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20130909110141.GM31370@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/debug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index e076bdd..1965599 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -124,7 +124,7 @@ print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
 		SEQ_printf(m, " ");
 
 	SEQ_printf(m, "%15s %5d %9Ld.%06ld %9Ld %5d ",
-		p->comm, p->pid,
+		p->comm, task_pid_nr(p),
 		SPLIT_NS(p->se.vruntime),
 		(long long)(p->nvcsw + p->nivcsw),
 		p->prio);
@@ -289,7 +289,7 @@ do {									\
 	P(nr_load_updates);
 	P(nr_uninterruptible);
 	PN(next_balance);
-	P(curr->pid);
+	SEQ_printf(m, "  .%-30s: %ld\n", "curr->pid", (long)(task_pid_nr(rq->curr)));
 	PN(clock);
 	P(cpu_load[0]);
 	P(cpu_load[1]);
@@ -492,7 +492,7 @@ void proc_sched_show_task(struct task_struct *p, struct seq_file *m)
 {
 	unsigned long nr_switches;
 
-	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
+	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_nr(p),
 						get_nr_threads(p));
 	SEQ_printf(m,
 		"---------------------------------------------------------"

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-09-12 18:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-03 14:22 /proc/$PID/sched does not take PID namespace into account Emmanuel Deloget
2013-09-09 11:01 ` Peter Zijlstra
2013-09-09 13:58   ` Emmanuel Deloget
2013-09-10  9:18   ` Peter Zijlstra
2013-09-12 18:05   ` [tip:sched/core] sched/debug: Take " tip-bot for Peter Zijlstra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.