2015-12-15 03:30:50

by Jeff Merkey

[permalink] [raw]
Subject: [PATCH] Fix spurious hard lockup events while in debugger

The current touch_nmi_watchdog() function in /kernel/watchdog.c does
not always catch all cases when a processor is spinning in the nmi
handler inside either KGDB, KDB, or MDB, in particular, the case where
a processor is being held by a debugger inside an int1 handler.

The hrtimer_interrupts_saved count can still end up matching the
hrtime value in some cases, resulting in the hard lockup detector
tagging processors inside a debugger and executing a panic.

The patch below corrects this problem. I did not add this to
the touch_nmi_function directly becuase of possible affects on
timing issues since the function is widely used by drivers and
modules.

I have tested this patch and it fixes the problem for kernel debuggers
stopping errant hard lockup events when processors are spinning inside
the debugger.

Signed-off-by: Jeff Merkey <[email protected]>
---
kernel/watchdog.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 18f34cf..b682aab 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -283,6 +283,13 @@ static bool is_hardlockup(void)
__this_cpu_write(hrtimer_interrupts_saved, hrint);
return false;
}
+
+void touch_hardlockup_watchdog(void)
+{
+ __this_cpu_write(hrtimer_interrupts_saved, 0);
+}
+EXPORT_SYMBOL_GPL(touch_hardlockup_watchdog);
+
#endif

static int is_softlockup(unsigned long touch_ts)
--
1.8.3.1


2015-12-16 16:58:05

by Jeff Merkey

[permalink] [raw]
Subject: Re: [PATCH] Fix spurious hard lockup events while in debugger

On 12/14/15, Jeff Merkey <[email protected]> wrote:
> The current touch_nmi_watchdog() function in /kernel/watchdog.c does
> not always catch all cases when a processor is spinning in the nmi
> handler inside either KGDB, KDB, or MDB, in particular, the case where
> a processor is being held by a debugger inside an int1 handler.
>
> The hrtimer_interrupts_saved count can still end up matching the
> hrtime value in some cases, resulting in the hard lockup detector
> tagging processors inside a debugger and executing a panic.
>
> The patch below corrects this problem. I did not add this to
> the touch_nmi_function directly becuase of possible affects on
> timing issues since the function is widely used by drivers and
> modules.
>
> I have tested this patch and it fixes the problem for kernel debuggers
> stopping errant hard lockup events when processors are spinning inside
> the debugger.
>
> Signed-off-by: Jeff Merkey <[email protected]>
> ---
> kernel/watchdog.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 18f34cf..b682aab 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -283,6 +283,13 @@ static bool is_hardlockup(void)
> __this_cpu_write(hrtimer_interrupts_saved, hrint);
> return false;
> }
> +
> +void touch_hardlockup_watchdog(void)
> +{
> + __this_cpu_write(hrtimer_interrupts_saved, 0);
> +}
> +EXPORT_SYMBOL_GPL(touch_hardlockup_watchdog);
> +
> #endif
>
> static int is_softlockup(unsigned long touch_ts)
> --
> 1.8.3.1
>
>


I stared at the function that detects hardlockups until my eyes have
turned red and the code looks ok. I am still trying to figure why
this is happening. I call touch_nmi_watchdog() without the other
function and the hard lockup fires off.

I'll try to debug it and determine why it is happening. Having the
flag thing for the touch operating independent of the counter may be
the clue. I'll look into it a little more today and see if I can
figure out why it is happening.

Jeff

2015-12-16 17:22:20

by Jeff Merkey

[permalink] [raw]
Subject: Re: [PATCH] Fix spurious hard lockup events while in debugger

On 12/14/15, Jeff Merkey <[email protected]> wrote:
> The current touch_nmi_watchdog() function in /kernel/watchdog.c does
> not always catch all cases when a processor is spinning in the nmi
> handler inside either KGDB, KDB, or MDB, in particular, the case where
> a processor is being held by a debugger inside an int1 handler.
>
> The hrtimer_interrupts_saved count can still end up matching the
> hrtime value in some cases, resulting in the hard lockup detector
> tagging processors inside a debugger and executing a panic.
>
> The patch below corrects this problem. I did not add this to
> the touch_nmi_function directly becuase of possible affects on
> timing issues since the function is widely used by drivers and
> modules.
>
> I have tested this patch and it fixes the problem for kernel debuggers
> stopping errant hard lockup events when processors are spinning inside
> the debugger.
>
> Signed-off-by: Jeff Merkey <[email protected]>
> ---
> kernel/watchdog.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 18f34cf..b682aab 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -283,6 +283,13 @@ static bool is_hardlockup(void)
> __this_cpu_write(hrtimer_interrupts_saved, hrint);
> return false;
> }
> +
> +void touch_hardlockup_watchdog(void)
> +{
> + __this_cpu_write(hrtimer_interrupts_saved, 0);
> +}
> +EXPORT_SYMBOL_GPL(touch_hardlockup_watchdog);
> +
> #endif
>
> static int is_softlockup(unsigned long touch_ts)
> --
> 1.8.3.1
>
>

I got to the bottom of it. It's related to the hardware I am using.
One of the processors is faulting and hanging due to an existing bug
in the hw_breakpoint handler not setting the resume flag (I have
previously reported it and submitted a patch). This breaks your code,
but there's nothing you can do about it.

There is a severe bug in hw_breakpoint.c that causes int1 recursion
and this whole "lazy debug register switching" nonsense does not work
properly. I am probably the first person to actually test this code
path robustly. I applied the patch that fixes this bug in
hw_breakpoint.c and the problem with your code firing off and ignoring
the touch flag
went away.

Jeff

2015-12-16 17:50:15

by Don Zickus

[permalink] [raw]
Subject: Re: [PATCH] Fix spurious hard lockup events while in debugger

On Wed, Dec 16, 2015 at 10:22:18AM -0700, Jeff Merkey wrote:
> On 12/14/15, Jeff Merkey <[email protected]> wrote:
> > The current touch_nmi_watchdog() function in /kernel/watchdog.c does
> > not always catch all cases when a processor is spinning in the nmi
> > handler inside either KGDB, KDB, or MDB, in particular, the case where
> > a processor is being held by a debugger inside an int1 handler.
> >
> > The hrtimer_interrupts_saved count can still end up matching the
> > hrtime value in some cases, resulting in the hard lockup detector
> > tagging processors inside a debugger and executing a panic.
> >
> > The patch below corrects this problem. I did not add this to
> > the touch_nmi_function directly becuase of possible affects on
> > timing issues since the function is widely used by drivers and
> > modules.
> >
> > I have tested this patch and it fixes the problem for kernel debuggers
> > stopping errant hard lockup events when processors are spinning inside
> > the debugger.
> >
> > Signed-off-by: Jeff Merkey <[email protected]>
> > ---
> > kernel/watchdog.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> > index 18f34cf..b682aab 100644
> > --- a/kernel/watchdog.c
> > +++ b/kernel/watchdog.c
> > @@ -283,6 +283,13 @@ static bool is_hardlockup(void)
> > __this_cpu_write(hrtimer_interrupts_saved, hrint);
> > return false;
> > }
> > +
> > +void touch_hardlockup_watchdog(void)
> > +{
> > + __this_cpu_write(hrtimer_interrupts_saved, 0);
> > +}
> > +EXPORT_SYMBOL_GPL(touch_hardlockup_watchdog);
> > +
> > #endif
> >
> > static int is_softlockup(unsigned long touch_ts)
> > --
> > 1.8.3.1
> >
> >
>
> I got to the bottom of it. It's related to the hardware I am using.
> One of the processors is faulting and hanging due to an existing bug
> in the hw_breakpoint handler not setting the resume flag (I have
> previously reported it and submitted a patch). This breaks your code,
> but there's nothing you can do about it.
>
> There is a severe bug in hw_breakpoint.c that causes int1 recursion
> and this whole "lazy debug register switching" nonsense does not work
> properly. I am probably the first person to actually test this code
> path robustly. I applied the patch that fixes this bug in
> hw_breakpoint.c and the problem with your code firing off and ignoring
> the touch flag
> went away.

Ah, good to know. Thanks! I'll drop this patch then.

Cheers,
Don

2015-12-16 17:55:13

by Jeff Merkey

[permalink] [raw]
Subject: Re: [PATCH] Fix spurious hard lockup events while in debugger

On 12/16/15, Jeff Merkey <[email protected]> wrote:
> On 12/14/15, Jeff Merkey <[email protected]> wrote:
>> The current touch_nmi_watchdog() function in /kernel/watchdog.c does
>> not always catch all cases when a processor is spinning in the nmi
>> handler inside either KGDB, KDB, or MDB, in particular, the case where
>> a processor is being held by a debugger inside an int1 handler.
>>
>> The hrtimer_interrupts_saved count can still end up matching the
>> hrtime value in some cases, resulting in the hard lockup detector
>> tagging processors inside a debugger and executing a panic.
>>
>> The patch below corrects this problem. I did not add this to
>> the touch_nmi_function directly becuase of possible affects on
>> timing issues since the function is widely used by drivers and
>> modules.
>>
>> I have tested this patch and it fixes the problem for kernel debuggers
>> stopping errant hard lockup events when processors are spinning inside
>> the debugger.
>>
>> Signed-off-by: Jeff Merkey <[email protected]>
>> ---
>> kernel/watchdog.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
>> index 18f34cf..b682aab 100644
>> --- a/kernel/watchdog.c
>> +++ b/kernel/watchdog.c
>> @@ -283,6 +283,13 @@ static bool is_hardlockup(void)
>> __this_cpu_write(hrtimer_interrupts_saved, hrint);
>> return false;
>> }
>> +
>> +void touch_hardlockup_watchdog(void)
>> +{
>> + __this_cpu_write(hrtimer_interrupts_saved, 0);
>> +}
>> +EXPORT_SYMBOL_GPL(touch_hardlockup_watchdog);
>> +
>> #endif
>>
>> static int is_softlockup(unsigned long touch_ts)
>> --
>> 1.8.3.1
>>
>>
>
> I got to the bottom of it. It's related to the hardware I am using.
> One of the processors is faulting and hanging due to an existing bug
> in the hw_breakpoint handler not setting the resume flag (I have
> previously reported it and submitted a patch). This breaks your code,
> but there's nothing you can do about it.
>
> There is a severe bug in hw_breakpoint.c that causes int1 recursion
> and this whole "lazy debug register switching" nonsense does not work
> properly. I am probably the first person to actually test this code
> path robustly. I applied the patch that fixes this bug in
> hw_breakpoint.c and the problem with your code firing off and ignoring
> the touch flag
> went away.
>
> Jeff
>

Wow, I figured it out. What's really needed here is the ability to
touch all the processors from just one processor. That's what's
missing. This per processor nonsense doesn't fly here. A debugger
needs to be able to turn off your stuff (it needs an on/off switch)
completely when needed.

I'll submit a patch for that. I'll just maintain the working version
in my patch for the debugger so people can get a working, stable,
debugable kernel and not a broken one until fixes start showing up in
the tree.

Jeff