[PATCH 1/2] [POWERPC] smp_call_function_map should be static

[PATCH 1/2] [POWERPC] smp_call_function_map should be static

[Olof Johansson]

smp_call_function_map should be static, and for consistency prepend it
with __ like other local helper functions in the same file.

Signed-off-by: Olof Johansson <olof@lixom.net>

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 338950a..cefeee8 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -181,12 +181,13 @@ static struct call_data_struct {
  * <wait> If true, wait (atomically) until function has completed on other CPUs.
  * [RETURNS] 0 on success, else a negative status code. Does not return until
  * remote CPUs are nearly ready to execute <<func>> or are or have executed.
+ * <map> is a cpu map of the cpus to send IPI to.
  *
  * You must not call this function with disabled interrupts or from a
  * hardware interrupt handler or from a bottom half handler.
  */
-int smp_call_function_map(void (*func) (void *info), void *info, int nonatomic,
-			int wait, cpumask_t map)
+static int __smp_call_function_map(void (*func) (void *info), void *info,
+				   int nonatomic, int wait, cpumask_t map)
 {
 	struct call_data_struct data;
 	int ret = -1, num_cpus;
@@ -265,7 +266,8 @@ int smp_call_function_map(void (*func) (void *info), void *info, int nonatomic,
 static int __smp_call_function(void (*func)(void *info), void *info,
 			       int nonatomic, int wait)
 {
-	return smp_call_function_map(func,info,nonatomic,wait,cpu_online_map);
+	return __smp_call_function_map(func, info, nonatomic, wait,
+				       cpu_online_map);
 }
 
 int smp_call_function(void (*func) (void *info), void *info, int nonatomic,
@@ -278,8 +280,8 @@ int smp_call_function(void (*func) (void *info), void *info, int nonatomic,
 }
 EXPORT_SYMBOL(smp_call_function);
 
-int smp_call_function_single(int cpu, void (*func) (void *info), void *info, int nonatomic,
-			int wait)
+int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
+			     int nonatomic, int wait)
 {
 	cpumask_t map = CPU_MASK_NONE;
 	int ret = 0;
@@ -292,7 +294,7 @@ int smp_call_function_single(int cpu, void (*func) (void *info), void *info, int
 
 	cpu_set(cpu, map);
 	if (cpu != get_cpu())
-		ret = smp_call_function_map(func,info,nonatomic,wait,map);
+		ret = __smp_call_function_map(func, info, nonatomic, wait, map);
 	else {
 		local_irq_disable();
 		func(info);

[PATCH 2/2] [POWERPC] Make smp_send_stop() handle panic and xmon reboot

[Olof Johansson]

smp_send_stop() will send an IPI to all other cpus to shut them down.
However, for the case of xmon-based reboots (as well as potentially some
panics), the other cpus are (or might be) spinning with interrupts off,
and won't take the IPI.

Current code will drop us into the debugger when the IPI fails, which
means we're in an infinite loop that we can't get out of without an
external reset of some sort.

Instead, make the smp_send_stop() IPI call path just print the warning
about being unable to send IPIs, but make it return so the rest of the
shutdown sequence can continue. It's not perfect, but the lesser of
two evils.

Also move the call_lock handling outside of smp_call_function_map so we
can avoid deadlocks in smp_send_stop().


Signed-off-by: Olof Johansson <olof@lixom.net>

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index cefeee8..8730687 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -76,6 +76,8 @@ void smp_call_function_interrupt(void);
 
 int smt_enabled_at_boot = 1;
 
+static int ipi_fail_ok;
+
 static void (*crash_ipi_function_ptr)(struct pt_regs *) = NULL;
 
 #ifdef CONFIG_PPC64
@@ -204,8 +206,6 @@ static int __smp_call_function_map(void (*func) (void *info), void *info,
 	if (wait)
 		atomic_set(&data.finished, 0);
 
-	spin_lock(&call_lock);
-
 	/* remove 'self' from the map */
 	if (cpu_isset(smp_processor_id(), map))
 		cpu_clear(smp_processor_id(), map);
@@ -232,7 +232,8 @@ static int __smp_call_function_map(void (*func) (void *info), void *info,
 			printk("smp_call_function on cpu %d: other cpus not "
 				"responding (%d)\n", smp_processor_id(),
 				atomic_read(&data.started));
-			debugger(NULL);
+			if (!ipi_fail_ok)
+				debugger(NULL);
 			goto out;
 		}
 	}
@@ -259,15 +260,18 @@ static int __smp_call_function_map(void (*func) (void *info), void *info,
  out:
 	call_data = NULL;
 	HMT_medium();
-	spin_unlock(&call_lock);
 	return ret;
 }
 
 static int __smp_call_function(void (*func)(void *info), void *info,
 			       int nonatomic, int wait)
 {
-	return __smp_call_function_map(func, info, nonatomic, wait,
+	int ret;
+	spin_lock(&call_lock);
+	ret =__smp_call_function_map(func, info, nonatomic, wait,
 				       cpu_online_map);
+	spin_unlock(&call_lock);
+	return ret;
 }
 
 int smp_call_function(void (*func) (void *info), void *info, int nonatomic,
@@ -293,9 +297,11 @@ int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
 		return -EINVAL;
 
 	cpu_set(cpu, map);
-	if (cpu != get_cpu())
+	if (cpu != get_cpu()) {
+		spin_lock(&call_lock);
 		ret = __smp_call_function_map(func, info, nonatomic, wait, map);
-	else {
+		spin_unlock(&call_lock);
+	} else {
 		local_irq_disable();
 		func(info);
 		local_irq_enable();
@@ -307,7 +313,22 @@ EXPORT_SYMBOL(smp_call_function_single);
 
 void smp_send_stop(void)
 {
-	__smp_call_function(stop_this_cpu, NULL, 1, 0);
+	int nolock;
+
+	/* It's OK to fail sending the IPI, since the alternative is to
+	 * be stuck forever waiting on the other CPU to take the interrupt.
+	 *
+	 * It's better to at least continue and go through reboot, since this
+	 * function is usually called at panic or reboot time in the first
+	 * place.
+	 */
+	ipi_fail_ok = 1;
+
+	/* Don't deadlock in case we got called through panic */
+	nolock = !spin_trylock(&call_lock);
+	__smp_call_function_map(stop_this_cpu, NULL, 1, 0, cpu_online_map);
+	if (!nolock)
+		spin_unlock(&call_lock);
 }
 
 void smp_call_function_interrupt(void)

Re: [PATCH 1/2] [POWERPC] smp_call_function_map should be static

[Benjamin Herrenschmidt]

On Thu, 2007-12-27 at 22:08 -0600, Olof Johansson wrote:
> smp_call_function_map should be static, and for consistency prepend it
> with __ like other local helper functions in the same file.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

NAK. I want it exported to other arch code, I'll need it in some stuff
that I haven't released yet, like some BookE SMP stuff.

Ben.

Re: [PATCH 1/2] [POWERPC] smp_call_function_map should be static

[Olof Johansson]

On Wed, Jan 02, 2008 at 07:28:33AM +1100, Benjamin Herrenschmidt wrote:
> 
> On Thu, 2007-12-27 at 22:08 -0600, Olof Johansson wrote:
> > smp_call_function_map should be static, and for consistency prepend it
> > with __ like other local helper functions in the same file.
> > 
> > Signed-off-by: Olof Johansson <olof@lixom.net>
> 
> NAK. I want it exported to other arch code, I'll need it in some stuff
> that I haven't released yet, like some BookE SMP stuff.

With the locking changes in 2/2, it will need a new exported function
that does the locking around the call to __*. With that, it'd be easier
to just merge this.

I.e. __smp_call_function_map() will be the internal (lockless) version,
and you'll end up adding a nonstatic smp_call_function_map() that does
locking and calls the __* one.

Is that OK with you?


-Olof

Re: [PATCH 1/2] [POWERPC] smp_call_function_map should be static

[Benjamin Herrenschmidt]

> With the locking changes in 2/2, it will need a new exported function
> that does the locking around the call to __*. With that, it'd be easier
> to just merge this.
> 
> I.e. __smp_call_function_map() will be the internal (lockless) version,
> and you'll end up adding a nonstatic smp_call_function_map() that does
> locking and calls the __* one.
> 
> Is that OK with you?

Yup, it's fine.

Cheers,
Ben.