Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1323627 > unrolled thread

[PATCH v4 4/4] pr_emerg add WARN_XX() debugger options

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 4/4] pr_emerg add WARN_XX() debugger options Jeffrey Merkey <jeffmerkey@gmail.com> - 2016-02-02 00:40 +0100
    Re: [PATCH v4 4/4] pr_emerg add WARN_XX() debugger options Jeffrey Merkey <jeffmerkey@gmail.com> - 2016-02-02 03:10 +0100
      Re: [PATCH v4 4/4] pr_emerg add WARN_XX() debugger options "Maciej W. Rozycki" <macro@linux-mips.org> - 2016-02-02 20:50 +0100
        Re: [PATCH v4 4/4] pr_emerg add WARN_XX() debugger options Jeffrey Merkey <jeffmerkey@gmail.com> - 2016-02-02 23:20 +0100
    Re: [PATCH v4 4/4] pr_emerg add WARN_XX() debugger options Steven Rostedt <rostedt@goodmis.org> - 2016-02-02 03:10 +0100

#1323627 — [PATCH v4 4/4] pr_emerg add WARN_XX() debugger options

This patch series adds config options which can be set during compile to
direct the compiler to output a breakpoint instruction anywhere a BUG()
or WARN() macro has been placed in the kernel to trigger the system to
enter a debugger if a bug is detected by the system.  Use of this
compile time option also allows conditional breakpoints to be set in the
kernel with these currently used macros.

This addition is extremely useful for debugging hard and soft lockups
real time and quickly from a console debugger, and other areas of the
kernel.

Signed-off-by: Jeffrey Merkey <jeffmerkey@gmail.com>
---
 include/linux/printk.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 51dd6b8..dcfb270 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -252,8 +252,16 @@ extern asmlinkage void dump_stack(void) __cold;
  * and other debug macros are compiled out unless either DEBUG is defined
  * or CONFIG_DYNAMIC_DEBUG is set.
  */
+#ifdef CONFIG_DEBUG_WARN
+#define pr_emerg(fmt, ...) \
+({								\
+	printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__);		\
+	BUG();							\
+})
+#else
 #define pr_emerg(fmt, ...) \
 	printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
+#endif
 #define pr_alert(fmt, ...) \
 	printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_crit(fmt, ...) \
-- 
1.8.3.1

[toc] | [next] | [standalone]

#1323700

On 2/1/16, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon,  1 Feb 2016 16:35:52 -0700
> Jeffrey Merkey <jeffmerkey@gmail.com> wrote:
>> ---
>>  include/linux/printk.h | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/include/linux/printk.h b/include/linux/printk.h
>> index 51dd6b8..dcfb270 100644
>> --- a/include/linux/printk.h
>> +++ b/include/linux/printk.h
>> @@ -252,8 +252,16 @@ extern asmlinkage void dump_stack(void) __cold;
>>   * and other debug macros are compiled out unless either DEBUG is
>> defined
>>   * or CONFIG_DYNAMIC_DEBUG is set.
>>   */
>> +#ifdef CONFIG_DEBUG_WARN
>> +#define pr_emerg(fmt, ...) \
>> +({								\
>> +	printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__);		\
>> +	BUG();							\
>
> This look rather heavy handed for a debug feature. This will crash any
> kernel on a pr-emerg(), similar to a panic on warning. Not only that,
> for cases that cal pr_emerg() more than once, it crashes on the first
> instance.
>
> What about adding a condition that can be set set in /proc/sys/kernel/
> A file called something like crash_on_print_emerg ?
>
> 	BUG_ON(crash_on_pr_emerg);
>
> -- Steve
>

If a debugger is loaded it will not crash, just enter the debugger.
But yes, it will int3 if set and no debugger has been loaded to handle
the int3 condition.  Hmmm.  Maybe its better just to skip calling
pr_emerg and put this logic as a single call somewhere else.

Jeff

[toc] | [prev] | [next] | [standalone]

#1324569

On Mon, 1 Feb 2016, Jeffrey Merkey wrote:

> If a debugger is loaded it will not crash, just enter the debugger.
> But yes, it will int3 if set and no debugger has been loaded to handle
> the int3 condition.  Hmmm.  Maybe its better just to skip calling
> pr_emerg and put this logic as a single call somewhere else.

 What's the point?  If you have a debugger loaded, then surely you can 
just set a breakpoint anywhere you like using whatever user interface the 
debugger provides for setting breakpoints.  You can actually set any 
number of software breakpoints you like wherever you like, depending on 
what you actually want to debug.  I fail to see why it would have to be 
prearranged within the kernel -- do you have a configuration where you run 
the kernel from ROM by any chance?

  Maciej

[toc] | [prev] | [next] | [standalone]

#1324688

On 2/2/16, Maciej W. Rozycki <macro@linux-mips.org> wrote:
> On Mon, 1 Feb 2016, Jeffrey Merkey wrote:
>
>> If a debugger is loaded it will not crash, just enter the debugger.
>> But yes, it will int3 if set and no debugger has been loaded to handle
>> the int3 condition.  Hmmm.  Maybe its better just to skip calling
>> pr_emerg and put this logic as a single call somewhere else.
>
>  What's the point?  If you have a debugger loaded, then surely you can
> just set a breakpoint anywhere you like using whatever user interface the
> debugger provides for setting breakpoints.  You can actually set any
> number of software breakpoints you like wherever you like, depending on
> what you actually want to debug.  I fail to see why it would have to be
> prearranged within the kernel -- do you have a configuration where you run
> the kernel from ROM by any chance?
>
>   Maciej
>

No ROM here.  I resubmitted this as series 5 and dropped the change to
pr_emerg for the very reasons you stated.  I verified what you said
was correct by reviewing several code paths and its not a good place
for that.  Folks expect this function to act like a printk on
steroids, not as a terminal placement of a BUG().

Jeff

[toc] | [prev] | [next] | [standalone]

#1323701

On Mon,  1 Feb 2016 16:35:52 -0700
Jeffrey Merkey <jeffmerkey@gmail.com> wrote:
> ---
>  include/linux/printk.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index 51dd6b8..dcfb270 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -252,8 +252,16 @@ extern asmlinkage void dump_stack(void) __cold;
>   * and other debug macros are compiled out unless either DEBUG is defined
>   * or CONFIG_DYNAMIC_DEBUG is set.
>   */
> +#ifdef CONFIG_DEBUG_WARN
> +#define pr_emerg(fmt, ...) \
> +({								\
> +	printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__);		\
> +	BUG();							\

This look rather heavy handed for a debug feature. This will crash any
kernel on a pr-emerg(), similar to a panic on warning. Not only that,
for cases that cal pr_emerg() more than once, it crashes on the first
instance.

What about adding a condition that can be set set in /proc/sys/kernel/
A file called something like crash_on_print_emerg ?

	BUG_ON(crash_on_pr_emerg);

-- Steve

> +})
> +#else
>  #define pr_emerg(fmt, ...) \
>  	printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
> +#endif
>  #define pr_alert(fmt, ...) \
>  	printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
>  #define pr_crit(fmt, ...) \

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web