From: Jason Baron <jbaron@redhat.com>
To: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Cc: akpm@linux-foundation.org, Ingo Molnar <mingo@elte.hu>,
linux-kernel@vger.kernel.org,
Rusty Russell <rusty@rustcorp.com.au>
Subject: [patch 2/2] implement immediate updating via stop_machine_run()
Date: Thu, 28 Feb 2008 11:37:01 -0500 [thread overview]
Message-ID: <20080228163701.GB6195@redhat.com> (raw)
In-Reply-To: <20080227190519.GA14335@Krystal>
-Updating immediate values, cannot rely on smp_call_function() b/c synchronizing
cpus using IPIs leads to deadlocks. Process A held a read lock on
tasklist_lock, then process B called apply_imv_update(). Process A received the
IPI and begins executing ipi_busy_loop(). Then process C takes a write lock
irq on the task list lock, before receiving the IPI. Thus, process A holds up
process C, and C can't get an IPI b/c interrupts are disabled. Solve this
problem by using a new 'ALL_CPUS' parameter to stop_machine_run(). Which
runs a function on all cpus after they are busy looping and have disabled
irqs. Since this is done in a new process context, we don't have to worry
about interrupted spin_locks. Also, less lines of code. Has survived 24 hours+
of testing...
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
kernel/immediate.c | 80 ++++++++++++++--------------------------------------
1 files changed, 21 insertions(+), 59 deletions(-)
diff --git a/kernel/immediate.c b/kernel/immediate.c
index 4c36a89..378a452 100644
--- a/kernel/immediate.c
+++ b/kernel/immediate.c
@@ -20,6 +20,7 @@
#include <linux/immediate.h>
#include <linux/memory.h>
#include <linux/cpu.h>
+#include <linux/stop_machine.h>
#include <asm/cacheflush.h>
@@ -27,48 +28,33 @@
* Kernel ready to execute the SMP update that may depend on trap and ipi.
*/
static int imv_early_boot_complete;
+static int wrote_text;
extern const struct __imv __start___imv[];
extern const struct __imv __stop___imv[];
+static int stop_machine_imv_update(void *imv_ptr)
+{
+ struct __imv *imv = imv_ptr;
+
+ if (!started) {
+ text_poke((void *)imv->imv, (void *)imv->var, imv->size);
+ wrote_text = 1;
+ smp_wmb(); /* make sure other cpus see that this has run */
+ } else
+ sync_core();
+
+ flush_icache_range(imv->imv, imv->imv + imv->size);
+
+ return 0;
+}
+
/*
* imv_mutex nests inside module_mutex. imv_mutex protects builtin
* immediates and module immediates.
*/
static DEFINE_MUTEX(imv_mutex);
-static atomic_t wait_sync;
-
-struct ipi_loop_data {
- long value;
- const struct __imv *imv;
-} loop_data;
-
-static void ipi_busy_loop(void *arg)
-{
- unsigned long flags;
-
- local_irq_save(flags);
- atomic_dec(&wait_sync);
- do {
- /* Make sure the wait_sync gets re-read */
- smp_mb();
- } while (atomic_read(&wait_sync) > loop_data.value);
- atomic_dec(&wait_sync);
- do {
- /* Make sure the wait_sync gets re-read */
- smp_mb();
- } while (atomic_read(&wait_sync) > 0);
- /*
- * Issuing a synchronizing instruction must be done on each CPU before
- * reenabling interrupts after modifying an instruction. Required by
- * Intel's errata.
- */
- sync_core();
- flush_icache_range(loop_data.imv->imv,
- loop_data.imv->imv + loop_data.imv->size);
- local_irq_restore(flags);
-}
/**
* apply_imv_update - update one immediate value
@@ -82,9 +68,6 @@ static void ipi_busy_loop(void *arg)
*/
static int apply_imv_update(const struct __imv *imv)
{
- unsigned long flags;
- long online_cpus;
-
/*
* If the variable and the instruction have the same value, there is
* nothing to do.
@@ -111,30 +94,9 @@ static int apply_imv_update(const struct __imv *imv)
if (imv_early_boot_complete) {
kernel_text_lock();
- get_online_cpus();
- online_cpus = num_online_cpus();
- atomic_set(&wait_sync, 2 * online_cpus);
- loop_data.value = online_cpus;
- loop_data.imv = imv;
- smp_call_function(ipi_busy_loop, NULL, 1, 0);
- local_irq_save(flags);
- atomic_dec(&wait_sync);
- do {
- /* Make sure the wait_sync gets re-read */
- smp_mb();
- } while (atomic_read(&wait_sync) > online_cpus);
- text_poke((void *)imv->imv, (void *)imv->var,
- imv->size);
- /*
- * Make sure the modified instruction is seen by all CPUs before
- * we continue (visible to other CPUs and local interrupts).
- */
- wmb();
- atomic_dec(&wait_sync);
- flush_icache_range(imv->imv,
- imv->imv + imv->size);
- local_irq_restore(flags);
- put_online_cpus();
+ wrote_text = 0;
+ stop_machine_run(stop_machine_imv_update, (void *)imv,
+ ALL_CPUS);
kernel_text_unlock();
} else
text_poke_early((void *)imv->imv, (void *)imv->var,
next prev parent reply other threads:[~2008-02-28 16:39 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-02-02 21:08 [patch 0/7] Immediate Values Mathieu Desnoyers
2008-02-02 21:08 ` [patch 1/7] Immediate Values - Architecture Independent Code Mathieu Desnoyers
2008-02-26 22:52 ` Jason Baron
2008-02-26 23:12 ` Mathieu Desnoyers
2008-02-26 23:34 ` Mathieu Desnoyers
2008-02-27 16:44 ` Jason Baron
2008-02-27 17:01 ` Jason Baron
2008-02-27 19:05 ` Mathieu Desnoyers
2008-02-28 16:33 ` [patch 1/2] add ALL_CPUS option to stop_machine_run() Jason Baron
2008-02-28 22:09 ` Max Krasnyanskiy
2008-02-28 22:14 ` Mathieu Desnoyers
2008-02-29 2:39 ` Jason Baron
2008-02-29 9:00 ` Ingo Molnar
2008-02-29 18:24 ` Max Krasnyanskiy
2008-02-29 19:15 ` Ingo Molnar
2008-02-29 19:58 ` Max Krasnyanskiy
2008-03-03 4:12 ` Rusty Russell
2008-03-04 0:30 ` Max Krasnyanskiy
2008-03-04 2:36 ` Rusty Russell
2008-03-04 4:11 ` Max Krasnyansky
2008-03-02 23:32 ` Rusty Russell
2008-02-28 16:37 ` Jason Baron [this message]
2008-02-29 13:43 ` [patch 2/2] implement immediate updating via stop_machine_run() Mathieu Desnoyers
2008-02-28 16:50 ` [patch 1/7] Immediate Values - Architecture Independent Code Jason Baron
2008-02-02 21:08 ` [patch 2/7] Immediate Values - Kconfig menu in EMBEDDED Mathieu Desnoyers
2008-02-02 21:08 ` [patch 3/7] Immediate Values - x86 Optimization Mathieu Desnoyers
2008-02-02 21:08 ` [patch 4/7] Add text_poke and sync_core to powerpc Mathieu Desnoyers
2008-02-02 21:08 ` [patch 5/7] Immediate Values - Powerpc Optimization Mathieu Desnoyers
2008-02-02 21:08 ` [patch 6/7] Immediate Values - Documentation Mathieu Desnoyers
2008-02-02 21:08 ` [patch 7/7] Scheduler Profiling - Use Immediate Values Mathieu Desnoyers
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=20080228163701.GB6195@redhat.com \
--to=jbaron@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@polymtl.ca \
--cc=mingo@elte.hu \
--cc=rusty@rustcorp.com.au \
/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
Powered by JetHome