Ensure single IPI generation for SMP call single
PATCH 2/2 Ensure single IPI generation for SMP call single
--- arch/ia64/kernel/smp.c | 8 ++++++-- arch/x86/kernel/smp_32.c | 8 ++++++-- arch/x86/kernel/smp_64.c | 8 ++++++-- include/linux/smp.h | 1 + kernel/smp.c | 34 ++++++++++++++++++++-------------- 5 files changed, 39 insertions(+), 20 deletions(-)
arch/ia64/kernel/smp.c +6 -2
@@ -364,7 +364,8 @@ void __smp_call_function_single(int cpu, struct call_single_data *data) {struct call_single_queue *dst;unsigned long flags; -int ipi, wait_done = data->flags &CSD_FLAG_WAIT ; +int ipi =0 ; +int wait_done = data->flags &CSD_FLAG_WAIT ;/* Can deadlock when called with interrupts disabled */ WARN_ON ((data->flags &CSD_FLAG_WAIT ) &&irqs_disabled ());
@@ -373,8 +374,11 @@ void __smp_call_function_single(int cpu, struct call_single_data *data) dst = &per_cpu (call_single_queue, cpu);spin_lock_irqsave (&dst->lock, flags); - ipi =list_empty (&dst->list);list_add_tail (&data->list, &dst->list); +if (!dst->activated) { + dst->activated =1 ; + ipi =1 ; + }spin_unlock_irqrestore (&dst->lock, flags);if (ipi)
arch/x86/kernel/smp_32.c +6 -2
@@ -629,7 +629,8 @@ void __smp_call_function_single(int cpu, struct call_single_data *data)cpumask_t mask =cpumask_of_cpu (cpu);struct call_single_queue *dst;unsigned long flags; -int ipi, wait_done = data->flags &CSD_FLAG_WAIT ; +int ipi =0 ; +int wait_done = data->flags &CSD_FLAG_WAIT ;/* Can deadlock when called with interrupts disabled */ WARN_ON ((data->flags &CSD_FLAG_WAIT ) &&irqs_disabled ());
@@ -638,8 +639,11 @@ void __smp_call_function_single(int cpu, struct call_single_data *data) dst = &per_cpu (call_single_queue, cpu);spin_lock_irqsave (&dst->lock, flags); - ipi =list_empty (&dst->list);list_add_tail (&data->list, &dst->list); +if (!dst->activated) { + dst->activated =1 ; + ipi =1 ; + }spin_unlock_irqrestore (&dst->lock, flags);if (ipi)
arch/x86/kernel/smp_64.c +6 -2
@@ -449,7 +449,8 @@ void __smp_call_function_single(int cpu, struct call_single_data *data)cpumask_t mask =cpumask_of_cpu (cpu);struct call_single_queue *dst;unsigned long flags; -int ipi, wait_done = data->flags &CSD_FLAG_WAIT ; +int ipi =0 ; +int wait_done = data->flags &CSD_FLAG_WAIT ;/* Can deadlock when called with interrupts disabled */ WARN_ON ((data->flags &CSD_FLAG_WAIT ) &&irqs_disabled ());
@@ -458,8 +459,11 @@ void __smp_call_function_single(int cpu, struct call_single_data *data) dst = &per_cpu (call_single_queue, cpu);spin_lock_irqsave (&dst->lock, flags); - ipi =list_empty (&dst->list);list_add_tail (&data->list, &dst->list); +if (!dst->activated) { + dst->activated =1 ; + ipi =1 ; + }spin_unlock_irqrestore (&dst->lock, flags);if (ipi)
include/linux/smp.h +1 -0
@@ -65,6 +65,7 @@ extern void smp_cpus_done(unsigned int max_cpus);struct call_single_queue {spinlock_t lock;struct list_head list; +int activated; };/*
kernel/smp.c +20 -14
@@ -23,28 +23,34 @@ void __cpuinit generic_init_call_single_data(void)void generic_smp_call_function_single_interrupt (void ) { -struct call_single_queue *q;unsigned long flags;LIST_HEAD (list); +struct call_single_queue *q = &__get_cpu_var (call_single_queue); - q = &__get_cpu_var (call_single_queue);spin_lock_irqsave (&q->lock, flags); -list_replace_init (&q->list, &list); -spin_unlock_irqrestore (&q->lock, flags); +while (!list_empty (&q->list)) { +list_replace_init (&q->list, &list); +spin_unlock_irqrestore (&q->lock, flags); -while (!list_empty (&list)) { -struct call_single_data *data; +do { +struct call_single_data *data; - data = list_entry(list.next, struct call_single_data, list); -list_del (&data->list); + data = list_entry(list.next, struct call_single_data, + list); +list_del (&data->list); - data->func (data->info); -if (data->flags &CSD_FLAG_WAIT ) { -smp_wmb (); - data->flags =0 ; - }else if (data->flags &CSD_FLAG_ALLOC ) -kfree (data); + data->func (data->info); +if (data->flags &CSD_FLAG_WAIT ) { +smp_wmb (); + data->flags =0 ; + }else if (data->flags &CSD_FLAG_ALLOC ) +kfree (data); + }while (!list_empty (&list)); + +spin_lock_irqsave (&q->lock, flags); } + q->activated =0 ; +spin_unlock_irqrestore (&q->lock, flags); } int generic_smp_call_function_single(int cpu, void (*func) (void *info),
Signature · 1 line
1.5.2.5
Quoted 41 lines · Alan D. Brunelle
kernel/smp.c +12
-14@@ -23,28 +23,34 @@ void __cpuinit generic_init_call_single_data(void)void generic_smp_call_function_single_interrupt (void ) { -struct call_single_queue *q;unsigned long flags;LIST_HEAD (list); +struct call_single_queue *q = &__get_cpu_var (call_single_queue); - q = &__get_cpu_var (call_single_queue);spin_lock_irqsave (&q->lock, flags); -list_replace_init (&q->list, &list); -spin_unlock_irqrestore (&q->lock, flags); +while (!list_empty (&q->list)) { +list_replace_init (&q->list, &list); +spin_unlock_irqrestore (&q->lock, flags); -while (!list_empty (&list)) { -struct call_single_data *data; + do { + struct call_single_data *data; - data = list_entry(list.next, struct call_single_data, list); -list_del (&data->list); + data = list_entry(list.next, struct call_single_data, + list); + list_del(&data->list); - data->func (data->info); -if (data->flags &CSD_FLAG_WAIT ) { -smp_wmb (); - data->flags =0 ; - }else if (data->flags &CSD_FLAG_ALLOC ) -kfree (data); + data->func(data->info); + if (data->flags & CSD_FLAG_WAIT) { + smp_wmb();
kernel/smp.c void __cpuinit generic_init_call_single_data(void) · line 54
Quoted 10 lines
kernel/smp.c +8
-0+ data->flags =0 ; + }else if (data->flags &CSD_FLAG_ALLOC ) +kfree (data); + }while (!list_empty (&list)); + +spin_lock_irqsave (&q->lock, flags); } + q->activated =0 ; +spin_unlock_irqrestore (&q->lock, flags); }
I agree with doing it this way, re-checking and doing another run (or more). However I think we can improve it a bit so we don't always have to grab the dst lock at least twice - it should be safe enough to include the lock only inside the first loop, doing an smp_mb() before the list_empty() check and again at the bottom before looping around and doing the list_empty() check again.
I've rolled a new patch series here:
http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=io-cpu-affinity
(or just pull the io-cpu-affinity branch), it also includes a bunch of other cleanups like porting the faster smp_call_function() to ia64 and powerpc as well. So the kernel/smp.c generic helpers have grown a bit, while the arch bits are smaller.
I'm curious if it now boots on ia64, since I killed the hack to manually call the __init manually there. If you could check, I would appreciate it ;-)
I've built all 4 supported archs and they compile and link fine, but nothing has been booted yet.
Signature · 1 line
Jens Axboe
Quoted 69 lines · Jens Axboe
Quoted 41 lines · Alan D. Brunelle
kernel/smp.c +12
-14@@ -23,28 +23,34 @@ void __cpuinit generic_init_call_single_data(void)void generic_smp_call_function_single_interrupt (void ) { -struct call_single_queue *q;unsigned long flags;LIST_HEAD (list); +struct call_single_queue *q = &__get_cpu_var (call_single_queue); - q = &__get_cpu_var (call_single_queue);spin_lock_irqsave (&q->lock, flags); -list_replace_init (&q->list, &list); -spin_unlock_irqrestore (&q->lock, flags); +while (!list_empty (&q->list)) { +list_replace_init (&q->list, &list); +spin_unlock_irqrestore (&q->lock, flags); -while (!list_empty (&list)) { -struct call_single_data *data; + do { + struct call_single_data *data; - data = list_entry(list.next, struct call_single_data, list); -list_del (&data->list); + data = list_entry(list.next, struct call_single_data, + list); + list_del(&data->list); - data->func (data->info); -if (data->flags &CSD_FLAG_WAIT ) { -smp_wmb (); - data->flags =0 ; - }else if (data->flags &CSD_FLAG_ALLOC ) -kfree (data); + data->func(data->info); + if (data->flags & CSD_FLAG_WAIT) { + smp_wmb();kernel/smp.c void __cpuinit generic_init_call_single_data(void) · line 54
Quoted 10 lines
kernel/smp.c +8
-0+ data->flags =0 ; + }else if (data->flags &CSD_FLAG_ALLOC ) +kfree (data); + }while (!list_empty (&list)); + +spin_lock_irqsave (&q->lock, flags); } + q->activated =0 ; +spin_unlock_irqrestore (&q->lock, flags); }I agree with doing it this way, re-checking and doing another run (or more). However I think we can improve it a bit so we don't always have to grab the dst lock at least twice - it should be safe enough to include the lock only inside the first loop, doing an
smp_mb()before thelist_empty()check and again at the bottom before looping around and doing thelist_empty()check again.I've rolled a new patch series here:
http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=io-cpu-affinity
(or just pull the io-cpu-affinity branch), it also includes a bunch of other cleanups like porting the faster
smp_call_function()to ia64 and powerpc as well. So thekernel/smp.cgeneric helpers have grown a bit, while the arch bits are smaller.
Quoted 6 lines
I'm curious if it now boots on ia64, since I killed the hack to manually call the
__initmanually there. If you could check, I would appreciate it ;-)I've built all 4 supported archs and they compile and link fine, but nothing has been booted yet.
x86 and x86-64 boot just fine, so powerpc should work as well (will do performance testing on a 4-way ppc tomorrow). ia64 should also work, as long as the init_call_single_data() gets called correctly.
If it doesn't work on tha ia64, try and change the core_initcall() to a postcore_initcall() or even an arch_initcall(). It now resides in kernel/smp.c.
Signature · 1 line
Jens Axboe