[for-6.4] printk: Unregister boot consoles on register of 1st real console

Message ID 13e4bdf7677924c689a70d0b7ad970a3255a8d41.1677213245.git.lukas@wunner.de
State New
Headers
Series [for-6.4] printk: Unregister boot consoles on register of 1st real console |

Commit Message

Lukas Wunner Feb. 24, 2022, 4:40 a.m. UTC
  The code comment preceding register_console() claims that:

   "There are two types of consoles - bootconsoles (early_printk) and
    "real" consoles (everything which is not a bootconsole) which are
    handled differently. [...]
    As soon as a "real" console is registered, all bootconsoles
    will be unregistered automatically."

But that's not what the code does:  The code unregisters bootconsoles
only when the *preferred* console registers, i.e. the last one on the
command line.  If that console's driver never registers (e.g. because
it is disabled in the kernel config), bootconsoles stay around
indefinitely.  Should the command line contain both a bootconsole as
well as a real console on the same serial port, all messages are logged
twice once the real console registers.

Moreover, the log buffer is replayed once the real console registers
even though the messages were already emitted by the bootconsole.

Amend the code to be congruent with the above-quoted code comment and
thereby avoid these issues.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 kernel/printk/printk.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
  

Comments

Petr Mladek March 9, 2023, 11:15 a.m. UTC | #1
On Thu 2022-02-24 05:40:33, Lukas Wunner wrote:
> The code comment preceding register_console() claims that:
> 
>    "There are two types of consoles - bootconsoles (early_printk) and
>     "real" consoles (everything which is not a bootconsole) which are
>     handled differently. [...]
>     As soon as a "real" console is registered, all bootconsoles
>     will be unregistered automatically."
> 
> But that's not what the code does:  The code unregisters bootconsoles
> only when the *preferred* console registers, i.e. the last one on the
> command line.

Yes

> If that console's driver never registers (e.g. because
> it is disabled in the kernel config), bootconsoles stay around
> indefinitely.

They are actually unregistered by a late_initcall, see
printk_late_init().

> Should the command line contain both a bootconsole as
> well as a real console on the same serial port, all messages are logged
> twice once the real console registers.
> 
> Moreover, the log buffer is replayed once the real console registers
> even though the messages were already emitted by the bootconsole.

Yes, the messages might get duplicated until the printk_late_init()
is called. And the log might get replayed even though it was already
printed on the boot console.

> Amend the code to be congruent with the above-quoted code comment and
> thereby avoid these issues.

I am afraid that we could not change the behavior. It would cause
the opposite problem. It might might remove the boot consoles too early
and some messages might be missing on the corresponding console.
Also the log might get replayed on the "preferred" console.
People would see this as regression.

The only acceptable solution would be to do the right thing.
I mean to match the corresponding boot and real consoles.
Then we could remove the boot console exactly when it is replaced
by the real one. It would prevent any duplicated or lost messages.

In the meantime, we could only fix the comment so that it describes
the current behavior.

Best Regards,
Petr

Best Regards,
Petr
  

Patch

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index fd0c9f913940..f89e865c6b23 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3423,10 +3423,8 @@  void register_console(struct console *newcon)
 	 * the real console are the same physical device, it's annoying to
 	 * see the beginning boot messages twice
 	 */
-	if (bootcon_registered &&
-	    ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV)) {
+	if (bootcon_registered && !(newcon->flags & CON_BOOT))
 		newcon->flags &= ~CON_PRINTBUFFER;
-	}
 
 	newcon->dropped = 0;
 	console_init_seq(newcon, bootcon_registered);
@@ -3465,8 +3463,7 @@  void register_console(struct console *newcon)
 	 * went to the bootconsole (that they do not see on the real console)
 	 */
 	con_printk(KERN_INFO, newcon, "enabled\n");
-	if (bootcon_registered &&
-	    ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV) &&
+	if (bootcon_registered && !(newcon->flags & CON_BOOT) &&
 	    !keep_bootcon) {
 		struct hlist_node *tmp;