Re: nmi_watchdog suspicious

Cyrill Gorcunov
2008-06-10T19:13:22Z

[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@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 -

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 -