Re: nmi_watchdog suspicious

[Cyrill Gorcunov]

[Cyrill Gorcunov - Tue, Jun 10, 2008 at 10:57:59PM +0400]
| Ingo, Maciej, I've a really bad suspicious about commit
| 
| ---
| commit 3ed3f06295e69700fa808396f7b350bff2b69de0
| Author: Cyrill Gorcunov <gorcunov@xxxxxxxxx>
| Date:   Wed Jun 4 01:00:47 2008 +0400
| 
|     x86: nmi - consolidate nmi_watchdog_default for 32bit mode
|     
|     64bit mode bootstrap code does set nmi_watchdog to NMI_NONE
|     by default and doing the same on 32bit mode is safe too.
|     Such an action saves us from several #ifdef.
|     
|     Btw, my previous commit
|     
|     commit 19ec673ced067316b9732bc6d1c4ff4052e5f795
|     Author: Cyrill Gorcunov <gorcunov@xxxxxxxxx>
|     Date:   Wed May 28 23:00:47 2008 +0400
|     
|         x86: nmi - fix incorrect NMI watchdog used by default
|     
|     did not fix the problem completely, moreover it
|     introduced additional bug - nmi_watchdog would be
|     set to either NMI_LOCAL_APIC or NMI_IO_APIC
|     _regardless_ to boot option if being enabled thru
|     /proc/sys/kernel/nmi_watchdog. Sorry for that.
|     Fix it too.
| ---
| 
| actually it changed nmi behaviour much more then I thought.
| Look, since by default nmi_watchdog now becomes NMI_NONE it
| does mean nmi.c:touch_nmi_watchdog() will not reset apic timers
| anymore even being requested by some other code.
| 
| This commit does affect 32bit mode _only_. So now I'm guessing
| what mode was implemented in correct way originally.
| 
| On 64bit mode nmi_watchdog=NMI_NONE by default (in case if APIC enabled).
| On 32bit mode nmi_watchdog=NMI_DEFAULT was by default (in any case,
| but could be set to NMI_NONE in check_timer(), but we don't take
| this case now).
| 
| So lets take a look on touch_nmi_watchdog().
| There is the condition
| 
| 	if (nmi_watchdog > 0)
| 		...tell to reset counters in nmi_watchdog_tick()
| 
| this condition is not taken on 64bit mode, but *was* taken on
| 32bit mode by default! So who was right then? 64bit version or 32bit?
| 
| Maciej, could you take a look please? Maybe I just missing figure
| in general - ie how nmi_watchdog _should_ work.
| 
| 		- Cyrill -

btw, since in general we support nmi_watchdog for two choises:
lapic and io-apic - i think about to change this condition to

	if (nmi_watchdog == NMI_LOCAL_APIC ||
		nmi_watchdog == NMI_IO_APIC) {

but still in doubts...

		- Cyrill -
--
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/