From: Suresh Siddha <suresh.b.siddha@intel.com>
To: mingo@elte.hu, tglx@linutronix.de, hpa@zytor.com,
trenn@novell.com, prarit@redhat.com, tj@kernel.org,
rusty@rustcorp.com.au, a.p.zijlstra@chello.nl,
akpm@linux-foundation.org, torvalds@linux-foundation.org
Cc: linux-kernel@vger.kernel.org, suresh.b.siddha@intel.com,
youquan.song@intel.com, stable@kernel.org
Subject: [patch 1/4] x86, mtrr: lock stop machine during MTRR rendezvous sequence
Date: Wed, 22 Jun 2011 15:20:22 -0700 [thread overview]
Message-ID: <20110622222043.862589370@sbsiddha-MOBL3.sc.intel.com> (raw)
In-Reply-To: <20110622222021.904952469@sbsiddha-MOBL3.sc.intel.com>
[-- Attachment #1: mtrr_stop_machine_quick_fix.patch --]
[-- Type: text/plain, Size: 4025 bytes --]
From: Suresh Siddha <suresh.b.siddha@intel.com>
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
(will be forwarded to stable series for inclusion in kernels v2.6.35-v2.6.39
after some testing in mainline).
Reported-by: Vadim Kotelnikov <vadimuzzz@inbox.ru>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: stable@kernel.org # 2.6.35+, backport a week or two after this gets more testing in mainline
---
arch/x86/kernel/cpu/mtrr/main.c | 16 ++++++++++++++++
include/linux/stop_machine.h | 2 ++
kernel/stop_machine.c | 2 +-
3 files changed, 19 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,18 @@ set_mtrr(unsigned int reg, unsigned long
unsigned long flags;
int cpu;
+#ifdef CONFIG_SMP
+ /*
+ * If we are not yet online, then there can be no stop_machine() in
+ * parallel. Stop machine ensures this by using get_online_cpus().
+ *
+ * If we are online, then we need to prevent a stop_machine() happening
+ * in parallel by taking the stop cpus mutex.
+ */
+ if (cpu_online(raw_smp_processor_id()))
+ mutex_lock(&stop_cpus_mutex);
+#endif
+
preempt_disable();
data.smp_reg = reg;
@@ -330,6 +342,10 @@ set_mtrr(unsigned int reg, unsigned long
local_irq_restore(flags);
preempt_enable();
+#ifdef CONFIG_SMP
+ if (cpu_online(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)
next prev parent reply other threads:[~2011-06-22 22:31 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-22 22:20 [patch 0/4] MTRR rendezvous deadlock fix and cleanups using stop_machine() Suresh Siddha
2011-06-22 22:20 ` Suresh Siddha [this message]
2011-06-23 9:05 ` [patch 1/4] x86, mtrr: lock stop machine during MTRR rendezvous sequence Peter Zijlstra
2011-06-23 9:33 ` Thomas Gleixner
2011-06-23 9:41 ` Peter Zijlstra
2011-06-23 18:16 ` Suresh Siddha
2011-06-22 22:20 ` [patch 2/4] stop_machine: reorganize stop_cpus() implementation Suresh Siddha
2011-06-22 22:20 ` [patch 3/4] stop_machine: implement stop_machine_from_offline_cpu() Suresh Siddha
2011-06-23 9:25 ` Peter Zijlstra
2011-06-23 9:28 ` Tejun Heo
2011-06-23 9:31 ` Peter Zijlstra
2011-06-23 18:19 ` Suresh Siddha
2011-06-24 7:45 ` Peter Zijlstra
2011-06-24 17:55 ` Suresh Siddha
2011-06-22 22:20 ` [patch 4/4] x86, mtrr: use stop_machine() for doing MTRR rendezvous Suresh Siddha
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110622222043.862589370@sbsiddha-MOBL3.sc.intel.com \
--to=suresh.b.siddha@intel.com \
--cc=a.p.zijlstra@chello.nl \
--cc=akpm@linux-foundation.org \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=prarit@redhat.com \
--cc=rusty@rustcorp.com.au \
--cc=stable@kernel.org \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=trenn@novell.com \
--cc=youquan.song@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®