Hi Russell,

On Fri, Jul 27, 2012 at 10:06:37PM +0100, Russell King - ARM Linux wrote:

On Fri, Jul 27, 2012 at 09:44:47PM +0100, Will Deacon wrote:

quoted

I did comment on this one:

  http://thread.gmane.org/gmane.linux.kernel/1303115

and I really think we should fix the cause of the problem, rather than
point patching this instance of it.

What do you think needs fixing there?

Well, we certainly need to fix the NULL dereference and the original patch
does do that. I just think it might be nicer to remove the possibility of a
NULL dereference instead.

We support booting a kernel on systems with or without SMP support, even
with a SMP kernel.  When the kernel is booted on such a system, it is
undefined whether smp_cross_call() is a valid function pointer.

So let's define it to point at a dummy function which explodes with a BUG if
the cpumask passed in isn't empty. That allows SMP kernels to do things like
`cross call to all other cores' without having to worry about whether there
are any other cores or not.

In any case, when we have only one CPU online in the system, it is
pointless even calling smp_cross_call().

Pointless, but also error-prone and requiring explicit cpumask checks at
each call-site.

That is why I explicitly suggested this solution.  This is the solution
_I_ want, because it is the most sane solution all round.

Adding a dummy implementation is straightforward [ok, this is untested]:

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 2c7217d..ffa411f 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c

@@ -329,7 +329,13 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
        }
 }
 
-static void (*smp_cross_call)(const struct cpumask *, unsigned int);
+static void dummy_smp_cross_call(const struct cpumask *mask, unsigned int irq)
+{
+       BUG_ON(!cpumask_empty(mask));
+}
+
+static void (*smp_cross_call)(const struct cpumask *, unsigned int) =
+       dummy_smp_cross_call;
 
 void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int))
 {

If you still prefer checking at the call-site then the original patch will
certainly work. Otherwise, I'm happy to submit the above after some testing.

Will