printk: Use symbolic defines for console loglevels
Subsystems (6)
Borislav Petkov
maintainerPATCH 1/1 printk: Use symbolic defines for console loglevels
From: Borislav Petkov <[email protected]>
... instead of naked numbers.
Stuff in sysrq.c used to set it to 8 which is supposed to mean above default level so set it to DEBUG instead as we're terminating/killing all tasks and we want to be verbose there.
There should be no functionality change resulting from this patch.
--- arch/x86/kernel/head64.c | 2 +- arch/x86/platform/uv/uv_nmi.c | 2 +- drivers/nubus/nubus.c | 18 +++++++++--------- drivers/tty/sysrq.c | 8 ++++---- include/linux/printk.h | 15 +++++++++++++-- init/main.c | 4 ++-- kernel/debug/kdb/kdb_bt.c | 2 +- kernel/debug/kdb/kdb_io.c | 2 +- kernel/debug/kdb/kdb_main.c | 2 +- kernel/printk/printk.c | 13 +++---------- 10 files changed, 36 insertions(+), 32 deletions(-)
arch/x86/kernel/head64.c +1 -1
@@ -172,7 +172,7 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)*/ load_ucode_bsp (); -if (console_loglevel ==10 ) +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG )early_printk ("Kernel alive\n" );clear_page (init_level4_pgt);
arch/x86/platform/uv/uv_nmi.c +1 -1
@@ -85,7 +85,7 @@ static cpumask_var_t uv_nmi_cpu_mask;* Default is all stack dumps go to the console and buffer. * Lower level to send to log buffer only. */ -static int uv_nmi_loglevel =7 ; +static int uv_nmi_loglevel =CONSOLE_LOGLEVEL_DEFAULT ;module_param_named (dump_loglevel, uv_nmi_loglevel, int,0644 );/*
drivers/nubus/nubus.c +9 -9
@@ -473,7 +473,7 @@ static struct nubus_dev* __initif (slot ==0 && (unsigned long )dir.base %2 ) dir.base +=1 ; -if (console_loglevel >=10 ) +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG )printk (KERN_DEBUG "nubus_get_functional_resource: parent is 0x%p, dir is 0x%p\n" , parent->base, dir.base);
@@ -568,7 +568,7 @@ static int __init nubus_get_vidnames(struct nubus_board* board,printk (KERN_INFO " video modes supported:\n" );nubus_get_subdir (parent, &dir); -if (console_loglevel >=10 ) +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG )printk (KERN_DEBUG "nubus_get_vidnames: parent is 0x%p, dir is 0x%p\n" , parent->base, dir.base);
@@ -629,7 +629,7 @@ static int __init nubus_get_vendorinfo(struct nubus_board* board,printk (KERN_INFO " vendor info:\n" );nubus_get_subdir (parent, &dir); -if (console_loglevel >=10 ) +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG )printk (KERN_DEBUG "nubus_get_vendorinfo: parent is 0x%p, dir is 0x%p\n" , parent->base, dir.base);
@@ -654,7 +654,7 @@ static int __init nubus_get_board_resource(struct nubus_board* board, int slot,struct nubus_dirent ent;nubus_get_subdir (parent, &dir); -if (console_loglevel >=10 ) +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG )printk (KERN_DEBUG "nubus_get_board_resource: parent is 0x%p, dir is 0x%p\n" , parent->base, dir.base);
@@ -753,19 +753,19 @@ static void __init nubus_find_rom_dir(struct nubus_board* board)if (nubus_readdir (&dir, &ent) ==-1 ) goto badrom; -if (console_loglevel >=10 ) +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG )printk (KERN_INFO "nubus_get_rom_dir: entry %02x %06x\n" , ent.type, ent.data);/* This one takes us to where we want to go. */ if (nubus_readdir (&dir, &ent) ==-1 ) goto badrom; -if (console_loglevel >=10 ) +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG )printk (KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n" , ent.type, ent.data);nubus_get_subdir (&ent, &dir);/* Resource ID 01, also an "Unknown Macintosh" */ if (nubus_readdir (&dir, &ent) ==-1 ) goto badrom; -if (console_loglevel >=10 ) +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG )printk (KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n" , ent.type, ent.data);/* FIXME: the first one is *not* always the right one. We
@@ -780,7 +780,7 @@ static void __init nubus_find_rom_dir(struct nubus_board* board)path to that address... */ if (nubus_readdir (&dir, &ent) ==-1 ) goto badrom; -if (console_loglevel >=10 ) +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG )printk (KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n" , ent.type, ent.data);/* Bwahahahaha... */
@@ -816,7 +816,7 @@ static struct nubus_board* __init nubus_add_board(int slot, int bytelanes) board->fblock = rp;/* Dump the format block for debugging purposes */ -if (console_loglevel >=10 ) { +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG ) {int i;printk (KERN_DEBUG "Slot %X, format block at 0x%p\n" , slot, rp);
drivers/tty/sysrq.c +4 -4
@@ -88,7 +88,7 @@ static void sysrq_handle_loglevel(int key)int i; i = key -'0' ; - console_loglevel =7 ; + console_loglevel =CONSOLE_LOGLEVEL_DEFAULT ;printk ("Loglevel set to %d\n" , i); console_loglevel = i; }
@@ -343,7 +343,7 @@ static void send_sig_all(int sig)static void sysrq_handle_term (int key) {send_sig_all (SIGTERM ); - console_loglevel =8 ; + console_loglevel =CONSOLE_LOGLEVEL_DEBUG ; }static struct sysrq_key_op sysrq_term_op = { .handler = sysrq_handle_term,
@@ -387,7 +387,7 @@ static struct sysrq_key_op sysrq_thaw_op = {static void sysrq_handle_kill (int key) {send_sig_all (SIGKILL ); - console_loglevel =8 ; + console_loglevel =CONSOLE_LOGLEVEL_DEBUG ; }static struct sysrq_key_op sysrq_kill_op = { .handler = sysrq_handle_kill,
@@ -520,7 +520,7 @@ void __handle_sysrq(int key, bool check_mask)* routing in the consumers of /proc/kmsg. */ orig_log_level = console_loglevel; - console_loglevel =7 ; + console_loglevel =CONSOLE_LOGLEVEL_DEFAULT ;printk (KERN_INFO "SysRq : " ); op_p =__sysrq_get_key_op (key);
include/linux/printk.h +13 -2
@@ -30,6 +30,17 @@ static inline const char *printk_skip_level(const char *buffer)return buffer; } +/* printk's without a loglevel use this.. */ +#define DEFAULT_MESSAGE_LOGLEVEL CONFIG_DEFAULT_MESSAGE_LOGLEVEL + +/* We show everything that is MORE important than this.. */ +#define CONSOLE_LOGLEVEL_SILENT 0/* Mum's the word */ +#define CONSOLE_LOGLEVEL_MIN 1/* Minimum loglevel we let people use */ +#define CONSOLE_LOGLEVEL_QUIET 4/* Shhh ..., when booted with "quiet" */ +#define CONSOLE_LOGLEVEL_DEFAULT 7/* anything MORE serious than KERN_DEBUG */ +#define CONSOLE_LOGLEVEL_DEBUG 10/* issue debug messages */ +#define CONSOLE_LOGLEVEL_MOTORMOUTH 15/* You can't shut this one up */ +extern int console_printk[];#define console_loglevel (console_printk[0])
@@ -39,13 +50,13 @@ extern int console_printk[];static inline void console_silent (void ) { - console_loglevel =0 ; + console_loglevel =CONSOLE_LOGLEVEL_SILENT ; }static inline void console_verbose (void ) {if (console_loglevel) - console_loglevel =15 ; + console_loglevel =CONSOLE_LOGLEVEL_MOTORMOUTH ; }struct va_format {
init/main.c +2 -2
@@ -203,13 +203,13 @@ EXPORT_SYMBOL(loops_per_jiffy);static int __init debug_kernel (char *str) { - console_loglevel =10 ; + console_loglevel =CONSOLE_LOGLEVEL_DEBUG ;return 0 ; }static int __init quiet_kernel (char *str) { - console_loglevel =4 ; + console_loglevel =CONSOLE_LOGLEVEL_QUIET ;return 0 ; }
kernel/debug/kdb/kdb_bt.c +1 -1
@@ -21,7 +21,7 @@static void kdb_show_stack (struct task_struct *p,void *addr) {int old_lvl = console_loglevel; - console_loglevel =15 ; + console_loglevel =CONSOLE_LOGLEVEL_MOTORMOUTH ; kdb_trap_printk++;kdb_set_current_task (p);if (addr) {
kernel/debug/kdb/kdb_io.c +1 -1
@@ -710,7 +710,7 @@ kdb_printit: }if (logging) { saved_loglevel = console_loglevel; - console_loglevel =0 ; + console_loglevel =CONSOLE_LOGLEVEL_SILENT ;printk (KERN_INFO "%s" , kdb_buffer); }
kernel/debug/kdb/kdb_main.c +1 -1
@@ -1091,7 +1091,7 @@ static int kdb_reboot(int argc, const char **argv)static void kdb_dumpregs (struct pt_regs *regs) {int old_lvl = console_loglevel; - console_loglevel =15 ; + console_loglevel =CONSOLE_LOGLEVEL_MOTORMOUTH ; kdb_trap_printk++;show_regs (regs); kdb_trap_printk--;
kernel/printk/printk.c +3 -10
@@ -54,18 +54,11 @@#include "console_cmdline.h" #include "braille.h" -/* printk's without a loglevel use this.. */ -#define DEFAULT_MESSAGE_LOGLEVEL CONFIG_DEFAULT_MESSAGE_LOGLEVEL - -/* We show everything that is MORE important than this.. */ -#define MINIMUM_CONSOLE_LOGLEVEL 1/* Minimum loglevel we let people use */ -#define DEFAULT_CONSOLE_LOGLEVEL 7/* anything MORE serious than KERN_DEBUG */ -int console_printk[4 ] = { -DEFAULT_CONSOLE_LOGLEVEL ,/* console_loglevel */ +CONSOLE_LOGLEVEL_DEFAULT ,/* console_loglevel */ DEFAULT_MESSAGE_LOGLEVEL ,/* default_message_loglevel */ -MINIMUM_CONSOLE_LOGLEVEL ,/* minimum_console_loglevel */ -DEFAULT_CONSOLE_LOGLEVEL ,/* default_console_loglevel */ +CONSOLE_LOGLEVEL_MIN ,/* minimum_console_loglevel */ +CONSOLE_LOGLEVEL_DEFAULT ,/* default_console_loglevel */ };/*
Signature · 1 line
1.9.0
Quoted 1 line · Borislav Petkov
... instead of naked numbers.
Seems sensible.
include/linux/printk.h
Quoted 1 line
include/linux/printk.h +0
-0
[]
include/linux/printk.h static inline const char *printk_skip_level(const char *buffer) · line 34
Quoted 6 lines
include/linux/printk.h +2
-0@@ -30,6 +30,17 @@ static inline const char *printk_skip_level(const char *buffer)return buffer; } +/* printk's without a loglevel use this.. */ +#define DEFAULT_MESSAGE_LOGLEVEL CONFIG_DEFAULT_MESSAGE_LOGLEVEL
Does anyone actually set CONFIG_DEFAULT_MESSAGE_LOGLEVEL to something other than 4?
include/linux/printk.h static inline const char *printk_skip_level(const char *buffer) · line 36
Quoted 1 line
include/linux/printk.h +1
-0+/* We show everything that is MORE important than this.. */
It might be nice to show somewhere that larger values are less important.
include/linux/printk.h static inline const char *printk_skip_level(const char *buffer) · line 42
Quoted 6 lines
include/linux/printk.h +6
-0+#define CONSOLE_LOGLEVEL_SILENT 0/* Mum's the word */ +#define CONSOLE_LOGLEVEL_MIN 1/* Minimum loglevel we let people use */ +#define CONSOLE_LOGLEVEL_QUIET 4/* Shhh ..., when booted with "quiet" */ +#define CONSOLE_LOGLEVEL_DEFAULT 7/* anything MORE serious than KERN_DEBUG */ +#define CONSOLE_LOGLEVEL_DEBUG 10/* issue debug messages */ +#define CONSOLE_LOGLEVEL_MOTORMOUTH 15/* You can't shut this one up */
It may be nicer to have #defines for the
KERN_<LEVEL> numbers from kern_levels.h
and use those here.
Maybe add:
#define LOGLEVEL_EMERG 0
#define LOGLEVEL_ALERT 1
#define LOGLEVEL_CRIT 2
#define LOGLEVEL_ERR 3
#define LOGLEVEL_WARNING 4
#define LOGLEVEL_NOTICE 5
#define LOGLEVEL_INFO 6
#define LOGLEVEL_DEBUG 7
That could clean up a few uses of
magic numbers like 7 in dev_printk_emit
Borislav Petkov
maintainerQuoted 44 lines · Joe Perches
Quoted 1 line · Borislav Petkov
... instead of naked numbers.
Seems sensible.
include/linux/printk.h
Quoted 1 line
include/linux/printk.h +0
-0[]
include/linux/printk.h static inline const char *printk_skip_level(const char *buffer) · line 34
Quoted 6 lines
include/linux/printk.h +2
-0@@ -30,6 +30,17 @@ static inline const char *printk_skip_level(const char *buffer)return buffer; } +/* printk's without a loglevel use this.. */ +#define DEFAULT_MESSAGE_LOGLEVEL CONFIG_DEFAULT_MESSAGE_LOGLEVELDoes anyone actually set
CONFIG_DEFAULT_MESSAGE_LOGLEVELto something other than 4?include/linux/printk.h static inline const char *printk_skip_level(const char *buffer) · line 36
Quoted 1 line
include/linux/printk.h +1
-0+/* We show everything that is MORE important than this.. */ It might be nice to show somewhere that larger values are less important.
include/linux/printk.h static inline const char *printk_skip_level(const char *buffer) · line 42
Quoted 6 lines
include/linux/printk.h +6
-0+#define CONSOLE_LOGLEVEL_SILENT 0/* Mum's the word */ +#define CONSOLE_LOGLEVEL_MIN 1/* Minimum loglevel we let people use */ +#define CONSOLE_LOGLEVEL_QUIET 4/* Shhh ..., when booted with "quiet" */ +#define CONSOLE_LOGLEVEL_DEFAULT 7/* anything MORE serious than KERN_DEBUG */ +#define CONSOLE_LOGLEVEL_DEBUG 10/* issue debug messages */ +#define CONSOLE_LOGLEVEL_MOTORMOUTH 15/* You can't shut this one up */ It may be nicer to have #defines for the
KERN_<LEVEL> numbers fromkern_levels.h
and use those here.Maybe add:
#define
LOGLEVEL_EMERG0
#defineLOGLEVEL_ALERT1
#defineLOGLEVEL_CRIT2
#defineLOGLEVEL_ERR3
#defineLOGLEVEL_WARNING4
#defineLOGLEVEL_NOTICE5
#defineLOGLEVEL_INFO6
#defineLOGLEVEL_DEBUG7
Quoted 2 lines
That could clean up a few uses of
magic numbers like 7 indev_printk_emit
This patch is supposed to be as obvious as possible - all other things which need/must/will be done should come ontop, not here.
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 10 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
... instead of naked numbers.
Stuff in
sysrq.cused to set it to 8 which is supposed to mean above default level so set it to DEBUG instead as we're terminating/killing all tasks and we want to be verbose there.There should be no functionality change resulting from this patch.
Quoted 15 lines
Cc Linus Torvalds <[email protected]>Cc Andrew Morton <[email protected]>Cc Randy Dunlap <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Jiri Slaby <[email protected]>Cc Jason Wessel <[email protected]>Cc Hedi Berriche <[email protected]>Cc Mike Travis <[email protected]>Cc Dimitri Sivanich <[email protected]>Cc Steven Rostedt <[email protected]>Cc Kees Cook <[email protected]>Cc Peter Zijlstra <[email protected]>Signed-off-by Borislav Petkov <[email protected]>
Yes, please. Good clean-up. :)
Signature · 2 lines
Kees Cook Chrome OS Security
Quoted 10 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
... instead of naked numbers.
Stuff in
sysrq.cused to set it to 8 which is supposed to mean above default level so set it to DEBUG instead as we're terminating/killing all tasks and we want to be verbose there.There should be no functionality change resulting from this patch.
Thanks.
Quoted 27 lines
Cc Linus Torvalds <[email protected]>Cc Andrew Morton <[email protected]>Cc Randy Dunlap <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Jiri Slaby <[email protected]>Cc Jason Wessel <[email protected]>Cc Hedi Berriche <[email protected]>Cc Mike Travis <[email protected]>Cc Dimitri Sivanich <[email protected]>Cc Steven Rostedt <[email protected]>Cc Kees Cook <[email protected]>Cc Peter Zijlstra <[email protected]>Signed-off-by Borislav Petkov <[email protected]>--- arch/x86/kernel/head64.c | 2 +- arch/x86/platform/uv/uv_nmi.c | 2 +- drivers/nubus/nubus.c | 18 +++++++++--------- drivers/tty/sysrq.c | 8 ++++---- include/linux/printk.h | 15 +++++++++++++-- init/main.c | 4 ++-- kernel/debug/kdb/kdb_bt.c | 2 +- kernel/debug/kdb/kdb_io.c | 2 +- kernel/debug/kdb/kdb_main.c | 2 +- kernel/printk/printk.c | 13 +++---------- 10 files changed, 36 insertions(+), 32 deletions(-)
Signature · 1 line
~Randy
Quoted 9 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
... instead of naked numbers.
Stuff in
sysrq.cused to set it to 8 which is supposed to mean above default level so set it to DEBUG instead as we're terminating/killing all tasks and we want to be verbose there.There should be no functionality change resulting from this patch.
After a quick eyeballing, looks like it, except for:
Review asmlinkage __visible void __init x86_64_start_kernel(char
Quoted 2 lines
+++
b/arch/x86/kernel/head64.c(quoted diff) +0
-0@@ -172,7 +172,7 @@ asmlinkage __visible void __init x86_64_start_kernel(char
* real_mode_data)
arch/x86/kernel/head64.c asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data) · line 176
Quoted 6 lines
arch/x86/kernel/head64.c +1
-1*/ load_ucode_bsp (); -if (console_loglevel ==10 ) +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG )early_printk ("Kernel alive\n" );
But this looks like a bugfix that changes the behavior for loglevels > 11.
Having said that, it's probably OK to not split the bugfix out to a separate patch....
Steven Rostedt
reviewerQuoted 11 lines · On Mon, 19 May 2014 20:27:32 +0200 Borislav Petkov <[email protected]> wrote:
From: Borislav Petkov <[email protected]>
There should be no functionality change resulting from this patch.
arch/x86/kernel/head64.c +0
-0
arch/x86/kernel/head64.c asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data) · line 175
Quoted 5 lines
arch/x86/kernel/head64.c +1
-1@@ -172,7 +172,7 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)*/ load_ucode_bsp (); -if (console_loglevel ==10 ) +if (console_loglevel >=CONSOLE_LOGLEVEL_DEBUG )
This looks like functionality change to me.
Please make the fix of "==" --> ">=" a separate patch.
-- Steve
Borislav Petkov
maintainerQuoted 3 lines · Steven Rostedt
This looks like functionality change to me.
Please make the fix of "==" --> ">=" a separate patch.
Yeah, that's actually a fix for console_loglevel values > 10. I don't think it is worth the effort of splitting though ... unless you have a real good reason.
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 8 lines · Borislav Petkov
Quoted 3 lines · Steven Rostedt
This looks like functionality change to me.
Please make the fix of "==" --> ">=" a separate patch.
Yeah, that's actually a fix for
console_loglevelvalues > 10. I don't think it is worth the effort of splitting though ... unless you have a real good reason.
If you don't split it out, you should at least mention the bugfix in the changelog....
Borislav Petkov
maintainerQuoted 2 lines · On Tue, May 27, 2014 at 06:36:24PM -0400, [email protected] wrote:
If you don't split it out, you should at least mention the bugfix in the changelog....
Ok ok, I give up. Steve is bugging me on IRC about it, too. Damn. Ok, I got it.
@Andrew: can you please replace the sentence:
"There should be no functionality change resulting from this patch."
with
"Also, correct the check in x86_64_start_kernel which should be >= as we're clearly issuing the string there for all debug levels, not only the magical 10."
so that Steve can sleep tonight? :-)
Or do you want me to resend with corrected changelog?
Thanks.
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 9 lines · On Wed, 28 May 2014 00:59:57 +0200 Borislav Petkov <[email protected]> wrote:
@Andrew: can you please replace the sentence:
"There should be no functionality change resulting from this patch."
with
"Also, correct the check in
x86_64_start_kernelwhich should be >= as we're clearly issuing the string there for all debug levels, not only the magical 10."
Done.
Quoted 1 line
so that Steve can sleep tonight? :-)
I think Steve only comes out at night.
Steven Rostedt
reviewerQuoted 3 lines · On Tue, 27 May 2014 16:26:44 -0700 Andrew Morton <[email protected]>…
Quoted 1 line
so that Steve can sleep tonight? :-)
I think Steve only comes out at night.
Of course. That's because night time is the best time to hit those Japanese heated toilet seats.
-- Steve
Borislav Petkov
maintainerQuoted 6 lines · Andrew Morton
Quoted 4 lines · On Wed, 28 May 2014 00:59:57 +0200 Borislav Petkov <[email protected]> wrote:
@Andrew: can you please replace the sentence:
"There should be no functionality change resulting from this patch."
Quoted 7 lines
Quoted 5 lines
with
"Also, correct the check in
x86_64_start_kernelwhich should be >= as we're clearly issuing the string there for all debug levels, not only the magical 10."Done.
Thanks Andrew!
Quoted 3 lines
Quoted 1 line
so that Steve can sleep tonight? :-)
I think Steve only comes out at night.
Yep, that's him: Steve "The Japanese Vampire on a heated toilet seat" Rostedt!
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--