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

[Thomas Schlichter]

Am Dienstag, 9. März 2004 08:01 schrieb Philippe Elie:
> On Mon, 08 Mar 2004 at 16:29 +0000, Andrew Morton wrote:

~~ snip ~~

> > Rename Thomas's script to crappy-code-detector.sh and its hit rate goes
> > to 100% ;)
>
> Was this patch so crappy ? http://tinyurl.com/2jbe4 ,
>
> -				check_nmi_watchdog();
> +				if (check_nmi_watchdog() < 0);
> +					timer_ack = !cpu_has_tsc

Well, exactly this code made me look for other possible problems... ;-)
As I wrote a few days ago I have problems with that ChangeSet,
  (http://marc.theaimsgroup.com/?l=linux-kernel&m=107840458123059&w=2)
so I did examine it closer.

My results are:
1. The semicolons behind the if()'s cannot be there intentionally.
2. To fix my problem, timer_ack must be set to 1 for my (integrated) APIC, and 
as my CPU has a TSC, this cannot be correct for me:
-	timer_ack = 1;
+	if (nmi_watchdog == NMI_IO_APIC && !APIC_INTEGRATED(ver))
+		timer_ack = 1;
+	else
+		timer_ack = !cpu_has_tsc;

I changed that if(...) to
	if (nmi_watchdog == NMI_IO_APIC || APIC_INTEGRATED(ver))
which works fine for me here, but I am not 100% sure if this is what the 
author of the original patch ment and if it still fixes the original 
problem...

The patch which makes these changes is attached. Perhaps someone else wants to 
test it, too...

   Thomas Schlichter

P.S.: I tested it with an AMD Athlon 3000+ on a board with a VIA KT400 chipset 
and enabled ACPI and IO-APIC.
--- linux-2.6.4-rc2-mm1/arch/i386/kernel/io_apic.c.orig	2004-03-08 16:29:14.000000000 +0100
+++ linux-2.6.4-rc2-mm1/arch/i386/kernel/io_apic.c	2004-03-08 17:26:40.000000000 +0100
@@ -2181,7 +2181,7 @@ static inline void check_timer(void)
 	 */
 	apic_write_around(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_EXTINT);
 	init_8259A(1);
-	if (nmi_watchdog == NMI_IO_APIC && !APIC_INTEGRATED(ver))
+	if (nmi_watchdog == NMI_IO_APIC || APIC_INTEGRATED(ver))
 		timer_ack = 1;
 	else
 		timer_ack = !cpu_has_tsc;
@@ -2202,7 +2202,7 @@ static inline void check_timer(void)
 				disable_8259A_irq(0);
 				setup_nmi();
 				enable_8259A_irq(0);
-				if (check_nmi_watchdog() < 0);
+				if (check_nmi_watchdog() < 0)
 					timer_ack = !cpu_has_tsc;
 			}
 			return;
@@ -2226,7 +2226,7 @@ static inline void check_timer(void)
 				add_pin_to_irq(0, 0, pin2);
 			if (nmi_watchdog == NMI_IO_APIC) {
 				setup_nmi();
-				if (check_nmi_watchdog() < 0);
+				if (check_nmi_watchdog() < 0)
 					timer_ack = !cpu_has_tsc;
 			}
 			return;