Re: [PATCH RFC 0/3] printk: Don't flush messages using write_atomic during console registration if kthreads have not been started yet.

[John Ogness <john.ogness@linutronix.de>]

On 2025-05-30, Michael Cobb <mcobb@thegoodpenguin.co.uk> wrote:
> On Fri, 16 May 2025 at 10:44, John Ogness <john.ogness@linutronix.de> wrote:
>> What if we create the kthread _before_ printing the message. Something
>> like the below (untested) changes. Does this also address the issue?
>
> Yes, that works and avoids the atomic flush, however we then lose the
> "console [ttyS0] enabled" message on any boot consoles that we are
> about to unregister.

Right. I was going about it wrong. Really we should start the threads as
soon as the console is fully registered. Waiting until after the boot
consoles are unregistered is wrong. So how about this change instead?

Note that printk_kthreads_check_locked() is only needed for the newly
added console. If any boot console is unregistered,
printk_kthreads_check_locked() is called again in
unregister_console_locked().

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 1eea80d0648ed..b142d69330de2 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -4115,6 +4115,9 @@ void register_console(struct console *newcon)
 
 	console_sysfs_notify();
 
+	/* Changed console list, may require printer threads to start/stop. */
+	printk_kthreads_check_locked();
+
 	/*
 	 * By unregistering the bootconsoles after we enable the real console
 	 * we get the "console xxx enabled" message on all the consoles -
@@ -4133,9 +4136,6 @@ void register_console(struct console *newcon)
 				unregister_console_locked(con);
 		}
 	}
-
-	/* Changed console list, may require printer threads to start/stop. */
-	printk_kthreads_check_locked();
 unlock:
 	console_list_unlock();
 }


> I am worried that by avoiding calling printk() to not trigger a flush,
> this might not be robust enough?

The problem is that we cannot trust that the kthreads will start.

I think my above change is correct because it starts the kthreads before
unregistering the boot console, so if there are any problems, the boot
consoles are still around to report them.

In your case you have no boot consoles, so you really just want to avoid
all atomic printing until the kthread has had a chance. Something like
this change might be more appropriate:

diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index 48a24e7b309db..7462a6d179850 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -240,7 +240,7 @@ static inline void printk_get_console_flush_type(struct console_flush_type *ft)
 	switch (nbcon_get_default_prio()) {
 	case NBCON_PRIO_NORMAL:
 		if (have_nbcon_console && !have_boot_console) {
-			if (printk_kthreads_running)
+			if (printk_kthreads_running || printk_kthreads_pending_start)
 				ft->nbcon_offload = true;
 			else
 				ft->nbcon_atomic = true;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 1eea80d0648ed..9c0378dc88c4c 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -4072,6 +4072,14 @@ void register_console(struct console *newcon)
 	if (newcon->flags & CON_BOOT)
 		have_boot_console = true;
 
+	/*
+	 * If this is the first console, avoid flushing the backlog
+	 * until the printing kthread has had a chance to start via
+	 * printk_kthreads_check_locked() below.
+	 */
+	if (hlist_empty(&console_list) && (newcon->flags & CON_NBCON))
+		printk_kthread_pending_start = true;
+
 	/*
 	 * If another context is actively using the hardware of this new
 	 * console, it will not be aware of the nbcon synchronization. This
@@ -4115,6 +4123,10 @@ void register_console(struct console *newcon)
 
 	console_sysfs_notify();
 
+	/* Changed console list, may require printer threads to start/stop. */
+	printk_kthreads_check_locked();
+	printk_kthread_pending_start = false;
+
 	/*
 	 * By unregistering the bootconsoles after we enable the real console
 	 * we get the "console xxx enabled" message on all the consoles -
@@ -4133,9 +4145,6 @@ void register_console(struct console *newcon)
 				unregister_console_locked(con);
 		}
 	}
-
-	/* Changed console list, may require printer threads to start/stop. */
-	printk_kthreads_check_locked();
 unlock:
 	console_list_unlock();
 }

Notice that I kept the control of the new flag inside the register
function.

I do not really like this because printk_get_console_flush_type() is now
checking an extra flag every time for something that is a special case
and only at boot. Although, theoretically, it can happen anytime that
all consoles are removed during runtime and then later added.

Thoughts?

John