Re: [git-pull -tip] x86: mpparse cleanup

[Ingo Molnar <mingo@elte.hu>]

* Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:

> On Wed, 2009-03-18 at 17:39 +0100, Ingo Molnar wrote:
> > * Ingo Molnar <mingo@elte.hu> wrote:
> > 
> > > >  1 files changed, 120 insertions(+), 141 deletions(-)
> > > 
> > > Looks good - pulled, thanks Jaswinder!
> > > 
> > > Would you be interested in resurrecting the more mechanic cleanups 
> > > you did as well, on top of this patch?
> > > 
> > > They are still valuable, we just have to keep them separate from a 
> > > potential breakage-causing change like the above one, so that if a 
> > > tester bisects to it, we have an as single-purpose and focused 
> > > commit to look at as possible.
> > 
> > So it's helpful to have a small commit when we have a regression.
> > 
> > Such as in this case - a -tip testbox crashed with:
> > 
> > weird, boot CPU (#0) not listed by the BIOS.
> > Kernel panic - not syncing: Boot APIC ID in local APIC unexpected (0 vs 4)
> > Pid: 1, comm: swapper Not tainted 
> > 2.6.29-rc8-tip-02595-g170ae27-dirty #21111
> > Call Trace:
> >  [<c0599964>] ? printk+0x14/0x18
> >  [<c05998a6>] panic+0x3e/0xe8
> >  [<c08fc9e4>] native_smp_prepare_cpus+0x13e/0x1f8
> >  [<c08f335a>] kernel_init+0x47/0xd8
> >  [<c08f3313>] ? kernel_init+0x0/0xd8
> >  [<c0103ea3>] kernel_thread_helper+0x7/0x10
> > 
> > config and full crashlog attached.
> > 
> 
> As I told you earlier, I cannot check this on my side. Only 
> suspect will be check_irq_src() please test this patch if this 
> does not work I will revert check_irq_src():

no need to test on your side. If a patch is broken the first step is 
to take a look at the patch and think about it. For example this 
block:

                case MP_PROCESSOR:
-                       {
-                               struct mpc_cpu *m = (struct mpc_cpu *)mpt;
-                               /* ACPI may have already provided this data */
-                               if (!acpi_lapic)
-                                       MP_processor_info(m);
-                               mpt += sizeof(*m);
-                               count += sizeof(*m);
-                               break;
-                       }
+                       /* ACPI may have already provided this data */
+                       if (!acpi_lapic)
+                               MP_processor_info((struct mpc_cpu *)&mpt);
+                       skip_entry(&mpt, &count, sizeof(struct mpc_cpu));
+                       break;


is buggy.

	Ingo