Re: [PATCH v5 2/2] printk: console: Support console-specific loglevels

From: Chris Down
Date: Tue May 02 2023 - 10:26:49 EST


Thanks for the detailed feedback! All comments not explicitly replied to are acked and will be updated in v6.

Petr Mladek writes:
@@ -2293,7 +2429,14 @@ asmlinkage int vprintk_emit(int facility, int level,
in_sched = true;
}

- printk_delay(level);
+ cookie = console_srcu_read_lock();
+ for_each_console_srcu(con) {
+ if (!suppress_message_printing(level, con)) {
+ printk_delay();

This would repeat the delay for each registered and non-supressed console.
But it should be called only once when there is at least one
non-suppressed console.

+ break;
+ }
+ }
+ console_srcu_read_unlock(cookie);

Doesn't the `break' below avoid that already? Maybe I'm missing something?

[...]

+static int printk_sysctl_deprecated(struct ctl_table *table, int write,
+ void *buffer, size_t *lenp, loff_t *ppos)
+{
+ int res = proc_dointvec(table, write, buffer, lenp, ppos);
+
+ if (write)
+ pr_warn_once(
+ "printk: The kernel.printk sysctl is deprecated. Consider using kernel.console_loglevel or kernel.default_message_loglevel instead.\n");

Nit: Please, put it on the same line as pr_warn_once("printk: The kernel....
The small shift left did not help much ;-)

I think this is done by clang-format with kernel default config, since my preference is single line as well. Maybe worth discussing changing the config?

How does this sounds, please?

Sounds great! Thank you again for the very detailed feedback :-)