Re: [2.6.4-rc2] bogus semicolon behind if()

[Mikael Pettersson]

On Thu, 18 Mar 2004 01:52:36 -0800, Andrew Morton wrote:
>Mikael Pettersson <mikpe@xxxxxxxxx> wrote:
>>
>> Maciej W. Rozycki writes:
>>  > On Wed, 17 Mar 2004, Andrew Morton wrote:
>>  > 
>>  > > I still have a couple of NMI patches in -mm:
>>  > > 
>>  > > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5-rc1/2.6.5-rc1-mm1/broken-out/nmi_watchdog-local-apic-fix.patch
>>  > > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5-rc1/2.6.5-rc1-mm1/broken-out/nmi-1-hz.patch
>>  > > 
>>  > > What should we do with these?
>>  > 
>>  >  I think we should ask Mikael Pettersson as he is the local APIC watchdog 
>>  > expert.  Mikael?
>> 
>> Will do. Is there a problem with them, or do you just want them
>> reviewed for merging into 2.6.5-rc?
>> 
>
>They seem to work OK - I did a batch of testing with various setups.  But a
>retest wouldn't hurt.
>
>We mainly need a review and general finish-it-off-and-bless-it please.

'nmi_watchdog-local-apic-fix.patch' looks Ok, although I
would prefer if it didn't put a 'static' on nmi_perfctr_msr:
the perfctr driver needs access to it.

I'm not happy with 'nmi-1-hz.patch'. The real problem is that
SMP with nmi_watchdog=2 initialises the lapic NMI watchdog but
doesn't check it and therefore doesn't reduce nmi_hz.
This is an SMP bug, but instead of fixing it the patch removes
the check from UP and changes nmi.c to compensate.

This would be Ok if check_nmi_watchdog() with nmi_watchdog=2
was redundant, but I don't believe it is. It has exposed bugs
and unexpected hardware changes before, and so should remain.

I propose the patch below instead. It changes smpboot.c to do a
check_nmi_watchdog() at the appropriate place, which fixes the
high NMI frequency problem w/o changing anything else.
I've verified that it solves the problem on my MP-capable UP box.

/Mikael

diff -ruN linux-2.6.5-rc2/arch/i386/kernel/smpboot.c linux-2.6.5-rc2.smpboot-lapic-watchdog-fix/arch/i386/kernel/smpboot.c
--- linux-2.6.5-rc2/arch/i386/kernel/smpboot.c	2004-03-11 14:01:25.000000000 +0100
+++ linux-2.6.5-rc2.smpboot-lapic-watchdog-fix/arch/i386/kernel/smpboot.c	2004-03-20 17:21:30.113862000 +0100
@@ -1107,6 +1107,9 @@
 		}
 	}
 
+	if (nmi_watchdog == NMI_LOCAL_APIC)
+		check_nmi_watchdog();
+
 	smpboot_setup_io_apic();
 
 	setup_boot_APIC_clock();
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/