Messages in this thread | | | Date | Fri, 28 Mar 2008 16:08:54 -0500 | From | Jack Steiner <> | Subject | Re: [PATCH 8/8] x86_64: Support for new UV apic |
| |
On Fri, Mar 28, 2008 at 09:45:33PM +0100, Ingo Molnar wrote: > > * Jack Steiner <steiner@sgi.com> wrote: > > > On Fri, Mar 28, 2008 at 09:15:32PM +0100, Ingo Molnar wrote: > > > > > > * Jack Steiner <steiner@sgi.com> wrote: > > > > > > > Index: linux/arch/x86/kernel/apic_64.c > > > > =================================================================== > > > > --- linux.orig/arch/x86/kernel/apic_64.c 2008-03-28 13:00:22.000000000 -0500 > > > > +++ linux/arch/x86/kernel/apic_64.c 2008-03-28 13:06:12.000000000 -0500 > > > > @@ -738,6 +738,7 @@ void __cpuinit setup_local_APIC(void) > > > > unsigned int value; > > > > int i, j; > > > > > > > > + preempt_disable(); > > > > value = apic_read(APIC_LVR); > > > > > > > > BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f); > > > > @@ -831,6 +832,7 @@ void __cpuinit setup_local_APIC(void) > > > > else > > > > value = APIC_DM_NMI | APIC_LVT_MASKED; > > > > apic_write(APIC_LVT1, value); > > > > + preempt_enable(); > > > > } > > > > > > hm, this looks a bit weird - why are all the preempt-disable/enable > > > calls needed? > > > > The first patch had a preempt disable/enable in the function that > > reads apicid (see read_apic_id() in arch/x86/kernel/genapic_64.c). > > This seemed necessary since large system generate an apicid by reading > > the live id & concatenating it with extra bits. > > > > One of the review comments suggested that I change the preempt to a > > WARN() since reading apic_id really should be done with preemtion > > disabled. The added code eliminates the warnings. > > could you post the stacktraces of the warnings as they occured? All > those codepaths should be running with preemption disabled in some > natural way. > > Ingo
Here is one of the pathes. I have not been able to reproduce the other (smp_sanity_check()). It's possible there was cleanup that eliminated the message, or I might have forgotten the receipe. I know it was early in boot:
WARNING: at arch/x86/kernel/genapic_64.c:97 read_apic_id+0x30/0x6a() Modules linked in: Pid: 1, comm: swapper Not tainted 2.6.25-rc7-x86-latest.git-00829-gfb5f592-dirty #15
Call Trace: [<ffffffff80231e69>] warn_on_slowpath+0x51/0x63 [<ffffffff80578d37>] _spin_lock+0xe/0x24 [<ffffffff8022b289>] task_rq_lock+0x3d/0x6f [<ffffffff80579133>] _spin_unlock_irqrestore+0x12/0x2c [<ffffffff8022d7fd>] set_cpus_allowed+0xd9/0xe6 [<ffffffff8057319c>] set_cpu_sibling_map+0x2f4/0x30c [<ffffffff8021f434>] read_apic_id+0x30/0x6a [<ffffffff807cf898>] native_smp_prepare_cpus+0xc4/0x2d4 [<ffffffff807c580f>] kernel_init+0x4e/0x2cb [<ffffffff8020bee8>] child_rip+0xa/0x12
--- jack
| |