[PATCH] powerpc: smp_call_function might switch cpu

Hugh Dickins hugh at veritas.com
Sat May 19 02:47:01 EST 2007


smp_call_function_map() was not safe against preemption to another
cpu: its test for removing self from map was outside the spinlock.
Rearrange it a little to fix that.

smp_call_function_single() was also wrong: now get_cpu() before
excluding self, as other architectures do.

Signed-off-by: Hugh Dickins <hugh at veritas.com>
---
I guess most callers actually have preemption disabled already: I only
noticed this for the first time in 2.6.22-rc1-mm1, where mm/page_alloc.c
has a drain_all_local_pages(), which triggered a warning with my PowerPC
DEBUG_PREEMPT patch.

 arch/powerpc/kernel/smp.c |   34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

--- 2.6.22-rc1-git7/arch/powerpc/kernel/smp.c	2007-05-13 05:40:52.000000000 +0100
+++ linux/arch/powerpc/kernel/smp.c	2007-05-18 17:07:51.000000000 +0100
@@ -201,13 +201,6 @@ int smp_call_function_map(void (*func) (
 	/* Can deadlock when called with interrupts disabled */
 	WARN_ON(irqs_disabled());
 
-	/* remove 'self' from the map */
-	if (cpu_isset(smp_processor_id(), map))
-		cpu_clear(smp_processor_id(), map);
-
-	/* sanity check the map, remove any non-online processors. */
-	cpus_and(map, map, cpu_online_map);
-
 	if (unlikely(smp_ops == NULL))
 		return ret;
 
@@ -222,10 +215,17 @@ int smp_call_function_map(void (*func) (
 	/* Must grab online cpu count with preempt disabled, otherwise
 	 * it can change. */
 	num_cpus = num_online_cpus() - 1;
-	if (!num_cpus || cpus_empty(map)) {
-		ret = 0;
-		goto out;
-	}
+	if (!num_cpus)
+		goto done;
+
+	/* remove 'self' from the map */
+	if (cpu_isset(smp_processor_id(), map))
+		cpu_clear(smp_processor_id(), map);
+
+	/* sanity check the map, remove any non-online processors. */
+	cpus_and(map, map, cpu_online_map);
+	if (cpus_empty(map))
+		goto done;
 
 	call_data = &data;
 	smp_wmb();
@@ -263,6 +263,7 @@ int smp_call_function_map(void (*func) (
 		}
 	}
 
+ done:
 	ret = 0;
 
  out:
@@ -282,16 +283,17 @@ EXPORT_SYMBOL(smp_call_function);
 int smp_call_function_single(int cpu, void (*func) (void *info), void *info, int nonatomic,
 			int wait)
 {
-	cpumask_t map=CPU_MASK_NONE;
+	cpumask_t map = CPU_MASK_NONE;
+	int ret = -EBUSY;
 
 	if (!cpu_online(cpu))
 		return -EINVAL;
 
-	if (cpu == smp_processor_id())
-		return -EBUSY;
-
 	cpu_set(cpu, map);
-	return smp_call_function_map(func,info,nonatomic,wait,map);
+	if (cpu != get_cpu())
+		ret = smp_call_function_map(func,info,nonatomic,wait,map);
+	put_cpu();
+	return ret;
 }
 EXPORT_SYMBOL(smp_call_function_single);
 



More information about the Linuxppc-dev mailing list