* [PATCH v2 0/10] Fixes for common mistakes w/ for_each_process and task->mm
From: Anton Vorontsov @ 2012-03-24 10:26 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
linuxppc-dev, linux-arm-kernel
Hi all,
This is a reincarnation of the task->mm fixes. Several architectures
were traverse the tasklist in an unsafe manner, plus there are a
few cases of unsafe access to task->mm.
In v2 I decided to introduce a small helper in cpu.c: most arches
duplicate the same [buggy] code snippet, so it's better to fix it
and move the logic into a common function.
Thanks!
--
Anton Vorontsov
Email: cbouatmailru@gmail.com
^ permalink raw reply
* [PATCH 01/10] cpu: Introduce clear_tasks_mm_cpumask() helper
From: Anton Vorontsov @ 2012-03-24 10:27 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324102609.GA28356@lizard>
Many architctures clear tasks' mm_cpumask like this:
read_lock(&tasklist_lock);
for_each_process(p) {
if (p->mm)
cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
}
read_unlock(&tasklist_lock);
The code above has several problems, such as:
1. Working with task->mm w/o getting mm or grabing the task lock is
dangerous as ->mm might disappear (exit_mm() assigns NULL under
task_lock(), so tasklist lock is not enough).
2. Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
This patch implements a small helper function that does things
correctly, i.e.:
1. We take the task's lock while whe handle its mm (we can't use
get_task_mm()/mmput() pair as mmput() might sleep);
2. To catch exited main thread case, we use find_lock_task_mm(),
which walks up all threads and returns an appropriate task
(with task lock held).
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
include/linux/cpu.h | 1 +
kernel/cpu.c | 18 ++++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 1f65875..941e865 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -171,6 +171,7 @@ extern void put_online_cpus(void);
#define hotcpu_notifier(fn, pri) cpu_notifier(fn, pri)
#define register_hotcpu_notifier(nb) register_cpu_notifier(nb)
#define unregister_hotcpu_notifier(nb) unregister_cpu_notifier(nb)
+void clear_tasks_mm_cpumask(int cpu);
int cpu_down(unsigned int cpu);
#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 2060c6e..5255936 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -10,6 +10,7 @@
#include <linux/sched.h>
#include <linux/unistd.h>
#include <linux/cpu.h>
+#include <linux/oom.h>
#include <linux/export.h>
#include <linux/kthread.h>
#include <linux/stop_machine.h>
@@ -171,6 +172,23 @@ void __ref unregister_cpu_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL(unregister_cpu_notifier);
+void clear_tasks_mm_cpumask(int cpu)
+{
+ struct task_struct *p;
+
+ read_lock(&tasklist_lock);
+ for_each_process(p) {
+ struct task_struct *t;
+
+ t = find_lock_task_mm(p);
+ if (!t)
+ continue;
+ cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
+ task_unlock(t);
+ }
+ read_unlock(&tasklist_lock);
+}
+
static inline void check_for_tasks(int cpu)
{
struct task_struct *p;
--
1.7.9.2
^ permalink raw reply related
* [PATCH 02/10] arm: Use clear_tasks_mm_cpumask()
From: Anton Vorontsov @ 2012-03-24 10:28 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324102609.GA28356@lizard>
Current CPU hotplug code has some task->mm handling issues:
1. Working with task->mm w/o getting mm or grabing the task lock is
dangerous as ->mm might disappear (exit_mm() assigns NULL under
task_lock(), so tasklist lock is not enough).
We can't use get_task_mm()/mmput() pair as mmput() might sleep,
so we must take the task lock while handle its mm.
2. Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
To fix this we would need to use find_lock_task_mm(), which would
walk up all threads and returns an appropriate task (with task
lock held).
clear_tasks_mm_cpumask() has all the issues fixed, so let's use it.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/arm/kernel/smp.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index cdeb727..880b93a 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -136,7 +136,6 @@ static void percpu_timer_stop(void);
int __cpu_disable(void)
{
unsigned int cpu = smp_processor_id();
- struct task_struct *p;
int ret;
ret = platform_cpu_disable(cpu);
@@ -166,12 +165,7 @@ int __cpu_disable(void)
flush_cache_all();
local_flush_tlb_all();
- read_lock(&tasklist_lock);
- for_each_process(p) {
- if (p->mm)
- cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
- }
- read_unlock(&tasklist_lock);
+ clear_tasks_mm_cpumask(cpu);
return 0;
}
--
1.7.9.2
^ permalink raw reply related
* [PATCH 03/10] powerpc: Use clear_tasks_mm_cpumask()
From: Anton Vorontsov @ 2012-03-24 10:28 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324102609.GA28356@lizard>
Current CPU hotplug code has some task->mm handling issues:
1. Working with task->mm w/o getting mm or grabing the task lock is
dangerous as ->mm might disappear (exit_mm() assigns NULL under
task_lock(), so tasklist lock is not enough).
We can't use get_task_mm()/mmput() pair as mmput() might sleep,
so we must take the task lock while handle its mm.
2. Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
To fix this we would need to use find_lock_task_mm(), which would
walk up all threads and returns an appropriate task (with task
lock held).
clear_tasks_mm_cpumask() has all the issues fixed, so let's use it.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/powerpc/mm/mmu_context_nohash.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
index 5b63bd3..e779642 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -333,9 +333,7 @@ static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self,
unsigned long action, void *hcpu)
{
unsigned int cpu = (unsigned int)(long)hcpu;
-#ifdef CONFIG_HOTPLUG_CPU
- struct task_struct *p;
-#endif
+
/* We don't touch CPU 0 map, it's allocated at aboot and kept
* around forever
*/
@@ -358,12 +356,7 @@ static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self,
stale_map[cpu] = NULL;
/* We also clear the cpu_vm_mask bits of CPUs going away */
- read_lock(&tasklist_lock);
- for_each_process(p) {
- if (p->mm)
- cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
- }
- read_unlock(&tasklist_lock);
+ clear_tasks_mm_cpumask(cpu);
break;
#endif /* CONFIG_HOTPLUG_CPU */
}
--
1.7.9.2
^ permalink raw reply related
* [PATCH 04/10] sh: Use clear_tasks_mm_cpumask()
From: Anton Vorontsov @ 2012-03-24 10:28 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324102609.GA28356@lizard>
Current CPU hotplug code has some task->mm handling issues:
1. Working with task->mm w/o getting mm or grabing the task lock is
dangerous as ->mm might disappear (exit_mm() assigns NULL under
task_lock(), so tasklist lock is not enough).
We can't use get_task_mm()/mmput() pair as mmput() might sleep,
so we must take the task lock while handle its mm.
2. Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
To fix this we would need to use find_lock_task_mm(), which would
walk up all threads and returns an appropriate task (with task
lock held).
clear_tasks_mm_cpumask() has all the issues fixed, so let's use it.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/sh/kernel/smp.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/sh/kernel/smp.c b/arch/sh/kernel/smp.c
index f624174..2470c83 100644
--- a/arch/sh/kernel/smp.c
+++ b/arch/sh/kernel/smp.c
@@ -123,7 +123,6 @@ void native_play_dead(void)
int __cpu_disable(void)
{
unsigned int cpu = smp_processor_id();
- struct task_struct *p;
int ret;
ret = mp_ops->cpu_disable(cpu);
@@ -153,11 +152,7 @@ int __cpu_disable(void)
flush_cache_all();
local_flush_tlb_all();
- read_lock(&tasklist_lock);
- for_each_process(p)
- if (p->mm)
- cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
- read_unlock(&tasklist_lock);
+ clear_tasks_mm_cpumask(cpu);
return 0;
}
--
1.7.9.2
^ permalink raw reply related
* [PATCH 05/10] blackfin: A couple of task->mm handling fixes
From: Anton Vorontsov @ 2012-03-24 10:29 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324102609.GA28356@lizard>
The patch fixes two problems:
1. Working with task->mm w/o getting mm or grabing the task lock is
dangerous as ->mm might disappear (exit_mm() assigns NULL under
task_lock(), so tasklist lock is not enough).
We can't use get_task_mm()/mmput() pair as mmput() might sleep,
so we have to take the task lock while handle its mm.
2. Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
To catch this we use find_lock_task_mm(), which walks up all
threads and returns an appropriate task (with task lock held).
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/blackfin/kernel/trace.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/blackfin/kernel/trace.c b/arch/blackfin/kernel/trace.c
index 050db44..5102cdf 100644
--- a/arch/blackfin/kernel/trace.c
+++ b/arch/blackfin/kernel/trace.c
@@ -10,6 +10,8 @@
#include <linux/hardirq.h>
#include <linux/thread_info.h>
#include <linux/mm.h>
+#include <linux/oom.h>
+#include <linux/sched.h>
#include <linux/uaccess.h>
#include <linux/module.h>
#include <linux/kallsyms.h>
@@ -27,7 +29,6 @@ void decode_address(char *buf, unsigned long address)
struct task_struct *p;
struct mm_struct *mm;
unsigned long flags, offset;
- unsigned char in_atomic = (bfin_read_IPEND() & 0x10) || in_atomic();
struct rb_node *n;
#ifdef CONFIG_KALLSYMS
@@ -113,15 +114,15 @@ void decode_address(char *buf, unsigned long address)
*/
write_lock_irqsave(&tasklist_lock, flags);
for_each_process(p) {
- mm = (in_atomic ? p->mm : get_task_mm(p));
- if (!mm)
- continue;
+ struct task_struct *t;
- if (!down_read_trylock(&mm->mmap_sem)) {
- if (!in_atomic)
- mmput(mm);
+ t = find_lock_task_mm(p);
+ if (!t)
continue;
- }
+
+ mm = t->mm;
+ if (!down_read_trylock(&mm->mmap_sem))
+ goto __continue;
for (n = rb_first(&mm->mm_rb); n; n = rb_next(n)) {
struct vm_area_struct *vma;
@@ -130,7 +131,7 @@ void decode_address(char *buf, unsigned long address)
if (address >= vma->vm_start && address < vma->vm_end) {
char _tmpbuf[256];
- char *name = p->comm;
+ char *name = t->comm;
struct file *file = vma->vm_file;
if (file) {
@@ -163,8 +164,7 @@ void decode_address(char *buf, unsigned long address)
name, vma->vm_start, vma->vm_end);
up_read(&mm->mmap_sem);
- if (!in_atomic)
- mmput(mm);
+ task_unlock(t);
if (buf[0] == '\0')
sprintf(buf, "[ %s ] dynamic memory", name);
@@ -174,8 +174,8 @@ void decode_address(char *buf, unsigned long address)
}
up_read(&mm->mmap_sem);
- if (!in_atomic)
- mmput(mm);
+__continue:
+ task_unlock(t);
}
/*
--
1.7.9.2
^ permalink raw reply related
* [PATCH 06/10] blackfin: Fix possible deadlock in decode_address()
From: Anton Vorontsov @ 2012-03-24 10:30 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324102609.GA28356@lizard>
Oleg Nesterov found an interesting deadlock possibility:
> sysrq_showregs_othercpus() does smp_call_function(showacpu)
> and showacpu() show_stack()->decode_address(). Now suppose that IPI
> interrupts the task holding read_lock(tasklist).
To fix this, blackfin should not grab the write_ variant of the
tasklist lock, read_ one is enough.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/blackfin/kernel/trace.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/blackfin/kernel/trace.c b/arch/blackfin/kernel/trace.c
index 5102cdf..9cc9302 100644
--- a/arch/blackfin/kernel/trace.c
+++ b/arch/blackfin/kernel/trace.c
@@ -28,7 +28,7 @@ void decode_address(char *buf, unsigned long address)
{
struct task_struct *p;
struct mm_struct *mm;
- unsigned long flags, offset;
+ unsigned long offset;
struct rb_node *n;
#ifdef CONFIG_KALLSYMS
@@ -112,7 +112,7 @@ void decode_address(char *buf, unsigned long address)
* mappings of all our processes and see if we can't be a whee
* bit more specific
*/
- write_lock_irqsave(&tasklist_lock, flags);
+ read_lock(&tasklist_lock);
for_each_process(p) {
struct task_struct *t;
@@ -185,7 +185,7 @@ __continue:
sprintf(buf, "/* kernel dynamic memory */");
done:
- write_unlock_irqrestore(&tasklist_lock, flags);
+ read_unlock(&tasklist_lock);
}
#define EXPAND_LEN ((1 << CONFIG_DEBUG_BFIN_HWTRACE_EXPAND_LEN) * 256 - 1)
--
1.7.9.2
^ permalink raw reply related
* [PATCH 07/10] um: Should hold tasklist_lock while traversing processes
From: Anton Vorontsov @ 2012-03-24 10:30 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324102609.GA28356@lizard>
Traversing the tasks requires holding tasklist_lock, otherwise it
is unsafe.
p.s. However, I'm not sure that calling os_kill_ptraced_process()
in the atomic context is correct. It seem to work, but please
take a closer look.
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/um/kernel/reboot.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/um/kernel/reboot.c b/arch/um/kernel/reboot.c
index 4d93dff..66d754c 100644
--- a/arch/um/kernel/reboot.c
+++ b/arch/um/kernel/reboot.c
@@ -4,6 +4,7 @@
*/
#include "linux/sched.h"
+#include "linux/spinlock.h"
#include "linux/slab.h"
#include "kern_util.h"
#include "os.h"
@@ -22,6 +23,7 @@ static void kill_off_processes(void)
struct task_struct *p;
int pid;
+ read_lock(&tasklist_lock);
for_each_process(p) {
if (p->mm == NULL)
continue;
@@ -29,6 +31,7 @@ static void kill_off_processes(void)
pid = p->mm->context.id.u.pid;
os_kill_ptraced_process(pid, 1);
}
+ read_unlock(&tasklist_lock);
}
}
--
1.7.9.2
^ permalink raw reply related
* [PATCH 08/10] um: Fix possible race on task->mm
From: Anton Vorontsov @ 2012-03-24 10:30 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324102609.GA28356@lizard>
Checking for task->mm is dangerous as ->mm might disappear (exit_mm()
assigns NULL under task_lock(), so tasklist lock is not enough).
We can't use get_task_mm()/mmput() pair as mmput() might sleep,
so let's take the task lock while we care about its mm.
Note that we should also use find_lock_task_mm() to check all process'
threads for a valid mm, but for uml we'll do it in a separate patch.
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/um/kernel/reboot.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/um/kernel/reboot.c b/arch/um/kernel/reboot.c
index 66d754c..1411f4e 100644
--- a/arch/um/kernel/reboot.c
+++ b/arch/um/kernel/reboot.c
@@ -25,10 +25,13 @@ static void kill_off_processes(void)
read_lock(&tasklist_lock);
for_each_process(p) {
- if (p->mm == NULL)
+ task_lock(p);
+ if (!p->mm) {
+ task_unlock(p);
continue;
-
+ }
pid = p->mm->context.id.u.pid;
+ task_unlock(p);
os_kill_ptraced_process(pid, 1);
}
read_unlock(&tasklist_lock);
--
1.7.9.2
^ permalink raw reply related
* [PATCH 09/10] um: Properly check all process' threads for a live mm
From: Anton Vorontsov @ 2012-03-24 10:31 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324102609.GA28356@lizard>
kill_off_processes() might miss a valid process, this is because
checking for process->mm is not enough. Process' main thread may
exit or detach its mm via use_mm(), but other threads may still
have a valid mm.
To catch this we use find_lock_task_mm(), which walks up all
threads and returns an appropriate task (with task lock held).
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/um/kernel/reboot.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/um/kernel/reboot.c b/arch/um/kernel/reboot.c
index 1411f4e..3d15243 100644
--- a/arch/um/kernel/reboot.c
+++ b/arch/um/kernel/reboot.c
@@ -6,6 +6,7 @@
#include "linux/sched.h"
#include "linux/spinlock.h"
#include "linux/slab.h"
+#include "linux/oom.h"
#include "kern_util.h"
#include "os.h"
#include "skas.h"
@@ -25,13 +26,13 @@ static void kill_off_processes(void)
read_lock(&tasklist_lock);
for_each_process(p) {
- task_lock(p);
- if (!p->mm) {
- task_unlock(p);
+ struct task_struct *t;
+
+ t = find_lock_task_mm(p);
+ if (!t)
continue;
- }
- pid = p->mm->context.id.u.pid;
- task_unlock(p);
+ pid = t->mm->context.id.u.pid;
+ task_unlock(t);
os_kill_ptraced_process(pid, 1);
}
read_unlock(&tasklist_lock);
--
1.7.9.2
^ permalink raw reply related
* [PATCH 10/10] oom: Make find_lock_task_mm() sparse-aware
From: Anton Vorontsov @ 2012-03-24 10:31 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324102609.GA28356@lizard>
This is needed so that callers would not get 'context imbalance'
warnings from the sparse tool.
As a side effect, this patch fixes the following sparse warnings:
CHECK mm/oom_kill.c
mm/oom_kill.c:201:28: warning: context imbalance in 'oom_badness' -
unexpected unlock
include/linux/rcupdate.h:249:30: warning: context imbalance in
'dump_tasks' - unexpected unlock
mm/oom_kill.c:453:9: warning: context imbalance in 'oom_kill_task' -
unexpected unlock
CHECK mm/memcontrol.c
...
mm/memcontrol.c:1130:17: warning: context imbalance in
'task_in_mem_cgroup' - unexpected unlock
p.s. I know Peter Zijlstra detest the __cond_lock() stuff, but untill
we have anything better in sparse, let's use it. This particular
patch helped me to detect one bug that I myself made during
task->mm fixup series. So, it is useful.
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
include/linux/oom.h | 12 +++++++++++-
mm/oom_kill.c | 2 +-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/include/linux/oom.h b/include/linux/oom.h
index 552fba9..26cf628 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -21,6 +21,7 @@
#ifdef __KERNEL__
+#include <linux/compiler.h>
#include <linux/sched.h>
#include <linux/types.h>
#include <linux/nodemask.h>
@@ -65,7 +66,16 @@ static inline void oom_killer_enable(void)
oom_killer_disabled = false;
}
-extern struct task_struct *find_lock_task_mm(struct task_struct *p);
+extern struct task_struct *__find_lock_task_mm(struct task_struct *p);
+
+static inline struct task_struct *find_lock_task_mm(struct task_struct *p)
+{
+ struct task_struct *ret;
+
+ ret = __find_lock_task_mm(p);
+ (void)__cond_lock(&ret->alloc_lock, ret);
+ return ret;
+}
/* sysctls */
extern int sysctl_oom_dump_tasks;
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 2958fd8..0ebb383 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -136,7 +136,7 @@ static bool has_intersects_mems_allowed(struct task_struct *tsk,
* pointer. Return p, or any of its subthreads with a valid ->mm, with
* task_lock() held.
*/
-struct task_struct *find_lock_task_mm(struct task_struct *p)
+struct task_struct *__find_lock_task_mm(struct task_struct *p)
{
struct task_struct *t = p;
--
1.7.9.2
^ permalink raw reply related
* Re: [PATCH 09/10] um: Properly check all process' threads for a live mm
From: Richard Weinberger @ 2012-03-24 11:12 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Oleg Nesterov, linux-kernel, linux-mm, Paul Mundt, John Stultz,
KOSAKI Motohiro, uclinux-dist-devel, Russell King, Andrew Morton,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324103110.GI29067@lizard>
[-- Attachment #1: Type: text/plain, Size: 593 bytes --]
Am 24.03.2012 11:31, schrieb Anton Vorontsov:
> kill_off_processes() might miss a valid process, this is because
> checking for process->mm is not enough. Process' main thread may
> exit or detach its mm via use_mm(), but other threads may still
> have a valid mm.
>
> To catch this we use find_lock_task_mm(), which walks up all
> threads and returns an appropriate task (with task lock held).
>
> Suggested-by: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
Acked-by: Richard Weinberger <richard@nod.at>
Thanks,
//richard
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 08/10] um: Fix possible race on task->mm
From: Richard Weinberger @ 2012-03-24 11:12 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Oleg Nesterov, linux-kernel, linux-mm, Paul Mundt, John Stultz,
KOSAKI Motohiro, uclinux-dist-devel, Russell King, Andrew Morton,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324103050.GH29067@lizard>
[-- Attachment #1: Type: text/plain, Size: 608 bytes --]
Am 24.03.2012 11:30, schrieb Anton Vorontsov:
> Checking for task->mm is dangerous as ->mm might disappear (exit_mm()
> assigns NULL under task_lock(), so tasklist lock is not enough).
>
> We can't use get_task_mm()/mmput() pair as mmput() might sleep,
> so let's take the task lock while we care about its mm.
>
> Note that we should also use find_lock_task_mm() to check all process'
> threads for a valid mm, but for uml we'll do it in a separate patch.
>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
Acked-by: Richard Weinberger <richard@nod.at>
Thanks,
//richard
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 07/10] um: Should hold tasklist_lock while traversing processes
From: Richard Weinberger @ 2012-03-24 11:12 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Oleg Nesterov, linux-kernel, linux-mm, Paul Mundt, John Stultz,
KOSAKI Motohiro, uclinux-dist-devel, Russell King, Andrew Morton,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324103030.GG29067@lizard>
[-- Attachment #1: Type: text/plain, Size: 475 bytes --]
Am 24.03.2012 11:30, schrieb Anton Vorontsov:
> Traversing the tasks requires holding tasklist_lock, otherwise it
> is unsafe.
>
> p.s. However, I'm not sure that calling os_kill_ptraced_process()
> in the atomic context is correct. It seem to work, but please
> take a closer look.
os_kill_ptraced_process() calls a host function.
From UML's point of view nothing sleeps, so this is fine.
Acked-by: Richard Weinberger <richard@nod.at>
Thanks,
//richard
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 01/10] cpu: Introduce clear_tasks_mm_cpumask() helper
From: Peter Zijlstra @ 2012-03-24 12:43 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Mike Frysinger, user-mode-linux-devel, linux-sh,
Richard Weinberger, linuxppc-dev, Oleg Nesterov, linux-kernel,
linux-mm, Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
Andrew Morton, uclinux-dist-devel, linux-arm-kernel
In-Reply-To: <20120324102751.GA29067@lizard>
On Sat, 2012-03-24 at 14:27 +0400, Anton Vorontsov wrote:
> +void clear_tasks_mm_cpumask(int cpu)
> +{
> + struct task_struct *p;
> +
> + read_lock(&tasklist_lock);
> + for_each_process(p) {
> + struct task_struct *t;
> +
> + t =3D find_lock_task_mm(p);
> + if (!t)
> + continue;
> + cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
> + task_unlock(t);
> + }
> + read_unlock(&tasklist_lock);
> +}=20
Why bother with the tasklist_lock at all anymore, afaict you could use
rcu_read_lock() here. This all is called after the cpu is taken down and
marked offline, so its not like new tasks will ever get this cpu set in
their mm mask.
^ permalink raw reply
* Re: [PATCH 07/10] um: Should hold tasklist_lock while traversing processes
From: Peter Zijlstra @ 2012-03-24 12:48 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Mike Frysinger, user-mode-linux-devel, linux-sh,
Richard Weinberger, linuxppc-dev, Oleg Nesterov, linux-kernel,
linux-mm, Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
Andrew Morton, uclinux-dist-devel, linux-arm-kernel
In-Reply-To: <20120324103030.GG29067@lizard>
On Sat, 2012-03-24 at 14:30 +0400, Anton Vorontsov wrote:
> Traversing the tasks requires holding tasklist_lock, otherwise it
> is unsafe.=20
No it doesn't, it either requires tasklist_lock or rcu_read_lock().
^ permalink raw reply
* Re: [PATCH 10/10] oom: Make find_lock_task_mm() sparse-aware
From: Peter Zijlstra @ 2012-03-24 12:52 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Mike Frysinger, user-mode-linux-devel, linux-sh,
Richard Weinberger, linuxppc-dev, Oleg Nesterov, linux-kernel,
linux-mm, Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
Andrew Morton, uclinux-dist-devel, linux-arm-kernel
In-Reply-To: <20120324103127.GJ29067@lizard>
On Sat, 2012-03-24 at 14:31 +0400, Anton Vorontsov wrote:
> This is needed so that callers would not get 'context imbalance'
> warnings from the sparse tool.
>=20
> As a side effect, this patch fixes the following sparse warnings:
>=20
> CHECK mm/oom_kill.c
> mm/oom_kill.c:201:28: warning: context imbalance in 'oom_badness' -
> unexpected unlock
> include/linux/rcupdate.h:249:30: warning: context imbalance in
> 'dump_tasks' - unexpected unlock
> mm/oom_kill.c:453:9: warning: context imbalance in 'oom_kill_task' -
> unexpected unlock
> CHECK mm/memcontrol.c
> ...
> mm/memcontrol.c:1130:17: warning: context imbalance in
> 'task_in_mem_cgroup' - unexpected unlock
>=20
> p.s. I know Peter Zijlstra detest the __cond_lock() stuff, but untill
> we have anything better in sparse, let's use it. This particular
> patch helped me to detect one bug that I myself made during
> task->mm fixup series. So, it is useful.
Yeah, so Nacked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Also, why didn't lockdep catch it?
Fix sparse already instead of smearing ugly all over.
^ permalink raw reply
* Re: [PATCH 10/10] oom: Make find_lock_task_mm() sparse-aware
From: Anton Vorontsov @ 2012-03-24 16:21 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Mike Frysinger, user-mode-linux-devel, linux-sh,
Richard Weinberger, linuxppc-dev, Oleg Nesterov, Linus Torvalds,
linux-kernel, linux-mm, Paul Mundt, John Stultz, KOSAKI Motohiro,
Russell King, Andrew Morton, uclinux-dist-devel, linux-arm-kernel
In-Reply-To: <1332593574.16159.31.camel@twins>
On Sat, Mar 24, 2012 at 01:52:54PM +0100, Peter Zijlstra wrote:
[...]
> > p.s. I know Peter Zijlstra detest the __cond_lock() stuff, but untill
> > we have anything better in sparse, let's use it. This particular
> > patch helped me to detect one bug that I myself made during
> > task->mm fixup series. So, it is useful.
>
> Yeah, so Nacked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
>
> Also, why didn't lockdep catch it?
Because patch authors test their patches on architectures they own
(well, sometimes I do check patches on exotic architectures w/ qemu,
but it is less convenient than just build/sparse-test the patch w/
a cross compiler).
And since lockdep is a runtime checker, it is not very useful.
Sparse is a build-time checker, so it is even better in the sense
that it is able to catch bugs even in code that is executed rarely.
> Fix sparse already instead of smearing ugly all over.
Just wonder how do you see the feature implemented?
Something like this?
#define __ret_cond_locked(l, c) __attribute__((ret_cond_locked(l, c)))
#define __ret_value __attribute__((ret_value))
#define __ret_locked_nonnull(l) __ret_cond_locked(l, __ret_value);
extern struct task_struct *find_lock_task_mm(struct task_struct *p)
__ret_locked_nonnull(&__ret_value->alloc_lock);
Thanks,
--
Anton Vorontsov
Email: cbouatmailru@gmail.com
^ permalink raw reply
* Re: [PATCH 10/10] oom: Make find_lock_task_mm() sparse-aware
From: Peter Zijlstra @ 2012-03-24 16:43 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Mike Frysinger, user-mode-linux-devel, linux-sh,
Richard Weinberger, linuxppc-dev, Oleg Nesterov, Linus Torvalds,
linux-kernel, linux-mm, Paul Mundt, John Stultz, KOSAKI Motohiro,
Russell King, Andrew Morton, uclinux-dist-devel, linux-arm-kernel
In-Reply-To: <20120324162151.GA3640@lizard>
On Sat, 2012-03-24 at 20:21 +0400, Anton Vorontsov wrote:
> Just wonder how do you see the feature implemented?
>=20
> Something like this?
>=20
> #define __ret_cond_locked(l, c) __attribute__((ret_cond_locked(l, c)))
> #define __ret_value __attribute__((ret_value))
> #define __ret_locked_nonnull(l) __ret_cond_locked(l, __ret_value);
>=20
> extern struct task_struct *find_lock_task_mm(struct task_struct *p)
> __ret_locked_nonnull(&__ret_value->alloc_lock);
Yeah, see the email I just CC'ed you on to linux-sparse.
Basically extend __attribute__((context())) to allow things similar to
what you proposed.
^ permalink raw reply
* [PATCH v2.1 01/10] cpu: Introduce clear_tasks_mm_cpumask() helper
From: Anton Vorontsov @ 2012-03-24 16:43 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Mike Frysinger, user-mode-linux-devel, linux-sh,
Richard Weinberger, linuxppc-dev, Oleg Nesterov, linux-kernel,
linux-mm, Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
Andrew Morton, uclinux-dist-devel, linux-arm-kernel
In-Reply-To: <1332593021.16159.27.camel@twins>
Many architctures clear tasks' mm_cpumask like this:
read_lock(&tasklist_lock);
for_each_process(p) {
if (p->mm)
cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
}
read_unlock(&tasklist_lock);
The code above has several problems, such as:
1. Working with task->mm w/o getting mm or grabing the task lock is
dangerous as ->mm might disappear (exit_mm() assigns NULL under
task_lock(), so tasklist lock is not enough).
2. Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
This patch implements a small helper function that does things
correctly, i.e.:
1. We take the task's lock while whe handle its mm (we can't use
get_task_mm()/mmput() pair as mmput() might sleep);
2. To catch exited main thread case, we use find_lock_task_mm(),
which walks up all threads and returns an appropriate task
(with task lock held).
Also, Per Peter Zijlstra's idea, now we don't grab tasklist_lock in
the new helper, instead we take the rcu read lock. We can do this
because the function is called after the cpu is taken down and marked
offline, so no new tasks will get this cpu set in their mm mask.
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
On Sat, Mar 24, 2012 at 01:43:41PM +0100, Peter Zijlstra wrote:
> On Sat, 2012-03-24 at 14:27 +0400, Anton Vorontsov wrote:
> > + read_unlock(&tasklist_lock);
> > +}
>
> Why bother with the tasklist_lock at all anymore, afaict you could use
> rcu_read_lock() here. This all is called after the cpu is taken down and
> marked offline, so its not like new tasks will ever get this cpu set in
> their mm mask.
I guess you're right. Well, this would make the code a bit fragile,
but a comment might help.
How about this version?
include/linux/cpu.h | 1 +
kernel/cpu.c | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 1f65875..941e865 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -171,6 +171,7 @@ extern void put_online_cpus(void);
#define hotcpu_notifier(fn, pri) cpu_notifier(fn, pri)
#define register_hotcpu_notifier(nb) register_cpu_notifier(nb)
#define unregister_hotcpu_notifier(nb) unregister_cpu_notifier(nb)
+void clear_tasks_mm_cpumask(int cpu);
int cpu_down(unsigned int cpu);
#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 2060c6e..ecdf499 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -10,6 +10,8 @@
#include <linux/sched.h>
#include <linux/unistd.h>
#include <linux/cpu.h>
+#include <linux/oom.h>
+#include <linux/rcupdate.h>
#include <linux/export.h>
#include <linux/kthread.h>
#include <linux/stop_machine.h>
@@ -171,6 +173,30 @@ void __ref unregister_cpu_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL(unregister_cpu_notifier);
+void clear_tasks_mm_cpumask(int cpu)
+{
+ struct task_struct *p;
+
+ /*
+ * This function is called after the cpu is taken down and marked
+ * offline, so its not like new tasks will ever get this cpu set in
+ * their mm mask. -- Peter Zijlstra
+ * Thus, we may use rcu_read_lock() here, instead of grabbing
+ * full-fledged tasklist_lock.
+ */
+ rcu_read_lock();
+ for_each_process(p) {
+ struct task_struct *t;
+
+ t = find_lock_task_mm(p);
+ if (!t)
+ continue;
+ cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
+ task_unlock(t);
+ }
+ rcu_read_unlock();
+}
+
static inline void check_for_tasks(int cpu)
{
struct task_struct *p;
--
1.7.9.2
^ permalink raw reply related
* Re: [PATCH 07/10] um: Should hold tasklist_lock while traversing processes
From: Anton Vorontsov @ 2012-03-24 16:43 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Mike Frysinger, user-mode-linux-devel, linux-sh,
Richard Weinberger, linuxppc-dev, Oleg Nesterov, linux-kernel,
linux-mm, Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
Andrew Morton, uclinux-dist-devel, linux-arm-kernel
In-Reply-To: <1332593303.16159.28.camel@twins>
On Sat, Mar 24, 2012 at 01:48:23PM +0100, Peter Zijlstra wrote:
> On Sat, 2012-03-24 at 14:30 +0400, Anton Vorontsov wrote:
> > Traversing the tasks requires holding tasklist_lock, otherwise it
> > is unsafe.
>
> No it doesn't, it either requires tasklist_lock or rcu_read_lock().
Well, currently the code does neither. :-)
--
Anton Vorontsov
Email: cbouatmailru@gmail.com
^ permalink raw reply
* Re: kilauea compilation breaks with v3.3 kernel and ELDK 4.2
From: Benjamin Herrenschmidt @ 2012-03-24 23:53 UTC (permalink / raw)
To: Wolfgang Denk; +Cc: linuxppc-dev, robert.karl.berger
In-Reply-To: <20120321162549.8C1D0202A4D@gemini.denx.de>
On Wed, 2012-03-21 at 17:25 +0100, Wolfgang Denk wrote:
> > > The problem is that for ppc-linux-gcc (GCC) 4.2.2 (which comes
> with the
> > > ELDK 4.2) this assembly instruction is not known and the build
> breaks.
> >
> > Sigh. GCC 4.2 is pretty old at this point.
>
> But still rock-solid ... We use it a lot, especially as reference for
> more recent (and sometimes more broken) versions of GCC.
Isn't this a binutils rather than gcc issue ?
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc/44x: Support CPM for Maui APM821xxx SoC and Bluestone board
From: Grant Likely @ 2012-03-24 19:48 UTC (permalink / raw)
To: Mai La, Benjamin Herrenschmidt, Paul Mackerras, Josh Boyer,
Matt Porter, Tirumala R Marri, Michael Neuling, Kumar Gala,
Anton Blanchard, linuxppc-dev, linux-kernel
Cc: Mai La
In-Reply-To: <1332296472-24734-1-git-send-email-mla@apm.com>
On Wed, 21 Mar 2012 09:21:12 +0700, Mai La <mla@apm.com> wrote:
> In this patch:
> - Add CPM node for Maui APM821xxx SoC and Bluestone board in DTS
>
> Signed-off-by: Mai La <mla@apm.com>
> ---
> arch/powerpc/boot/dts/bluestone.dts | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/bluestone.dts b/arch/powerpc/boot/dts/bluestone.dts
> index 2a56a0d..15a9516 100644
> --- a/arch/powerpc/boot/dts/bluestone.dts
> +++ b/arch/powerpc/boot/dts/bluestone.dts
> @@ -117,6 +117,15 @@
> dcr-reg = <0x00c 0x002>;
> };
>
> + CPM0: cpm {
> + compatible = "ibm, cpm-apm821xx", "ibm,cpm";
^
Is that a typo?
> + cell-index = <0>;
> + dcr-reg = <0x160 0x003>;
> + er-offset = <0>;
> + idle-doze = <0x02000000>;
> + standby = <0x302570F0>;
> + };
> +
> plb {
> compatible = "ibm,plb4";
> #address-cells = <2>;
> --
> 1.7.3.4
>
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.
^ permalink raw reply
* Re: [PATCH v2.1 01/10] cpu: Introduce clear_tasks_mm_cpumask() helper
From: Oleg Nesterov @ 2012-03-25 17:42 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Mike Frysinger, Peter Zijlstra, user-mode-linux-devel, linux-sh,
Richard Weinberger, linux-kernel, uclinux-dist-devel, linux-mm,
Paul Mundt, John Stultz, KOSAKI Motohiro, Russell King,
Andrew Morton, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120324164316.GB3640@lizard>
On 03/24, Anton Vorontsov wrote:
>
> Many architctures clear tasks' mm_cpumask like this:
>
> read_lock(&tasklist_lock);
> for_each_process(p) {
> if (p->mm)
> cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
> }
> read_unlock(&tasklist_lock);
Namely arm, powerpc, and sh.
> The code above has several problems, such as:
>
> 1. Working with task->mm w/o getting mm or grabing the task lock is
> dangerous as ->mm might disappear (exit_mm() assigns NULL under
> task_lock(), so tasklist lock is not enough).
This is not actually true for arm and sh, afaics. They do not even
need tasklist or rcu lock for for_each_process().
__cpu_disable() is called by __stop_machine(), we know that nobody
can preempt us and other CPUs can do nothing.
> 2. Checking for process->mm is not enough because process' main
> thread may exit or detach its mm via use_mm(), but other threads
> may still have a valid mm.
Yes,
> Also, Per Peter Zijlstra's idea, now we don't grab tasklist_lock in
> the new helper, instead we take the rcu read lock. We can do this
> because the function is called after the cpu is taken down and marked
> offline, so no new tasks will get this cpu set in their mm mask.
And only powerpc needs rcu_read_lock() and task_lock().
OTOH, I do not understand why powepc does this on CPU_DEAD...
And probably CPU_UP_CANCELED doesn't need to clear mm_cpumask().
That said, personally I think these patches are fine, the common
helper makes sense.
Oleg.
^ permalink raw reply
* RE: [PATCH] powerpc/85xx: Add machine check handler to fix PCIe erratum on mpc85xx
From: Jia Hongtao-B38951 @ 2012-03-26 6:14 UTC (permalink / raw)
To: Kumar Gala, <shuo.liu@freesacle.com>
Cc: soniccat.liu@gmail.com, Wood Scott-B07421,
linuxppc-dev@lists.ozlabs.org, Zang Roy-R61911,
Zhao Chenhui-B35336
In-Reply-To: <5A2DE148-B029-4FF0-A49B-29172F234874@kernel.crashing.org>
> -----Original Message-----
> From: linuxppc-dev-bounces+b38951=3Dfreescale.com@lists.ozlabs.org
> [mailto:linuxppc-dev-bounces+b38951=3Dfreescale.com@lists.ozlabs.org] On
> Behalf Of Kumar Gala
> Sent: Saturday, March 17, 2012 5:04 AM
> To: <shuo.liu@freesacle.com>
> Cc: Zhao Chenhui-B35336; Liu Shuo-B35362; Zang Roy-R61911; Liu Shuo-
> B35362; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH] powerpc/85xx: Add machine check handler to fix PCIe
> erratum on mpc85xx
>=20
>=20
> On Feb 10, 2012, at 2:09 AM, <shuo.liu@freesacle.com>
> <shuo.liu@freesacle.com> wrote:
>=20
> > From: Liu Shuo <shuo.liu@freescale.com>
> >
> > A PCIe erratum of mpc85xx may causes a core hang when a link of PCIe
> > goes down. when the link goes down, Non-posted transactions issued
> > via the ATMU requiring completion result in an instruction stall.
> > At the same time a machine-check exception is generated to the core
> > to allow further processing by the handler. We implements the handler
> > which skips the instruction caused the stall.
> >
> > Signed-off-by: Zhao Chenhui <b35336@freescale.com>
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > Signed-off-by: Liu Shuo <b35362@freescale.com>
> > ---
> > arch/powerpc/kernel/cpu_setup_fsl_booke.S | 2 +-
> > arch/powerpc/kernel/traps.c | 3 ++
> > arch/powerpc/sysdev/fsl_pci.c | 36
> +++++++++++++++++++++++++++++
> > arch/powerpc/sysdev/fsl_pci.h | 6 +++++
> > 4 files changed, 46 insertions(+), 1 deletions(-)
>=20
> Should we have a check to make sure the hose is for PCIe and not PCI?
>=20
> - k
Due to errata spec pcie check is necessary.
I will send a new version of this patch to solve this problem soon.
Thanks.
Btw, from now on I will follow this patch instead of Liu Shuo.
- Jia Hongtao.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox