[PATCH] don't call console drivers on non-online CPUs

George Anzinger identified the following problem: when a secondary CPU is
coming up, it calls printk() before it is "online".  It calls the console
drivers before its per-cpu storage has been prepared.  And the vga console
driver does a mod_timer().  This CPU's timers have not yet been initialised;
it is not clear why this doesn't oops - George thinks it is because virtual
address zero is still accessible at that time.

I believe the right way to fix this is to change printk so that a not-online
CPU will not call the console drivers.  Because printk should always be
callable.  If the CPU is not online the message is buffered, so the next
caller to printk who is online will actually display it.

ia64 has been doing exactly this for ages, so we can remove the
arch_consoles_callable() hook and just open-code the cpu_online() test in
printk.

That fixes things up for the secondary CPUs.  But this change causes a
problem for the boot CPU: it is being marked online very late in boot, so the
printk buffer is being displayed much later than we would like.

I believe that the solution to this is to mark the boot CPU online much
earlier.  So in this patch we call the new arch-provided function
smp_prepare_boot_cpu() immediately after the boot CPU's per-cpu areas are set
up.  Its mandate is to (at least) mark the boot CPU "online".

The change has been reviewed by davem and rth.  No comments were received
from the other arch maintainers.

diff --git a/arch/alpha/kernel/smp.c b/arch/alpha/kernel/smp.c
index a6ab21ca5bb8f410405baca600a4dbf034b325fb..148aaaf310f32234905aba61d24ad54b44d3aab8 100644 (file)
--- a/arch/alpha/kernel/smp.c
+++ b/arch/alpha/kernel/smp.c
@@ -607,6 +607,12 @@ smp_prepare_cpus(unsigned int max_cpus)
        smp_boot_cpus();
 }
 
+void __devinit
+smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_present_mask);
+}
+
 int __devinit
 __cpu_up(unsigned int cpu)
 {

diff --git a/arch/i386/kernel/smpboot.c b/arch/i386/kernel/smpboot.c
index 7c08f768281c5d557b11ba218cf7fa382ba41f50..b602e889879f8dffc6d83757c2dae1bd2c8e1b73 100644 (file)
--- a/arch/i386/kernel/smpboot.c
+++ b/arch/i386/kernel/smpboot.c
@@ -995,11 +995,6 @@ static void __init smp_boot_cpus(unsigned int max_cpus)
        printk("CPU%d: ", 0);
        print_cpu_info(&cpu_data[0]);
 
-       /*
-        * We have the boot CPU online for sure.
-        */
-       set_bit(0, &cpu_online_map);
-       set_bit(0, &cpu_callout_map);
        boot_cpu_logical_apicid = logical_smp_processor_id();
        map_cpu_to_boot_apicid(0, boot_cpu_apicid);
 
@@ -1172,6 +1167,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
        smp_boot_cpus(max_cpus);
 }
 
+void __devinit smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_online_map);
+       set_bit(smp_processor_id(), &cpu_callout_map);
+}
+
 int __devinit __cpu_up(unsigned int cpu)
 {
        /* This only works at boot for x86.  See "rewrite" above. */

diff --git a/arch/i386/mach-voyager/voyager_smp.c b/arch/i386/mach-voyager/voyager_smp.c
index 6875c8346171269f9fe9b77c2ae2d7584160b563..a6355ae320fd65ecb6ac927700a3b478b3d091e9 100644 (file)
--- a/arch/i386/mach-voyager/voyager_smp.c
+++ b/arch/i386/mach-voyager/voyager_smp.c
@@ -1942,6 +1942,12 @@ smp_prepare_cpus(unsigned int max_cpus)
        smp_boot_cpus();
 }
 
+void __devinit smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_online_map);
+       set_bit(smp_processor_id(), &cpu_callout_map);
+}
+
 int __devinit
 __cpu_up(unsigned int cpu)
 {

diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index e1c9a5ead71b88e73d33cf561d56af07c1baf7a2..6223825e0bebbfc00456e36b7234e344e0fe4824 100644 (file)
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -506,6 +506,12 @@ smp_prepare_cpus (unsigned int max_cpus)
        }
 }
 
+void __devinit smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_online_map);
+       set_bit(smp_processor_id(), &cpu_callin_map);
+}
+
 void
 smp_cpus_done (unsigned int dummy)
 {

diff --git a/arch/parisc/kernel/smp.c b/arch/parisc/kernel/smp.c
index 50dd06819c854fc4a431f62f53cb2650cf668e0c..7ea4390aed85fca978ecd66acdab3365b8ea5fec 100644 (file)
--- a/arch/parisc/kernel/smp.c
+++ b/arch/parisc/kernel/smp.c
@@ -706,6 +706,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
        smp_boot_cpus();
 }
 
+void __devinit smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_online_map);
+       set_bit(smp_processor_id(), &cpu_present_mask);
+}
+
 int __devinit __cpu_up(unsigned int cpu)
 {
        return cpu_online(cpu) ? 0 : -ENOSYS;

diff --git a/arch/ppc/kernel/smp.c b/arch/ppc/kernel/smp.c
index 6a10bcbe7579f6e1180990c8059e620a5c8aafa4..8b5270afcdba08948b7f682233466732c28be241 100644 (file)
--- a/arch/ppc/kernel/smp.c
+++ b/arch/ppc/kernel/smp.c
@@ -351,6 +351,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
                smp_ops->space_timers(num_cpus);
 }
 
+void __devinit smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_online_map);
+       set_bit(smp_processor_id(), &cpu_possible_map);
+}
+
 int __init setup_profiling_timer(unsigned int multiplier)
 {
        return 0;

diff --git a/arch/ppc64/kernel/smp.c b/arch/ppc64/kernel/smp.c
index 7db75666d20549d2d06620ec62e64a179ce0d491..eeae9cb81e58e7d568fd9c4bdebc5e28b57e090b 100644 (file)
--- a/arch/ppc64/kernel/smp.c
+++ b/arch/ppc64/kernel/smp.c
@@ -604,6 +604,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
        smp_space_timers(max_cpus);
 }
 
+void __devinit smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_online_map);
+       /* FIXME: what about cpu_possible()? */
+}
+
 int __devinit __cpu_up(unsigned int cpu)
 {
        struct pt_regs regs;

diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
index a9dc74a7ca5db3e342aa8bd28450cf05336b7098..e02e5c8c47fe7316e1a93936144a6df447b56374 100644 (file)
--- a/arch/s390/kernel/smp.c
+++ b/arch/s390/kernel/smp.c
@@ -572,6 +572,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
        set_prefix((u32) lowcore_ptr[smp_processor_id()]);
 }
 
+void __devinit smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_online_map);
+       set_bit(smp_processor_id(), &cpu_possible_map);
+}
+
 void smp_cpus_done(unsigned int max_cpus)
 {
 }

diff --git a/arch/s390x/kernel/smp.c b/arch/s390x/kernel/smp.c
index 11b0d8f1d24e4dd745b0f92766b45720356c278c..8abfde52cb9a869b66fda3a6ce3ec47d5b645175 100644 (file)
--- a/arch/s390x/kernel/smp.c
+++ b/arch/s390x/kernel/smp.c
@@ -554,6 +554,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
        set_prefix((u32)(u64) lowcore_ptr[smp_processor_id()]);
 }
 
+void __devinit smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_online_map);
+       set_bit(smp_processor_id(), &cpu_possible_map);
+}
+
 void smp_cpus_done(unsigned int max_cpis)
 {
 }

diff --git a/arch/sparc64/kernel/smp.c b/arch/sparc64/kernel/smp.c
index 64963cd4e949af0373e4d7ccfc9ff63583426bf0..52c390c34dd890795394198c39ae88705030ea18 100644 (file)
--- a/arch/sparc64/kernel/smp.c
+++ b/arch/sparc64/kernel/smp.c
@@ -1246,6 +1246,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
        smp_store_cpu_info(boot_cpu_id);
 }
 
+void __devinit smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_online_map);
+       set_bit(smp_processor_id(), &phys_cpu_present_map);
+}
+
 int __devinit __cpu_up(unsigned int cpu)
 {
        int ret = smp_boot_one_cpu(cpu);

diff --git a/arch/um/kernel/smp.c b/arch/um/kernel/smp.c
index 3503ed13f59ad92679e4490c27c75746cd4bb26a..ad740a718957af9ec78a78ab8e30f34289442895 100644 (file)
--- a/arch/um/kernel/smp.c
+++ b/arch/um/kernel/smp.c
@@ -186,6 +186,11 @@ void smp_prepare_cpus(unsigned int maxcpus)
        }
 }
 
+void __devinit smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_online_map);
+}
+
 int __cpu_up(unsigned int cpu)
 {
        set_bit(cpu, &smp_commenced_mask);

diff --git a/arch/x86_64/kernel/smpboot.c b/arch/x86_64/kernel/smpboot.c
index e7acfed4bf2afe643a9129313c4eccc6487b5ee2..a305c3916e64e1ed591bc23cfdfbb527f7b34a27 100644 (file)
--- a/arch/x86_64/kernel/smpboot.c
+++ b/arch/x86_64/kernel/smpboot.c
@@ -962,6 +962,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
        smp_boot_cpus(max_cpus);
 }
 
+void __devinit smp_prepare_boot_cpu(void)
+{
+       set_bit(smp_processor_id(), &cpu_online_map);
+       set_bit(smp_processor_id(), &cpu_callout_map);
+}
+
 int __devinit __cpu_up(unsigned int cpu)
 {
        /* This only works at boot for x86.  See "rewrite" above. */

diff --git a/include/asm-ia64/system.h b/include/asm-ia64/system.h
index d09f11cb14ec2c05827ffa5a361146eba3b6f735..e621c5c08b941e41b1c5a733617ade919b31e14c 100644 (file)
--- a/include/asm-ia64/system.h
+++ b/include/asm-ia64/system.h
@@ -234,9 +234,6 @@ extern void ia64_load_extra (struct task_struct *task);
 
 #ifdef CONFIG_SMP
 
-/* Return true if this CPU can call the console drivers in printk() */
-#define arch_consoles_callable() (cpu_online_map & (1UL << smp_processor_id()))
-
 /*
  * In the SMP case, we save the fph state when context-switching
  * away from a thread that modified fph.  This way, when the thread

diff --git a/include/linux/smp.h b/include/linux/smp.h
index 7a1e22b170c27c654e7a801f49b52785515cc022..8243c6bec13066ed05f968144fb2b503428a1d52 100644 (file)
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -78,6 +78,13 @@ extern int register_cpu_notifier(struct notifier_block *nb);
 extern void unregister_cpu_notifier(struct notifier_block *nb);
 
 int cpu_up(unsigned int cpu);
+
+/*
+ * Mark the boot cpu "online" so that it can call console drivers in
+ * printk() and can access its per-cpu storage.
+ */
+void smp_prepare_boot_cpu(void);
+
 #else /* !SMP */
 
 /*
@@ -94,7 +101,8 @@ static inline void smp_send_reschedule_all(void) { }
 #define cpu_online(cpu)                                ({ BUG_ON((cpu) != 0); 1; })
 #define num_online_cpus()                      1
 #define num_booting_cpus()                     1
-#define cpu_possible(cpu)                              ({ BUG_ON((cpu) != 0); 1; })
+#define cpu_possible(cpu)                      ({ BUG_ON((cpu) != 0); 1; })
+#define smp_prepare_boot_cpu()                 do {} while (0)
 
 struct notifier_block;
 

diff --git a/init/main.c b/init/main.c
index 6770292ed29a26f3411c7eec6a4bab2130124998..b07fac4e9f02d951ac9e433cbf609896bf196dd4 100644 (file)
--- a/init/main.c
+++ b/init/main.c
@@ -377,6 +377,13 @@ asmlinkage void __init start_kernel(void)
        printk(linux_banner);
        setup_arch(&command_line);
        setup_per_cpu_areas();
+
+       /*
+        * Mark the boot cpu "online" so that it can call console drivers in
+        * printk() and can access its per-cpu storage.
+        */
+       smp_prepare_boot_cpu();
+
        build_all_zonelists();
        page_alloc_init();
        printk("Kernel command line: %s\n", saved_command_line);

diff --git a/kernel/printk.c b/kernel/printk.c
index bb1bcb0d723f2d4ef14f7a83f87d90cb20d6d353..ffe724fabc0e80b45328e8392917981eb61fbade 100644 (file)
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -27,6 +27,7 @@
 #include <linux/interrupt.h>                   /* For in_interrupt() */
 #include <linux/config.h>
 #include <linux/delay.h>
+#include <linux/smp.h>
 
 #include <asm/uaccess.h>
 
@@ -42,10 +43,6 @@
 
 #define LOG_BUF_MASK   (LOG_BUF_LEN-1)
 
-#ifndef arch_consoles_callable
-#define arch_consoles_callable() (1)
-#endif
-
 /* printk's without a loglevel use this.. */
 #define DEFAULT_MESSAGE_LOGLEVEL 4 /* KERN_WARNING */
 
@@ -447,10 +444,12 @@ asmlinkage int printk(const char *fmt, ...)
                        log_level_unknown = 1;
        }
 
-       if (!arch_consoles_callable()) {
+       if (!cpu_online(smp_processor_id())) {
                /*
-                * On some architectures, the consoles are not usable
-                * on secondary CPUs early in the boot process.
+                * Some console drivers may assume that per-cpu resources have
+                * been allocated.  So don't allow them to be called by this
+                * CPU until it is officially up.  We shouldn't be calling into
+                * random console drivers on a CPU which doesn't exist yet..
                 */
                spin_unlock_irqrestore(&logbuf_lock, flags);
                goto out;
@@ -638,7 +637,8 @@ void register_console(struct console * console)
        }
        if (console->flags & CON_PRINTBUFFER) {
                /*
-                * release_console_sem() will print out the buffered messages for us.
+                * release_console_sem() will print out the buffered messages
+                * for us.
                 */
                spin_lock_irqsave(&logbuf_lock, flags);
                con_start = log_start;