[tip: sched/rt] printk: nbcon: Use driver synchronization while (un)registering

From: tip-bot2 for John Ogness
Date: Mon Sep 09 2024 - 13:36:57 EST


The following commit has been merged into the sched/rt branch of tip:

Commit-ID: e55c3bcf380c7593d768f31994eff63935081d06
Gitweb: https://git.kernel.org/tip/e55c3bcf380c7593d768f31994eff63935081d06
Author: John Ogness <john.ogness@xxxxxxxxxxxxx>
AuthorDate: Tue, 20 Aug 2024 08:35:35 +02:06
Committer: Petr Mladek <pmladek@xxxxxxxx>
CommitterDate: Wed, 21 Aug 2024 14:56:23 +02:00

printk: nbcon: Use driver synchronization while (un)registering

Console drivers typically have to deal with access to the
hardware via user input/output (such as an interactive login
shell) and output of kernel messages via printk() calls.

They use some classic driver-specific locking mechanism in most
situations. But console->write_atomic() callbacks, used by nbcon
consoles, are synchronized only by acquiring the console
context.

The synchronization via the console context ownership is possible
only when the console driver is registered. It is when a
particular device driver is connected with a particular console
driver.

The two synchronization mechanisms must be synchronized between
each other. It is tricky because the console context ownership
is quite special. It might be taken over by a higher priority
context. Also CPU migration must be disabled. The most tricky
part is to (dis)connect these two mechanisms during the console
(un)registration.

Use the driver-specific locking callbacks: device_lock(),
device_unlock(). They allow taking the device-specific lock
while the device is being (un)registered by the related console
driver.

For example, these callbacks lock/unlock the port lock for
serial port drivers.

Note that the driver-specific locking is only needed during
(un)register if it is an nbcon console with the write_atomic()
callback implemented. If write_atomic() is not implemented, the
driver should never attempt to access the hardware without
first acquiring its driver-specific lock.

Signed-off-by: John Ogness <john.ogness@xxxxxxxxxxxxx>
Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>
Link: https://lore.kernel.org/r/20240820063001.36405-10-john.ogness@xxxxxxxxxxxxx
Signed-off-by: Petr Mladek <pmladek@xxxxxxxx>
---
kernel/printk/printk.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 20c3950..4cd2c50 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3548,9 +3548,11 @@ static int unregister_console_locked(struct console *console);
*/
void register_console(struct console *newcon)
{
- struct console *con;
+ bool use_device_lock = (newcon->flags & CON_NBCON) && newcon->write_atomic;
bool bootcon_registered = false;
bool realcon_registered = false;
+ struct console *con;
+ unsigned long flags;
u64 init_seq;
int err;

@@ -3638,6 +3640,19 @@ void register_console(struct console *newcon)
}

/*
+ * If another context is actively using the hardware of this new
+ * console, it will not be aware of the nbcon synchronization. This
+ * is a risk that two contexts could access the hardware
+ * simultaneously if this new console is used for atomic printing
+ * and the other context is still using the hardware.
+ *
+ * Use the driver synchronization to ensure that the hardware is not
+ * in use while this new console transitions to being registered.
+ */
+ if (use_device_lock)
+ newcon->device_lock(newcon, &flags);
+
+ /*
* Put this console in the list - keep the
* preferred driver at the head of the list.
*/
@@ -3661,6 +3676,10 @@ void register_console(struct console *newcon)
* register_console() completes.
*/

+ /* This new console is now registered. */
+ if (use_device_lock)
+ newcon->device_unlock(newcon, flags);
+
console_sysfs_notify();

/*
@@ -3689,6 +3708,8 @@ EXPORT_SYMBOL(register_console);
/* Must be called under console_list_lock(). */
static int unregister_console_locked(struct console *console)
{
+ bool use_device_lock = (console->flags & CON_NBCON) && console->write_atomic;
+ unsigned long flags;
int res;

lockdep_assert_console_list_lock_held();
@@ -3707,8 +3728,18 @@ static int unregister_console_locked(struct console *console)
if (!console_is_registered_locked(console))
return -ENODEV;

+ /*
+ * Use the driver synchronization to ensure that the hardware is not
+ * in use while this console transitions to being unregistered.
+ */
+ if (use_device_lock)
+ console->device_lock(console, &flags);
+
hlist_del_init_rcu(&console->node);

+ if (use_device_lock)
+ console->device_unlock(console, flags);
+
/*
* <HISTORICAL>
* If this isn't the last console and it has CON_CONSDEV set, we