[PATCH 01/31] Add hard/soft lockup debugger entry points

[PATCH 01/31] Add hard/soft lockup debugger entry points

[Jeffrey Merkey]

This patch series adds an export which can be set by system debuggers to
direct the hard lockup and soft lockup detector to trigger a breakpoint
exception and enter a debugger if one is active.  It is assumed that if
someone sets this variable, then an breakpoint handler of some sort will
be actively loaded or registered via the notify die handler chain.

This addition is extremely useful for debugging hard and soft lockups
real time and quickly from a console debugger.

Signed-off-by: Jeffrey Merkey <jeffmerkey@gmail.com>
---
 kernel/watchdog.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index b3ace6e..1dd5902 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -23,6 +23,7 @@
 #include <linux/workqueue.h>
 
 #include <asm/irq_regs.h>
+#include <asm/kdebug.h>
 #include <linux/kvm_para.h>
 #include <linux/perf_event.h>
 #include <linux/kthread.h>
@@ -108,6 +109,9 @@ static DEFINE_PER_CPU(struct perf_event *, watchdog_ev);
 #endif
 static unsigned long soft_lockup_nmi_warn;
 
+int debug_watchdog_lockups;
+EXPORT_SYMBOL_GPL(debug_watchdog_lockups);
+
 /* boot commands */
 /*
  * Should we panic when a soft-lockup or hard-lockup occurs:
@@ -358,6 +362,9 @@ static void watchdog_overflow_callback(struct perf_event *event,
 		else
 			dump_stack();
 
+		if (debug_watchdog_lockups)
+			arch_breakpoint();
+
 		/*
 		 * Perform all-CPU dump only once to avoid multiple hardlockups
 		 * generating interleaving traces
@@ -478,6 +485,9 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
 		else
 			dump_stack();
 
+		if (debug_watchdog_lockups)
+			arch_breakpoint();
+
 		if (softlockup_all_cpu_backtrace) {
 			/* Avoid generating two back traces for current
 			 * given that one is already made above
-- 
1.8.3.1

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Thomas Gleixner]

On Thu, 28 Jan 2016, Jeffrey Merkey wrote:

> This patch series adds an export which can be set by system debuggers to
> direct the hard lockup and soft lockup detector to trigger a breakpoint
> exception and enter a debugger if one is active.  It is assumed that if
> someone sets this variable, then an breakpoint handler of some sort will
> be actively loaded or registered via the notify die handler chain.
> 
> This addition is extremely useful for debugging hard and soft lockups
> real time and quickly from a console debugger.

Why do we need an extra breakpoint instruction in the code to enter a
debugger? Don't have debuggers mechanisms to install breakpoints?

I'm probably missing something obvious here.

Thanks,

	tglx

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Jeffrey Merkey]

On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 28 Jan 2016, Jeffrey Merkey wrote:
>
>> This patch series adds an export which can be set by system debuggers to
>> direct the hard lockup and soft lockup detector to trigger a breakpoint
>> exception and enter a debugger if one is active.  It is assumed that if
>> someone sets this variable, then an breakpoint handler of some sort will
>> be actively loaded or registered via the notify die handler chain.
>>
>> This addition is extremely useful for debugging hard and soft lockups
>> real time and quickly from a console debugger.
>
> Why do we need an extra breakpoint instruction in the code to enter a
> debugger? Don't have debuggers mechanisms to install breakpoints?
>
> I'm probably missing something obvious here.
>
> Thanks,
>
> 	tglx
>
>
>

It's a pain in the butt to grep around through assembly language in a
function in watchdog.c that has everything declared static with no
symbols.  It's a lot easier just to insert an INT3 in the section of
code that has the mouse caught in the trap (inside a function that
triggers the hard lockup) -- after all -- that's what the instruction
is for.

Jeff

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Thomas Gleixner]

On Thu, 28 Jan 2016, Jeffrey Merkey wrote:
> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
> > I'm probably missing something obvious here.
> 
> It's a pain in the butt to grep around through assembly language in a
> function in watchdog.c that has everything declared static with no
> symbols.  It's a lot easier just to insert an INT3 in the section of
> code that has the mouse caught in the trap (inside a function that
> triggers the hard lockup) -- after all -- that's what the instruction
> is for.

AFAICT, debuggers can set breakpoints on arbitrary code lines without grepping
through assembly language. If you don't have the debug information available,
then using a debugger is pointless anyway.

This is beyond silly. If we follow your argumentation we need another
gazillion of conditional breakpoints in the kernel. Definitely not.

Thanks,

	tglx

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Jeffrey Merkey]

On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 28 Jan 2016, Jeffrey Merkey wrote:
>> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > I'm probably missing something obvious here.
>>
>> It's a pain in the butt to grep around through assembly language in a
>> function in watchdog.c that has everything declared static with no
>> symbols.  It's a lot easier just to insert an INT3 in the section of
>> code that has the mouse caught in the trap (inside a function that
>> triggers the hard lockup) -- after all -- that's what the instruction
>> is for.
>
> AFAICT, debuggers can set breakpoints on arbitrary code lines without
> grepping
> through assembly language. If you don't have the debug information
> available,
> then using a debugger is pointless anyway.
>
> This is beyond silly. If we follow your argumentation we need another
> gazillion of conditional breakpoints in the kernel. Definitely not.
>
> Thanks,
>
> 	tglx
>
>
>

If you don't get it Thomas, I don't know what else to say.  Right now
the only debugger that provides disassembly on a single running live
Linux system is the one I use unless you want to use a serial
connection with kgdb.  All you are convincing me of is that you don't
use a debugger or sit around looking at dissassembly all day long on
live linux systems looking for bugs or you would understand why this
is so helpful.  So I totally understand why you don't get this.

Think of it like git.  Before git was around, everything was done with
manual patches.  Now we have git, and everything can be automated.
Same thing here.  Why do I want to grep around looking for a bug when
I can have linux find it for me.

Jeff

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Thomas Gleixner]

On Thu, 28 Jan 2016, Jeffrey Merkey wrote:
> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
> If you don't get it Thomas, I don't know what else to say.  Right now
> the only debugger that provides disassembly on a single running live
> Linux system is the one I use unless you want to use a serial
> connection with kgdb.  All you are convincing me of is that you don't
> use a debugger or sit around looking at dissassembly all day long on
> live linux systems looking for bugs or you would understand why this
> is so helpful.  So I totally understand why you don't get this.

It does not matter a whit whether I use a debugger or not. It matters that I
can find out without too much hassle where this stupid address is where I want
to set my breakpoint. It's not rocket science.
 
> Think of it like git.  Before git was around, everything was done with
> manual patches.  Now we have git, and everything can be automated.
> Same thing here.  Why do I want to grep around looking for a bug when
> I can have linux find it for me.

Using the proper tools you can figure that address out fully automated without
rumaging in disassembly.

You obviously completely ignored this part of my reply:

> > If we follow your argumentation we need another gazillion of conditional
> > breakpoints in the kernel.

Thanks,

	tglx

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Jeffrey Merkey]

>
> You obviously completely ignored this part of my reply:
>
>> > If we follow your argumentation we need another gazillion of
>> > conditional
>> > breakpoints in the kernel

Actually, I was not suggesting this at all.  But now that you mention
it, there is already a BUG() macro that inserts a a two byte u2DA
instruction instead of a one byte CC (int3) breakpoint instruction
that would be a good candidate for setting int3 breakpoints since the
code is already there and its a macro change build option.

Jeff

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Ingo Molnar]

* Jeffrey Merkey <jeffmerkey@gmail.com> wrote:

> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Thu, 28 Jan 2016, Jeffrey Merkey wrote:
> >> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> > I'm probably missing something obvious here.
> >>
> >> It's a pain in the butt to grep around through assembly language in a 
> >> function in watchdog.c that has everything declared static with no symbols.  
> >> It's a lot easier just to insert an INT3 in the section of code that has the 
> >> mouse caught in the trap (inside a function that triggers the hard lockup) -- 
> >> after all -- that's what the instruction is for.
> >
> > AFAICT, debuggers can set breakpoints on arbitrary code lines without grepping 
> > through assembly language. If you don't have the debug information available, 
> > then using a debugger is pointless anyway.
> >
> > This is beyond silly. If we follow your argumentation we need another 
> > gazillion of conditional breakpoints in the kernel. Definitely not.
> >
> > Thanks,
> >
> > 	tglx
> 
> If you don't get it Thomas, I don't know what else to say. [...]

He provided specific technical arguments:

> > AFAICT, debuggers can set breakpoints on arbitrary code lines without grepping 
> > through assembly language.

Thomas's argument is that live kernel debuggers are already able to insert 
breakpoints just fine, without us having to artificially uglify the source code 
like your patch series does.

... but instead of addressing his technical point (which is perfectly valid), you 
replied with a condescending tone. You are quickly establishing yourself as a 
contributor who is difficult to work with.

As to Thomas's point: on typical distro kernels we at minimum have the kallsyms 
data, but also the System.map in general on packaged kernels. Having function 
symbols is more than enough to start a disassembly from, and the breakpoint can be 
set from there.

If you intentionally and completely throw away all symbol data then debuggability 
decreases drastically. That's nothing new - don't do that. Note that disassembly 
from a live debugger is generally _still_ possible: as function entries are 
usually pretty easy to recognize signatures - and generally there's enough padding 
for cache alignment reasons for even a 'blind' disassembly starting say 1KB before 
the intended breakpoint to actually show correct disassembly.

So I don't see any technical reason to apply your patch-set in that form.

> [...]  Right now the only debugger that provides disassembly on a single running 
> live Linux system is the one I use unless you want to use a serial connection 
> with kgdb. [...]

Given that at least in the x86 space most systems have a real or an emulated 
serial line (the latter via management interfaces), this isn't a big limitation in 
practice.

> [...] All you are convincing me of is that you don't use a debugger or sit 
> around looking at dissassembly all day long on live linux systems looking for 
> bugs or you would understand why this is so helpful.  So I totally understand 
> why you don't get this.

Just for the record, I don't see the point of the many artificial and ugly 
breakpoints either that your series adds, so I'm NAK-ing this intrusive form, 
until better justification is given:

  NAKed-by: Ingo Molnar <mingo@kernel.org>

> Think of it like git.  Before git was around, everything was done with manual 
> patches.  Now we have git, and everything can be automated. Same thing here.  
> Why do I want to grep around looking for a bug when I can have linux find it for 
> me.

Non sequitur: uglifying kernel source code (which has a very real cost for only 
marginal benefit - making it a net negative) has very little to do with the 
utility of Git (which has small cost for a big benefit, which makes it a net 
positive).

Thanks,

	Ingo

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Jeffrey Merkey]

On 1/29/16, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Jeffrey Merkey <jeffmerkey@gmail.com> wrote:
>
>> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Thu, 28 Jan 2016, Jeffrey Merkey wrote:
>> >> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
>> >> > I'm probably missing something obvious here.
>> >>
>> >> It's a pain in the butt to grep around through assembly language in a
>> >> function in watchdog.c that has everything declared static with no
>> >> symbols.
>> >> It's a lot easier just to insert an INT3 in the section of code that
>> >> has the
>> >> mouse caught in the trap (inside a function that triggers the hard
>> >> lockup) --
>> >> after all -- that's what the instruction is for.
>> >
>> > AFAICT, debuggers can set breakpoints on arbitrary code lines without
>> > grepping
>> > through assembly language. If you don't have the debug information
>> > available,
>> > then using a debugger is pointless anyway.
>> >
>> > This is beyond silly. If we follow your argumentation we need another
>> > gazillion of conditional breakpoints in the kernel. Definitely not.
>> >
>> > Thanks,
>> >
>> > 	tglx
>>
>> If you don't get it Thomas, I don't know what else to say. [...]
>
> He provided specific technical arguments:
>
>> > AFAICT, debuggers can set breakpoints on arbitrary code lines without
>> > grepping
>> > through assembly language.
>
> Thomas's argument is that live kernel debuggers are already able to insert
> breakpoints just fine, without us having to artificially uglify the source
> code
> like your patch series does.
>
> ... but instead of addressing his technical point (which is perfectly
> valid), you
> replied with a condescending tone. You are quickly establishing yourself as
> a
> contributor who is difficult to work with.
>
> As to Thomas's point: on typical distro kernels we at minimum have the
> kallsyms
> data, but also the System.map in general on packaged kernels. Having
> function
> symbols is more than enough to start a disassembly from, and the breakpoint
> can be
> set from there.
>
> If you intentionally and completely throw away all symbol data then
> debuggability
> decreases drastically. That's nothing new - don't do that. Note that
> disassembly
> from a live debugger is generally _still_ possible: as function entries are
>
> usually pretty easy to recognize signatures - and generally there's enough
> padding
> for cache alignment reasons for even a 'blind' disassembly starting say 1KB
> before
> the intended breakpoint to actually show correct disassembly.
>
> So I don't see any technical reason to apply your patch-set in that form.
>
>> [...]  Right now the only debugger that provides disassembly on a single
>> running
>> live Linux system is the one I use unless you want to use a serial
>> connection
>> with kgdb. [...]
>
> Given that at least in the x86 space most systems have a real or an emulated
>
> serial line (the latter via management interfaces), this isn't a big
> limitation in
> practice.
>
>> [...] All you are convincing me of is that you don't use a debugger or sit
>>
>> around looking at dissassembly all day long on live linux systems looking
>> for
>> bugs or you would understand why this is so helpful.  So I totally
>> understand
>> why you don't get this.
>
> Just for the record, I don't see the point of the many artificial and ugly
> breakpoints either that your series adds, so I'm NAK-ing this intrusive
> form,
> until better justification is given:
>
>   NAKed-by: Ingo Molnar <mingo@kernel.org>
>
>> Think of it like git.  Before git was around, everything was done with
>> manual
>> patches.  Now we have git, and everything can be automated. Same thing
>> here.
>> Why do I want to grep around looking for a bug when I can have linux find
>> it for
>> me.
>
> Non sequitur: uglifying kernel source code (which has a very real cost for
> only
> marginal benefit - making it a net negative) has very little to do with the
>
> utility of Git (which has small cost for a big benefit, which makes it a net
>
> positive).
>
> Thanks,
>
> 	Ingo
>

You were not included on the post since you are not a maintainer of
watchdog.c so I am confused as to why you are nacking and trolling me
on something not in your area.

Jeff

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Jeffrey Merkey]

On 1/29/16, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Jeffrey Merkey <jeffmerkey@gmail.com> wrote:
>
>> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Thu, 28 Jan 2016, Jeffrey Merkey wrote:
>> >> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
>> >> > I'm probably missing something obvious here.
>> >>
>> >> It's a pain in the butt to grep around through assembly language in a
>> >> function in watchdog.c that has everything declared static with no
>> >> symbols.
>> >> It's a lot easier just to insert an INT3 in the section of code that
>> >> has the
>> >> mouse caught in the trap (inside a function that triggers the hard
>> >> lockup) --
>> >> after all -- that's what the instruction is for.
>> >
>> > AFAICT, debuggers can set breakpoints on arbitrary code lines without
>> > grepping
>> > through assembly language. If you don't have the debug information
>> > available,
>> > then using a debugger is pointless anyway.
>> >
>> > This is beyond silly. If we follow your argumentation we need another
>> > gazillion of conditional breakpoints in the kernel. Definitely not.
>> >
>> > Thanks,
>> >
>> > 	tglx
>>
>> If you don't get it Thomas, I don't know what else to say. [...]
>
> He provided specific technical arguments:
>
>> > AFAICT, debuggers can set breakpoints on arbitrary code lines without
>> > grepping
>> > through assembly language.
>
> Thomas's argument is that live kernel debuggers are already able to insert
> breakpoints just fine, without us having to artificially uglify the source
> code
> like your patch series does.
>
> ... but instead of addressing his technical point (which is perfectly
> valid), you
> replied with a condescending tone. You are quickly establishing yourself as
> a
> contributor who is difficult to work with.
>
> As to Thomas's point: on typical distro kernels we at minimum have the
> kallsyms
> data, but also the System.map in general on packaged kernels. Having
> function
> symbols is more than enough to start a disassembly from, and the breakpoint
> can be
> set from there.
>
> If you intentionally and completely throw away all symbol data then
> debuggability
> decreases drastically. That's nothing new - don't do that. Note that
> disassembly
> from a live debugger is generally _still_ possible: as function entries are
>
> usually pretty easy to recognize signatures - and generally there's enough
> padding
> for cache alignment reasons for even a 'blind' disassembly starting say 1KB
> before
> the intended breakpoint to actually show correct disassembly.
>
> So I don't see any technical reason to apply your patch-set in that form.
>
>> [...]  Right now the only debugger that provides disassembly on a single
>> running
>> live Linux system is the one I use unless you want to use a serial
>> connection
>> with kgdb. [...]
>
> Given that at least in the x86 space most systems have a real or an emulated
>
> serial line (the latter via management interfaces), this isn't a big
> limitation in
> practice.
>
>> [...] All you are convincing me of is that you don't use a debugger or sit
>>
>> around looking at dissassembly all day long on live linux systems looking
>> for
>> bugs or you would understand why this is so helpful.  So I totally
>> understand
>> why you don't get this.
>
> Just for the record, I don't see the point of the many artificial and ugly
> breakpoints either that your series adds, so I'm NAK-ing this intrusive
> form,
> until better justification is given:
>
>   NAKed-by: Ingo Molnar <mingo@kernel.org>
>
>> Think of it like git.  Before git was around, everything was done with
>> manual
>> patches.  Now we have git, and everything can be automated. Same thing
>> here.
>> Why do I want to grep around looking for a bug when I can have linux find
>> it for
>> me.
>
> Non sequitur: uglifying kernel source code (which has a very real cost for
> only
> marginal benefit - making it a net negative) has very little to do with the
>
> utility of Git (which has small cost for a big benefit, which makes it a net
>
> positive).
>
> Thanks,
>
> 	Ingo
>

It's not intrusive to use the BUG() macro to insert a breakpoint
instead of the ud2a instruction to trigger a breakpoint.    All of
thomas technical arguments were addressed to the extent he
communicated any other than political banter.

Jeff

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Jeffrey Merkey]

On 1/29/16, Jeffrey Merkey <jeffmerkey@gmail.com> wrote:
> On 1/29/16, Ingo Molnar <mingo@kernel.org> wrote:
>>
>> * Jeffrey Merkey <jeffmerkey@gmail.com> wrote:
>>
>>> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
>>> > On Thu, 28 Jan 2016, Jeffrey Merkey wrote:
>>> >> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
>>> >> > I'm probably missing something obvious here.
>>> >>
>>> >> It's a pain in the butt to grep around through assembly language in a
>>> >> function in watchdog.c that has everything declared static with no
>>> >> symbols.
>>> >> It's a lot easier just to insert an INT3 in the section of code that
>>> >> has the
>>> >> mouse caught in the trap (inside a function that triggers the hard
>>> >> lockup) --
>>> >> after all -- that's what the instruction is for.
>>> >
>>> > AFAICT, debuggers can set breakpoints on arbitrary code lines without
>>> > grepping
>>> > through assembly language. If you don't have the debug information
>>> > available,
>>> > then using a debugger is pointless anyway.
>>> >
>>> > This is beyond silly. If we follow your argumentation we need another
>>> > gazillion of conditional breakpoints in the kernel. Definitely not.
>>> >
>>> > Thanks,
>>> >
>>> > 	tglx
>>>
>>> If you don't get it Thomas, I don't know what else to say. [...]
>>
>> He provided specific technical arguments:
>>
>>> > AFAICT, debuggers can set breakpoints on arbitrary code lines without
>>> > grepping
>>> > through assembly language.
>>
>> Thomas's argument is that live kernel debuggers are already able to
>> insert
>> breakpoints just fine, without us having to artificially uglify the
>> source
>> code
>> like your patch series does.
>>
>> ... but instead of addressing his technical point (which is perfectly
>> valid), you
>> replied with a condescending tone. You are quickly establishing yourself
>> as
>> a
>> contributor who is difficult to work with.
>>
>> As to Thomas's point: on typical distro kernels we at minimum have the
>> kallsyms
>> data, but also the System.map in general on packaged kernels. Having
>> function
>> symbols is more than enough to start a disassembly from, and the
>> breakpoint
>> can be
>> set from there.
>>
>> If you intentionally and completely throw away all symbol data then
>> debuggability
>> decreases drastically. That's nothing new - don't do that. Note that
>> disassembly
>> from a live debugger is generally _still_ possible: as function entries
>> are
>>
>> usually pretty easy to recognize signatures - and generally there's
>> enough
>> padding
>> for cache alignment reasons for even a 'blind' disassembly starting say
>> 1KB
>> before
>> the intended breakpoint to actually show correct disassembly.
>>
>> So I don't see any technical reason to apply your patch-set in that form.
>>
>>> [...]  Right now the only debugger that provides disassembly on a single
>>> running
>>> live Linux system is the one I use unless you want to use a serial
>>> connection
>>> with kgdb. [...]
>>
>> Given that at least in the x86 space most systems have a real or an
>> emulated
>>
>> serial line (the latter via management interfaces), this isn't a big
>> limitation in
>> practice.
>>
>>> [...] All you are convincing me of is that you don't use a debugger or
>>> sit
>>>
>>> around looking at dissassembly all day long on live linux systems
>>> looking
>>> for
>>> bugs or you would understand why this is so helpful.  So I totally
>>> understand
>>> why you don't get this.
>>
>> Just for the record, I don't see the point of the many artificial and
>> ugly
>> breakpoints either that your series adds, so I'm NAK-ing this intrusive
>> form,
>> until better justification is given:
>>
>>   NAKed-by: Ingo Molnar <mingo@kernel.org>
>>
>>> Think of it like git.  Before git was around, everything was done with
>>> manual
>>> patches.  Now we have git, and everything can be automated. Same thing
>>> here.
>>> Why do I want to grep around looking for a bug when I can have linux
>>> find
>>> it for
>>> me.
>>
>> Non sequitur: uglifying kernel source code (which has a very real cost
>> for
>> only
>> marginal benefit - making it a net negative) has very little to do with
>> the
>>
>> utility of Git (which has small cost for a big benefit, which makes it a
>> net
>>
>> positive).
>>
>> Thanks,
>>
>> 	Ingo
>>
>
> It's not intrusive to use the BUG() macro to insert a breakpoint
> instead of the ud2a instruction to trigger a breakpoint.    All of
> thomas technical arguments were addressed to the extent he
> communicated any other than political banter.
>
> Jeff
>


Sending version 3 with use of the BUG() macro.  Then I don;t have to
touch all these arches and it's totally non-intrusive.

Jeff

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Jeffrey Merkey]

On 1/29/16, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Jeffrey Merkey <jeffmerkey@gmail.com> wrote:
>
>> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Thu, 28 Jan 2016, Jeffrey Merkey wrote:
>> >> On 1/28/16, Thomas Gleixner <tglx@linutronix.de> wrote:
>> >> > I'm probably missing something obvious here.
>> >>
>> >> It's a pain in the butt to grep around through assembly language in a
>> >> function in watchdog.c that has everything declared static with no
>> >> symbols.
>> >> It's a lot easier just to insert an INT3 in the section of code that
>> >> has the
>> >> mouse caught in the trap (inside a function that triggers the hard
>> >> lockup) --
>> >> after all -- that's what the instruction is for.
>> >
>> > AFAICT, debuggers can set breakpoints on arbitrary code lines without
>> > grepping
>> > through assembly language. If you don't have the debug information
>> > available,
>> > then using a debugger is pointless anyway.
>> >
>> > This is beyond silly. If we follow your argumentation we need another
>> > gazillion of conditional breakpoints in the kernel. Definitely not.
>> >
>> > Thanks,
>> >
>> > 	tglx
>>
>> If you don't get it Thomas, I don't know what else to say. [...]
>
> He provided specific technical arguments:
>
>> > AFAICT, debuggers can set breakpoints on arbitrary code lines without
>> > grepping
>> > through assembly language.
>
> Thomas's argument is that live kernel debuggers are already able to insert
> breakpoints just fine, without us having to artificially uglify the source
> code
> like your patch series does.
>

I agree that this patch series is less than ideal.

> ... but instead of addressing his technical point (which is perfectly
> valid), you
> replied with a condescending tone. You are quickly establishing yourself as
> a
> contributor who is difficult to work with.
>

Well, Ingo, I guess we both have articles smeared all over the internet about
how we are hard to deal with.  You have a few, I have a few.  Other people have
them to.  People who make a difference ruffle feathers.  It's the
nature of change.
The only person that likes change is a wet baby.

> As to Thomas's point: on typical distro kernels we at minimum have the
> kallsyms
> data, but also the System.map in general on packaged kernels. Having
> function
> symbols is more than enough to start a disassembly from, and the breakpoint
> can be
> set from there.
>

Well, I can certainly do all that, all I was suggesting was let linux
find the bugs
for you and pop into a debugger if one is active.

> If you intentionally and completely throw away all symbol data then
> debuggability
> decreases drastically. That's nothing new - don't do that. Note that
> disassembly
> from a live debugger is generally _still_ possible: as function entries are
>
> usually pretty easy to recognize signatures - and generally there's enough
> padding
> for cache alignment reasons for even a 'blind' disassembly starting say 1KB
> before
> the intended breakpoint to actually show correct disassembly.
>
> So I don't see any technical reason to apply your patch-set in that form.
>

I would agree that something more elegant is needed.

>> [...]  Right now the only debugger that provides disassembly on a single
>> running
>> live Linux system is the one I use unless you want to use a serial
>> connection
>> with kgdb. [...]
>
> Given that at least in the x86 space most systems have a real or an emulated
>
> serial line (the latter via management interfaces), this isn't a big
> limitation in
> practice.
>

A live debugger on actual hardware is a far cry from a simulated UML/KVM/QEMU
style environment.  It's just not the same thing.  And I use kgdb with
a virtual
serial connection, but GDB has got to be one of the most user hostile interfaces
on the planet.  It's plain hard to use with long commands and piss
poor command recall.

>> [...] All you are convincing me of is that you don't use a debugger or sit
>>
>> around looking at dissassembly all day long on live linux systems looking
>> for
>> bugs or you would understand why this is so helpful.  So I totally
>> understand
>> why you don't get this.
>
> Just for the record, I don't see the point of the many artificial and ugly
> breakpoints either that your series adds, so I'm NAK-ing this intrusive
> form,
> until better justification is given:

How about I go off and refine this idea and submit something later.

>
>   NAKed-by: Ingo Molnar <mingo@kernel.org>
>
>> Think of it like git.  Before git was around, everything was done with
>> manual
>> patches.  Now we have git, and everything can be automated. Same thing
>> here.
>> Why do I want to grep around looking for a bug when I can have linux find
>> it for
>> me.
>
> Non sequitur: uglifying kernel source code (which has a very real cost for
> only
> marginal benefit - making it a net negative) has very little to do with the
>
> utility of Git (which has small cost for a big benefit, which makes it a net
>
> positive).

There are thousands of BUG(), WARN(), BUG_ON() macros uglifying the code
already.    BFD.

Jeff

>
> Thanks,
>
> 	Ingo
>

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]

Hi Jeffrey,

[auto build test ERROR on v4.5-rc1]
[also build test ERROR on next-20160128]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Jeffrey-Merkey/Add-hard-soft-lockup-debugger-entry-points/20160129-035852
config: alpha-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=alpha 

All errors (new ones prefixed by >>):

   kernel/watchdog.c: In function 'watchdog_timer_fn':
>> kernel/watchdog.c:489:4: error: implicit declaration of function 'arch_breakpoint' [-Werror=implicit-function-declaration]
       arch_breakpoint();
       ^
   cc1: some warnings being treated as errors

vim +/arch_breakpoint +489 kernel/watchdog.c

   483			if (regs)
   484				show_regs(regs);
   485			else
   486				dump_stack();
   487	
   488			if (debug_watchdog_lockups)
 > 489				arch_breakpoint();
   490	
   491			if (softlockup_all_cpu_backtrace) {
   492				/* Avoid generating two back traces for current

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 45062 bytes --]

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Chris Metcalf]

On 01/28/2016 02:46 PM, Jeffrey Merkey wrote:
> This patch series adds an export which can be set by system debuggers to
> direct the hard lockup and soft lockup detector to trigger a breakpoint
> exception and enter a debugger if one is active.  It is assumed that if
> someone sets this variable, then an breakpoint handler of some sort will
> be actively loaded or registered via the notify die handler chain.
>
> This addition is extremely useful for debugging hard and soft lockups
> real time and quickly from a console debugger.

I'm concerned that you are duplicating the breakpoint instructions
for all the platforms.  Could you make kgdb.h include kdebug.h and
just move the arch_kgdb_breakpoint() implementations in kgdb.h to
arch_breakpoint() in kdebug.h?  Then each platform can just put an
appropriate define in kgdb.h, e.g. "#define arch_kgdb_breakpoint
arch_breakpoint", unless (like mips) they have a more complicated
requirement.

I'm concerned that in some cases (e.g. arm64) there is a perfectly good
breakpoint defined in kgdb.h but you are providing a no-op in kdebug.h.
You should probably do another check across all the architectures for
this case.

You should probably add your no-op implementation of arch_breakpoint()
to asm-generic/kdebug.h, and then add "generic-y" lines to the Kbuild
files for the architectures that you are creating new empty files for.
I'm a little ambivalent about the "silent no-op" implementation, but I'm
not really sure there's a better option.

For mips, I'm pretty sure you don't want to create a global "breakinst"
symbol every time you insert a breakpoint into code.  I think this is an
example of where you need to have a different implementation of
arch_breakpoint() and arch_kgdb_breakpoint(), since mips makes its
breakpoint magical by knowing what the address used for that specific
instruction is, and you can't do that for arch_breakpoint().

As a general rule, you probably want to provide header guards in new
headers that you create, but if you just use asm-generic instead, it
actually won't matter for this case.

I should add that I didn't do a thorough review of the patch series,
just a quick skim of a few of the architectures.

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com

Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

[Jeffrey Merkey]

On 1/28/16, Chris Metcalf <cmetcalf@ezchip.com> wrote:
> On 01/28/2016 02:46 PM, Jeffrey Merkey wrote:
>> This patch series adds an export which can be set by system debuggers to
>> direct the hard lockup and soft lockup detector to trigger a breakpoint
>> exception and enter a debugger if one is active.  It is assumed that if
>> someone sets this variable, then an breakpoint handler of some sort will
>> be actively loaded or registered via the notify die handler chain.
>>
>> This addition is extremely useful for debugging hard and soft lockups
>> real time and quickly from a console debugger.
>
> I'm concerned that you are duplicating the breakpoint instructions
> for all the platforms.  Could you make kgdb.h include kdebug.h and
> just move the arch_kgdb_breakpoint() implementations in kgdb.h to
> arch_breakpoint() in kdebug.h?  Then each platform can just put an
> appropriate define in kgdb.h, e.g. "#define arch_kgdb_breakpoint
> arch_breakpoint", unless (like mips) they have a more complicated
> requirement.
>
> I'm concerned that in some cases (e.g. arm64) there is a perfectly good
> breakpoint defined in kgdb.h but you are providing a no-op in kdebug.h.
> You should probably do another check across all the architectures for
> this case.
>
> You should probably add your no-op implementation of arch_breakpoint()
> to asm-generic/kdebug.h, and then add "generic-y" lines to the Kbuild
> files for the architectures that you are creating new empty files for.
> I'm a little ambivalent about the "silent no-op" implementation, but I'm
> not really sure there's a better option.
>
> For mips, I'm pretty sure you don't want to create a global "breakinst"
> symbol every time you insert a breakpoint into code.  I think this is an
> example of where you need to have a different implementation of
> arch_breakpoint() and arch_kgdb_breakpoint(), since mips makes its
> breakpoint magical by knowing what the address used for that specific
> instruction is, and you can't do that for arch_breakpoint().
>
> As a general rule, you probably want to provide header guards in new
> headers that you create, but if you just use asm-generic instead, it
> actually won't matter for this case.
>
> I should add that I didn't do a thorough review of the patch series,
> just a quick skim of a few of the architectures.
>
> --
> Chris Metcalf, EZChip Semiconductor
> http://www.ezchip.com
>
>

todo:

* Adding header guards (if file does not exist)
* reimplemeting arch_breakpoint
* move arch_breakpoint to asm-generic/kdebug.h
* implement as a macro if possible
* study MIPS breakpoint instruction.  Ask for help is not sure.
* fix syntax for INT3 instruction for x86 to use proper gas semantics
* fix kbuild test robot error for SPARC (move to asm-generic fixes)

List so far.

Jeff