Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu

[Neil Horman <nhorman@tuxdriver.com>]

On Wed, Dec 12, 2007 at 03:21:32PM +0100, Andi Kleen wrote:
> > +	htcfg = read_pci_config(num, slot, func, 0x68);
> > +	if (htcfg & (1 << 18)) {	
> > +		printk(KERN_INFO "Detected use of extended apic ids on hypertransport bus\n");
> > +		if ((htcfg & (1 << 17)) == 0) {
> > +			printk(KERN_INFO "Enabling hypertransport extended apic interrupt broadcast\n");
> > +			printk(KERN_INFO "Note this is a bios bug, please contact your hw vendor\n");
> 
> I'm not convinced the message is correct. e.g. on a system with only a dual core not enabling
> that is fine, but the extended IDs might be still set.
> 
I'm not sure that would be fine.  In the situation you describe, not setting
this bit means the second core won't receive interrupts.  If we crash on that
core and boot the kdump kernel with it, we get exactly the same problem that we
currently see.


> >  #endif /* CONFIG_ACPI */
> >  
> > -static void __init nvidia_bugs(void)
> > +static void __init nvidia_bugs(int num, int slot, int func)
> >  {
> > +	static int fix_applied = 0;
> > +
> > +	if (fix_applied++)
> > +		return;
> 
> This looks like the wrong place to do this. Better add a flag or something
> in the structure. Dito others.
> 
I suppose I can, but I'm not sure what benefit that provides.  Can you
elaborate?


> Also while not a problem here in general it's bad style to add potential
> wrapping bugs like this. Never use ++ for flags.
> 
I can fix that up.  I'll hold off though until ben redoes all his testing.  He
mentioned earlier this morning, that some of the results he was getting may have
been caused by a kexec utility bug.  He's re-confirming that this patch solves
the reported problem.  Once he does, I'll repost.

Thanks & Regards
Neil

> -Andi

-- 
/***************************************************
 *Neil Horman
 *nhorman@tuxdriver.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/