"Martin J. Bligh" wrote:
>
> >> Yes, this caused for me, a completely reliable boot time panic with 2.5.46.
> >> The problem is that per_cpu areas aren't initiallised until cpu_up is called,
> >> so a cpu cannot now take an interrupt before cpu_up is called.
> >
> > Rusty's da man on this, but I think the fix is to not turn on
> > the interrupts (at the APIC level) until cpu_up() has called
> > __cpu_up(). Look at cpu_up():
> >
> > ret = notifier_call_chain(&cpu_chain, CPU_UP_PREPARE, hcpu);
> > if (ret == NOTIFY_BAD) {
> > printk("%s: attempt to bring up CPU %u failed\n",
> > __FUNCTION__, cpu);
> > ret = -EINVAL;
> > goto out_notify;
> > }
> >
> > /* Arch-specific enabling code. */
> > ret = __cpu_up(cpu);
> >
> > The softirq storage is initialised inside the CPU_UP_PREPARE call.
> > So we're ready for interrupts on that CPU when your architecture's
> > __cpu_up() is called. And no sooner than this.
>
> All interrupts, or just softints?
>
I don't know. This sequencing really needs to be thought about
and written down, else we'll just have an ongoing fiasco trying
to graft stuff onto it.
In this case I'd say "all interrupts". The secondary really
should be 100% dormant until all CPU_UP_PREPARE callouts have
been run and have returned NOTIFY_OK.
At least, that's how I'd have designed it.
> I don't know. This sequencing really needs to be thought about
> and written down, else we'll just have an ongoing fiasco trying
> to graft stuff onto it.
OK, well here's a first cut at a rough diagram of the sequencing.
I'm told arches other than i386 have less problems (not sure why
we all feel the need to be different on this issue, but no doubt
there's some good reason. Or not). Whilst digging around, I found
the following comment, which was amusing, if frustrating:
/* These are wrappers to interface to the new boot process. Someone
who understands all this stuff should rewrite it properly. --RR 15/Jul/02 */
---------------------
BOOT CPU:
init
smp_prepare_cpus
smp_boot_cpus
setup_local_APIC
foreach cpu
do_boot_cpu
wakeup_secondary_via_(NMI|INIT)
{set bit in cpu_callout_map}
{spin on cpu_callin_map}
setup_IO_APIC
synchronize_tsc_bp
smp_init
foreach cpu
cpu_up
notifier_call_chain(CPU_UP_PREPARE)
__cpu_up
{set bit in smp_commenced_mask}
{spin on cpu_online_map}
notifier_call_chain(CPU_ONLINE)
smp_cpus_done
smp_commence
------------------------
SECONDARY CPU:
start_secondary
cpu_init
smp_callin
{spin on cpu_callout_map}
setup_local_APIC
local_irq_enable
calibrate_delay
disable_APIC_timer
{set our bit in cpu_callin_map}
synchronise_tsc_api
{spin on smp_commenced_mask}
enable_APIC_timer
{set our bit in cpu_online_map}
idle
> In this case I'd say "all interrupts". The secondary really
> should be 100% dormant until all CPU_UP_PREPARE callouts have
> been run and have returned NOTIFY_OK.
>
> At least, that's how I'd have designed it.
Well that makes sense to me. Apart from when I started doing it,
it seems that in smp_callin we do calibrate_delay, which looks
like it needs interrupts enabled (I could be wrong). I suppose
I could just disable them at the end of smp_callin again, but
it's all rather ugly. Maybe after we do disable_APIC_timer in
smp_callin.
M.
On Wed, Nov 06, 2002 at 12:04:44PM -0800, Andrew Morton wrote:
> > >
> > > The softirq storage is initialised inside the CPU_UP_PREPARE call.
> > > So we're ready for interrupts on that CPU when your architecture's
> > > __cpu_up() is called. And no sooner than this.
> >
> > All interrupts, or just softints?
> >
>
> I don't know. This sequencing really needs to be thought about
> and written down, else we'll just have an ongoing fiasco trying
> to graft stuff onto it.
>
> In this case I'd say "all interrupts". The secondary really
> should be 100% dormant until all CPU_UP_PREPARE callouts have
> been run and have returned NOTIFY_OK.
Well, the secondaries atleast cannot take the local timer interrupt
since things hooked off it, like timers and RCU, are initiliazed
using CPU_UP_PREPARE callouts. I would agree that it is unsafe to
allow *any* interrupt until CPU_UP_PREPARE callouts are completed.
So, the current way of enabling interrupts in secondaries during
delay calibration is unsafe. For that matter, it seems to me
that with no proper mixed CPU support, calibrate_delay() for
secondaries is meaningless.
Thanks
Dipankar