From: Suresh Siddha <[email protected]>
MTRR rendezvous sequence using stop_one_cpu_nowait() can potentially
happen in parallel with another system wide rendezvous using
stop_machine(). This can lead to deadlock (The order in which
works are queued can be different on different cpu's. Some cpu's
will be running the first rendezvous handler and others will be running
the second rendezvous handler. Each set waiting for the other set to join
for the system wide rendezvous, leading to a deadlock).
MTRR rendezvous sequence is not implemented using stop_machine() as this
gets called both from the process context aswell as the cpu online paths
(where the cpu has not come online and the interrupts are disabled etc).
stop_machine() works with only online cpus.
For now, take the stop_machine mutex in the MTRR rendezvous sequence that
gets called from an online cpu (here we are in the process context
and can potentially sleep while taking the mutex). And the MTRR rendezvous
that gets triggered during cpu online doesn't need to take this stop_machine
lock (as the stop_machine() already ensures that there is no cpu hotplug
going on in parallel by doing get_online_cpus())
TBD: Pursue a cleaner solution of extending the stop_machine()
infrastructure to handle the case where the calling cpu is
still not online and use this for MTRR rendezvous sequence.
fixes: https://bugzilla.novell.com/show_bug.cgi?id=672008
Reported-by: Vadim Kotelnikov <[email protected]>
Signed-off-by: Suresh Siddha <[email protected]>
Cc: [email protected] # 2.6.35+, backport a week or two after this gets more testing in mainline
---
arch/x86/kernel/cpu/mtrr/main.c | 23 +++++++++++++++++++++++
include/linux/stop_machine.h | 2 ++
kernel/stop_machine.c | 2 +-
3 files changed, 26 insertions(+), 1 deletion(-)
Index: linux-2.6-tip/arch/x86/kernel/cpu/mtrr/main.c
===================================================================
--- linux-2.6-tip.orig/arch/x86/kernel/cpu/mtrr/main.c
+++ linux-2.6-tip/arch/x86/kernel/cpu/mtrr/main.c
@@ -248,6 +248,25 @@ set_mtrr(unsigned int reg, unsigned long
unsigned long flags;
int cpu;
+#ifdef CONFIG_SMP
+ /*
+ * If this cpu is not yet active, we are in the cpu online path. There
+ * can be no stop_machine() in parallel, as stop machine ensures this
+ * by using get_online_cpus(). We can skip taking the stop_cpus_mutex,
+ * as we don't need it and also we can't afford to block while waiting
+ * for the mutex.
+ *
+ * If this cpu is active, we need to prevent stop_machine() happening
+ * in parallel by taking the stop cpus mutex.
+ *
+ * Also, this is called in the context of cpu online path or in the
+ * context where cpu hotplug is prevented. So checking the active status
+ * of the raw_smp_processor_id() is safe.
+ */
+ if (cpu_active(raw_smp_processor_id()))
+ mutex_lock(&stop_cpus_mutex);
+#endif
+
preempt_disable();
data.smp_reg = reg;
@@ -330,6 +349,10 @@ set_mtrr(unsigned int reg, unsigned long
local_irq_restore(flags);
preempt_enable();
+#ifdef CONFIG_SMP
+ if (cpu_active(raw_smp_processor_id()))
+ mutex_unlock(&stop_cpus_mutex);
+#endif
}
/**
Index: linux-2.6-tip/include/linux/stop_machine.h
===================================================================
--- linux-2.6-tip.orig/include/linux/stop_machine.h
+++ linux-2.6-tip/include/linux/stop_machine.h
@@ -27,6 +27,8 @@ struct cpu_stop_work {
struct cpu_stop_done *done;
};
+extern struct mutex stop_cpus_mutex;
+
int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg);
void stop_one_cpu_nowait(unsigned int cpu, cpu_stop_fn_t fn, void *arg,
struct cpu_stop_work *work_buf);
Index: linux-2.6-tip/kernel/stop_machine.c
===================================================================
--- linux-2.6-tip.orig/kernel/stop_machine.c
+++ linux-2.6-tip/kernel/stop_machine.c
@@ -132,8 +132,8 @@ void stop_one_cpu_nowait(unsigned int cp
cpu_stop_queue_work(&per_cpu(cpu_stopper, cpu), work_buf);
}
+DEFINE_MUTEX(stop_cpus_mutex);
/* static data for stop_cpus */
-static DEFINE_MUTEX(stop_cpus_mutex);
static DEFINE_PER_CPU(struct cpu_stop_work, stop_cpus_work);
int __stop_cpus(const struct cpumask *cpumask, cpu_stop_fn_t fn, void *arg)
Commit-ID: 6d3321e8e2b3bf6a5892e2ef673c7bf536e3f904
Gitweb: http://git.kernel.org/tip/6d3321e8e2b3bf6a5892e2ef673c7bf536e3f904
Author: Suresh Siddha <[email protected]>
AuthorDate: Thu, 23 Jun 2011 11:19:26 -0700
Committer: H. Peter Anvin <[email protected]>
CommitDate: Mon, 27 Jun 2011 14:00:46 -0700
x86, mtrr: lock stop machine during MTRR rendezvous sequence
MTRR rendezvous sequence using stop_one_cpu_nowait() can potentially
happen in parallel with another system wide rendezvous using
stop_machine(). This can lead to deadlock (The order in which
works are queued can be different on different cpu's. Some cpu's
will be running the first rendezvous handler and others will be running
the second rendezvous handler. Each set waiting for the other set to join
for the system wide rendezvous, leading to a deadlock).
MTRR rendezvous sequence is not implemented using stop_machine() as this
gets called both from the process context aswell as the cpu online paths
(where the cpu has not come online and the interrupts are disabled etc).
stop_machine() works with only online cpus.
For now, take the stop_machine mutex in the MTRR rendezvous sequence that
gets called from an online cpu (here we are in the process context
and can potentially sleep while taking the mutex). And the MTRR rendezvous
that gets triggered during cpu online doesn't need to take this stop_machine
lock (as the stop_machine() already ensures that there is no cpu hotplug
going on in parallel by doing get_online_cpus())
TBD: Pursue a cleaner solution of extending the stop_machine()
infrastructure to handle the case where the calling cpu is
still not online and use this for MTRR rendezvous sequence.
fixes: https://bugzilla.novell.com/show_bug.cgi?id=672008
Reported-by: Vadim Kotelnikov <[email protected]>
Signed-off-by: Suresh Siddha <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Cc: [email protected] # 2.6.35+, backport a week or two after this gets more testing in mainline
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/kernel/cpu/mtrr/main.c | 23 +++++++++++++++++++++++
include/linux/stop_machine.h | 2 ++
kernel/stop_machine.c | 2 +-
3 files changed, 26 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 929739a..3d17bc7 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -248,6 +248,25 @@ set_mtrr(unsigned int reg, unsigned long base, unsigned long size, mtrr_type typ
unsigned long flags;
int cpu;
+#ifdef CONFIG_SMP
+ /*
+ * If this cpu is not yet active, we are in the cpu online path. There
+ * can be no stop_machine() in parallel, as stop machine ensures this
+ * by using get_online_cpus(). We can skip taking the stop_cpus_mutex,
+ * as we don't need it and also we can't afford to block while waiting
+ * for the mutex.
+ *
+ * If this cpu is active, we need to prevent stop_machine() happening
+ * in parallel by taking the stop cpus mutex.
+ *
+ * Also, this is called in the context of cpu online path or in the
+ * context where cpu hotplug is prevented. So checking the active status
+ * of the raw_smp_processor_id() is safe.
+ */
+ if (cpu_active(raw_smp_processor_id()))
+ mutex_lock(&stop_cpus_mutex);
+#endif
+
preempt_disable();
data.smp_reg = reg;
@@ -330,6 +349,10 @@ set_mtrr(unsigned int reg, unsigned long base, unsigned long size, mtrr_type typ
local_irq_restore(flags);
preempt_enable();
+#ifdef CONFIG_SMP
+ if (cpu_active(raw_smp_processor_id()))
+ mutex_unlock(&stop_cpus_mutex);
+#endif
}
/**
diff --git a/include/linux/stop_machine.h b/include/linux/stop_machine.h
index 092dc9b..14d3524 100644
--- a/include/linux/stop_machine.h
+++ b/include/linux/stop_machine.h
@@ -27,6 +27,8 @@ struct cpu_stop_work {
struct cpu_stop_done *done;
};
+extern struct mutex stop_cpus_mutex;
+
int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg);
void stop_one_cpu_nowait(unsigned int cpu, cpu_stop_fn_t fn, void *arg,
struct cpu_stop_work *work_buf);
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index e3516b2..0cae1cc 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -132,8 +132,8 @@ void stop_one_cpu_nowait(unsigned int cpu, cpu_stop_fn_t fn, void *arg,
cpu_stop_queue_work(&per_cpu(cpu_stopper, cpu), work_buf);
}
+DEFINE_MUTEX(stop_cpus_mutex);
/* static data for stop_cpus */
-static DEFINE_MUTEX(stop_cpus_mutex);
static DEFINE_PER_CPU(struct cpu_stop_work, stop_cpus_work);
int __stop_cpus(const struct cpumask *cpumask, cpu_stop_fn_t fn, void *arg)