Re: [PATCH] i386/x86-64: Fix timer SMP bootup race

[Rusty Russell <rusty@rustcorp.com.au>]

On Sun, 2005-01-16 at 06:34 +0100, Andi Kleen wrote:
> On Sun, Jan 16, 2005 at 03:20:47PM +1100, Rusty Russell wrote:
> > But adding a bizarro "pre-prepare" notifier verged on nonsensical 8(.  I
> 
> I don't see the big issue. Preparse is just not as early as you thought.

State machine for non-boot CPUs currently goes:

 ------------
|            v
|	CPU_UP_PREPARE <-----
|	     |		     |
|	     v		     |
|	__cpu_up() ---> CPU_UP_CANCELED
|	     |
|	     v
|	CPU_ONLINE
|	     |
|	     v
|	CPU_DOWN_PREPARE <----
|	     |		      |
|	     v	              |
|	__cpu_down()---> CPU_DOWN_FAILED
|	     |
|	     v
|	CPU_DEAD
|	     |
 ------------

You've tacked a CPU_UP_PREPARE_EARLY above this state for two
architectures, which only gets called at boot, and only is used by
timers, to fix the fact that those archs don't actually follow this
state machine yet.

And sure enough, you've added a potential bug, since timer_jiffies
doesn't get reset to jiffies when a CPU comes back up.  Really, a
specific hack as below is required.

> > I'm also not clear on why we need to enable interrupts around
> > calibrate_delay() on secondary processors, but I'll try that too and
> > find out 8)
> 
> Because you cannot calibrate the timer without a timer tick.

AFAICT from a quick reading, that's not true.  CPU0 will bump the
jiffies, which is all you need: local interrupts not required.

> Even if you changed that it wouldn't help because the race can
> as well happen in the idle loop on the secondaries.

Again, I don't think so: the other CPUs are sitting in start_secondary()
waiting for __cpu_up().

As I said, I will try it and see.  It would certainly be the nicest
solution.  If not, I'll code a "timer_smp_boot_init()" function for x86
myself.

Cheers,
Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman