On Mon, May 05, 2008 at 06:15:33AM +0200, Nick Piggin wrote:
OK... But the wait=1 case already unconditionally stores its data on the
stack. In the wait=0 case, I would guess that it would be good to supply
a function that gets called after a grace period elapses after the last
CPU invokes the to-be-called-by-all-CPUs function:
int smp_call_function_nowait(void (*func)(void *), void *info, int natomic, void (*cleanup)(struct rcu_head *rcu))
Is this what you are looking for? My guess is that the idea is to
pre-allocate the memory before entering a bunch of irq-diabled code that
is difficult to unwind from.
Well, that would explain my confusion!!! ;-)
It is also not hard to support irq-disabled calls for the wait case one
of two ways:
o Expand the scope of call_function_lock to cover the call
to csd_flag_wait(). In the irq-disabled-wait case, use
spin_trylock_irqsave() to acquire this lock. If lock acquisition
fails, hand back -EBUSY.
o Use the polling trick. This avoids deadlock and failure as well,
but means that one form of irqs happens behind your back despite
irqs being disabled.
So just saying "no" until a need arises makes sense here.
The fallback is to waiting in the current patch (see below). This version
disallows calls with irqs disabled, so parallel calls to smp_call_function()
will process each other's function calls concurrently. Might be a bit of
a surprise to some existing uses -- but is the price of parallelism.
That makes a lot of sense -- especially if we need to introduce a cleanup
function to handle memory being passed into smp_call_function_disabled()
or whatever it ends up being called.
Good!
OK. So you are willing to live with smp_call_function() and
smp_call_function_mask() being globally serialized? That would
simplify things a bit. I think...
Yep, it is one way to permit the irq-disabled-wait case without -EBUSY.
But it does add some hair, so I agree that it should wait until someone
needs it.
OK. The below still permits parallel smp_call_function()
and smp_call_function_mask() as well as permitting parallel
smp_call_function_single(). It prohibits the irq-disabled case.
Thoughts?
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
arch/Kconfig | 2 -
kernel/smp.c | 87 +++++++++++------------------------------------------------
2 files changed, 18 insertions(+), 71 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index a5a0184..5ae9360 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -38,4 +38,4 @@ config HAVE_KRETPROBES
def_bool n
config USE_GENERIC_SMP_HELPERS
- def_bool n
+ def_bool y
diff --git a/kernel/smp.c b/kernel/smp.c
index 36d3eca..ef6de3d 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -17,7 +17,6 @@ __cacheline_aligned_in_smp DEFINE_SPINLOCK(call_function_lock);
enum {
CSD_FLAG_WAIT = 0x01,
CSD_FLAG_ALLOC = 0x02,
- CSD_FLAG_FALLBACK = 0x04,
};
struct call_function_data {
@@ -33,9 +32,6 @@ struct call_single_queue {
spinlock_t lock;
};
-static DEFINE_PER_CPU(struct call_function_data, cfd_fallback);
-static DEFINE_PER_CPU(unsigned long, cfd_fallback_used);
-
void __cpuinit init_call_single_data(void)
{
int i;
@@ -84,48 +80,13 @@ static void generic_exec_single(int cpu, struct call_single_data *data)
csd_flag_wait(data);
}
-/*
- * We need to have a global per-cpu fallback of call_function_data, so
- * we can safely proceed with smp_call_function() if dynamic allocation
- * fails and we cannot fall back to on-stack allocation (if wait == 0).
- */
-static noinline void acquire_cpu_fallback(int cpu)
-{
- while (test_and_set_bit_lock(0, &per_cpu(cfd_fallback_used, cpu)))
- cpu_relax();
-}
-
-static noinline void free_cpu_fallback(struct call_single_data *csd)
-{
- struct call_function_data *data;
- int cpu;
-
- data = container_of(csd, struct call_function_data, csd);
-
- /*
- * We could drop this loop by embedding a cpu variable in
- * csd, but this should happen so extremely rarely (if ever)
- * that this seems like a better idea
- */
- for_each_possible_cpu(cpu) {
- if (&per_cpu(cfd_fallback, cpu) != data)
- continue;
-
- clear_bit_unlock(0, &per_cpu(cfd_fallback_used, cpu));
- break;
- }
-}
-
static void rcu_free_call_data(struct rcu_head *head)
{
struct call_function_data *data;
data = container_of(head, struct call_function_data, rcu_head);
- if (data->csd.flags & CSD_FLAG_ALLOC)
- kfree(data);
- else
- free_cpu_fallback(&data->csd);
+ kfree(data);
}
/*
@@ -222,8 +183,6 @@ void generic_smp_call_function_single_interrupt(void)
data->flags &= ~CSD_FLAG_WAIT;
} else if (data_flags & CSD_FLAG_ALLOC)
kfree(data);
- else if (data_flags & CSD_FLAG_FALLBACK)
- free_cpu_fallback(data);
}
/*
* See comment on outer loop
@@ -244,35 +203,29 @@ void generic_smp_call_function_single_interrupt(void)
int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
int retry, int wait)
{
+ struct call_single_data d;
unsigned long flags;
/* prevent preemption and reschedule on another processor */
int me = get_cpu();
/* Can deadlock when called with interrupts disabled */
- WARN_ON(wait && irqs_disabled());
+ WARN_ON(irqs_disabled());
if (cpu == me) {
local_irq_save(flags);
func(info);
local_irq_restore(flags);
} else {
- struct call_single_data *data;
+ struct call_single_data *data = NULL;
- if (wait) {
- struct call_single_data d;
-
- data = &d;
- data->flags = CSD_FLAG_WAIT;
- } else {
+ if (!wait) {
data = kmalloc(sizeof(*data), GFP_ATOMIC);
if (data)
data->flags = CSD_FLAG_ALLOC;
- else {
- acquire_cpu_fallback(me);
-
- data = &per_cpu(cfd_fallback, me).csd;
- data->flags = CSD_FLAG_FALLBACK;
- }
+ }
+ if (!data) {
+ data = &d;
+ data->flags = CSD_FLAG_WAIT;
}
data->func = func;
@@ -320,13 +273,14 @@ void __smp_call_function_single(int cpu, struct call_single_data *data)
int smp_call_function_mask(cpumask_t mask, void (*func)(void *), void *info,
int wait)
{
- struct call_function_data *data;
+ struct call_function_data d;
+ struct call_function_data *data = NULL;
cpumask_t allbutself;
unsigned long flags;
int cpu, num_cpus;
/* Can deadlock when called with interrupts disabled */
- WARN_ON(wait && irqs_disabled());
+ WARN_ON(irqs_disabled());
cpu = smp_processor_id();
allbutself = cpu_online_map;
@@ -345,21 +299,14 @@ int smp_call_function_mask(cpumask_t mask, void (*func)(void *), void *info,
return smp_call_function_single(cpu, func, info, 0, wait);
}
- if (wait) {
- struct call_function_data d;
-
- data = &d;
- data->csd.flags = CSD_FLAG_WAIT;
- } else {
+ if (!wait) {
data = kmalloc(sizeof(*data), GFP_ATOMIC);
if (data)
data->csd.flags = CSD_FLAG_ALLOC;
- else {
- acquire_cpu_fallback(cpu);
-
- data = &per_cpu(cfd_fallback, cpu);
- data->csd.flags = CSD_FLAG_FALLBACK;
- }
+ }
+ if (!data) {
+ data = &d;
+ data->csd.flags = CSD_FLAG_WAIT;
}
spin_lock_init(&data->lock);
--
