nmi_watchdog suspicious

Cyrill Gorcunov
2008-06-10T18:58:26Z

Ingo, Maciej, I've a really bad suspicious about commit

---
commit 3ed3f06295e69700fa808396f7b350bff2b69de0
Author: Cyrill Gorcunov <gorcunov@gmail.com>
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@gmail.com>
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 -