[BUG] events process running on wrong cpu

[BUG] events process running on wrong cpu

[Heiko Carstens]

Hi,

with todays git from time to time I get this (s390 defconfig):


BUG: using smp_processor_id() in preemptible [00000000] code: events/1/16
caller is vmstat_update+0x28/0x74
CPU: 0 Not tainted 2.6.31-06618-gebc79c4-dirty #3
Process events/1 (pid: 16, task: 000000003fcbea00, ksp: 000000003fcd7cd8)
000000003fcd7da8 000000003fcd7d28 0000000000000002 0000000000000000 
       000000003fcd7dc8 000000003fcd7d40 000000003fcd7d40 00000000003fe46a 
       000000000001ffff 000000003fcd7e70 000000000192b128 ffffffffffffc000 
       000000000000000d 000000000000000c 000000003fcd7d90 0000000000000000 
       0000000000000000 0000000000016bda 000000003fcd7d28 000000003fcd7d68 
Call Trace:
([<0000000000016aea>] show_trace+0xe6/0x134)
 [<0000000000202a5e>] debug_smp_processor_id+0x112/0x12c
 [<00000000000b302c>] vmstat_update+0x28/0x74
 [<000000000005fb9a>] worker_thread+0x19a/0x27c
 [<0000000000065f80>] kthread+0xa0/0xa8
 [<000000000001b786>] kernel_thread_starter+0x6/0xc
 [<000000000001b780>] kernel_thread_starter+0x0/0xc

Please note that events/1 is running in cpu 0 (!).

But (with a patched procfs output):

[root@h4245004 ~]# cat /proc/16/status 
Name:   events/1
State:  S (sleeping)
Tgid:   16
Pid:    16
PPid:   2
TracerPid:      0
Uid:    0       0       0       0
Gid:    0       0       0       0
FDSize: 64
Groups:
Threads:        1
SigQ:   0/4096
SigPnd: 0000000000000000
ShdPnd: 0000000000000000
SigBlk: 0000000000000000
SigIgn: ffffffffffffffff
SigCgt: 0000000000000000
CapInh: 0000000000000000
CapPrm: ffffffffffffffff
CapEff: fffffffffffffeff
CapBnd: fffffffffffffeff
Cpus_allowed:   00000002  <----------
Cpus_allowed_list:      1 <----------
task: 000000003fcbea00, ksp: 000000003fcd7cd8
User PSW : 0704000180000000 000000000001b780
User GPRS: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
           0000000000000000 0000000000000000 0000000000000000 0000000000000000
           0000000000000000 0000000000065ee0 000000003fe73cf8 000000000004d414
           0000000000000000 0000000000000000 0000000000000000 000000003fcd7eb8
User ACRS: 00000000 00000000 00000000 00000000
           00000000 00000000 00000000 00000000
           00000000 00000000 00000000 00000000
           00000000 00000000 00000000 00000000
voluntary_ctxt_switches:        55
nonvoluntary_ctxt_switches:     1

So the cpus_allowed mask is correct (later on at least). Looks like the
created kthread ran before its cpus_allowed mask was set.
Looking at the code this shouldn't happen. But still...

Re: [BUG] events process running on wrong cpu

[Peter Zijlstra]

On Mon, 2009-09-21 at 10:51 +0200, Heiko Carstens wrote:
> Hi,
> 
> with todays git from time to time I get this (s390 defconfig):

Yep, I messed up..

Does your kernel contain the below?


---
commit 3f04e8cd5b24727a2500f8ab8f3de730ba47b02c
Author: Mike Galbraith <efault@gmx.de>
Date:   Sat Sep 19 16:52:35 2009 +0200

    sched: Re-add lost cpu_allowed check to sched_fair.c::select_task_rq_fair()
    
    While doing some testing, I pinned mplayer, only to find it
    following X around like a puppy. Looking at commit c88d591, I found
    a cpu_allowed check that went AWOL.  I plugged it back in where it
    looks like it needs to go, and now when I say "sit, stay!", mplayer
    obeys again.
    
    'c88d591 sched: Merge select_task_rq_fair() and
    sched_balance_self()' accidentally dropped the check, causing
    wake_affine() to pull pinned tasks - put it back.
    
    [ v2: use a cheaper version from Peter ]
    
    Signed-off-by: Mike Galbraith <efault@gmx.de>
    Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
    LKML-Reference: <new-submission>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 29b35a7..566e3bb 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -1339,7 +1339,8 @@ static int select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flag
 	int sync = wake_flags & WF_SYNC;
 
 	if (sd_flag & SD_BALANCE_WAKE) {
-		if (sched_feat(AFFINE_WAKEUPS))
+		if (sched_feat(AFFINE_WAKEUPS) &&
+		    cpumask_test_cpu(cpu, &p->cpus_allowed))
 			want_affine = 1;
 		new_cpu = prev_cpu;
 	}

Re: [BUG] events process running on wrong cpu

[Ingo Molnar]

* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> On Mon, 2009-09-21 at 10:51 +0200, Heiko Carstens wrote:
> > Hi,
> > 
> > with todays git from time to time I get this (s390 defconfig):
> 
> Yep, I messed up..
> 
> Does your kernel contain the below?

FYI, that fix will go Linuswards later today.

	Ingo

[PATCH] Always show Cpus_allowed field in /proc/<pid>/status.

[Heiko Carstens]

Subject: [PATCH] Always show Cpus_allowed fields in /proc/<pid>/status.

From: Heiko Carstens <heiko.carstens@de.ibm.com>

The Cpus_allowed fields in /proc/<pid>/status is currently only shown in
case of CONFIG_CPUSETS. However their contents are also useful for the
!CONFIG_CPUSETS case.
So change the current behaviour and always show these fields.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 fs/proc/array.c |   11 +++++++++++
 kernel/cpuset.c |    8 +-------
 2 files changed, 12 insertions(+), 7 deletions(-)

Index: linux-2.6/fs/proc/array.c
===================================================================
--- linux-2.6.orig/fs/proc/array.c
+++ linux-2.6/fs/proc/array.c
@@ -321,6 +321,16 @@ static inline void task_context_switch_c
 			p->nivcsw);
 }
 
+void task_cpus_allowed(struct seq_file *m, struct task_struct *task)
+{
+	seq_printf(m, "Cpus_allowed:\t");
+	seq_cpumask(m, &task->cpus_allowed);
+	seq_printf(m, "\n");
+	seq_printf(m, "Cpus_allowed_list:\t");
+	seq_cpumask_list(m, &task->cpus_allowed);
+	seq_printf(m, "\n");
+}
+
 int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
 			struct pid *pid, struct task_struct *task)
 {
@@ -335,6 +345,7 @@ int proc_pid_status(struct seq_file *m, 
 	}
 	task_sig(m, task);
 	task_cap(m, task);
+	task_cpus_allowed(m, task);
 	cpuset_task_status_allowed(m, task);
 #if defined(CONFIG_S390)
 	task_show_regs(m, task);
Index: linux-2.6/kernel/cpuset.c
===================================================================
--- linux-2.6.orig/kernel/cpuset.c
+++ linux-2.6/kernel/cpuset.c
@@ -2499,15 +2499,9 @@ const struct file_operations proc_cpuset
 };
 #endif /* CONFIG_PROC_PID_CPUSET */
 
-/* Display task cpus_allowed, mems_allowed in /proc/<pid>/status file. */
+/* Display task mems_allowed in /proc/<pid>/status file. */
 void cpuset_task_status_allowed(struct seq_file *m, struct task_struct *task)
 {
-	seq_printf(m, "Cpus_allowed:\t");
-	seq_cpumask(m, &task->cpus_allowed);
-	seq_printf(m, "\n");
-	seq_printf(m, "Cpus_allowed_list:\t");
-	seq_cpumask_list(m, &task->cpus_allowed);
-	seq_printf(m, "\n");
 	seq_printf(m, "Mems_allowed:\t");
 	seq_nodemask(m, &task->mems_allowed);
 	seq_printf(m, "\n");

Re: [BUG] events process running on wrong cpu

[Heiko Carstens]

On Mon, Sep 21, 2009 at 11:03:24AM +0200, Ingo Molnar wrote:
> 
> * Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> 
> > On Mon, 2009-09-21 at 10:51 +0200, Heiko Carstens wrote:
> > > Hi,
> > > 
> > > with todays git from time to time I get this (s390 defconfig):
> > 
> > Yep, I messed up..
> > 
> > Does your kernel contain the below?
> 
> FYI, that fix will go Linuswards later today.

Ok, so it's already fixed. Thanks! ;)

Re: [PATCH] Always show Cpus_allowed field in /proc/<pid>/status.

[Heiko Carstens]

On Mon, Sep 21, 2009 at 11:06:27AM +0200, Heiko Carstens wrote:

> +void task_cpus_allowed(struct seq_file *m, struct task_struct *task)

sigh. This should have been static.. updated patch below.


Subject: [PATCH] Always show Cpus_allowed fields in /proc/<pid>/status.

From: Heiko Carstens <heiko.carstens@de.ibm.com>

The Cpus_allowed fields in /proc/<pid>/status is currently only shown in
case of CONFIG_CPUSETS. However their contents are also useful for the
!CONFIG_CPUSETS case.
So change the current behaviour and always show these fields.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 fs/proc/array.c |   11 +++++++++++
 kernel/cpuset.c |    8 +-------
 2 files changed, 12 insertions(+), 7 deletions(-)

Index: linux-2.6/fs/proc/array.c
===================================================================
--- linux-2.6.orig/fs/proc/array.c
+++ linux-2.6/fs/proc/array.c
@@ -321,6 +321,16 @@ static inline void task_context_switch_c
 			p->nivcsw);
 }
 
+static void task_cpus_allowed(struct seq_file *m, struct task_struct *task)
+{
+	seq_printf(m, "Cpus_allowed:\t");
+	seq_cpumask(m, &task->cpus_allowed);
+	seq_printf(m, "\n");
+	seq_printf(m, "Cpus_allowed_list:\t");
+	seq_cpumask_list(m, &task->cpus_allowed);
+	seq_printf(m, "\n");
+}
+
 int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
 			struct pid *pid, struct task_struct *task)
 {
@@ -335,6 +345,7 @@ int proc_pid_status(struct seq_file *m, 
 	}
 	task_sig(m, task);
 	task_cap(m, task);
+	task_cpus_allowed(m, task);
 	cpuset_task_status_allowed(m, task);
 #if defined(CONFIG_S390)
 	task_show_regs(m, task);
Index: linux-2.6/kernel/cpuset.c
===================================================================
--- linux-2.6.orig/kernel/cpuset.c
+++ linux-2.6/kernel/cpuset.c
@@ -2499,15 +2499,9 @@ const struct file_operations proc_cpuset
 };
 #endif /* CONFIG_PROC_PID_CPUSET */
 
-/* Display task cpus_allowed, mems_allowed in /proc/<pid>/status file. */
+/* Display task mems_allowed in /proc/<pid>/status file. */
 void cpuset_task_status_allowed(struct seq_file *m, struct task_struct *task)
 {
-	seq_printf(m, "Cpus_allowed:\t");
-	seq_cpumask(m, &task->cpus_allowed);
-	seq_printf(m, "\n");
-	seq_printf(m, "Cpus_allowed_list:\t");
-	seq_cpumask_list(m, &task->cpus_allowed);
-	seq_printf(m, "\n");
 	seq_printf(m, "Mems_allowed:\t");
 	seq_nodemask(m, &task->mems_allowed);
 	seq_printf(m, "\n");

[tip:sched/core] sched: Always show Cpus_allowed field in /proc/<pid>/status

[tip-bot for Heiko Carstens]

Commit-ID:  d01d4827858cdc2e1c437c87ab65ec0a00fd40f8
Gitweb:     http://git.kernel.org/tip/d01d4827858cdc2e1c437c87ab65ec0a00fd40f8
Author:     Heiko Carstens <heiko.carstens@de.ibm.com>
AuthorDate: Mon, 21 Sep 2009 11:06:27 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 21 Sep 2009 11:37:27 +0200

sched: Always show Cpus_allowed field in /proc/<pid>/status

The Cpus_allowed fields in /proc/<pid>/status is currently only
shown in case of CONFIG_CPUSETS. However their contents are also
useful for the !CONFIG_CPUSETS case.

So change the current behaviour and always show these fields.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <20090921090627.GD4649@osiris.boeblingen.de.ibm.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 fs/proc/array.c |   11 +++++++++++
 kernel/cpuset.c |    8 +-------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 725a650..762aea9 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -321,6 +321,16 @@ static inline void task_context_switch_counts(struct seq_file *m,
 			p->nivcsw);
 }
 
+static void task_cpus_allowed(struct seq_file *m, struct task_struct *task)
+{
+	seq_printf(m, "Cpus_allowed:\t");
+	seq_cpumask(m, &task->cpus_allowed);
+	seq_printf(m, "\n");
+	seq_printf(m, "Cpus_allowed_list:\t");
+	seq_cpumask_list(m, &task->cpus_allowed);
+	seq_printf(m, "\n");
+}
+
 int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
 			struct pid *pid, struct task_struct *task)
 {
@@ -335,6 +345,7 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
 	}
 	task_sig(m, task);
 	task_cap(m, task);
+	task_cpus_allowed(m, task);
 	cpuset_task_status_allowed(m, task);
 #if defined(CONFIG_S390)
 	task_show_regs(m, task);
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 7e75a41..b81f7f0 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -2499,15 +2499,9 @@ const struct file_operations proc_cpuset_operations = {
 };
 #endif /* CONFIG_PROC_PID_CPUSET */
 
-/* Display task cpus_allowed, mems_allowed in /proc/<pid>/status file. */
+/* Display task mems_allowed in /proc/<pid>/status file. */
 void cpuset_task_status_allowed(struct seq_file *m, struct task_struct *task)
 {
-	seq_printf(m, "Cpus_allowed:\t");
-	seq_cpumask(m, &task->cpus_allowed);
-	seq_printf(m, "\n");
-	seq_printf(m, "Cpus_allowed_list:\t");
-	seq_cpumask_list(m, &task->cpus_allowed);
-	seq_printf(m, "\n");
 	seq_printf(m, "Mems_allowed:\t");
 	seq_nodemask(m, &task->mems_allowed);
 	seq_printf(m, "\n");

Re: [PATCH] Always show Cpus_allowed field in /proc/<pid>/status.

[Paul Menage]

On Mon, Sep 21, 2009 at 2:17 AM, Heiko Carstens
<heiko.carstens@de.ibm.com> wrote:
> On Mon, Sep 21, 2009 at 11:06:27AM +0200, Heiko Carstens wrote:
>
>> +void task_cpus_allowed(struct seq_file *m, struct task_struct *task)
>
> sigh. This should have been static.. updated patch below.
>
>
> Subject: [PATCH] Always show Cpus_allowed fields in /proc/<pid>/status.
>
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
>
> The Cpus_allowed fields in /proc/<pid>/status is currently only shown in
> case of CONFIG_CPUSETS. However their contents are also useful for the
> !CONFIG_CPUSETS case.
> So change the current behaviour and always show these fields.
>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>

Acked-by: Paul Menage <menage@google.com>

Sounds reasonable.