Messages in this thread | | | Date | Tue, 10 Jun 2008 23:10:25 +0400 | From | Cyrill Gorcunov <> | Subject | Re: nmi_watchdog suspicious |
| |
[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 -
| |