2007-10-06 20:10:34

by Jan Engelhardt

[permalink] [raw]
Subject: [PATCH 2/2] Colored kernel output (run3)


Colored kernel message output (2/2)

By popular request, this patch adds per-loglevel coloring.
The user may set values using vt.printk_color= or by modifying
the sysfs file in the running system.

Signed-off-by: Jan Engelhardt <[email protected]>

---
arch/x86_64/kernel/early_printk.c | 11 +++++++----
drivers/char/Kconfig | 4 +++-
drivers/char/vt.c | 32 ++++++++++++++++++++++++--------
drivers/net/netconsole.c | 3 ++-
drivers/serial/8250.c | 3 ++-
drivers/serial/8250_early.c | 3 ++-
include/linux/console.h | 2 +-
kernel/printk.c | 12 +++++++-----
8 files changed, 48 insertions(+), 22 deletions(-)

Index: linux-2.6.23/arch/x86_64/kernel/early_printk.c
===================================================================
--- linux-2.6.23.orig/arch/x86_64/kernel/early_printk.c
+++ linux-2.6.23/arch/x86_64/kernel/early_printk.c
@@ -20,7 +20,8 @@
static int max_ypos = 25, max_xpos = 80;
static int current_ypos = 25, current_xpos = 0;

-static void early_vga_write(struct console *con, const char *str, unsigned n)
+static void early_vga_write(struct console *con, const char *str, unsigned n,
+ unsigned int loglevel)
{
char c;
int i, k, j;
@@ -89,7 +90,8 @@ static int early_serial_putc(unsigned ch
return timeout ? 0 : -1;
}

-static void early_serial_write(struct console *con, const char *s, unsigned n)
+static void early_serial_write(struct console *con, const char *s, unsigned n,
+ unsigned int loglevel)
{
while (*s && n-- > 0) {
if (*s == '\n')
@@ -185,7 +187,8 @@ static void __init simnow_init(char *str
simnow_fd = simnow(XOPEN, (unsigned long)fn, O_WRONLY|O_APPEND|O_CREAT, 0644);
}

-static void simnow_write(struct console *con, const char *s, unsigned n)
+static void simnow_write(struct console *con, const char *s, unsigned n,
+ unsigned int loglevel)
{
simnow(XWRITE, simnow_fd, (unsigned long)s, n);
}
@@ -209,7 +212,7 @@ void early_printk(const char *fmt, ...)

va_start(ap,fmt);
n = vscnprintf(buf,512,fmt,ap);
- early_console->write(early_console,buf,n);
+ early_console->write(early_console, buf, n, 0);
va_end(ap);
}

Index: linux-2.6.23/drivers/char/Kconfig
===================================================================
--- linux-2.6.23.orig/drivers/char/Kconfig
+++ linux-2.6.23/drivers/char/Kconfig
@@ -75,7 +75,9 @@ config VT_PRINTK_COLOR
default 0x07
---help---
This option defines with which color kernel messages will be
- printed to the console.
+ printed to the console. This applies to all log levels.
+ Colors for independent log levels can be set using the
+ vt.printk_color runtime option.

The value you need to enter here is the value is composed
(OR-ed) of a foreground and a background color.
Index: linux-2.6.23/drivers/char/vt.c
===================================================================
--- linux-2.6.23.orig/drivers/char/vt.c
+++ linux-2.6.23/drivers/char/vt.c
@@ -2346,8 +2346,17 @@ struct tty_driver *console_driver;
#ifdef CONFIG_VT_CONSOLE

#ifdef CONFIG_VT_CKO
-static unsigned int printk_color __read_mostly = CONFIG_VT_PRINTK_COLOR;
-module_param(printk_color, uint, S_IRUGO | S_IWUSR);
+static unsigned int printk_color[8] __read_mostly = {
+ CONFIG_VT_PRINTK_COLOR, /* KERN_EMERG */
+ CONFIG_VT_PRINTK_COLOR, /* KERN_ALERT */
+ CONFIG_VT_PRINTK_COLOR, /* KERN_CRIT */
+ CONFIG_VT_PRINTK_COLOR, /* KERN_ERR */
+ CONFIG_VT_PRINTK_COLOR, /* KERN_WARNING */
+ CONFIG_VT_PRINTK_COLOR, /* KERN_NOTICE */
+ CONFIG_VT_PRINTK_COLOR, /* KERN_INFO */
+ CONFIG_VT_PRINTK_COLOR, /* KERN_DEBUG */
+};
+module_param_array(printk_color, uint, NULL, S_IRUGO | S_IWUSR);

static void vc_set_color(struct vc_data *vc, unsigned char color)
{
@@ -2357,7 +2366,7 @@ static void vc_set_color(struct vc_data
update_attr(vc);
}
#else
-static unsigned int printk_color;
+static unsigned int printk_color[8];
static inline void vc_set_color(const struct vc_data *vc, unsigned char c)
{
}
@@ -2369,10 +2378,11 @@ static inline void vc_set_color(const st
* The console must be locked when we get here.
*/

-static void vt_console_print(struct console *co, const char *b, unsigned count)
+static void vt_console_print(struct console *co, const char *b, unsigned count,
+ unsigned int loglevel)
{
struct vc_data *vc = vc_cons[fg_console].d;
- unsigned char c;
+ unsigned char current_color, c;
static unsigned long printing;
const ushort *start;
ushort cnt = 0;
@@ -2403,7 +2413,13 @@ static void vt_console_print(struct cons
hide_cursor(vc);

start = (ushort *)vc->vc_pos;
- vc_set_color(vc, printk_color);
+
+ /*
+ * We always get a valid loglevel - <8> and "no level" is transformed
+ * to <4> in the typical kernel.
+ */
+ current_color = printk_color[loglevel];
+ vc_set_color(vc, current_color);

/* Contrived structure to try to emulate original need_wrap behaviour
* Problems caused when we have need_wrap set on '\n' character */
@@ -2423,7 +2439,7 @@ static void vt_console_print(struct cons
bs(vc);
start = (ushort *)vc->vc_pos;
myx = vc->vc_x;
- vc_set_color(vc, printk_color);
+ vc_set_color(vc, current_color);
continue;
}
if (c != 13)
@@ -2431,7 +2447,7 @@ static void vt_console_print(struct cons
cr(vc);
start = (ushort *)vc->vc_pos;
myx = vc->vc_x;
- vc_set_color(vc, printk_color);
+ vc_set_color(vc, current_color);
if (c == 10 || c == 13)
continue;
}
Index: linux-2.6.23/drivers/net/netconsole.c
===================================================================
--- linux-2.6.23.orig/drivers/net/netconsole.c
+++ linux-2.6.23/drivers/net/netconsole.c
@@ -65,7 +65,8 @@ static int configured = 0;

#define MAX_PRINT_CHUNK 1000

-static void write_msg(struct console *con, const char *msg, unsigned int len)
+static void write_msg(struct console *con, const char *msg, unsigned int len,
+ unsigned int loglevel)
{
int frag, left;
unsigned long flags;
Index: linux-2.6.23/drivers/serial/8250.c
===================================================================
--- linux-2.6.23.orig/drivers/serial/8250.c
+++ linux-2.6.23/drivers/serial/8250.c
@@ -2464,7 +2464,8 @@ static void serial8250_console_putchar(s
* The console_lock must be held when we get here.
*/
static void
-serial8250_console_write(struct console *co, const char *s, unsigned int count)
+serial8250_console_write(struct console *co, const char *s, unsigned int count,
+ unsigned int loglevel)
{
struct uart_8250_port *up = &serial8250_ports[co->index];
unsigned long flags;
Index: linux-2.6.23/drivers/serial/8250_early.c
===================================================================
--- linux-2.6.23.orig/drivers/serial/8250_early.c
+++ linux-2.6.23/drivers/serial/8250_early.c
@@ -82,7 +82,8 @@ static void __init putc(struct uart_port
serial_out(port, UART_TX, c);
}

-static void __init early_serial8250_write(struct console *console, const char *s, unsigned int count)
+static void __init early_serial8250_write(struct console *console,
+ const char *s, unsigned int count, unsigned int loglevel)
{
struct uart_port *port = &early_device.port;
unsigned int ier;
Index: linux-2.6.23/include/linux/console.h
===================================================================
--- linux-2.6.23.orig/include/linux/console.h
+++ linux-2.6.23/include/linux/console.h
@@ -93,7 +93,7 @@ void give_up_console(const struct consw

struct console {
char name[16];
- void (*write)(struct console *, const char *, unsigned);
+ void (*write)(struct console *, const char *, unsigned, unsigned int);
int (*read)(struct console *, char *, unsigned);
struct tty_driver *(*device)(struct console *, int *);
void (*unblank)(void);
Index: linux-2.6.23/kernel/printk.c
===================================================================
--- linux-2.6.23.orig/kernel/printk.c
+++ linux-2.6.23/kernel/printk.c
@@ -320,7 +320,8 @@ asmlinkage long sys_syslog(int type, cha
/*
* Call the console drivers on a range of log_buf
*/
-static void __call_console_drivers(unsigned long start, unsigned long end)
+static void __call_console_drivers(unsigned long start, unsigned long end,
+ unsigned int loglevel)
{
struct console *con;

@@ -328,7 +329,7 @@ static void __call_console_drivers(unsig
if ((con->flags & CON_ENABLED) && con->write &&
(cpu_online(smp_processor_id()) ||
(con->flags & CON_ANYTIME)))
- con->write(con, &LOG_BUF(start), end - start);
+ con->write(con, &LOG_BUF(start), end - start, loglevel);
}
}

@@ -355,10 +356,11 @@ static void _call_console_drivers(unsign
if ((start & LOG_BUF_MASK) > (end & LOG_BUF_MASK)) {
/* wrapped write */
__call_console_drivers(start & LOG_BUF_MASK,
- log_buf_len);
- __call_console_drivers(0, end & LOG_BUF_MASK);
+ log_buf_len, msg_log_level);
+ __call_console_drivers(0, end & LOG_BUF_MASK,
+ msg_log_level);
} else {
- __call_console_drivers(start, end);
+ __call_console_drivers(start, end, msg_log_level);
}
}
}


2007-10-06 21:11:39

by Oleg Verych

[permalink] [raw]
Subject: Re: [PATCH 2/2] Colored kernel output (run3)

Thanks for dealing with my acidness in the first patch :)

But what about this one?

On Sat, Oct 06, 2007 at 10:10:01PM +0200, Jan Engelhardt wrote:
>
> Colored kernel message output (2/2)
>
> By popular request, this patch adds per-loglevel coloring.
> The user may set values using vt.printk_color= or by modifying
> the sysfs file in the running system.
>
> Signed-off-by: Jan Engelhardt <[email protected]>
>
> ---
> arch/x86_64/kernel/early_printk.c | 11 +++++++----
> drivers/char/Kconfig | 4 +++-
> drivers/char/vt.c | 32 ++++++++++++++++++++++++--------
> drivers/net/netconsole.c | 3 ++-
> drivers/serial/8250.c | 3 ++-
> drivers/serial/8250_early.c | 3 ++-
> include/linux/console.h | 2 +-
> kernel/printk.c | 12 +++++++-----
> 8 files changed, 48 insertions(+), 22 deletions(-)

Making this amount changes and not thinking about more intelligent
coding of the functionality?

> Index: linux-2.6.23/arch/x86_64/kernel/early_printk.c
> ===================================================================
> --- linux-2.6.23.orig/arch/x86_64/kernel/early_printk.c
> +++ linux-2.6.23/arch/x86_64/kernel/early_printk.c
> @@ -20,7 +20,8 @@
> static int max_ypos = 25, max_xpos = 80;
> static int current_ypos = 25, current_xpos = 0;
>
> -static void early_vga_write(struct console *con, const char *str, unsigned n)
> +static void early_vga_write(struct console *con, const char *str, unsigned n,
> + unsigned int loglevel)
> {

I mean, *write() have nothing to do with loglevels. If they do
(suddenly), then why not to use a char in the *str to make it possible? I
might be wrong, but there are already macros before format strings in
printk().

And this is not `loglevel' thing any more. It's attributes, which can be
used by many other drivers/file systems/schedulers/ what ever, if
developers, like Ingo, will extend printk() output to be more nice in
subsystems they develop.

So, maybe they can be extended, and functionality implemented in the
function bodies (which will be `do { } while (0)' if config is NO), but
*NOT* by passing additional argument to the functions.
___

2007-10-06 21:28:08

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH 2/2] Colored kernel output (run3)


On Oct 6 2007 23:25, Oleg Verych wrote:
>> ---
>> arch/x86_64/kernel/early_printk.c | 11 +++++++----
>> drivers/char/Kconfig | 4 +++-
>> drivers/char/vt.c | 32 ++++++++++++++++++++++++--------
>> drivers/net/netconsole.c | 3 ++-
>> drivers/serial/8250.c | 3 ++-
>> drivers/serial/8250_early.c | 3 ++-
>> include/linux/console.h | 2 +-
>> kernel/printk.c | 12 +++++++-----
>> 8 files changed, 48 insertions(+), 22 deletions(-)
>
>Making this amount changes and not thinking about more intelligent
>coding of the functionality?

Well humm.. in an unmodified kernel, the last function that has
knowledge of the loglevel is _call_console_drivers() in kernel/printk.c.
>From there, the call chain is directly -> __call_console_drivers() ->
drivers/char/vt.c:vt_console_print().

_call_console_drivers() skips the <N> substring and passes on the rest of the
message:

if (msg_level < 0 && ((end - cur_index) > 2) &&
LOG_BUF(cur_index + 0) == '<' &&
LOG_BUF(cur_index + 1) >= '0' &&
LOG_BUF(cur_index + 1) <= '7' &&
LOG_BUF(cur_index + 2) == '>') {
msg_level = LOG_BUF(cur_index + 1) - '0';
cur_index += 3;
start_print = cur_index;
}

>I mean, *write() have nothing to do with loglevels. If they do
>(suddenly), then why not to use a char in the *str to make it possible? I
>might be wrong, but there are already macros before format strings in
>printk().
>
>And this is not `loglevel' thing any more. It's attributes, which can be
>used by many other drivers/file systems/schedulers/ what ever, if
>developers, like Ingo, will extend printk() output to be more nice in
>subsystems they develop.

If I interpret you correctly, you want me to remove cur_index+=3
and instead reparse <N> in drivers/char/vt.c. But that's not good,
because if you use serial, we won't go to vt.c, and in the end,
8250.c would also need to parse <N>, which I think is just walking
around the hill.


kernel/printk.c -> 8250.c -> grab_loglevel_from_string -> ignore it and print
message
kernel/printk.c -> vt.c -> grab_loglevel_from_string -> print color according
to loglevel

VS

kernel/printk.c -> pass loglevel directly to 8250.c -> ignore loglevel and
print message
kernel/printk.c -> pass loglevel directly to vt.c -> print color according to
loglevel

2007-10-06 22:14:28

by Oleg Verych

[permalink] [raw]
Subject: Re: On text size and run time if config is "n", [PATCH 2/2] Colored kernel output (run3)

On Sat, Oct 06, 2007 at 11:27:54PM +0200, Jan Engelhardt wrote:
[]
> _call_console_drivers() skips the <N> substring and passes on the rest of the
> message:
>
> if (msg_level < 0 && ((end - cur_index) > 2) &&
> LOG_BUF(cur_index + 0) == '<' &&
> LOG_BUF(cur_index + 1) >= '0' &&
> LOG_BUF(cur_index + 1) <= '7' &&
> LOG_BUF(cur_index + 2) == '>') {
> msg_level = LOG_BUF(cur_index + 1) - '0';
> cur_index += 3;
> start_print = cur_index;
> }
>
> >I mean, *write() have nothing to do with loglevels. If they do
> >(suddenly), then why not to use a char in the *str to make it possible? I
> >might be wrong, but there are already macros before format strings in
> >printk().
> >
> >And this is not `loglevel' thing any more. It's attributes, which can be
> >used by many other drivers/file systems/schedulers/ what ever, if
> >developers, like Ingo, will extend printk() output to be more nice in
> >subsystems they develop.
>
> If I interpret you correctly, you want me to remove cur_index+=3
> and instead reparse <N> in drivers/char/vt.c. But that's not good,
> because if you use serial, we won't go to vt.c, and in the end,
> 8250.c would also need to parse <N>, which I think is just walking
> around the hill.

I thought, i was talking about *write() functions, that got one
additional unrelated, non config removable API change in face of
`unsigned int loglevel'.

Idea. Extend those macro defines before format string with one
additional macro, that will be empty, if config is NO and having
colour byte otherwise.

In the *write() functions, have

color = str[0];
++str;

in case if config is YES. And nothing otherwise.


Sorry, i don't know much about stuff, you've presented:

> kernel/printk.c -> 8250.c -> grab_loglevel_from_string -> ignore it and print
> message
> kernel/printk.c -> vt.c -> grab_loglevel_from_string -> print color according
> to loglevel

so maybe i just am an ignorant. Anyway this:

> VS
>
> kernel/printk.c -> pass loglevel directly to 8250.c -> ignore loglevel and
> print message
> kernel/printk.c -> pass loglevel directly to vt.c -> print color according to
> loglevel

have no compile or run time optimization possible.
____

2007-10-06 22:21:10

by Jan Engelhardt

[permalink] [raw]
Subject: Re: On text size and run time if config is "n", [PATCH 2/2] Colored kernel output (run3)


On Oct 7 2007 00:28, Oleg Verych wrote:
>
>I thought, i was talking about *write() functions, that got one
>additional unrelated, non config removable API change in face of
>`unsigned int loglevel'.

Documentation/stable_api_nonsense.txt ;-)

>Idea. Extend those macro defines before format string with one
>additional macro, that will be empty, if config is NO and having
>colour byte otherwise.
>
>In the *write() functions, have
>
> color = str[0];
> ++str;

This is just as "bad" as the loglevel parameter. Because you'd have
to parse str[0] in _every_ write() function. Furthermore, the string
that is being printed also goes to klogd/syslog, including the <N>
tag already. If you now add a color byte in the KERN_* macros, then
that color byte would also go to syslog. Not good.

>Sorry, i don't know much about stuff, you've presented:

(Paraphrased one paragraph above.)