Re: [PATCH 3/3] printk: Add ability to set loglevel via "console=" cmdline
From: Petr Mladek <pmladek@suse.com>
Date: 2017-11-03 15:15:57
Also in:
lkml
On Thu 2017-09-28 17:43:57, Calvin Owens wrote:
quoted hunk ↗ jump to hunk
This extends the "console=" interface to allow setting the per-console loglevel by adding "/N" to the string, where N is the desired loglevel expressed as a base 10 integer. Invalid values are silently ignored. Cc: Petr Mladek <pmladek@suse.com> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Sergey Senozhatsky <redacted> Signed-off-by: Calvin Owens <redacted> --- Documentation/admin-guide/kernel-parameters.txt | 6 ++--- kernel/printk/console_cmdline.h | 1 + kernel/printk/printk.c | 30 ++++++++++++++++++++----- 3 files changed, 28 insertions(+), 9 deletions(-)diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 0549662..f22b992 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt@@ -607,10 +607,10 @@ ttyS<n>[,options] ttyUSB0[,options] Use the specified serial port. The options are of - the form "bbbbpnf", where "bbbb" is the baud rate, + the form "bbbbpnf/l", where "bbbb" is the baud rate, "p" is parity ("n", "o", or "e"), "n" is number of - bits, and "f" is flow control ("r" for RTS or - omit it). Default is "9600n8". + bits, "f" is flow control ("r" for RTS or omit it), + and "l" is the loglevel on [0,7]. Default is "9600n8".
If I get this correctly, the patch allows to define the loglevel for any
console. I think that we need to describe it in a generic
way. Something like:
console= [KNL] Output console device and options.
Format: name[,options][/min_loglevel]
Where "name" is the console name, "options"
are console-specific options, and "min_loglevel"
allows to increase the loglevel for a particular
console over the global one.
tty<n> Use the virtual console device <n>.
I would also add a cross reference into the loglevel= section
about that the global loglevel might be overridden by a higher
console-specific min_loglevel value.
quoted hunk ↗ jump to hunk
See Documentation/admin-guide/serial-console.rst for more information. Seediff --git a/kernel/printk/console_cmdline.h b/kernel/printk/console_cmdline.h index 2ca4a8b..269e666 100644 --- a/kernel/printk/console_cmdline.h +++ b/kernel/printk/console_cmdline.h@@ -5,6 +5,7 @@ struct console_cmdline { char name[16]; /* Name of the driver */ int index; /* Minor dev. to use */ + int loglevel; /* Loglevel to use */
Again, I would use "min_loglevel".
quoted hunk ↗ jump to hunk
char *options; /* Options for the driver */ #ifdef CONFIG_A11Y_BRAILLE_CONSOLE char *brl_options; /* Options for braille driver */diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 488bda3..4c14cf2 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c@@ -2541,6 +2552,12 @@ void register_console(struct console *newcon) if (newcon->index < 0) newcon->index = c->index; + /* + * Carry over the loglevel from the cmdline + */ + newcon->level = c->loglevel; + extant = true;
I would personally do the following: if (!newcon->min_loglevel) newcon->min_loglevel = c->min_loglevel; It is similar to newcon->index handling above. It will use the command line setting only when the console is registered for the first time. All this is based on my assumption that all non-initialized struct console members are zero. At least I do not see any location where some other members would be explicitly zeroed. And there might be candidates, e.g. data, match(), next. In each case, I do not know what the shortcut "extant" stands for and I feel confused ;-)
quoted hunk ↗ jump to hunk
if (_braille_register_console(newcon, c)) return;@@ -2572,8 +2589,9 @@ void register_console(struct console *newcon) /* * By default, the per-console minimum forces no messages through. */ - newcon->level = LOGLEVEL_EMERG; newcon->kobj = NULL; + if (!extant) + newcon->level = LOGLEVEL_EMERG;
I believe that this is not necessary. Otherwise, I really like this approach. I am surprised that all consoles might be supported so easily. Well, I am not 100% sure that '/' delimiter is the best choice. I hope that it will not cause any confusion. But I cannot think of anything better. Best Regards, Petr