[PATCH v2 0/2] printk: Add printk.console_verbose boot param

[PATCH v2 0/2] printk: Add printk.console_verbose boot param

[Dmitry Safonov]

v1 to v2 Changes:
- Add printk.console_verbose boot parameter instead of compile-time
  CONFIG_CONSOLE_LOGLEVEL_PANIC (see v1 discussion with Petr)
- I didn't rename console_verbose() to console_verbose_panic() as
  I need it to be always disabled regardless oops/panic/lockdep.
- I noticed console_silent() which is unused for long time, remove it.

v1: https://lore.kernel.org/lkml/20210622143350.1105701-1-dima@arista.com/

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Steven Rostedt <rostedt@goodmis.org>

Dmitry Safonov (2):
  printk: Remove console_silent()
  printk: Add printk.console_verbose boot parameter

 Documentation/admin-guide/kernel-parameters.txt | 9 +++++++++
 include/linux/printk.h                          | 7 ++-----
 kernel/printk/printk.c                          | 6 ++++++
 3 files changed, 17 insertions(+), 5 deletions(-)


base-commit: e73f0f0ee7541171d89f2e2491130c7771ba58d3
-- 
2.32.0

[PATCH v2 1/2] printk: Remove console_silent()

[Dmitry Safonov]

It' unused since removal of mn10300:
commit 739d875dd698 ("mn10300: Remove the architecture")
x86 stopped using it in v2.6.12 (see history git):
commit 7574828b3dbb ("[PATCH] x86_64: add nmi button support")

Let's clean it up from the header.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 include/linux/printk.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index e834d78f0478..a63f468a8239 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -70,11 +70,6 @@ extern int console_printk[];
 #define minimum_console_loglevel (console_printk[2])
 #define default_console_loglevel (console_printk[3])
 
-static inline void console_silent(void)
-{
-	console_loglevel = CONSOLE_LOGLEVEL_SILENT;
-}
-
 static inline void console_verbose(void)
 {
 	if (console_loglevel)
-- 
2.32.0

Re: [PATCH v2 1/2] printk: Remove console_silent()

[Petr Mladek]

On Tue 2021-07-13 02:15:10, Dmitry Safonov wrote:
> It' unused since removal of mn10300:
> commit 739d875dd698 ("mn10300: Remove the architecture")
> x86 stopped using it in v2.6.12 (see history git):
> commit 7574828b3dbb ("[PATCH] x86_64: add nmi button support")
> 
> Let's clean it up from the header.
> 
> Signed-off-by: Dmitry Safonov <dima@arista.com>

Reviewed-by: Petr Mladek <pmladek@suse.com>

I thought about this several times but never made a patch ;-)

Best Regards,
Petr

[PATCH v2 2/2] printk: Add printk.console_verbose boot parameter

[Dmitry Safonov]

console_verbose() increases console loglevel to CONSOLE_LOGLEVEL_MOTORMOUTH,
which provides more information to debug a panic/oops.

Unfortunately, in Arista we maintain some DUTs (Device Under Test) that
are configured to have 9600 baud rate. While verbose console messages
have their value to post-analyze crashes, on such setup they:
- may prevent panic/oops messages being printed
- take too long to flush on console resulting in watchdog reboot

In all our setups we use kdump which saves dmesg buffer after panic,
so in reality those extra messages on console provide no additional value,
but rather add risk of not getting to __crash_kexec().

Provide printk.console_verbose boot parameter, which allows to switch off
printk being verbose on oops/panic/lockdep (making it boot parameter
instead of compile-option suggested-by Petr).

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 9 +++++++++
 include/linux/printk.h                          | 4 +++-
 kernel/printk/printk.c                          | 6 ++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index bdb22006f713..9fae19b1edd8 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4167,6 +4167,15 @@
 			Format: <bool>  (1/Y/y=enable, 0/N/n=disable)
 			default: disabled
 
+	printk.console_verbose=
+			Raise console loglevel to highest on oops, panic or
+			lockdep-detected issues (only if lock debug is on).
+			With an exception to setups with low baudrate on
+			serial console, keeping this enabled is a good choice
+			in order to provide more debug information.
+			Format: <bool>  (1/Y/y=enable, 0/N/n=disable)
+			default: enabled
+
 	printk.devkmsg={on,off,ratelimit}
 			Control writing to /dev/kmsg.
 			on - unlimited logging to /dev/kmsg from userspace
diff --git a/include/linux/printk.h b/include/linux/printk.h
index a63f468a8239..9d0b8133a03c 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -70,9 +70,11 @@ extern int console_printk[];
 #define minimum_console_loglevel (console_printk[2])
 #define default_console_loglevel (console_printk[3])
 
+extern bool printk_console_verbose;
+
 static inline void console_verbose(void)
 {
-	if (console_loglevel)
+	if (console_loglevel && printk_console_verbose)
 		console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH;
 }
 
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 142a58d124d9..e321ee78855d 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2404,6 +2404,12 @@ module_param_named(console_suspend, console_suspend_enabled,
 MODULE_PARM_DESC(console_suspend, "suspend console during suspend"
 	" and hibernate operations");
 
+bool printk_console_verbose = true;
+EXPORT_SYMBOL(printk_console_verbose);
+
+module_param_named(console_verbose, printk_console_verbose, bool, 0644);
+MODULE_PARM_DESC(console_verbose, "Raise console loglevel to highest on oops/panic/etc");
+
 /**
  * suspend_console - suspend the console subsystem
  *
-- 
2.32.0

Re: [PATCH v2 2/2] printk: Add printk.console_verbose boot parameter

[Petr Mladek]

On Tue 2021-07-13 02:15:11, Dmitry Safonov wrote:
> console_verbose() increases console loglevel to CONSOLE_LOGLEVEL_MOTORMOUTH,
> which provides more information to debug a panic/oops.
> 
> Unfortunately, in Arista we maintain some DUTs (Device Under Test) that
> are configured to have 9600 baud rate. While verbose console messages
> have their value to post-analyze crashes, on such setup they:
> - may prevent panic/oops messages being printed
> - take too long to flush on console resulting in watchdog reboot
> 
> In all our setups we use kdump which saves dmesg buffer after panic,
> so in reality those extra messages on console provide no additional value,
> but rather add risk of not getting to __crash_kexec().

Yup, it makes sense.

> Provide printk.console_verbose boot parameter, which allows to switch off
> printk being verbose on oops/panic/lockdep (making it boot parameter
> instead of compile-option suggested-by Petr).
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: John Ogness <john.ogness@linutronix.de>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 9 +++++++++
>  include/linux/printk.h                          | 4 +++-
>  kernel/printk/printk.c                          | 6 ++++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index bdb22006f713..9fae19b1edd8 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4167,6 +4167,15 @@
>  			Format: <bool>  (1/Y/y=enable, 0/N/n=disable)
>  			default: disabled
>  
> +	printk.console_verbose=
> +			Raise console loglevel to highest on oops, panic or
> +			lockdep-detected issues (only if lock debug is on).
> +			With an exception to setups with low baudrate on
> +			serial console, keeping this enabled is a good choice
> +			in order to provide more debug information.
> +			Format: <bool>  (1/Y/y=enable, 0/N/n=disable)
> +			default: enabled

Hmm, the name suggests that the console should always be verbose.
It looks like a counterpart to the "quiet" option.

It actually is a counter part to the existing "quiet" option
except that it triggers in some situations only.

Hence, I would call it "no_auto_verbose":

   + "verbose" follows the simple naming scheme of the existing
     "quiet" option (no "printk" and no "console" in the name)

   + "no_auto" suggests that it disables some auto-verbose behavior
     which is exactly what it does.

Any better idea?

> +
>  	printk.devkmsg={on,off,ratelimit}
>  			Control writing to /dev/kmsg.
>  			on - unlimited logging to /dev/kmsg from userspace
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index a63f468a8239..9d0b8133a03c 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -70,9 +70,11 @@ extern int console_printk[];
>  #define minimum_console_loglevel (console_printk[2])
>  #define default_console_loglevel (console_printk[3])
>  
> +extern bool printk_console_verbose;
> +
>  static inline void console_verbose(void)
>  {
> -	if (console_loglevel)
> +	if (console_loglevel && printk_console_verbose)
>  		console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH;
>  }
>  
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 142a58d124d9..e321ee78855d 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2404,6 +2404,12 @@ module_param_named(console_suspend, console_suspend_enabled,
>  MODULE_PARM_DESC(console_suspend, "suspend console during suspend"
>  	" and hibernate operations");
>  
> +bool printk_console_verbose = true;

I would call it "console_auto_verbose".

> +EXPORT_SYMBOL(printk_console_verbose);

I would prefer to move console_verbose() into printk.c
and export the function instead of this variable.

> +module_param_named(console_verbose, printk_console_verbose, bool, 0644);
> +MODULE_PARM_DESC(console_verbose, "Raise console loglevel to highest on oops/panic/etc");

Best Regards,
Petr

Re: [PATCH v2 2/2] printk: Add printk.console_verbose boot parameter

[Dmitry Safonov]

On 7/21/21 3:46 PM, Petr Mladek wrote:
[..]
>> +	printk.console_verbose=
>> +			Raise console loglevel to highest on oops, panic or
>> +			lockdep-detected issues (only if lock debug is on).
>> +			With an exception to setups with low baudrate on
>> +			serial console, keeping this enabled is a good choice
>> +			in order to provide more debug information.
>> +			Format: <bool>  (1/Y/y=enable, 0/N/n=disable)
>> +			default: enabled
> 
> Hmm, the name suggests that the console should always be verbose.
> It looks like a counterpart to the "quiet" option.
> 
> It actually is a counter part to the existing "quiet" option
> except that it triggers in some situations only.
> 
> Hence, I would call it "no_auto_verbose":
> 
>    + "verbose" follows the simple naming scheme of the existing
>      "quiet" option (no "printk" and no "console" in the name)
> 
>    + "no_auto" suggests that it disables some auto-verbose behavior
>      which is exactly what it does.
> 
> Any better idea?

Yeah, ok. I've tried to avoid negative as a parameter as it sometimes
may be confusing. But I see you have this naming in mind and no hard
feelings from my side - I'll call it "no_auto_verbose" in v3.

[..]
>> @@ -2404,6 +2404,12 @@ module_param_named(console_suspend, console_suspend_enabled,
>>  MODULE_PARM_DESC(console_suspend, "suspend console during suspend"
>>  	" and hibernate operations");
>>  
>> +bool printk_console_verbose = true;
> 
> I would call it "console_auto_verbose".
> 
>> +EXPORT_SYMBOL(printk_console_verbose);
> 
> I would prefer to move console_verbose() into printk.c
> and export the function instead of this variable.

Makes sense, will do in v3.

Thanks,
          Dmitry

Re: [PATCH v2 2/2] printk: Add printk.console_verbose boot parameter

[Petr Mladek]

On Tue 2021-07-13 02:15:11, Dmitry Safonov wrote:
> console_verbose() increases console loglevel to CONSOLE_LOGLEVEL_MOTORMOUTH,
> which provides more information to debug a panic/oops.
> 
> Unfortunately, in Arista we maintain some DUTs (Device Under Test) that
> are configured to have 9600 baud rate. While verbose console messages
> have their value to post-analyze crashes, on such setup they:
> - may prevent panic/oops messages being printed
> - take too long to flush on console resulting in watchdog reboot
> 
> In all our setups we use kdump which saves dmesg buffer after panic,
> so in reality those extra messages on console provide no additional value,
> but rather add risk of not getting to __crash_kexec().
> 
> Provide printk.console_verbose boot parameter, which allows to switch off
> printk being verbose on oops/panic/lockdep (making it boot parameter
> instead of compile-option suggested-by Petr).
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: John Ogness <john.ogness@linutronix.de>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Dmitry Safonov <dima@arista.com>

Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH v2 0/2] printk: Add printk.console_verbose boot param

[Petr Mladek]

On Tue 2021-07-13 02:15:09, Dmitry Safonov wrote:
> v1 to v2 Changes:
> - Add printk.console_verbose boot parameter instead of compile-time
>   CONFIG_CONSOLE_LOGLEVEL_PANIC (see v1 discussion with Petr)
> - I didn't rename console_verbose() to console_verbose_panic() as
>   I need it to be always disabled regardless oops/panic/lockdep.
> - I noticed console_silent() which is unused for long time, remove it.
> 
> v1: https://lore.kernel.org/lkml/20210622143350.1105701-1-dima@arista.com/
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: John Ogness <john.ogness@linutronix.de>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> 
> Dmitry Safonov (2):
>   printk: Remove console_silent()
>   printk: Add printk.console_verbose boot parameter

The patchset is comitted in printk/linux.git, branch
for-5.15-verbose-console.

Best Regards,
Petr